Fix incursive deadlock in ~Snapshot by JaySon-Huang · Pull Request #2277 · pingcap/tiflash

JaySon-Huang · 2021-06-30T12:16:04Z

What problem does this PR solve?

Issue Number: close #2249

Problem Summary:
Running ./dbms/src/Storages/Page/tests/page_stress_page_storage "" 120 8 1024, it will create 8 writing threads and 1024 reading threads.

It will throw the same exception as #2249. The backtrace is[1].
We can see that inside ~Snapshot, it calls removeExpiredSnapshots and release another snapshot. It cause incursive deadlock on vset->read_write_mutex.
#2229 Add some logic in removeExpiredSnapshots and make it more likely to run into this case.

[1] backtrace

(gdb) bt
#0  0x00007ffff67da387 in raise () from /lib64/libc.so.6
#1  0x00007ffff67dba78 in abort () from /lib64/libc.so.6
#2  0x00000000006ff9d5 in __gnu_cxx::__verbose_terminate_handler () at ../../.././libstdc++-v3/libsupc++/vterminate.cc:95
#3  0x0000000000682226 in __cxxabiv1::__terminate (handler=<optimized out>) at ../../.././libstdc++-v3/libsupc++/eh_terminate.cc:47
#4  0x0000000000683329 in __cxa_call_terminate (ue_header=ue_header@entry=0x7ffd262cf100) at ../../.././libstdc++-v3/libsupc++/eh_call.cc:54
#5  0x00000000006829d8 in __cxxabiv1::__gxx_personality_v0 (version=<optimized out>, actions=6, exception_class=5138137972254386944, ue_header=0x7ffd262cf100, context=<optimized out>) at ../../.././libstdc++-v3/libsupc++/eh_personality.cc:676
#6  0x0000000000716573 in _Unwind_RaiseException_Phase2 (exc=exc@entry=0x7ffd262cf100, context=context@entry=0x7fff715bda10) at ../.././libgcc/unwind.inc:62
#7  0x0000000000716d8f in _Unwind_RaiseException (exc=exc@entry=0x7ffd262cf100) at ../.././libgcc/unwind.inc:131
#8  0x00000000006839b6 in __cxxabiv1::__cxa_throw (obj=obj@entry=0x7ffd262cf120, tinfo=tinfo@entry=0x9c61d8 <typeinfo for std::system_error>, dest=dest@entry=0x68b910 <std::system_error::~system_error()>) at ../../.././libstdc++-v3/libsupc++/eh_throw.cc:88
#9  0x000000000068bbde in std::__throw_system_error (__i=__i@entry=35) at ../../../.././libstdc++-v3/src/c++11/system_error.cc:81
#10 0x0000000000520735 in std::__shared_mutex_pthread::lock_shared (this=<optimized out>) at /usr/local/include/c++/7.3.0/shared_mutex:142
#11 std::shared_mutex::lock_shared (this=<optimized out>) at /usr/local/include/c++/7.3.0/shared_mutex:335
#12 std::shared_lock<std::shared_mutex>::shared_lock (__m=..., this=<optimized out>) at /usr/local/include/c++/7.3.0/shared_mutex:553

// try to get incursive lock on vset->read_write_mutex
#13 DB::MVCC::VersionSetWithDelta<DB::PageEntriesForDelta, DB::PageEntriesView, DB::PageEntriesEdit, DB::DeltaVersionEditAcceptor>::compactOnDeltaRelease (tail=..., this=<optimized out>)
    at /data1/jaysonhuang/tics/dbms/src/Storages/Page/mvcc/VersionSetWithDelta.h:275
#14 DB::MVCC::VersionSetWithDelta<DB::PageEntriesForDelta, DB::PageEntriesView, DB::PageEntriesEdit, DB::DeltaVersionEditAcceptor>::Snapshot::~Snapshot (this=0x7ffd3b96a010, __in_chrg=<optimized out>)
    at /data1/jaysonhuang/tics/dbms/src/Storages/Page/mvcc/VersionSetWithDelta.h:144
