From 2eb291a4614670b45ae12d25d933f4df342c742a Mon Sep 17 00:00:00 2001 From: Chris Miles Date: Sat, 28 Jun 2025 17:28:19 -0500 Subject: [PATCH] [Databuckets] Revert Caching Changes of #4917 (#4957) * [Databuckets] Revert Caching Changes of #4917 * Comment caching tests until addressed later --- common/data_bucket.cpp | 90 +++------------------------------- zone/cli/tests/databuckets.cpp | 52 ++++++++++---------- 2 files changed, 34 insertions(+), 108 deletions(-) diff --git a/common/data_bucket.cpp b/common/data_bucket.cpp index 166100549..bef06a124 100644 --- a/common/data_bucket.cpp +++ b/common/data_bucket.cpp @@ -19,37 +19,6 @@ extern WorldDatabase database; #error "You must define either ZONE or WORLD" #endif -// Key: compound cache key (e.g., account_id|character_id|zone_id|instance_id|top_key|full_key) -// Value: resolved DataBuckets with extracted nested value -static std::unordered_map g_nested_bucket_cache; - -static std::string MakeNestedCacheKey(const DataBucketKey &k, const std::string &full_key) { - return fmt::format( - "account_id:{}|character_id:{}|npc_id:{}|bot_id:{}|zone_id:{}|instance_id:{}|top_key:{}|full_key:{}", - k.account_id, k.character_id, k.npc_id, k.bot_id, k.zone_id, k.instance_id, - Strings::Split(full_key, NESTED_KEY_DELIMITER).front(), - full_key - ); -} - -static std::string MakeNestedCacheKeyPrefix(const DataBucketKey &k, const std::string &top_key) { - return fmt::format( - "account_id:{}|character_id:{}|npc_id:{}|bot_id:{}|zone_id:{}|instance_id:{}|top_key:{}|", - k.account_id, k.character_id, k.npc_id, k.bot_id, k.zone_id, k.instance_id, top_key - ); -} - -static void InvalidateNestedCacheForKey(const DataBucketKey &k, const std::string &top_key) { - std::string prefix = MakeNestedCacheKeyPrefix(k, top_key); - for (auto it = g_nested_bucket_cache.begin(); it != g_nested_bucket_cache.end(); ) { - if (it->first.find(prefix) == 0) { - it = g_nested_bucket_cache.erase(it); - } else { - ++it; - } - } -} - void DataBucket::SetData(const std::string &bucket_key, const std::string &bucket_value, std::string expires_time) { auto k = DataBucketKey{ @@ -167,15 +136,6 @@ void DataBucket::SetData(const DataBucketKey &k_) // Serialize JSON back to string b.value = json_value.dump(); b.key_ = top_key; // Use the top-level key - - if (CanCache(k_)) { - InvalidateNestedCacheForKey(k_, top_key); - std::string nested_cache_key = MakeNestedCacheKey(k_, k_.key); - auto extracted = ExtractNestedValue(b, k_.key); - if (extracted.id > 0) { - g_nested_bucket_cache[nested_cache_key] = extracted; - } - } } if (bucket_id) { @@ -291,27 +251,12 @@ DataBucketsRepository::DataBuckets DataBucket::GetData(const DataBucketKey &k_, LogDataBuckets("Returning key [{}] value [{}] from cache", e.key_, e.value); if (is_nested_key && !k_.key.empty()) { - std::string nested_cache_key = MakeNestedCacheKey(k_, k.key); - - auto it = g_nested_bucket_cache.find(nested_cache_key); - if (it != g_nested_bucket_cache.end()) { - LogDataBucketsDetail("Nested cache hit for key [{}]", nested_cache_key); - return it->second; - } - - auto extracted = ExtractNestedValue(e, k_.key); - if (extracted.id > 0) { - g_nested_bucket_cache[nested_cache_key] = extracted; - } - return extracted; + return ExtractNestedValue(e, k_.key); } return e; } } - - // if we can cache its assumed we didn't load this into the cache so we should not return a miss - return DataBucketsRepository::NewEntity(); // Not found in cache } // Fetch the value from the database @@ -370,42 +315,23 @@ DataBucketsRepository::DataBuckets DataBucket::GetData(const DataBucketKey &k_, } // Add the value to the cache if it doesn't exist - // If cacheable and not found in cache, short-circuit and assume it doesn't exist if (can_cache) { - bool found_in_cache = false; + bool has_cache = false; for (const auto &e : g_data_bucket_cache) { - if (CheckBucketMatch(e, k)) { - found_in_cache = true; + if (e.id == bucket.id) { + has_cache = true; break; } } - if (!found_in_cache) { - LogDataBuckets("Cache miss for key [{}] - skipping DB due to CanCache", k.key); - return DataBucketsRepository::NewEntity(); + if (!has_cache) { + g_data_bucket_cache.emplace_back(bucket); } } // Handle nested key extraction if (is_nested_key && !k_.key.empty()) { - if (CanCache(k_)) { - std::string nested_cache_key = MakeNestedCacheKey(k_, k.key); - - auto it = g_nested_bucket_cache.find(nested_cache_key); - if (it != g_nested_bucket_cache.end()) { - LogDataBucketsDetail("Nested cache hit for key [{}]", nested_cache_key); - return it->second; - } - - auto extracted = ExtractNestedValue(bucket, k_.key); - if (extracted.id > 0) { - g_nested_bucket_cache[nested_cache_key] = extracted; - } - return extracted; - } else { - // Not cacheable, just extract and return - return ExtractNestedValue(bucket, k_.key); - } + return ExtractNestedValue(bucket, k_.key); } return bucket; @@ -917,4 +843,4 @@ bool DataBucket::CanCache(const DataBucketKey &key) } return false; -} +} \ No newline at end of file diff --git a/zone/cli/tests/databuckets.cpp b/zone/cli/tests/databuckets.cpp index c51b99c6a..28a3604d4 100644 --- a/zone/cli/tests/databuckets.cpp +++ b/zone/cli/tests/databuckets.cpp @@ -318,32 +318,32 @@ void ZoneCLI::TestDataBuckets(int argc, char** argv, argh::parser& cmd, std::str ); // Cold cache test — should return "" - std::string cold_value = client->GetBucket(scoped_key); - RunTest("Cold Cache Scoped Key Returns Empty (Due to Skip DB)", "", cold_value); - - // ✅ Reload cache - client->LoadDataBucketsCache(); - - // Cache should now return the value - std::string hot_value = client->GetBucket(scoped_key); - RunTest("Post-BulkLoad Scoped Key Returns Value", "cached_value", hot_value); - - // Also test nested key after preload - client->DeleteBucket("ac_nested.test"); - client->SetBucket("ac_nested.test", "nested_val"); - - // Clear cache, then preload - DataBucket::ClearCache(); - client->LoadDataBucketsCache(); - - std::string nested_value = client->GetBucket("ac_nested.test"); - RunTest("Post-BulkLoad Nested Scoped Key Returns Value", "nested_val", nested_value); - - // Remove and check that cache misses properly again - client->DeleteBucket("ac_nested.test"); - DataBucket::ClearCache(); - std::string post_delete_check = client->GetBucket("ac_nested.test"); - RunTest("Post-Delete Nested Scoped Key Returns Empty", "", post_delete_check); + // std::string cold_value = client->GetBucket(scoped_key); + // RunTest("Cold Cache Scoped Key Returns Empty (Due to Skip DB)", "", cold_value); + // + // // ✅ Reload cache + // client->LoadDataBucketsCache(); + // + // // Cache should now return the value + // std::string hot_value = client->GetBucket(scoped_key); + // RunTest("Post-BulkLoad Scoped Key Returns Value", "cached_value", hot_value); + // + // // Also test nested key after preload + // client->DeleteBucket("ac_nested.test"); + // client->SetBucket("ac_nested.test", "nested_val"); + // + // // Clear cache, then preload + // DataBucket::ClearCache(); + // client->LoadDataBucketsCache(); + // + // std::string nested_value = client->GetBucket("ac_nested.test"); + // RunTest("Post-BulkLoad Nested Scoped Key Returns Value", "nested_val", nested_value); + // + // // Remove and check that cache misses properly again + // client->DeleteBucket("ac_nested.test"); + // DataBucket::ClearCache(); + // std::string post_delete_check = client->GetBucket("ac_nested.test"); + // RunTest("Post-Delete Nested Scoped Key Returns Empty", "", post_delete_check); std::cout << "\n===========================================\n";