]> git-server-git.apps.pok.os.sepia.ceph.com Git - rocksdb.git/commitdiff
Fix a bug where iterator can return incorrect data for DeleteRange() users (#11786)
authorChangyu Bi <changyubi@meta.com>
Fri, 1 Sep 2023 18:33:15 +0000 (11:33 -0700)
committerAndrew Kryczka <andrew.kryczka2@gmail.com>
Fri, 1 Sep 2023 19:08:33 +0000 (12:08 -0700)
Summary:
This should only affect iterator when
- user uses DeleteRange(),
- An iterator from level L has a non-ok status (such non-ok status may not be caught before the bug fix in https://github.com/facebook/rocksdb/pull/11783), and
- A range tombstone covers a key from level > L and triggers a reseek sets the status_ to OK in SeekImpl()/SeekPrevImpl() e.g. https://github.com/facebook/rocksdb/blob/bd6a8340c3a2db764620e90b3ac5be173fc68a0c/table/merging_iterator.cc#L801

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

Differential Revision: D48908830

Pulled By: cbi42

fbshipit-source-id: eb564be375af4e33dc27542eff753260186e6d5d

table/merging_iterator.cc
unreleased_history/bug_fixes/010_check_more_iter_status_for_delete_range.md [new file with mode: 0644]

index ae92aa19830ef021c72332b829d0d6071939c6b2..505cd76d38682091c812d53fc7b6bfd5f39aab26 100644 (file)
@@ -308,6 +308,7 @@ class MergingIterator : public InternalIterator {
     // holds after this call, and minHeap_.top().iter points to the
     // first key >= target among children_ that is not covered by any range
     // tombstone.
+    status_ = Status::OK();
     SeekImpl(target);
     FindNextVisibleKey();
 
@@ -321,6 +322,7 @@ class MergingIterator : public InternalIterator {
   void SeekForPrev(const Slice& target) override {
     assert(range_tombstone_iters_.empty() ||
            range_tombstone_iters_.size() == children_.size());
+    status_ = Status::OK();
     SeekForPrevImpl(target);
     FindPrevVisibleKey();
 
@@ -798,7 +800,6 @@ void MergingIterator::SeekImpl(const Slice& target, size_t starting_level,
     active_.erase(active_.lower_bound(starting_level), active_.end());
   }
 
-  status_ = Status::OK();
   IterKey current_search_key;
   current_search_key.SetInternalKey(target, false /* copy */);
   // Seek target might change to some range tombstone end key, so
@@ -1083,7 +1084,6 @@ void MergingIterator::SeekForPrevImpl(const Slice& target,
     active_.erase(active_.lower_bound(starting_level), active_.end());
   }
 
-  status_ = Status::OK();
   IterKey current_search_key;
   current_search_key.SetInternalKey(target, false /* copy */);
   // Seek target might change to some range tombstone end key, so
diff --git a/unreleased_history/bug_fixes/010_check_more_iter_status_for_delete_range.md b/unreleased_history/bug_fixes/010_check_more_iter_status_for_delete_range.md
new file mode 100644 (file)
index 0000000..3e060b6
--- /dev/null
@@ -0,0 +1 @@
+* Fix a bug where iterator may return incorrect result for DeleteRange() users if there was an error reading from a file.
\ No newline at end of file