#15 __gnu_cxx::new_allocator<DB::MVCC::VersionSetWithDelta<DB::PageEntriesForDelta, DB::PageEntriesView, DB::PageEntriesEdit, DB::DeltaVersionEditAcceptor>::Snapshot>::destroy<DB::MVCC::VersionSetWithDelta<DB::PageEntriesForDelta, DB::PageEntriesView, DB::PageEntriesEdit, DB::DeltaVersionEditAcceptor>::Snapshot> (this=<optimized out>, __p=<optimized out>) at /usr/local/include/c++/7.3.0/ext/new_allocator.h:140
#16 std::allocator_traits<std::allocator<DB::MVCC::VersionSetWithDelta<DB::PageEntriesForDelta, DB::PageEntriesView, DB::PageEntriesEdit, DB::DeltaVersionEditAcceptor>::Snapshot> >::destroy<DB::MVCC::VersionSetWithDelta<DB::PageEntriesForDelta, DB::PageEntriesView, DB::PageEntriesEdit, DB::DeltaVersionEditAcceptor>::Snapshot> (__a=..., __p=<optimized out>) at /usr/local/include/c++/7.3.0/bits/alloc_traits.h:487
#17 std::_Sp_counted_ptr_inplace<DB::MVCC::VersionSetWithDelta<DB::PageEntriesForDelta, DB::PageEntriesView, DB::PageEntriesEdit, DB::DeltaVersionEditAcceptor>::Snapshot, std::allocator<DB::MVCC::VersionSetWithDelta<DB::PageEntriesForDelta, DB::PageEntriesView, DB::PageEntriesEdit, DB::DeltaVersionEditAcceptor>::Snapshot>, (__gnu_cxx::_Lock_policy)2>::_M_dispose (this=0x7ffd3b96a000) at /usr/local/include/c++/7.3.0/bits/shared_ptr_base.h:535
#18 0x00000000005185d9 in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release (this=0x7ffd3b96a000) at /usr/local/include/c++/7.3.0/bits/shared_ptr_base.h:154
#19 std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count (this=<synthetic pointer>, __in_chrg=<optimized out>) at /usr/local/include/c++/7.3.0/bits/shared_ptr_base.h:684
#20 std::__shared_ptr<DB::MVCC::VersionSetWithDelta<DB::PageEntriesForDelta, DB::PageEntriesView, DB::PageEntriesEdit, DB::DeltaVersionEditAcceptor>::Snapshot, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr (this=<synthetic pointer>, __in_chrg=<optimized out>)
    at /usr/local/include/c++/7.3.0/bits/shared_ptr_base.h:1123
#21 std::shared_ptr<DB::MVCC::VersionSetWithDelta<DB::PageEntriesForDelta, DB::PageEntriesView, DB::PageEntriesEdit, DB::DeltaVersionEditAcceptor>::Snapshot>::~shared_ptr (this=<synthetic pointer>, __in_chrg=<optimized out>)
    at /usr/local/include/c++/7.3.0/bits/shared_ptr.h:93

// call `removeExpiredSnapshots`
#22 DB::MVCC::VersionSetWithDelta<DB::PageEntriesForDelta, DB::PageEntriesView, DB::PageEntriesEdit, DB::DeltaVersionEditAcceptor>::removeExpiredSnapshots (this=0x7ffff2e6f228) at /data1/jaysonhuang/tics/dbms/src/Storages/Page/mvcc/VersionSetWithDelta.h:320
#23 0x00000000005205dc in DB::MVCC::VersionSetWithDelta<DB::PageEntriesForDelta, DB::PageEntriesView, DB::PageEntriesEdit, DB::DeltaVersionEditAcceptor>::Snapshot::~Snapshot (this=0x7ffd26185790, __in_chrg=<optimized out>)
    at /data1/jaysonhuang/tics/dbms/src/Storages/Page/mvcc/VersionSetWithDelta.h:153
