-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Data race between ~Tablet and yb::tablet::Tablet::RegularDbFilesChanged()/ThreadPoolTask::Run #3519
Labels
area/docdb
YugabyteDB core features
Comments
Also reproducible with |
spolitov
added a commit
that referenced
this issue
Feb 25, 2020
…DbFilesChanged() Summary: DBImpl invokes FilesChanged after decrementing the number of background flushes and unlocking the mutex in BackgroundCallFlush. So we could get into a situation when DBImpl destructor already thinks that the background job is done. But we are calling FilesChanged and it will be invoked for a destroyed RocksDB, that will call the callback on a destroyed Tablet. Fixed by calling FilesChanged before decrementing number of background flushes. Did the same in BackgroundCallCompaction. Also moved TaskPriorityUpdater.Apply to the same place. It was safe invoke it from that place, but brought confusion that other code could be placed nearby. Test Plan: ybd tsan --cxx-test client_ql-tablet-test --gtest_filter QLTabletTest.SkewedClocks -n 100 -- -p 4 Reviewers: timur, bogdan, mikhail Reviewed By: mikhail Subscribers: raju, ybase Differential Revision: https://phabricator.dev.yugabyte.com/D8022
spolitov
added a commit
that referenced
this issue
Apr 18, 2020
…and yb::tablet::Tablet::RegularDbFilesChanged() Summary: DBImpl invokes FilesChanged after decrementing the number of background flushes and unlocking the mutex in BackgroundCallFlush. So we could get into a situation when DBImpl destructor already thinks that the background job is done. But we are calling FilesChanged and it will be invoked for a destroyed RocksDB, that will call the callback on a destroyed Tablet. Fixed by calling FilesChanged before decrementing number of background flushes. Did the same in BackgroundCallCompaction. Also moved TaskPriorityUpdater.Apply to the same place. It was safe invoke it from that place, but brought confusion that other code could be placed nearby. Test Plan: ybd tsan --cxx-test client_ql-tablet-test --gtest_filter QLTabletTest.SkewedClocks -n 100 -- -p 4 Jenkins: auto rebase: no Reviewers: bogdan Reviewed By: bogdan Subscribers: ybase Differential Revision: https://phabricator.dev.yugabyte.com/D8322
ngov17
pushed a commit
to ngov17/yugabyte-db
that referenced
this issue
Jun 26, 2020
…~Tablet and yb::tablet::Tablet::RegularDbFilesChanged() Summary: DBImpl invokes FilesChanged after decrementing the number of background flushes and unlocking the mutex in BackgroundCallFlush. So we could get into a situation when DBImpl destructor already thinks that the background job is done. But we are calling FilesChanged and it will be invoked for a destroyed RocksDB, that will call the callback on a destroyed Tablet. Fixed by calling FilesChanged before decrementing number of background flushes. Did the same in BackgroundCallCompaction. Also moved TaskPriorityUpdater.Apply to the same place. It was safe invoke it from that place, but brought confusion that other code could be placed nearby. Test Plan: ybd tsan --cxx-test client_ql-tablet-test --gtest_filter QLTabletTest.SkewedClocks -n 100 -- -p 4 Jenkins: auto rebase: no Reviewers: bogdan Reviewed By: bogdan Subscribers: ybase Differential Revision: https://phabricator.dev.yugabyte.com/D8322
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
https://jenkins.dev.yugabyte.com/job/github-yugabyte-db-phabricator/24436/artifact/build/tsan-clang-dynamic-ninja/yb-test-logs/tests-client__client-test/ClientTest_TestGetTabletServerBlacklist.log
The text was updated successfully, but these errors were encountered: