Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/main' into pr/12085
Browse files Browse the repository at this point in the history
  • Loading branch information
pdillinger committed Dec 15, 2023
2 parents 6f1e926 + cc069f2 commit 33cb1bb
Show file tree
Hide file tree
Showing 16 changed files with 88 additions and 38 deletions.
4 changes: 3 additions & 1 deletion cache/compressed_secondary_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ CompressedSecondaryCache::~CompressedSecondaryCache() {}
std::unique_ptr<SecondaryCacheResultHandle> CompressedSecondaryCache::Lookup(
const Slice& key, const Cache::CacheItemHelper* helper,
Cache::CreateContext* create_context, bool /*wait*/, bool advise_erase,
bool& kept_in_sec_cache) {
Statistics* stats, bool& kept_in_sec_cache) {
assert(helper);
// This is a minor optimization. Its ok to skip it in TSAN in order to
// avoid a false positive.
Expand All @@ -51,6 +51,7 @@ std::unique_ptr<SecondaryCacheResultHandle> CompressedSecondaryCache::Lookup(
void* handle_value = cache_->Value(lru_handle);
if (handle_value == nullptr) {
cache_->Release(lru_handle, /*erase_if_last_ref=*/false);
RecordTick(stats, COMPRESSED_SECONDARY_CACHE_DUMMY_HITS);
return nullptr;
}

Expand Down Expand Up @@ -137,6 +138,7 @@ std::unique_ptr<SecondaryCacheResultHandle> CompressedSecondaryCache::Lookup(
cache_->Release(lru_handle, /*erase_if_last_ref=*/false);
}
handle.reset(new CompressedSecondaryCacheResultHandle(value, charge));
RecordTick(stats, COMPRESSED_SECONDARY_CACHE_HITS);
return handle;
}

Expand Down
2 changes: 1 addition & 1 deletion cache/compressed_secondary_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ class CompressedSecondaryCache : public SecondaryCache {
std::unique_ptr<SecondaryCacheResultHandle> Lookup(
const Slice& key, const Cache::CacheItemHelper* helper,
Cache::CreateContext* create_context, bool /*wait*/, bool advise_erase,
bool& kept_in_sec_cache) override;
Statistics* stats, bool& kept_in_sec_cache) override;

bool SupportForceErase() const override { return true; }

Expand Down
26 changes: 13 additions & 13 deletions cache/compressed_secondary_cache_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class CompressedSecondaryCacheTestBase : public testing::Test,
// Lookup an non-existent key.
std::unique_ptr<SecondaryCacheResultHandle> handle0 =
sec_cache->Lookup(key0, GetHelper(), this, true, /*advise_erase=*/true,
kept_in_sec_cache);
/*stats=*/nullptr, kept_in_sec_cache);
ASSERT_EQ(handle0, nullptr);

Random rnd(301);
Expand All @@ -59,7 +59,7 @@ class CompressedSecondaryCacheTestBase : public testing::Test,

std::unique_ptr<SecondaryCacheResultHandle> handle1_1 =
sec_cache->Lookup(key1, GetHelper(), this, true, /*advise_erase=*/false,
kept_in_sec_cache);
/*stats=*/nullptr, kept_in_sec_cache);
ASSERT_EQ(handle1_1, nullptr);

// Insert and Lookup the item k1 for the second time and advise erasing it.
Expand All @@ -68,7 +68,7 @@ class CompressedSecondaryCacheTestBase : public testing::Test,

std::unique_ptr<SecondaryCacheResultHandle> handle1_2 =
sec_cache->Lookup(key1, GetHelper(), this, true, /*advise_erase=*/true,
kept_in_sec_cache);
/*stats=*/nullptr, kept_in_sec_cache);
ASSERT_NE(handle1_2, nullptr);
ASSERT_FALSE(kept_in_sec_cache);
if (sec_cache_is_compressed) {
Expand All @@ -89,7 +89,7 @@ class CompressedSecondaryCacheTestBase : public testing::Test,
// Lookup the item k1 again.
std::unique_ptr<SecondaryCacheResultHandle> handle1_3 =
sec_cache->Lookup(key1, GetHelper(), this, true, /*advise_erase=*/true,
kept_in_sec_cache);
/*stats=*/nullptr, kept_in_sec_cache);
ASSERT_EQ(handle1_3, nullptr);

// Insert and Lookup the item k2.
Expand All @@ -99,7 +99,7 @@ class CompressedSecondaryCacheTestBase : public testing::Test,
ASSERT_EQ(get_perf_context()->compressed_sec_cache_insert_dummy_count, 2);
std::unique_ptr<SecondaryCacheResultHandle> handle2_1 =
sec_cache->Lookup(key2, GetHelper(), this, true, /*advise_erase=*/false,
kept_in_sec_cache);
/*stats=*/nullptr, kept_in_sec_cache);
ASSERT_EQ(handle2_1, nullptr);

ASSERT_OK(sec_cache->Insert(key2, &item2, GetHelper(), false));
Expand All @@ -115,7 +115,7 @@ class CompressedSecondaryCacheTestBase : public testing::Test,
}
std::unique_ptr<SecondaryCacheResultHandle> handle2_2 =
sec_cache->Lookup(key2, GetHelper(), this, true, /*advise_erase=*/false,
kept_in_sec_cache);
/*stats=*/nullptr, kept_in_sec_cache);
ASSERT_NE(handle2_2, nullptr);
std::unique_ptr<TestItem> val2 =
std::unique_ptr<TestItem>(static_cast<TestItem*>(handle2_2->Value()));
Expand Down Expand Up @@ -196,14 +196,14 @@ class CompressedSecondaryCacheTestBase : public testing::Test,
bool kept_in_sec_cache{false};
std::unique_ptr<SecondaryCacheResultHandle> handle1 =
sec_cache->Lookup(key1, GetHelper(), this, true, /*advise_erase=*/false,
kept_in_sec_cache);
/*stats=*/nullptr, kept_in_sec_cache);
ASSERT_EQ(handle1, nullptr);

// Insert k2 and k1 is evicted.
ASSERT_OK(sec_cache->Insert(key2, &item2, GetHelper(), false));
std::unique_ptr<SecondaryCacheResultHandle> handle2 =
sec_cache->Lookup(key2, GetHelper(), this, true, /*advise_erase=*/false,
kept_in_sec_cache);
/*stats=*/nullptr, kept_in_sec_cache);
ASSERT_NE(handle2, nullptr);
std::unique_ptr<TestItem> val2 =
std::unique_ptr<TestItem>(static_cast<TestItem*>(handle2->Value()));
Expand All @@ -215,14 +215,14 @@ class CompressedSecondaryCacheTestBase : public testing::Test,

std::unique_ptr<SecondaryCacheResultHandle> handle1_1 =
sec_cache->Lookup(key1, GetHelper(), this, true, /*advise_erase=*/false,
kept_in_sec_cache);
/*stats=*/nullptr, kept_in_sec_cache);
ASSERT_EQ(handle1_1, nullptr);

// Create Fails.
SetFailCreate(true);
std::unique_ptr<SecondaryCacheResultHandle> handle2_1 =
sec_cache->Lookup(key2, GetHelper(), this, true, /*advise_erase=*/true,
kept_in_sec_cache);
/*stats=*/nullptr, kept_in_sec_cache);
ASSERT_EQ(handle2_1, nullptr);