#24 __gnu_cxx::new_allocator<DB::MVCC::VersionSetWithDelta<DB::PageEntriesForDelta, DB::PageEntriesView, DB::PageEntriesEdit, DB::DeltaVersionEditAcceptor>::Snapshot>::destroy<DB::MVCC::VersionSetWithDelta<DB::PageEntriesForDelta, DB::PageEntriesView, DB::PageEntriesEdit, DB::DeltaVersionEditAcceptor>::Snapshot> (this=<optimized out>, __p=<optimized out>) at /usr/local/include/c++/7.3.0/ext/new_allocator.h:140
#25 std::allocator_traits<std::allocator<DB::MVCC::VersionSetWithDelta<DB::PageEntriesForDelta, DB::PageEntriesView, DB::PageEntriesEdit, DB::DeltaVersionEditAcceptor>::Snapshot> >::destroy<DB::MVCC::VersionSetWithDelta<DB::PageEntriesForDelta, DB::PageEntriesView, DB::PageEntriesEdit, DB::DeltaVersionEditAcceptor>::Snapshot> (__a=..., __p=<optimized out>) at /usr/local/include/c++/7.3.0/bits/alloc_traits.h:487
#26 std::_Sp_counted_ptr_inplace<DB::MVCC::VersionSetWithDelta<DB::PageEntriesForDelta, DB::PageEntriesView, DB::PageEntriesEdit, DB::DeltaVersionEditAcceptor>::Snapshot, std::allocator<DB::MVCC::VersionSetWithDelta<DB::PageEntriesForDelta, DB::PageEntriesView, DB::PageEntriesEdit, DB::DeltaVersionEditAcceptor>::Snapshot>, (__gnu_cxx::_Lock_policy)2>::_M_dispose (this=0x7ffd26185780) at /usr/local/include/c++/7.3.0/bits/shared_ptr_base.h:535
#27 0x00000000004f5b09 in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release (this=0x7ffd26185780) at /usr/local/include/c++/7.3.0/bits/shared_ptr_base.h:154
#28 std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count (this=0x7fff715bdf48, __in_chrg=<optimized out>) at /usr/local/include/c++/7.3.0/bits/shared_ptr_base.h:684
#29 std::__shared_ptr<DB::MVCC::VersionSetWithDelta<DB::PageEntriesForDelta, DB::PageEntriesView, DB::PageEntriesEdit, DB::DeltaVersionEditAcceptor>::Snapshot, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr (this=0x7fff715bdf40, __in_chrg=<optimized out>)
    at /usr/local/include/c++/7.3.0/bits/shared_ptr_base.h:1123
#30 std::shared_ptr<DB::MVCC::VersionSetWithDelta<DB::PageEntriesForDelta, DB::PageEntriesView, DB::PageEntriesEdit, DB::DeltaVersionEditAcceptor>::Snapshot>::~shared_ptr (this=0x7fff715bdf40, __in_chrg=<optimized out>)
    at /usr/local/include/c++/7.3.0/bits/shared_ptr.h:93
#31 PSReader::run (this=0x7ffff2e27ca0) at /data1/jaysonhuang/tics/dbms/src/Storages/Page/tests/stress_page_stroage.cpp:180
#32 0x000000000063b0e7 in Poco::PooledThread::run (this=0x7ffff2f62600) at /data1/jaysonhuang/tics/contrib/poco/Foundation/src/ThreadPool.cpp:214
#33 0x0000000000635a58 in Poco::ThreadImpl::runnableEntry (pThread=<optimized out>) at /data1/jaysonhuang/tics/contrib/poco/Foundation/src/Thread_STD.cpp:139
#34 0x00000000006a9baf in std::execute_native_thread_routine (__p=0x7ffff2e8c1c0) at ../../../.././libstdc++-v3/src/c++11/thread.cc:83
#35 0x00007ffff795cea5 in start_thread () from /lib64/libpthread.so.0
#36 0x00007ffff68a29fd in clone () from /lib64/libc.so.6

[2] https://github.com/pingcap/tics/blob/8a4e9a6745e540144201b12194f820dea3ea0f11/dbms/src/Storages/Page/mvcc/VersionSetWithDelta.h#L149-L154

What is changed and how it works?

Do not call vset->removeExpiredSnapshots inside ~Snapshot, only remove useless snapshot weak_ptrs in PageEntriesVersionSetWithDelta::listAllLiveFiles
Add lock in MergeDeltaTaskPool::length (a small data race fixing)

Related changes

PR to update pingcap/docs/pingcap/docs-cn:
Need to cherry-pick to the release branch:

Check List

Tests

Manual test (add detailed scripts or steps below)
- Running ./dbms/src/Storages/Page/tests/page_stress_page_storage "" 120 8 1024, it won't throw error "Resource deadlock avoided"

Side effects

Release note

Fix the panic issue that occurs when the read load is heavy

Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>

JaySon-Huang · 2021-06-30T12:30:39Z

/run-all-tests

flowbehappy

LGTM

ti-srebot · 2021-06-30T12:46:42Z

cherry pick to release-4.0 in PR #2280

ti-srebot · 2021-06-30T12:47:08Z

cherry pick to release-5.1 in PR #2281

