Fix a MultiGet crash (#10688)

Summary:
Fix a bug in the async IO/coroutine version of MultiGet that may cause a segfault or assertion failure due to accessing an invalid file index in a LevelFilesBrief. The bug is that when a MultiGetRange is split into two, we may re-process keys in the original range that were already marked to be skipped (in ```current_level_range_```) due to not overlapping the level.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/10688

Reviewed By: gitbw95

Differential Revision: D39556131

Pulled By: anand1976

fbshipit-source-id: 65e79438508a283cb19e64eca5c91d0714b81458
main
anand76 2 years ago committed by Facebook GitHub Bot
parent 6ce782beaf
commit c206aebd0b
  1. 6
      db/version_set.cc

@ -511,6 +511,7 @@ class FilePickerMultiGet {
MultiGetRange& GetRange() { return range_; } MultiGetRange& GetRange() { return range_; }
void ReplaceRange(const MultiGetRange& other) { void ReplaceRange(const MultiGetRange& other) {
assert(curr_level_ == 0 || !RemainingOverlapInLevel());
range_ = other; range_ = other;
current_level_range_ = other; current_level_range_ = other;
} }
@ -2727,8 +2728,9 @@ Status Version::ProcessBatch(
f = fp.GetNextFileInLevel(); f = fp.GetNextFileInLevel();
} }
// Split the current batch only if some keys are likely in this level and // Split the current batch only if some keys are likely in this level and
// some are not. // some are not. Only split if we're done with this level, i.e f is null.
if (s.ok() && !leftover.empty() && !range.empty()) { // Otherwise, it means there are more files in this level to look at.
if (s.ok() && !f && !leftover.empty() && !range.empty()) {
fp.ReplaceRange(range); fp.ReplaceRange(range);
batches.emplace_back(&leftover, fp); batches.emplace_back(&leftover, fp);
to_process.emplace_back(batches.size() - 1); to_process.emplace_back(batches.size() - 1);

Loading…
Cancel
Save