jplevyak commented on code in PR #13893:
URL: https://github.com/apache/trafficserver/pull/13893#discussion_r4367862510
##########
src/iocore/cache/unit_tests/test_RamCacheCLFUS.cc:
##########
@@ -398,3 +456,295 @@ TEST_CASE("CLFUS value metric is a floating-point
density", "[cache][ramcache][c
static_assert(v_hot > v_one, "CLFUS value metric does not increase with
hits");
static_assert(v_small > v_large, "CLFUS value metric does not decrease with
size");
}
+
+namespace
+{
+
+CryptoHash
+make_key(uint64_t id)
+{
+ CryptoHash key;
+ key.u32[0] = static_cast<uint32_t>(0xc0ffee00 + id);
+ key.u32[2] = static_cast<uint32_t>(0xdeadbeef + id);
+ key.u32[3] = static_cast<uint32_t>(0x12345678 + id);
+ return key;
+}
+
+} // namespace
+
+TEST_CASE("CLFUS decays hit count when sparing victims during eviction",
"[cache][ramcache][clfus]")
+{
+ CacheDisk disk;
+ init_disk(disk);
+ StripeSM stripe{&disk, 10, 0};
+ CacheVol cache_vol;
+ wire_stripe(stripe, cache_vol);
+
+ cache_config_ram_cache_compress = CACHE_COMPRESSION_NONE;
+ cache_config_ram_cache_use_seen_filter = 0;
+
+ RamCacheCLFUS rc;
+ rc.init(32 * 1024, &stripe);
+
+ auto buf = make_buffer(compressible_bytes(8 * 1024));
+ auto key1 = make_key(1);
+ auto key2 = make_key(2);
+ auto key3 = make_key(3);
+
+ REQUIRE(rc.put(&key1, buf.get(), 8 * 1024) == 1);
+ REQUIRE(rc.put(&key2, buf.get(), 8 * 1024) == 1);
+ REQUIRE(rc.put(&key3, buf.get(), 8 * 1024) == 1);
+
+ RamCacheCLFUSEntry *e1 = RamCacheCLFUSTestAccess::find_entry(rc, key1);
+ REQUIRE(e1 != nullptr);
+ e1->hits = 16; // Give key1 high hits so it exceeds average value
+
+ // key4 triggers an eviction pass
+ auto key4 = make_key(4);
+ rc.put(&key4, buf.get(), 8 * 1024);
+
+ // e1 was spared on its first pass as a victim (16 -> 8), then taken on the
+ // second pass once the requeue budget ran out and put back by
+ // _requeue_victims() (8 -> 7). Without the spared-victim decay it would be
15.
+ CHECK(e1->hits == 7);
+}
+
+TEST_CASE("CLFUS unconditionally promotes resident entries to the tail on
hits", "[cache][ramcache][clfus]")
+{
+ CacheDisk disk;
+ init_disk(disk);
+ StripeSM stripe{&disk, 10, 0};
+ CacheVol cache_vol;
+ wire_stripe(stripe, cache_vol);
+
+ cache_config_ram_cache_compress = CACHE_COMPRESSION_NONE;
+ cache_config_ram_cache_use_seen_filter = 0;
+
+ RamCacheCLFUS rc;
+ rc.init(64 * 1024, &stripe);
+
+ auto buf = make_buffer(compressible_bytes(4 * 1024));
+ auto key1 = make_key(1);
+ auto key2 = make_key(2);
+ auto key3 = make_key(3);
+
+ REQUIRE(rc.put(&key1, buf.get(), 4 * 1024) == 1);
+ REQUIRE(rc.put(&key2, buf.get(), 4 * 1024) == 1);
+ REQUIRE(rc.put(&key3, buf.get(), 4 * 1024) == 1);
+
+ RamCacheCLFUSEntry *e1 = RamCacheCLFUSTestAccess::find_entry(rc, key1);
+ REQUIRE(e1 != nullptr);
+ // Initial order in _lru[0] has e1 at the head
+ CHECK(RamCacheCLFUSTestAccess::lru_head(rc) == e1);
+
+ // Inflate _average_value so e1's value is lower than average
+ RamCacheCLFUSTestAccess::set_average_value(rc, 1000.0);
+
+ // get() on key1 must unconditionally promote e1 to the tail of _lru[0]
+ Ptr<IOBufferData> got;
+ CHECK(rc.get(&key1, &got) == 1);
+ CHECK(RamCacheCLFUSTestAccess::lru_tail(rc) == e1);
+ CHECK(RamCacheCLFUSTestAccess::lru_head(rc) != e1);
+}
+
+TEST_CASE("CLFUS history entry is refreshed on get() hit",
"[cache][ramcache][clfus]")
+{
+ CacheDisk disk;
+ init_disk(disk);
+ StripeSM stripe{&disk, 10, 0};
+ CacheVol cache_vol;
+ wire_stripe(stripe, cache_vol);
+
+ cache_config_ram_cache_compress = CACHE_COMPRESSION_NONE;
+ cache_config_ram_cache_use_seen_filter = 0;
+
+ RamCacheCLFUS rc;
+ rc.init(12 * 1024, &stripe);
+
+ auto buf = make_buffer(compressible_bytes(8 * 1024));
+ auto key1 = make_key(1);
+ auto key2 = make_key(2);
+
+ REQUIRE(rc.put(&key1, buf.get(), 8 * 1024) == 1);
+ // key2 cannot fit in 12KB (8KB + 8KB > 12KB), routed to history (_lru[1])
+ CHECK(rc.put(&key2, buf.get(), 8 * 1024) == 0);
+
+ RamCacheCLFUSEntry *e2 = RamCacheCLFUSTestAccess::find_entry(rc, key2);
+ REQUIRE(e2 != nullptr);
+ REQUIRE(e2->flag_bits.lru == 1); // in history
+
+ uint64_t hits_before = e2->hits;
+ Ptr<IOBufferData> got;
Review Comment:
Fixed in 7c210ab41. The test now adds a second history entry (key3) and
requires it to be the history tail before the lookup, so get(key2) must move e2
to the tail. Verified that removing the remove/enqueue in get() makes the test
fail.
##########
src/iocore/cache/RamCacheCLFUS.cc:
##########
@@ -775,10 +799,18 @@ RamCacheCLFUS::put(CryptoHash *key, IOBufferData *data,
uint32_t len, bool copy,
if (!this->_max_bytes) {
return 0;
}
+ ink_hrtime now = ink_get_hrtime();
+ if (now - this->_last_decay_time > HRTIME_SECONDS(30)) {
+ if (this->_last_decay_time != 0) {
+ this->_decay_epoch();
+ }
+ this->_last_decay_time = now;
Review Comment:
Fixed in 420397d8f. _decay_epochs() now advances one epoch per whole elapsed
30s interval (carrying the partial remainder), and get() also checks the clock
so hit-only traffic ages entries. 833424347 additionally decays _average_value
by the full interval count rather than the 32-epoch cap.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
