Fix kBlockCacheTier read with table cache miss (#12443)

Summary:
Thanks ltamasi for pointing out this bug.

We were incorrectly overwriting `Status::Incomplete` with `Status::OK` after a table cache miss failed to open the file due to the read being memory-only (`kBlockCacheTier`). The fix is to simply stop overwriting the status.

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

Reviewed By: cbi42

Differential Revision: D54930128

Pulled By: ajkr

fbshipit-source-id: 52f912a2e93b46e71d79fc5968f8ca35b299213d
This commit is contained in:
Andrew Kryczka 2024-03-15 14:41:58 -07:00 committed by Facebook GitHub Bot
parent 3f5bd46a07
commit 4d5ebad971
3 changed files with 24 additions and 3 deletions

View File

@ -7808,6 +7808,27 @@ TEST_F(DBTest2, ZSTDChecksum) {
}
#endif
TEST_F(DBTest2, TableCacheMissDuringReadFromBlockCacheTier) {
Options options = CurrentOptions();
options.statistics = ROCKSDB_NAMESPACE::CreateDBStatistics();
Reopen(options);
// Give table cache zero capacity to prevent preloading tables. That way,
// `kBlockCacheTier` reads will fail due to table cache misses.
dbfull()->TEST_table_cache()->SetCapacity(0);
ASSERT_OK(Put("foo", "bar"));
ASSERT_OK(Flush());
uint64_t orig_num_file_opens = TestGetTickerCount(options, NO_FILE_OPENS);
ReadOptions non_blocking_opts;
non_blocking_opts.read_tier = kBlockCacheTier;
std::string value;
ASSERT_TRUE(db_->Get(non_blocking_opts, "foo", &value).IsIncomplete());
ASSERT_EQ(orig_num_file_opens, TestGetTickerCount(options, NO_FILE_OPENS));
}
} // namespace ROCKSDB_NAMESPACE
int main(int argc, char** argv) {

View File

@ -489,9 +489,8 @@ Status TableCache::Get(
s = t->Get(options, k, get_context, prefix_extractor.get(), skip_filters);
get_context->SetReplayLog(nullptr);
} else if (options.read_tier == kBlockCacheTier && s.IsIncomplete()) {
// Couldn't find Table in cache but treat as kFound if no_io set
// Couldn't find table in cache and couldn't open it because of no_io.
get_context->MarkKeyMayExist();
s = Status::OK();
done = true;
}
}
@ -729,4 +728,4 @@ uint64_t TableCache::ApproximateSize(
return result;
}
} // namespace ROCKSDB_NAMESPACE
} // namespace ROCKSDB_NAMESPACE

View File

@ -0,0 +1 @@
* Fixed `kBlockCacheTier` reads to return `Status::Incomplete` on table cache miss rather than incorrectly returning an empty value.