Skip to content

Commit 76a88c7

Browse files
rhubnerrhubner
committed
Fix misbehaviour of value_found in DBImpl::KeyMayExist.
Remove debugging code.
1 parent 877bd62 commit 76a88c7

4 files changed

Lines changed: 18 additions & 102 deletions

File tree

‎db/db_impl/db_impl.cc‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,6 @@
7878
#ifdef ROCKSDB_JEMALLOC
7979
#include "port/jemalloc_helper.h"
8080
#endif
81-
#include <iostream>
8281

8382
#include "port/port.h"
8483
#include "rocksdb/cache.h"
@@ -3827,11 +3826,11 @@ bool DBImpl::KeyMayExist(const ReadOptions& read_options,
38273826
value->assign(pinnable_val.data(), pinnable_val.size());
38283827
}
38293828

3830-
std::cout << "DBImpl::KeyMayExist s : " << s.ToString() << std::endl <<std::flush ;
38313829
// If block_cache is enabled and the index block of the table didn't
38323830
// not present in block_cache, the return value will be Status::Incomplete.
38333831
// In this case, key may still exist in the table.
3834-
if(value_found != nullptr && s.IsIncomplete()) {
3832+
if (value_found != nullptr && s.IsIncomplete()) {
3833+
// falsify later if key-may-exist but can't fetch value
38353834
*value_found = false;
38363835
}
38373836
return s.ok() || s.IsIncomplete();

‎java/rocksjni/rocksjni.cc‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1925,11 +1925,6 @@ jboolean key_exists_helper(JNIEnv* env, jlong jdb_handle, jlong jcf_handle,
19251925

19261926
const bool may_exist =
19271927
db->KeyMayExist(read_opts, cf_handle, key_slice, &value, &value_found);
1928-
1929-
std::cout << "Key may exist : " << may_exist << std::endl;
1930-
std::cout << "Value found : " << value_found << std::endl << std::flush;
1931-
1932-
19331928
if (may_exist) {
19341929
ROCKSDB_NAMESPACE::Status s;
19351930
{

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

Lines changed: 16 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
import java.nio.file.Path;
1515
import java.nio.file.Paths;
1616
import java.util.*;
17-
1817
import org.junit.*;
1918
import org.junit.rules.ExpectedException;
2019
import org.junit.rules.TemporaryFolder;
@@ -52,54 +51,44 @@ public void after() {
5251
}
5352

5453
@Test
55-
public void keyExistBug() throws RocksDBException{
56-
try(RocksDB db2 = RocksDB.open(dbFolder2.getRoot().getAbsolutePath())) {
54+
public void keyExistAfterDbOpen() throws RocksDBException {
55+
try (RocksDB db2 = RocksDB.open(dbFolder2.getRoot().getAbsolutePath())) {
5756
db2.put("key".getBytes(UTF_8), "value".getBytes(UTF_8));
5857
assertThat(db2.keyExists("key".getBytes(UTF_8))).isTrue();
5958
assertThat(db2.keyExists("key2".getBytes(UTF_8))).isFalse();
6059
assertThat(db2.keyMayExist("key".getBytes(UTF_8), null)).isTrue();
6160
}
62-
try(RocksDB db2 = RocksDB.open(dbFolder2.getRoot().getAbsolutePath())) {
61+
try (RocksDB db2 = RocksDB.open(dbFolder2.getRoot().getAbsolutePath())) {
6362
assertThat(db2.keyMayExist("key".getBytes(UTF_8), null)).isTrue();
6463
assertThat(db2.keyExists("key".getBytes(UTF_8))).isTrue();
6564
assertThat(db2.keyExists("key2".getBytes(UTF_8))).isFalse();
6665
}
67-
try(RocksDB db2 = RocksDB.open(dbFolder2.getRoot().getAbsolutePath())) {
66+
try (RocksDB db2 = RocksDB.open(dbFolder2.getRoot().getAbsolutePath())) {
6867
assertThat(db2.keyMayExist("key".getBytes(UTF_8), null)).isTrue();
6968
}
70-
try(RocksDB db2 = RocksDB.open(dbFolder2.getRoot().getAbsolutePath())) {
69+
try (RocksDB db2 = RocksDB.open(dbFolder2.getRoot().getAbsolutePath())) {
7170
assertThat(db2.keyExists("key".getBytes(UTF_8))).isTrue();
7271
}
73-
try(RocksDB db2 = RocksDB.open(dbFolder2.getRoot().getAbsolutePath())) {
72+
try (RocksDB db2 = RocksDB.open(dbFolder2.getRoot().getAbsolutePath())) {
7473
assertThat(db2.keyExists("key2".getBytes(UTF_8))).isFalse();
7574
}
7675
}
7776

7877
@Test
79-
public void keyExistGenerator() throws RocksDBException, IOException {
80-
81-
final byte[] KNOWN_KEY = "random_key_value".getBytes(UTF_8);
78+
public void keyExistInTtlDb() throws RocksDBException, IOException {
79+
final byte[] KNOWN_KEY = "random_key".getBytes(UTF_8);
8280
final long PSEUDO_RANDOM_SEED = 15875551233124l;
8381

84-
Path rocksDbPath = Paths.get("/tmp/rocks-test");
85-
if (Files.exists(rocksDbPath)) {
86-
Files.walk(rocksDbPath)
87-
.sorted(Comparator.reverseOrder())
88-
.map(Path::toFile)
89-
.forEach(x -> x.delete());
90-
}
91-
92-
try(Options options = new Options().setCreateIfMissing(true);
93-
TtlDB db2 = TtlDB.open(options, "/tmp/rocks-test", 86400, false);
94-
WriteOptions writeOptions = new WriteOptions()
95-
) {
82+
try (Options options = new Options().setCreateIfMissing(true);
83+
TtlDB db2 = TtlDB.open(options, dbFolder2.getRoot().getAbsolutePath(), 86400, false);
84+
WriteOptions writeOptions = new WriteOptions()) {
9685
writeOptions.setDisableWAL(true);
9786
db2.put(KNOWN_KEY, "value".getBytes(UTF_8));
9887

9988
ByteBuffer key = ByteBuffer.allocateDirect(16);
10089
ByteBuffer value = ByteBuffer.allocateDirect(16);
10190
Random r = new Random(PSEUDO_RANDOM_SEED);
102-
for(int i = 0 ; i < 1_000_000 ; i++){
91+
for (int i = 0; i < 1000; i++) {
10392
key.clear();
10493
key.putLong(r.nextLong());
10594
key.putLong(r.nextLong());
@@ -109,68 +98,16 @@ public void keyExistGenerator() throws RocksDBException, IOException {
10998
value.putLong(r.nextLong());
11099
value.putLong(r.nextLong());
111100
value.flip();
112-
113101
db2.put(writeOptions, key, value);
114-
if (i % 50000 == 0) {
115-
System.out.println(i);
116-
}
117102
}
118-
// System.out.print("Compacting ...");
119-
// db.compactRange();
120-
// System.out.println(" [OK]");
121-
}
122-
try(Options options = new Options().setCreateIfMissing(true);
123-
124-
//RocksDB db2 = RocksDB.open(options, "/tmp/rocks-test")
125-
//RocksDB db2 = RocksDB.openReadOnly(options, "/tmp/rocks-test")
126-
TtlDB db2 = TtlDB.open(options, "/tmp/rocks-test", 86400, true);
127-
128-
) {
129-
130-
// assertThat(db2.get("key".getBytes())).isEqualTo("value".getBytes(UTF_8));
131-
Random r = new Random(PSEUDO_RANDOM_SEED);
132-
ByteBuffer key = ByteBuffer.allocateDirect(16);
133-
// for (int i = 0; i < 100; i++) {
134-
// key.clear();
135-
// key.putLong(r.nextLong());
136-
// key.putLong(r.nextLong());
137-
// key.flip();
138-
//
139-
// r.nextLong();
140-
// r.nextLong();
141-
//
142-
// System.out.println(" key : " + i + " exist : " + db2.keyExists(key));
143-
//
144-
// }
145-
System.out.println("Key exist 1 : ");
146-
System.out.flush();
147-
db2.keyExists(KNOWN_KEY);
148-
db2.get(KNOWN_KEY);
149-
System.out.println("Key exists 2 : ");
150-
System.out.flush();
151-
db2.keyExists(KNOWN_KEY);
103+
db2.compactRange();
152104
}
153-
154-
}
155-
156-
@Test
157-
public void verify() throws RocksDBException {
158-
try(Options options = new Options().setCreateIfMissing(true);
159-
160-
//RocksDB db2 = RocksDB.open(options, "/tmp/rocks-test")
161-
//RocksDB db2 = RocksDB.openReadOnly(options, "/tmp/rocks-test")
162-
TtlDB db2 = TtlDB.open(options, "/tmp/rocks-test", 86400, true);
163-
164-
) {
165-
166-
// assertThat(db2.get("key".getBytes())).isEqualTo("value".getBytes(UTF_8));
167-
168-
assertThat(db2.keyExists("key".getBytes(UTF_8))).isTrue();
105+
try (Options options = new Options();
106+
TtlDB db2 = TtlDB.open(options, dbFolder2.getRoot().getAbsolutePath(), 86400, true);) {
107+
assertThat(db2.keyExists(KNOWN_KEY)).isTrue();
169108
}
170109
}
171110

172-
173-
174111
@Test
175112
public void keyExists() throws RocksDBException {
176113
db.put("key".getBytes(UTF_8), "value".getBytes(UTF_8));

‎utilities/ttl/db_ttl_impl.cc‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@
55

66
#include "utilities/ttl/db_ttl_impl.h"
77

8-
#include <iostream>
9-
108
#include "db/write_batch_internal.h"
119
#include "file/filename.h"
1210
#include "logging/logging.h"
@@ -543,22 +541,9 @@ bool DBWithTTLImpl::KeyMayExist(const ReadOptions& options,
543541
ColumnFamilyHandle* column_family,
544542
const Slice& key, std::string* value,
545543
bool* value_found) {
546-
std::cout << "DBWithTTLImpl::KeyMayExist" << std::endl <<std::flush ;
547-
548544
bool ret = db_->KeyMayExist(options, column_family, key, value, value_found);
549-
std::cout << " DBWithTTLImpl::KeyMayExist ret value : " << ret << std::endl <<std::flush ;
550-
if(value_found != nullptr) {
551-
std::cout << " DBWithTTLImpl::KeyMayExist value_found : " << *value_found << std::endl <<std::flush ;
552-
}
553-
if(value_found != nullptr && *value_found && value != nullptr) {
554-
std::cout << " DBWithTTLImpl::KeyMayExist value.size : " << value->size() << std::endl <<std::flush ;
555-
}
556-
557545
if (ret && value != nullptr && value_found != nullptr && *value_found) {
558-
// std::cout << " DBWithTTLImpl::KeyMayExist SanityCheckTimestamp : " << SanityCheckTimestamp(*value).ToString() << std::endl <<std::flush ;
559-
// std::cout << " DBWithTTLImpl::KeyMayExist StripTS(value) : " << StripTS(value).ToString() << std::endl <<std::flush ;
560546
if (!SanityCheckTimestamp(*value).ok() || !StripTS(value).ok()) {
561-
562547
return false;
563548
}
564549
}

0 commit comments

Comments
 (0)