Fix point lookup on range tombstone sentinel endpoint

facebook · Jan 3, 2019 · 4b031b7 · 4b031b7
1 parent ace543a
commit 4b031b7
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 0 deletions.
diff --git a/HISTORY.md b/HISTORY.md
@@ -14,6 +14,7 @@
 * Fix a memory leak when files with range tombstones are read in mmap mode and block cache is enabled
 * Fix handling of corrupt range tombstone blocks such that corruptions cannot cause deleted keys to reappear
 * Lock free MultiGet
+* Fix incorrect `NotFound` point lookup result when querying the endpoint of a file that has been extended by a range tombstone.
 
 ## 5.18.0 (11/30/2018)
 ### New Features

diff --git a/db/db_range_del_test.cc b/db/db_range_del_test.cc
@@ -1041,11 +1041,16 @@ TEST_F(DBRangeDelTest, RangeTombstoneEndKeyAsSstableUpperBound) {
     //   L2:
     //     [key000000#1,1, key000000#1,1]
     //     [key000002#6,1, key000004#72057594037927935,15]
+    //
+    // At the same time, verify the compaction does not cause the key at the
+    // endpoint (key000002#6,1) to disappear.
+    ASSERT_EQ(value, Get(Key(2)));
     auto begin_str = Key(3);
     const rocksdb::Slice begin = begin_str;
     dbfull()->TEST_CompactRange(1, &begin, nullptr);
     ASSERT_EQ(1, NumTableFilesAtLevel(1));
     ASSERT_EQ(2, NumTableFilesAtLevel(2));
+    ASSERT_EQ(value, Get(Key(2)));
   }
 
   {

diff --git a/db/version_set.cc b/db/version_set.cc
@@ -200,6 +200,13 @@ class FilePicker {
               break;
             }
           }
+          if (cmp_largest == 0 &&
+              GetInternalKeySeqno(f->largest_key) == kMaxSequenceNumber) {
+            assert(ExtractValueType(f->largest_key) == kTypeRangeDeletion);
+            // Key falls at the range tombstone sentinel endpoint. Proceed to
+            // next level.
+            break;
+          }
         }
 #ifndef NDEBUG
         // Sanity check to make sure that the files are correctly sorted