From f2cbed0cc4469712a99a1aed17b1de1b9252582b Mon Sep 17 00:00:00 2001
From: Changyu Bi <changyubi@meta.com>
Date: Fri, 1 Sep 2023 11:33:15 -0700
Subject: [PATCH] Fix a bug where iterator can return incorrect data for
 DeleteRange() users (#11786)

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                                     | 4 ++--
 .../bug_fixes/010_check_more_iter_status_for_delete_range.md  | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)
 create mode 100644 unreleased_history/bug_fixes/010_check_more_iter_status_for_delete_range.md

diff --git a/table/merging_iterator.cc b/table/merging_iterator.cc
index ae92aa198..505cd76d3 100644
--- a/table/merging_iterator.cc
+++ b/table/merging_iterator.cc
@@ -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
index 000000000..3e060b658
--- /dev/null
+++ b/unreleased_history/bug_fixes/010_check_more_iter_status_for_delete_range.md
@@ -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
-- 
2.47.3