andreacavalli/rocksdb
0
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

BlobDB: Avoid returning garbage value on key not found (#4321)

Browse Source 
Summary:
When reading an expired key using `Get(..., std::string* value)` API, BlobDB first read the index entry and decode expiration from it. In this case, although BlobDB reset the PinnableSlice, the index entry is stored in user provided string `value`. The value will be returned as a garbage value, despite status being NotFound. Fixing it by use a different PinnableSlice to read the index entry.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/4321

Differential Revision: D9519042

Pulled By: yiwu-arbug

fbshipit-source-id: f054c951a1fa98265228be94f931904ed7056677
This commit is contained in:
Yi Wu 2018-08-27 16:23:48 -07:00 committed by Facebook Github Bot
parent 6ed7f146c3
commit 38ad3c9f8a
2 changed files with 16 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
utilities/blob_db/blob_db_impl.cc
View File

@ -1115,9 +1115,10 @@ Status BlobDBImpl::GetImpl(const ReadOptions& read_options,
ReadOptions ro(read_options); ReadOptions ro(read_options);
bool snapshot_created = SetSnapshotIfNeeded(&ro); bool snapshot_created = SetSnapshotIfNeeded(&ro);
PinnableSlice index_entry;
Status s; Status s;
bool is_blob_index = false; bool is_blob_index = false;
s = db_impl_->GetImpl(ro, column_family, key, value, s = db_impl_->GetImpl(ro, column_family, key, &index_entry,
nullptr /*value_found*/, nullptr /*read_callback*/, nullptr /*value_found*/, nullptr /*read_callback*/,
&is_blob_index); &is_blob_index);
TEST_SYNC_POINT("BlobDBImpl::Get:AfterIndexEntryGet:1"); TEST_SYNC_POINT("BlobDBImpl::Get:AfterIndexEntryGet:1");
@ -1125,16 +1126,19 @@ Status BlobDBImpl::GetImpl(const ReadOptions& read_options,
if (expiration != nullptr) { if (expiration != nullptr) {
*expiration = kNoExpiration; *expiration = kNoExpiration;
} }
if (s.ok() && is_blob_index) { RecordTick(statistics_, BLOB_DB_NUM_KEYS_READ);
std::string index_entry = value->ToString(); if (s.ok()) {
value->Reset(); if (is_blob_index) {
s = GetBlobValue(key, index_entry, value, expiration); s = GetBlobValue(key, index_entry, value, expiration);
} else {
// The index entry is the value itself in this case.
value->PinSelf(index_entry);
}
RecordTick(statistics_, BLOB_DB_BYTES_READ, value->size());
} }
if (snapshot_created) { if (snapshot_created) {
db_->ReleaseSnapshot(ro.snapshot); db_->ReleaseSnapshot(ro.snapshot);
} }
RecordTick(statistics_, BLOB_DB_NUM_KEYS_READ);
RecordTick(statistics_, BLOB_DB_BYTES_READ, value->size());
return s; return s;
} }

5
utilities/blob_db/blob_db_test.cc
View File

@ -1413,6 +1413,11 @@ TEST_F(BlobDBTest, EvictExpiredFile) {
blob_db_impl()->TEST_DeleteObsoleteFiles(); blob_db_impl()->TEST_DeleteObsoleteFiles();
ASSERT_EQ(0, blob_db_impl()->TEST_GetBlobFiles().size()); ASSERT_EQ(0, blob_db_impl()->TEST_GetBlobFiles().size());
ASSERT_EQ(0, blob_db_impl()->TEST_GetObsoleteFiles().size()); ASSERT_EQ(0, blob_db_impl()->TEST_GetObsoleteFiles().size());
// Make sure we don't return garbage value after blob file being evicted,
// but the blob index still exists in the LSM tree.
std::string val = "";
ASSERT_TRUE(blob_db_->Get(ReadOptions(), "foo", &val).IsNotFound());
ASSERT_EQ("", val);
} }
TEST_F(BlobDBTest, DisableFileDeletions) { TEST_F(BlobDBTest, DisableFileDeletions) {