dolysis/rocksdb
3
0
Fork 0
mirror of https://github.com/facebook/rocksdb.git synced 2024-11-29 09:36:17 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix a bug where iterator status is not checked (#11782)

Browse source 
Summary:
This happens in (Compaction)MergingIterator layer, and can cause data loss during compaction or read/scan return incorrect result

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

Reviewed By: ajkr

Differential Revision: D48880575

Pulled By: cbi42

fbshipit-source-id: 2294ad284a6d653d3674bebe55380f12ee4b645b
This commit is contained in:
Changyu Bi 2023-09-01 09:34:08 -07:00 committed by Facebook GitHub Bot
parent 47be3ffffb
commit bd6a8340c3
3 changed files with 10 additions and 0 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
table/compaction_merging_iterator.cc
View file

@ -329,6 +329,7 @@ void CompactionMergingIterator::FindNextVisibleKey() {
assert(current->iter.status().ok()); assert(current->iter.status().ok());
minHeap_.replace_top(current); minHeap_.replace_top(current);
} else { } else {
considerStatus(current->iter.status());
minHeap_.pop(); minHeap_.pop();
} }
if (range_tombstone_iters_[current->level]) { if (range_tombstone_iters_[current->level]) {

8
table/merging_iterator.cc
View file

@ -931,6 +931,7 @@ bool MergingIterator::SkipNextDeleted() {
InsertRangeTombstoneToMinHeap(current->level, true /* start_key */, InsertRangeTombstoneToMinHeap(current->level, true /* start_key */,
true /* replace_top */); true /* replace_top */);
} else { } else {
// TruncatedRangeDelIterator does not have status
minHeap_.pop(); minHeap_.pop();
} }
return true /* current key deleted */; return true /* current key deleted */;
@ -988,6 +989,9 @@ bool MergingIterator::SkipNextDeleted() {
if (current->iter.Valid()) { if (current->iter.Valid()) {
assert(current->iter.status().ok()); assert(current->iter.status().ok());
minHeap_.push(current); minHeap_.push(current);
} else {
// TODO(cbi): check status and early return if non-ok.
considerStatus(current->iter.status());
} }
// Invariants (rti) and (phi) // Invariants (rti) and (phi)
if (range_tombstone_iters_[current->level] && if (range_tombstone_iters_[current->level] &&
@ -1027,6 +1031,7 @@ bool MergingIterator::SkipNextDeleted() {
if (current->iter.Valid()) { if (current->iter.Valid()) {
minHeap_.replace_top(current); minHeap_.replace_top(current);
} else { } else {
considerStatus(current->iter.status());
minHeap_.pop(); minHeap_.pop();
} }
return true /* current key deleted */; return true /* current key deleted */;
@ -1199,6 +1204,8 @@ bool MergingIterator::SkipPrevDeleted() {
if (current->iter.Valid()) { if (current->iter.Valid()) {
assert(current->iter.status().ok()); assert(current->iter.status().ok());
maxHeap_->push(current); maxHeap_->push(current);
} else {
considerStatus(current->iter.status());
} }
if (range_tombstone_iters_[current->level] && if (range_tombstone_iters_[current->level] &&
@ -1241,6 +1248,7 @@ bool MergingIterator::SkipPrevDeleted() {
if (current->iter.Valid()) { if (current->iter.Valid()) {
maxHeap_->replace_top(current); maxHeap_->replace_top(current);
} else { } else {
considerStatus(current->iter.status());
maxHeap_->pop(); maxHeap_->pop();
} }
return true /* current key deleted */; return true /* current key deleted */;

1
unreleased_history/bug_fixes/001_check_iter_status_data_loss.md Normal file
View file

@ -0,0 +1 @@
* Fix a bug where if there is an error reading from offset 0 of a file from L1+ and that the file is not the first file in the sorted run, data can be lost in compaction and read/scan can return incorrect results.