Fix Get() return status when block cache is disabled (#8485)

Summary:
This PR is for https://github.com/facebook/rocksdb/issues/8453

We need to update `s = biter.status();`  when `biter.status().IsIncomplete()` is true. By doing this, can fix the problem in issue.
Besides, we still need to update `db_statistics`  in `get_context.ReportCounters()` before return back.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/8485

Reviewed By: jay-zhuang

Differential Revision: D29604835

Pulled By: ajkr

fbshipit-source-id: c7f2f1cd058223ce1b507ec05d57cf264b9c9710
This commit is contained in:
hongrubb 2021-07-13 18:12:03 -07:00 committed by Facebook GitHub Bot
parent 7b9ecd4067
commit 870033291a
4 changed files with 8 additions and 2 deletions

View File

@ -8,6 +8,8 @@
* `GetLiveFilesMetaData()` now populates the `temperature`, `oldest_ancester_time`, and `file_creation_time` fields of its `LiveFileMetaData` results when the information is available. Previously these fields always contained zero indicating unknown. * `GetLiveFilesMetaData()` now populates the `temperature`, `oldest_ancester_time`, and `file_creation_time` fields of its `LiveFileMetaData` results when the information is available. Previously these fields always contained zero indicating unknown.
* Fix mismatches of OnCompaction{Begin,Completed} in case of DisableManualCompaction(). * Fix mismatches of OnCompaction{Begin,Completed} in case of DisableManualCompaction().
* Fix continuous logging of an existing background error on every user write * Fix continuous logging of an existing background error on every user write
* Fix a bug that `Get()` return Status::OK() and an empty value for non-existent key when `read_options.read_tier = kBlockCacheTier`.
* Fix a bug that stat in `get_context` didn't accumulate to statistics when query is failed.
### New Features ### New Features
* ldb has a new feature, `list_live_files_metadata`, that shows the live SST files, as well as their LSM storage level and the column family they belong to. * ldb has a new feature, `list_live_files_metadata`, that shows the live SST files, as well as their LSM storage level and the column family they belong to.

View File

@ -299,8 +299,8 @@ TEST_F(DBBlockCacheTest, TestWithCompressedBlockCache) {
// Load last key block. // Load last key block.
ASSERT_EQ("Result incomplete: Insert failed due to LRU cache being full.", ASSERT_EQ("Result incomplete: Insert failed due to LRU cache being full.",
Get(ToString(kNumBlocks - 1))); Get(ToString(kNumBlocks - 1)));
// Failure won't record the miss counter. // Failure will also record the miss counter.
CheckCacheCounters(options, 0, 0, 0, 1); CheckCacheCounters(options, 1, 0, 0, 1);
CheckCompressedCacheCounters(options, 1, 0, 1, 0); CheckCompressedCacheCounters(options, 1, 0, 1, 0);
// Clear strict capacity limit flag. This time we shall hit compressed block // Clear strict capacity limit flag. This time we shall hit compressed block

View File

@ -1939,6 +1939,9 @@ void Version::Get(const ReadOptions& read_options, const LookupKey& k,
fp.GetHitFileLevel()); fp.GetHitFileLevel());
} }
if (!status->ok()) { if (!status->ok()) {
if (db_statistics_ != nullptr) {
get_context.ReportCounters();
}
return; return;
} }

View File

@ -2341,6 +2341,7 @@ Status BlockBasedTable::Get(const ReadOptions& read_options, const Slice& key,
// Update Saver.state to Found because we are only looking for // Update Saver.state to Found because we are only looking for
// whether we can guarantee the key is not there when "no_io" is set // whether we can guarantee the key is not there when "no_io" is set
get_context->MarkKeyMayExist(); get_context->MarkKeyMayExist();
s = biter.status();
break; break;
} }
if (!biter.status().ok()) { if (!biter.status().ok()) {