loading_cache: implement a variation of least frequent recently used …

…(LFRU) eviction policy

This patch implements a simple variation of LFRU eviction policy:
  * We define 2 dynamic cache sections which total size should not exceed the maximum cache size.
  * New cache entry is always added to the "unprivileged" section.
  * After a cache entry is read more than SectionHitThreshold times it moves to the second cache section.
  * Both sections' entries obey expiration and reload rules in the same way as before this patch.
  * When cache entries need to be evicted due to a size restriction "unprivileged" section's
    least recently used entries are evicted first.

Note:
With a 2 sections cache it's not enough for a new entry to have the latest timestamp
in order not be evicted right after insertion: e.g. if all all other entries
are from the privileged section.

And obviously we want to allow new cache entries to be added to a cache.

Therefore we can no longer first add a new entry and then shrink the cache.
Switching the order of these two operations resolves the culprit.

Fixes scylladb#8674

Signed-off-by: Vlad Zolotarov <vladz@scylladb.com>

alternator/auth.hh

@@ -35,7 +35,7 @@ namespace alternator {
 
 using hmac_sha256_digest = std::array<char, 32>;
 
-using key_cache = utils::loading_cache<std::string, std::string>;
+using key_cache = utils::loading_cache<std::string, std::string, 1>;
 
 std::string get_signature(std::string_view access_key_id, std::string_view secret_access_key, std::string_view host, std::string_view method,
         std::string_view orig_datestamp, std::string_view signed_headers_str, const std::map<std::string_view, std::string_view>& signed_headers_map,

auth/permissions_cache.hh

@@ -67,6 +67,7 @@ class permissions_cache final {
     using cache_type = utils::loading_cache<
             std::pair<role_or_anonymous, resource>,
             permission_set,
+            1,
             utils::loading_cache_reload_enabled::yes,
             utils::simple_entry_size<permission_set>,
             utils::tuple_hash>;

cql3/authorized_prepared_statements_cache.hh

@@ -115,6 +115,7 @@ private:
     using checked_weak_ptr = typename statements::prepared_statement::checked_weak_ptr;
     using cache_type = utils::loading_cache<cache_key_type,
                                             checked_weak_ptr,
+                                            1,
                                             utils::loading_cache_reload_enabled::yes,
                                             authorized_prepared_statements_cache_size,
                                             std::hash<cache_key_type>,

cql3/prepared_statements_cache.hh

@@ -102,7 +102,14 @@ public:
 
 private:
     using cache_key_type = typename prepared_cache_key_type::cache_key_type;
-    using cache_type = utils::loading_cache<cache_key_type, prepared_cache_entry, utils::loading_cache_reload_enabled::no, prepared_cache_entry_size, utils::tuple_hash, std::equal_to<cache_key_type>, prepared_cache_stats_updater>;
+    // Keep the entry in the "unprivileged" cache section till 2 hits because
+    // every prepared statement is accessed at least twice in the cache:
+    //  1) During PREPARE
+    //  2) During EXECUTE
+    //
+    // Therefore a typical "pollution" (when a cache entry is used only once) would involve
+    // 2 cache hits.
+    using cache_type = utils::loading_cache<cache_key_type, prepared_cache_entry, 2, utils::loading_cache_reload_enabled::no, prepared_cache_entry_size, utils::tuple_hash, std::equal_to<cache_key_type>, prepared_cache_stats_updater>;
     using cache_value_ptr = typename cache_type::value_ptr;
     using checked_weak_ptr = typename statements::prepared_statement::checked_weak_ptr;
 

test/boost/loading_cache_test.cc

@@ -266,17 +266,24 @@ SEASTAR_TEST_CASE(test_loading_cache_loading_different_keys) {
 SEASTAR_TEST_CASE(test_loading_cache_loading_expiry_eviction) {
     return seastar::async([] {
         using namespace std::chrono;
-        utils::loading_cache<int, sstring> loading_cache(num_loaders, 20ms, testlog);
+        utils::loading_cache<int, sstring, 1> loading_cache(num_loaders, 20ms, testlog);
         auto stop_cache_reload = seastar::defer([&loading_cache] { loading_cache.stop().get(); });
 
         prepare().get();
 
         loading_cache.get_ptr(0, loader).discard_result().get();
 
+        // Check unprivileged section eviction
+        BOOST_REQUIRE(loading_cache.size() == 1);
+        sleep(20ms).get();
+        REQUIRE_EVENTUALLY_EQUAL(loading_cache.size(), 0);
+
+        // Check privileged section eviction
+        loading_cache.get_ptr(0, loader).discard_result().get();
         BOOST_REQUIRE(loading_cache.find(0) != nullptr);
 
         sleep(20ms).get();
-        REQUIRE_EVENTUALLY_EQUAL(loading_cache.find(0), nullptr);
+        REQUIRE_EVENTUALLY_EQUAL(loading_cache.size(), 0);
     });
 }
 
@@ -339,14 +346,31 @@ SEASTAR_TEST_CASE(test_loading_cache_move_item_to_mru_list_front_on_sync_op) {
     });
 }
 
-SEASTAR_TEST_CASE(test_loading_cache_loading_reloading) {
+SEASTAR_TEST_CASE(test_loading_cache_loading_reloading_privileged_gen) {
     return seastar::async([] {
         using namespace std::chrono;
         load_count = 0;
-        utils::loading_cache<int, sstring, utils::loading_cache_reload_enabled::yes> loading_cache(num_loaders, 100ms, 20ms, testlog, loader);
+        utils::loading_cache<int, sstring, 1, utils::loading_cache_reload_enabled::yes> loading_cache(num_loaders, 100ms, 20ms, testlog, loader);
         auto stop_cache_reload = seastar::defer([&loading_cache] { loading_cache.stop().get(); });
         prepare().get();
-        loading_cache.get_ptr(0, loader).discard_result().get();
+        // Push the entry into the privileged section. Make sure it's being reloaded.
+        loading_cache.get_ptr(0).discard_result().get();
+        loading_cache.get_ptr(0).discard_result().get();
+        sleep(60ms).get();
+        BOOST_REQUIRE(eventually_true([&] { return load_count >= 3; }));
+    });
+}
+
+SEASTAR_TEST_CASE(test_loading_cache_loading_reloading_unprivileged) {
+    return seastar::async([] {
+        using namespace std::chrono;
+        load_count = 0;
+        utils::loading_cache<int, sstring, 1, utils::loading_cache_reload_enabled::yes> loading_cache(num_loaders, 100ms, 20ms, testlog, loader);
+        auto stop_cache_reload = seastar::defer([&loading_cache] { loading_cache.stop().get(); });
+        prepare().get();
+        // Load one entry into the unprivileged section.
+        // Make sure it's reloaded.
+        loading_cache.get_ptr(0).discard_result().get();
         sleep(60ms).get();
         BOOST_REQUIRE(eventually_true([&] { return load_count >= 2; }));
     });
@@ -370,11 +394,58 @@ SEASTAR_TEST_CASE(test_loading_cache_max_size_eviction) {
     });
 }
 
+SEASTAR_TEST_CASE(test_loading_cache_max_size_eviction_unprivileged_first) {
+    return seastar::async([] {
+        using namespace std::chrono;
+        load_count = 0;
+        utils::loading_cache<int, sstring, 1> loading_cache(4, 1h, testlog);
+        auto stop_cache_reload = seastar::defer([&loading_cache] { loading_cache.stop().get(); });
+
+        prepare().get();
+
+        // Touch the value with the key "-1" twice
+        loading_cache.get_ptr(-1, loader).discard_result().get();
+        loading_cache.find(-1);
+
+        for (int i = 0; i < num_loaders; ++i) {
+            loading_cache.get_ptr(i, loader).discard_result().get();
+        }
+
+        BOOST_REQUIRE_EQUAL(load_count, num_loaders + 1);
+        BOOST_REQUIRE_EQUAL(loading_cache.size(), 4);
+        // Make sure that the value we touched twice is still in the cache
+        BOOST_REQUIRE(loading_cache.find(-1) != nullptr);
+    });
+}
+
+SEASTAR_TEST_CASE(test_loading_cache_eviction_unprivileged) {
+    return seastar::async([] {
+        using namespace std::chrono;
+        load_count = 0;
+        utils::loading_cache<int, sstring, 1> loading_cache(4, 10ms, testlog);
+        auto stop_cache_reload = seastar::defer([&loading_cache] { loading_cache.stop().get(); });
+
+        prepare().get();
+
+        // Touch the value with the key "-1" twice
+        loading_cache.get_ptr(-1, loader).discard_result().get();
+        loading_cache.find(-1);
+
+        for (int i = 0; i < num_loaders; ++i) {
+            loading_cache.get_ptr(i, loader).discard_result().get();
+        }
+
+        // Make sure that the value we touched twice is eventually evicted
+        REQUIRE_EVENTUALLY_EQUAL(loading_cache.find(-1), nullptr);
+        REQUIRE_EVENTUALLY_EQUAL(loading_cache.size(), 0);
+    });
+}
+
 SEASTAR_TEST_CASE(test_loading_cache_reload_during_eviction) {
     return seastar::async([] {
         using namespace std::chrono;
         load_count = 0;
-        utils::loading_cache<int, sstring, utils::loading_cache_reload_enabled::yes> loading_cache(1, 100ms, 10ms, testlog, loader);
+        utils::loading_cache<int, sstring, 0, utils::loading_cache_reload_enabled::yes> loading_cache(1, 100ms, 10ms, testlog, loader);
         auto stop_cache_reload = seastar::defer([&loading_cache] { loading_cache.stop().get(); });
 
         prepare().get();