// Save Fails.
Expand Down Expand Up @@ -912,9 +912,9 @@ TEST_P(CompressedSecondaryCacheTestWithCompressionParam, EntryRoles) {
ASSERT_EQ(get_perf_context()->compressed_sec_cache_insert_real_count, 1U);

bool kept_in_sec_cache{true};
std::unique_ptr<SecondaryCacheResultHandle> handle =
sec_cache->Lookup(ith_key, GetHelper(role), this, true,
/*advise_erase=*/true, kept_in_sec_cache);
std::unique_ptr<SecondaryCacheResultHandle> handle = sec_cache->Lookup(
ith_key, GetHelper(role), this, true,
/*advise_erase=*/true, /*stats=*/nullptr, kept_in_sec_cache);
ASSERT_NE(handle, nullptr);

// Lookup returns the right data
Expand Down
3 changes: 2 additions & 1 deletion cache/lru_cache_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1091,7 +1091,8 @@ class TestSecondaryCache : public SecondaryCache {
std::unique_ptr<SecondaryCacheResultHandle> Lookup(
const Slice& key, const Cache::CacheItemHelper* helper,
Cache::CreateContext* create_context, bool /*wait*/,
bool /*advise_erase*/, bool& kept_in_sec_cache) override {
bool /*advise_erase*/, Statistics* /*stats*/,
bool& kept_in_sec_cache) override {
std::string key_str = key.ToString();
TEST_SYNC_POINT_CALLBACK("TestSecondaryCache::Lookup", &key_str);

Expand Down
11 changes: 6 additions & 5 deletions cache/secondary_cache_adapter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,8 @@ Cache::Handle* CacheWithSecondaryAdapter::Lookup(const Slice& key,
bool kept_in_sec_cache = false;
std::unique_ptr<SecondaryCacheResultHandle> secondary_handle =
secondary_cache_->Lookup(key, helper, create_context, /*wait*/ true,
found_dummy_entry, /*out*/ kept_in_sec_cache);
found_dummy_entry, stats,
/*out*/ kept_in_sec_cache);
if (secondary_handle) {
result = Promote(std::move(secondary_handle), key, helper, priority,
stats, found_dummy_entry, kept_in_sec_cache);
Expand Down Expand Up @@ -348,10 +349,10 @@ void CacheWithSecondaryAdapter::StartAsyncLookupOnMySecondary(
assert(async_handle.result_handle == nullptr);

std::unique_ptr<SecondaryCacheResultHandle> secondary_handle =
secondary_cache_->Lookup(async_handle.key, async_handle.helper,
async_handle.create_context, /*wait*/ false,
async_handle.found_dummy_entry,
/*out*/ async_handle.kept_in_sec_cache);
secondary_cache_->Lookup(
async_handle.key, async_handle.helper, async_handle.create_context,
/*wait*/ false, async_handle.found_dummy_entry, async_handle.stats,
/*out*/ async_handle.kept_in_sec_cache);
if (secondary_handle) {
// TODO with stacked secondaries: Check & process if already ready?
async_handle.pending_handle = secondary_handle.release();
Expand Down
18 changes: 13 additions & 5 deletions cache/tiered_secondary_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

#include "cache/tiered_secondary_cache.h"

#include "monitoring/statistics_impl.h"

namespace ROCKSDB_NAMESPACE {

// Creation callback for use in the lookup path. It calls the upper layer
Expand All @@ -29,6 +31,9 @@ Status TieredSecondaryCache::MaybeInsertAndCreate(
// TODO: Don't hardcode the source
context->comp_sec_cache->InsertSaved(*context->key, data, type, source)
.PermitUncheckedError();
RecordTick(context->stats, COMPRESSED_SECONDARY_CACHE_PROMOTIONS);
} else {
RecordTick(context->stats, COMPRESSED_SECONDARY_CACHE_PROMOTION_SKIPS);
}
// Primary cache will accept the object, so call its helper to create
// the object
Expand All @@ -43,10 +48,10 @@ Status TieredSecondaryCache::MaybeInsertAndCreate(
std::unique_ptr<SecondaryCacheResultHandle> TieredSecondaryCache::Lookup(
const Slice& key, const Cache::CacheItemHelper* helper,
Cache::CreateContext* create_context, bool wait, bool advise_erase,
bool& kept_in_sec_cache) {
Statistics* stats, bool& kept_in_sec_cache) {
bool dummy = false;
std::unique_ptr<SecondaryCacheResultHandle> result =
target()->Lookup(key, helper, create_context, wait, advise_erase,
target()->Lookup(key, helper, create_context, wait, advise_erase, stats,
/*kept_in_sec_cache=*/dummy);
// We never want the item to spill back into the secondary cache
kept_in_sec_cache = true;
Expand All @@ -66,9 +71,10 @@ std::unique_ptr<SecondaryCacheResultHandle> TieredSecondaryCache::Lookup(
ctx.helper = helper;
ctx.inner_ctx = create_context;
ctx.comp_sec_cache = target();
ctx.stats = stats;

return nvm_sec_cache_->Lookup(key, outer_helper, &ctx, wait, advise_erase,
kept_in_sec_cache);
stats, kept_in_sec_cache);
}

// If wait is false, i.e its an async lookup, we have to allocate a result
Expand All @@ -80,8 +86,10 @@ std::unique_ptr<SecondaryCacheResultHandle> TieredSecondaryCache::Lookup(
handle->ctx()->helper = helper;
handle->ctx()->inner_ctx = create_context;
handle->ctx()->comp_sec_cache = target();
handle->SetInnerHandle(nvm_sec_cache_->Lookup(
key, outer_helper, handle->ctx(), wait, advise_erase, kept_in_sec_cache));
handle->ctx()->stats = stats;
handle->SetInnerHandle(
nvm_sec_cache_->Lookup(key, outer_helper, handle->ctx(), wait,
advise_erase, stats, kept_in_sec_cache));
if (!handle->inner_handle()) {
handle.reset();
} else {
Expand Down
3 changes: 2 additions & 1 deletion cache/tiered_secondary_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class TieredSecondaryCache : public SecondaryCacheWrapper {
virtual std::unique_ptr<SecondaryCacheResultHandle> Lookup(
const Slice& key, const Cache::CacheItemHelper* helper,
Cache::CreateContext* create_context, bool wait, bool advise_erase,
bool& kept_in_sec_cache) override;
Statistics* stats, bool& kept_in_sec_cache) override;

virtual void WaitAll(
std::vector<SecondaryCacheResultHandle*> handles) override;
Expand All @@ -72,6 +72,7 @@ class TieredSecondaryCache : public SecondaryCacheWrapper {
Cache::CreateContext* inner_ctx;
std::shared_ptr<SecondaryCacheResultHandle> inner_handle;
SecondaryCache* comp_sec_cache;
Statistics* stats;
};

class ResultHandle : public SecondaryCacheResultHandle {
Expand Down
2 changes: 1 addition & 1 deletion cache/tiered_secondary_cache_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ class TestSecondaryCache : public SecondaryCache {
std::unique_ptr<SecondaryCacheResultHandle> Lookup(
const Slice& key, const Cache::CacheItemHelper* helper,
Cache::CreateContext* create_context, bool wait, bool /*advise_erase*/,
bool& kept_in_sec_cache) override {
Statistics* /*stats*/, bool& kept_in_sec_cache) override {
std::string key_str = key.ToString();
TEST_SYNC_POINT_CALLBACK("TestSecondaryCache::Lookup", &key_str);

Expand Down
4 changes: 2 additions & 2 deletions db/blob/blob_source_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1220,7 +1220,7 @@ TEST_F(BlobSecondaryCacheTest, GetBlobsFromSecondaryCache) {
auto sec_handle0 = secondary_cache->Lookup(
key0, BlobSource::SharedCacheInterface::GetFullHelper(),
/*context*/ nullptr, true,
/*advise_erase=*/true, kept_in_sec_cache);
/*advise_erase=*/true, /*stats=*/nullptr, kept_in_sec_cache);
ASSERT_FALSE(kept_in_sec_cache);
ASSERT_NE(sec_handle0, nullptr);
ASSERT_TRUE(sec_handle0->IsReady());
Expand Down Expand Up @@ -1248,7 +1248,7 @@ TEST_F(BlobSecondaryCacheTest, GetBlobsFromSecondaryCache) {
auto sec_handle1 = secondary_cache->Lookup(
key1, BlobSource::SharedCacheInterface::GetFullHelper(),
/*context*/ nullptr, true,
/*advise_erase=*/true, kept_in_sec_cache);
/*advise_erase=*/true, /*stats=*/nullptr, kept_in_sec_cache);
ASSERT_FALSE(kept_in_sec_cache);
ASSERT_EQ(sec_handle1, nullptr);

Expand Down
6 changes: 3 additions & 3 deletions include/rocksdb/secondary_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ class SecondaryCache : public Customizable {
virtual std::unique_ptr<SecondaryCacheResultHandle> Lookup(
const Slice& key, const Cache::CacheItemHelper* helper,
Cache::CreateContext* create_context, bool wait, bool advise_erase,
bool& kept_in_sec_cache) = 0;
Statistics* stats, bool& kept_in_sec_cache) = 0;

// Indicate whether a handle can be erased in this secondary cache.
[[nodiscard]] virtual bool SupportForceErase() const = 0;
Expand Down Expand Up @@ -176,9 +176,9 @@ class SecondaryCacheWrapper : public SecondaryCache {
virtual std::unique_ptr<SecondaryCacheResultHandle> Lookup(
const Slice& key, const Cache::CacheItemHelper* helper,
Cache::CreateContext* create_context, bool wait, bool advise_erase,
bool& kept_in_sec_cache) override {
Statistics* stats, bool& kept_in_sec_cache) override {
return target()->Lookup(key, helper, create_context, wait, advise_erase,
kept_in_sec_cache);
stats, kept_in_sec_cache);
}

virtual bool SupportForceErase() const override {
Expand Down
6 changes: 6 additions & 0 deletions include/rocksdb/statistics.h
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,12 @@ enum Tickers : uint32_t {
// Number of FS reads avoided due to scan prefetching
PREFETCH_HITS,

// Compressed secondary cache related stats
COMPRESSED_SECONDARY_CACHE_DUMMY_HITS,
COMPRESSED_SECONDARY_CACHE_HITS,
COMPRESSED_SECONDARY_CACHE_PROMOTIONS,
COMPRESSED_SECONDARY_CACHE_PROMOTION_SKIPS,

TICKER_ENUM_MAX
};

Expand Down
20 changes: 20 additions & 0 deletions java/rocksjni/portal.h
Original file line number Diff line number Diff line change
Expand Up @@ -5175,6 +5175,15 @@ class TickerTypeJni {
return -0x41;
case ROCKSDB_NAMESPACE::Tickers::PREFETCH_HITS:
return -0x42;
case ROCKSDB_NAMESPACE::Tickers::COMPRESSED_SECONDARY_CACHE_DUMMY_HITS:
return -0x43;
case ROCKSDB_NAMESPACE::Tickers::COMPRESSED_SECONDARY_CACHE_HITS:
return -0x44;
case ROCKSDB_NAMESPACE::Tickers::COMPRESSED_SECONDARY_CACHE_PROMOTIONS:
return -0x45;
case ROCKSDB_NAMESPACE::Tickers::
COMPRESSED_SECONDARY_CACHE_PROMOTION_SKIPS:
return -0x46;
case ROCKSDB_NAMESPACE::Tickers::TICKER_ENUM_MAX:
// 0x5F was the max value in the initial copy of tickers to Java.
// Since these values are exposed directly to Java clients, we keep
Expand Down Expand Up @@ -5550,6 +5559,17 @@ class TickerTypeJni {
return ROCKSDB_NAMESPACE::Tickers::PREFETCH_BYTES_USEFUL;
case -0x42:
return ROCKSDB_NAMESPACE::Tickers::PREFETCH_HITS;
case -0x43:
return ROCKSDB_NAMESPACE::Tickers::
COMPRESSED_SECONDARY_CACHE_DUMMY_HITS;
case -0x44:
return ROCKSDB_NAMESPACE::Tickers::COMPRESSED_SECONDARY_CACHE_HITS;
case -0x45:
return ROCKSDB_NAMESPACE::Tickers::
COMPRESSED_SECONDARY_CACHE_PROMOTIONS;
case -0x46:
return ROCKSDB_NAMESPACE::Tickers::
COMPRESSED_SECONDARY_CACHE_PROMOTION_SKIPS;
case 0x5F:
// 0x5F was the max value in the initial copy of tickers to Java.
// Since these values are exposed directly to Java clients, we keep
Expand Down
8 changes: 8 additions & 0 deletions monitoring/statistics.cc
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,14 @@ const std::vector<std::pair<Tickers, std::string>> TickersNameMap = {
{PREFETCH_BYTES, "rocksdb.prefetch.bytes"},
{PREFETCH_BYTES_USEFUL, "rocksdb.prefetch.bytes.useful"},
{PREFETCH_HITS, "rocksdb.prefetch.hits"},
{COMPRESSED_SECONDARY_CACHE_DUMMY_HITS,
"rocksdb.compressed.secondary.cache.dummy.hits"},
{COMPRESSED_SECONDARY_CACHE_HITS,
"rocksdb.compressed.secondary.cache.hits"},
{COMPRESSED_SECONDARY_CACHE_PROMOTIONS,
"rocksdb.compressed.secondary.cache.promotions"},
{COMPRESSED_SECONDARY_CACHE_PROMOTION_SKIPS,
"rocksdb.compressed.secondary.cache.promotion.skips"},
};

const std::vector<std::pair<Histograms, std::string>> HistogramsNameMap = {
Expand Down
3 changes: 2 additions & 1 deletion options/customizable_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1241,7 +1241,8 @@ class TestSecondaryCache : public SecondaryCache {
std::unique_ptr<SecondaryCacheResultHandle> Lookup(
const Slice& /*key*/, const Cache::CacheItemHelper* /*helper*/,
Cache::CreateContext* /*create_context*/, bool /*wait*/,
bool /*advise_erase*/, bool& kept_in_sec_cache) override {
bool /*advise_erase*/, Statistics* /*stats*/,
bool& kept_in_sec_cache) override {
kept_in_sec_cache = true;
return nullptr;
}
Expand Down
8 changes: 5 additions & 3 deletions utilities/fault_injection_secondary_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -92,18 +92,20 @@ FaultInjectionSecondaryCache::Lookup(const Slice& key,
const Cache::CacheItemHelper* helper,
Cache::CreateContext* create_context,
bool wait, bool advise_erase,
Statistics* stats,
bool& kept_in_sec_cache) {
ErrorContext* ctx = GetErrorContext();
if (base_is_compressed_sec_cache_) {
if (ctx->rand.OneIn(prob_)) {
return nullptr;
} else {
return base_->Lookup(key, helper, create_context, wait, advise_erase,
kept_in_sec_cache);
stats, kept_in_sec_cache);
}
} else {
std::unique_ptr<SecondaryCacheResultHandle> hdl = base_->Lookup(
key, helper, create_context, wait, advise_erase, kept_in_sec_cache);
std::unique_ptr<SecondaryCacheResultHandle> hdl =
base_->Lookup(key, helper, create_context, wait, advise_erase, stats,
kept_in_sec_cache);
if (wait && ctx->rand.OneIn(prob_)) {
hdl.reset();
}
Expand Down
2 changes: 1 addition & 1 deletion utilities/fault_injection_secondary_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class FaultInjectionSecondaryCache : public SecondaryCache {
std::unique_ptr<SecondaryCacheResultHandle> Lookup(
const Slice& key, const Cache::CacheItemHelper* helper,
Cache::CreateContext* create_context, bool wait, bool advise_erase,
bool& kept_in_sec_cache) override;
Statistics* stats, bool& kept_in_sec_cache) override;

bool SupportForceErase() const override { return base_->SupportForceErase(); }

Expand Down

0 comments on commit 33cb1bb

Please sign in to comment.