* Fix data race on bkg tasks * Fix incursive deadlock Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>

* cherry pick #2277 to release-4.0 Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

JaySon-Huang · 2021-07-06T09:53:27Z

cherry pick to release-5.0 in PR #2274

JaySon-Huang added 2 commits June 30, 2021 20:14

Fix data race on bkg tasks

8288458

Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>

Fix incursive deadlock

5cff1aa

Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>

JaySon-Huang mentioned this pull request Jun 30, 2021

Fix incursive deadlock in ~Snapshot(#2277) #2274

Merged

JaySon-Huang self-assigned this Jun 30, 2021

JaySon-Huang added type/bugfix This PR fixes a bug. needs-cherry-pick-release-4.0 PR which needs to be cherry-picked to release-4.0 needs-cherry-pick-release-5.1 PR which needs to be cherry-picked to release-5.1 labels Jun 30, 2021

Merge branch 'master' into fix_incursive_deadlock

2851bf5

JaySon-Huang requested a review from flowbehappy June 30, 2021 12:17

flowbehappy approved these changes Jun 30, 2021

View reviewed changes

ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Jun 30, 2021

JaySon-Huang merged commit e9f28c7 into pingcap:master Jun 30, 2021

JaySon-Huang deleted the fix_incursive_deadlock branch June 30, 2021 12:46

ti-srebot mentioned this pull request Jun 30, 2021

Fix incursive deadlock in ~Snapshot (#2277) #2280

Merged

ti-srebot mentioned this pull request Jun 30, 2021

Fix incursive deadlock in ~Snapshot (#2277) #2281

Merged

JaySon-Huang added a commit that referenced this pull request Jun 30, 2021

Fix incursive deadlock in ~Snapshot(#2277) (#2274)

332467d

* Fix data race on bkg tasks * Fix incursive deadlock Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>

JaySon-Huang added a commit to JaySon-Huang/tiflash that referenced this pull request Jul 1, 2021

Fix incursive deadlock in ~Snapshot (pingcap#2277)

4031e6c

* Fix data race on bkg tasks * Fix incursive deadlock Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>

JaySon-Huang mentioned this pull request Jul 1, 2021

Fix incursive deadlock in ~Snapshot (#2277) #2290

Merged

LittleFall pushed a commit that referenced this pull request Jul 1, 2021

Fix incursive deadlock in ~Snapshot (#2277) (#2290)

96200cf

* Fix data race on bkg tasks * Fix incursive deadlock Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>

JaySon-Huang pushed a commit that referenced this pull request Jul 6, 2021

Fix incursive deadlock in ~Snapshot (#2277) (#2280)

858e93e

* cherry pick #2277 to release-4.0 Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

ti-srebot added a commit that referenced this pull request Jul 15, 2021

Fix incursive deadlock in ~Snapshot (#2277) (#2281)

b9c00ed

This was referenced Jul 22, 2021

Fix deadlock on removeExpiredSnapshots #2461

Merged

coredump with the error of "std::system_error what(): Resource deadlock avoided" #2456

Closed

ti-srebot mentioned this pull request Aug 4, 2021

Fix deadlock on removeExpiredSnapshots (#2461) #2567

Merged

JaySon-Huang mentioned this pull request Aug 30, 2021

[potential bug] TiFlash crash when the snapshot of Delta Tree Storage is free #1429

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix incursive deadlock in ~Snapshot#2277

Fix incursive deadlock in ~Snapshot#2277
JaySon-Huang merged 3 commits intopingcap:masterfrom
JaySon-Huang:fix_incursive_deadlock

JaySon-Huang commented Jun 30, 2021 •

edited

Loading

Uh oh!

JaySon-Huang commented Jun 30, 2021

Uh oh!

flowbehappy left a comment

Uh oh!

ti-srebot commented Jun 30, 2021

Uh oh!

ti-srebot commented Jun 30, 2021

Uh oh!

JaySon-Huang commented Jul 6, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

JaySon-Huang commented Jun 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What is changed and how it works?

Related changes

Check List

Release note

Uh oh!

JaySon-Huang commented Jun 30, 2021

Uh oh!

flowbehappy left a comment

Choose a reason for hiding this comment

Uh oh!

ti-srebot commented Jun 30, 2021

Uh oh!

ti-srebot commented Jun 30, 2021

Uh oh!

JaySon-Huang commented Jul 6, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

JaySon-Huang commented Jun 30, 2021 •

edited

Loading