-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
fix hanging after CompactFiles with L0 overlap #2849
Conversation
@ajkr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
ping, so far it seems like this fixes a percona issue. |
@ajkr does MyRocks use CompactFiles()? |
They expose a sysvar: force_flush_memtable_and_lzero_now. Let me ask Herman if it's used. |
@ajkr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
@siying it's not currently used in any automated way; it's only used in tests. |
* fix CompactFiles inclusion of older L0 files Summary: if we're moving any L0 files down, we need to include older L0 files since they may contain older versions of the keys being moved down. Closes facebook#2845 Differential Revision: D5773800 Pulled By: ajkr fbshipit-source-id: 9f0770a8eaaeea4c87df2e7a2a1d65bf9d7f4f7e * fix hanging after CompactFiles with L0 overlap Summary: Bug report: https://www.facebook.com/groups/rocksdb.dev/permalink/1389452781153232/ Non-empty `level0_compactions_in_progress_` was aborting `CompactFiles` after incrementing `bg_compaction_scheduled_`, and in that case we never decremented it. This blocked future compactions and prevented DB close as we wait for scheduled compactions to finish/abort during close. I eliminated `CompactFiles`'s dependency on `level0_compactions_in_progress_`. Since it takes a contiguous span of L0 files -- through the last L0 file if any L1+ files are included -- it's fine to run in parallel with other compactions involving L0. We make the same assumption in intra-L0 compaction. Closes facebook#2849 Differential Revision: D5780440 Pulled By: ajkr fbshipit-source-id: 15b15d3faf5a699aed4b82a58352d4a7bb23e027 * Check conflict at output level in CompactFiles (facebook#3926) Summary: CompactFiles checked whether the existing files conflicted with the chosen compaction. But it missed checking whether future files would conflict, i.e., when another compaction was simultaneously writing new files to the same range at the same output level. Closes facebook#3926 Differential Revision: D8218996 Pulled By: ajkr fbshipit-source-id: 21cb00a6fed4c8c62d3ed2ff810962e6bdc2fdfb
Bug report: https://www.facebook.com/groups/rocksdb.dev/permalink/1389452781153232/
Non-empty
level0_compactions_in_progress_
was abortingCompactFiles
after incrementingbg_compaction_scheduled_
, and in that case we never decremented it. This blocked future compactions and prevented DB close as we wait for scheduled compactions to finish/abort during close.I eliminated
CompactFiles
's dependency onlevel0_compactions_in_progress_
. Since it takes a contiguous span of L0 files -- through the last L0 file if any L1+ files are included -- it's fine to run in parallel with other compactions involving L0. We make the same assumption in intra-L0 compaction.Test Plan: