Fix misbehaviour of value_found in DBImpl::KeyMayExist.

db/db_impl/db_impl.cc

@@ -77,6 +77,7 @@
 #ifdef ROCKSDB_JEMALLOC
 #include "port/jemalloc_helper.h"
 #endif
+
 #include "port/port.h"
 #include "rocksdb/cache.h"
 #include "rocksdb/compaction_filter.h"
@@ -3794,6 +3795,9 @@ bool DBImpl::KeyMayExist(const ReadOptions& read_options,
   // If block_cache is enabled and the index block of the table didn't
   // not present in block_cache, the return value will be Status::Incomplete.
   // In this case, key may still exist in the table.
+  if (value_found != nullptr && s.IsIncomplete()) {
+    *value_found = false;
+  }
   return s.ok() || s.IsIncomplete();
 }
 

java/rocksjni/rocksjni.cc

@@ -1925,7 +1925,6 @@ jboolean key_exists_helper(JNIEnv* env, jlong jdb_handle, jlong jcf_handle,
 
   const bool may_exist =
       db->KeyMayExist(read_opts, cf_handle, key_slice, &value, &value_found);
-
   if (may_exist) {
     ROCKSDB_NAMESPACE::Status s;
     {

java/src/test/java/org/rocksdb/KeyExistsTest.java

@@ -7,10 +7,13 @@
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.assertj.core.api.Assertions.assertThat;
 
+import java.io.IOException;
 import java.nio.ByteBuffer;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.List;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.*;
 import org.junit.*;
 import org.junit.rules.ExpectedException;
 import org.junit.rules.TemporaryFolder;
@@ -21,6 +24,7 @@ public class KeyExistsTest {
       new RocksNativeLibraryResource();
 
   @Rule public TemporaryFolder dbFolder = new TemporaryFolder();
+  @Rule public TemporaryFolder dbFolder2 = new TemporaryFolder();
 
   @Rule public ExpectedException exceptionRule = ExpectedException.none();
 
@@ -46,6 +50,64 @@ public void after() {
     db.close();
   }
 
+  @Test
+  public void keyExistAfterDbOpen() throws RocksDBException {
+    try (RocksDB db2 = RocksDB.open(dbFolder2.getRoot().getAbsolutePath())) {
+      db2.put("key".getBytes(UTF_8), "value".getBytes(UTF_8));
+      assertThat(db2.keyExists("key".getBytes(UTF_8))).isTrue();
+      assertThat(db2.keyExists("key2".getBytes(UTF_8))).isFalse();
+      assertThat(db2.keyMayExist("key".getBytes(UTF_8), null)).isTrue();
+    }
+    try (RocksDB db2 = RocksDB.open(dbFolder2.getRoot().getAbsolutePath())) {
+      assertThat(db2.keyMayExist("key".getBytes(UTF_8), null)).isTrue();
+      assertThat(db2.keyExists("key".getBytes(UTF_8))).isTrue();
+      assertThat(db2.keyExists("key2".getBytes(UTF_8))).isFalse();
+    }
+    try (RocksDB db2 = RocksDB.open(dbFolder2.getRoot().getAbsolutePath())) {
+      assertThat(db2.keyMayExist("key".getBytes(UTF_8), null)).isTrue();
+    }
+    try (RocksDB db2 = RocksDB.open(dbFolder2.getRoot().getAbsolutePath())) {
+      assertThat(db2.keyExists("key".getBytes(UTF_8))).isTrue();
+    }
+    try (RocksDB db2 = RocksDB.open(dbFolder2.getRoot().getAbsolutePath())) {
+      assertThat(db2.keyExists("key2".getBytes(UTF_8))).isFalse();
+    }
+  }
+
+  @Test
+  public void keyExistInTtlDb() throws RocksDBException, IOException {
+    final byte[] KNOWN_KEY = "random_key".getBytes(UTF_8);
+    final long PSEUDO_RANDOM_SEED = 15875551233124l;
+
+    try (Options options = new Options().setCreateIfMissing(true);
+         TtlDB db2 = TtlDB.open(options, dbFolder2.getRoot().getAbsolutePath(), 86400, false);
+         WriteOptions writeOptions = new WriteOptions()) {
+      writeOptions.setDisableWAL(true);
+      db2.put(KNOWN_KEY, "value".getBytes(UTF_8));
+
+      ByteBuffer key = ByteBuffer.allocateDirect(16);
+      ByteBuffer value = ByteBuffer.allocateDirect(16);
+      Random r = new Random(PSEUDO_RANDOM_SEED);
+      for (int i = 0; i < 1000; i++) {
+        key.clear();
+        key.putLong(r.nextLong());
+        key.putLong(r.nextLong());
+        key.flip();
+
+        value.clear();
+        value.putLong(r.nextLong());
+        value.putLong(r.nextLong());
+        value.flip();
+        db2.put(writeOptions, key, value);
+      }
+      db2.compactRange();
+    }
+    try (Options options = new Options();
+         TtlDB db2 = TtlDB.open(options, dbFolder2.getRoot().getAbsolutePath(), 86400, true);) {
+      assertThat(db2.keyExists(KNOWN_KEY)).isTrue();
+    }
+  }
+
   @Test
   public void keyExists() throws RocksDBException {
     db.put("key".getBytes(UTF_8), "value".getBytes(UTF_8));

java/src/test/java/org/rocksdb/KeyMayExistTest.java

@@ -24,6 +24,8 @@ public class KeyMayExistTest {
   @Rule
   public TemporaryFolder dbFolder = new TemporaryFolder();
 
+  @Rule public TemporaryFolder dbFolder2 = new TemporaryFolder();
+
   @Rule public ExpectedException exceptionRule = ExpectedException.none();
 
   List<ColumnFamilyDescriptor> cfDescriptors;
@@ -541,4 +543,40 @@ public void keyMayExistNonUnicodeString() throws RocksDBException {
     exists = db.keyMayExist("key".getBytes(UTF_8), null);
     assertThat(exists).isTrue();
   }
+
+  /*
+    This test was created to verify bug with keyMaye exist and value_found.
+    It's probabilistic and expect that SST files are not loaded immediately to memory
+    after database opening.
+   */
+  @Test
+  public void keyMayExistNoCache() throws RocksDBException {
+    byte[] key = "key".getBytes(UTF_8);
+    byte[] value = "value".getBytes(UTF_8);
+
+    try (RocksDB db2 = RocksDB.open(dbFolder2.getRoot().getAbsolutePath())) {
+      db2.put(key, value);
+      db2.compactRange();
+    }
+
+    try (RocksDB db2 = RocksDB.open(dbFolder2.getRoot().getAbsolutePath())) {
+      Holder<byte[]> holder = new Holder<>();
+
+      boolean exists = db2.keyMayExist("key".getBytes(UTF_8), holder);
+
+      assertThat(exists).isTrue();
+
+      // keys is not in cache, so returned value must be null;
+      assertThat(holder.getValue()).isNull();
+
+      byte[] returned_value = db2.get(key);
+
+      assertThat(returned_value).isEqualTo(value);
+
+      exists = db2.keyMayExist("key".getBytes(UTF_8), holder);
+      assertThat(exists).isTrue();
+      // key is in cache, it should return value
+      assertThat(holder.getValue()).isEqualTo(value);
+    }
+  }
 }