Fix merge errors

Author: mrambacher

file/sst_file_manager_impl.h

@@ -44,11 +44,6 @@ class SstFileManagerImpl : public SstFileManager {
   Status OnAddFile(const std::string& file_path, uint64_t file_size,
                    bool compaction);
 
-  // Overload where size of the file is provided by the caller rather than
-  // queried from the filesystem. This is an optimization.
-  Status OnAddFile(const std::string& file_path, uint64_t file_size,
-                   bool compaction);
-
   // DB will call OnDeleteFile whenever an sst file is deleted.
   Status OnDeleteFile(const std::string& file_path);
 

table/block_based/block.cc

@@ -384,10 +384,19 @@ void IndexBlockIter::Seek(const Slice& target) {
   if (data_ == nullptr) {  // Not init yet
     return;
   }
+  status_ = Status::OK();
   uint32_t index = 0;
   bool ok = false;
   if (prefix_index_) {
-    ok = PrefixSeek(target, &index);
+    bool prefix_may_exist = true;
+    ok = PrefixSeek(target, &index, &prefix_may_exist);
+    if (!prefix_may_exist) {
+      // This is to let the caller to distinguish between non-existing prefix,
+      // and when key is larger than the last key, which both set Valid() to
+      // false.
+      current_ = restarts_;
+      status_ = Status::NotFound();
+    }
   } else if (value_delta_encoded_) {
     ok = BinarySeek<DecodeKeyV4>(seek_key, 0, num_restarts_ - 1, &index,
                                  comparator_);
@@ -453,6 +462,7 @@ void IndexBlockIter::SeekToFirst() {
   if (data_ == nullptr) {  // Not init yet
     return;
   }
+  status_ = Status::OK();
   SeekToRestartPoint(0);
   ParseNextIndexKey();
 }
@@ -471,6 +481,7 @@ void IndexBlockIter::SeekToLast() {
   if (data_ == nullptr) {  // Not init yet
     return;
   }
+  status_ = Status::OK();
   SeekToRestartPoint(num_restarts_ - 1);
   while (ParseNextIndexKey() && NextEntryOffset() < restarts_) {
     // Keep skipping
@@ -709,8 +720,12 @@ int IndexBlockIter::CompareBlockKey(uint32_t block_index, const Slice& target) {
 // with a key >= target
 bool IndexBlockIter::BinaryBlockIndexSeek(const Slice& target,
                                           uint32_t* block_ids, uint32_t left,
-                                          uint32_t right, uint32_t* index) {
+                                          uint32_t right, uint32_t* index,
+                                          bool* prefix_may_exist) {
   assert(left <= right);
+  assert(index);
+  assert(prefix_may_exist);
+  *prefix_may_exist = true;
   uint32_t left_bound = left;
 
   while (left <= right) {
@@ -744,21 +759,53 @@ bool IndexBlockIter::BinaryBlockIndexSeek(const Slice& target,
         (left == left_bound || block_ids[left - 1] != block_ids[left] - 1) &&
         CompareBlockKey(block_ids[left] - 1, target) > 0) {
       current_ = restarts_;
+      *prefix_may_exist = false;
       return false;
     }
 
     *index = block_ids[left];
     return true;
   } else {
     assert(left > right);
+
+    // If the next block key is larger than seek key, it is possible that
+    // no key shares the prefix with `target`, or all keys with the same
+    // prefix as `target` are smaller than prefix. In the latter case,
+    // we are mandated to set the position the same as the total order.
+    // In the latter case, either:
+    // (1) `target` falls into the range of the next block. In this case,
+    //     we can place the iterator to the next block, or
+    // (2) `target` is larger than all block keys. In this case we can
+    //     keep the iterator invalidate without setting `prefix_may_exist`
+    //     to false.
+    // We might sometimes end up with setting the total order position
+    // while there is no key sharing the prefix as `target`, but it
+    // still follows the contract.
+    uint32_t right_index = block_ids[right];
+    assert(right_index + 1 <= num_restarts_);
+    if (right_index + 1 < num_restarts_) {
+      if (CompareBlockKey(right_index + 1, target) >= 0) {
+        *index = right_index + 1;
+        return true;
+      } else {
+        // We have to set the flag here because we are not positioning
+        // the iterator to the total order position.
+        *prefix_may_exist = false;
+      }
+    }
+
     // Mark iterator invalid
     current_ = restarts_;
     return false;
   }
 }
 
-bool IndexBlockIter::PrefixSeek(const Slice& target, uint32_t* index) {
+bool IndexBlockIter::PrefixSeek(const Slice& target, uint32_t* index,
+                                bool* prefix_may_exist) {
+  assert(index);
+  assert(prefix_may_exist);
   assert(prefix_index_);
+  *prefix_may_exist = true;
   Slice seek_key = target;
   if (!key_includes_seq_) {
     seek_key = ExtractUserKey(target);
@@ -768,9 +815,12 @@ bool IndexBlockIter::PrefixSeek(const Slice& target, uint32_t* index) {
 
   if (num_blocks == 0) {
     current_ = restarts_;
+    *prefix_may_exist = false;
     return false;
   } else {
-    return BinaryBlockIndexSeek(seek_key, block_ids, 0, num_blocks - 1, index);
+    assert(block_ids);
+    return BinaryBlockIndexSeek(seek_key, block_ids, 0, num_blocks - 1, index,
+                                prefix_may_exist);
   }
 }
 

table/block_based/block.h

@@ -601,9 +601,16 @@ class IndexBlockIter final : public BlockIter<IndexValue> {
 
   std::unique_ptr<GlobalSeqnoState> global_seqno_state_;
 
-  bool PrefixSeek(const Slice& target, uint32_t* index);
+  // Set *prefix_may_exist to false if no key possibly share the same prefix
+  // as `target`. If not set, the result position should be the same as total
+  // order Seek.
+  bool PrefixSeek(const Slice& target, uint32_t* index, bool* prefix_may_exist);
+  // Set *prefix_may_exist to false if no key can possibly share the same
+  // prefix as `target`. If not set, the result position should be the same
+  // as total order seek.
   bool BinaryBlockIndexSeek(const Slice& target, uint32_t* block_ids,
-                            uint32_t left, uint32_t right, uint32_t* index);
+                            uint32_t left, uint32_t right, uint32_t* index,
+                            bool* prefix_may_exist);
   inline int CompareBlockKey(uint32_t block_index, const Slice& target);
 
   inline int Compare(const Slice& a, const Slice& b) const {

table/block_based/block_based_table_reader.cc

@@ -2908,12 +2908,22 @@ void BlockBasedTableIterator<TBlockIter, TValue>::SeekForPrev(
   index_iter_->Seek(target);
 
   if (!index_iter_->Valid()) {
-    if (!index_iter_->status().ok()) {
+    auto seek_status = index_iter_->status();
+    // Check for IO error
+    if (!seek_status.IsNotFound() && !seek_status.ok()) {
       ResetDataIter();
       return;
     }
 
-    index_iter_->SeekToLast();
+    // With prefix index, Seek() returns NotFound if the prefix doesn't exist
+    if (seek_status.IsNotFound()) {
+      // Any key less than the target is fine for prefix seek
+      ResetDataIter();
+      return;
+    } else {
+      index_iter_->SeekToLast();
+    }
+    // Check for IO error
     if (!index_iter_->Valid()) {
       ResetDataIter();
       return;
@@ -3465,7 +3475,7 @@ Status BlockBasedTable::Get(const ReadOptions& read_options, const Slice& key,
       PERF_COUNTER_BY_LEVEL_ADD(bloom_filter_full_true_positive, 1,
                                 rep_->level);
     }
-    if (s.ok()) {
+    if (s.ok() && !iiter->status().IsNotFound()) {
       s = iiter->status();
     }
   }

table/block_based/block_based_table_reader.h

@@ -688,7 +688,8 @@ class BlockBasedTableIterator : public InternalIteratorBase<TValue> {
     return block_iter_.value();
   }
   Status status() const override {
-    if (!index_iter_->status().ok()) {
+    // Prefix index set status to NotFound when the prefix does not exist
+    if (!index_iter_->status().ok() && !index_iter_->status().IsNotFound()) {
       return index_iter_->status();
     } else if (block_iter_points_to_real_block_) {
       return block_iter_.status();