Summary
This is a sibling of #408 (fixed by #420) on the latency side. When the KV cache evicts data from a higher tier into a lower one, kv_cache/cache.py::_demote_entry() performs real backend reads and writes but never appends the resulting timings to the per-tier latency lists (storage_write_latencies, storage_read_latencies, cpu_write_latencies, cpu_read_latencies, and the storage_*_device/host_latencies variants).
As a result, on the typical multi-tier setup where a tier is populated via eviction rather than direct allocation, _evaluate_storage_performance() (default performance_profile='latency') finds an empty storage_write_basis and silently skips the "Storage Tier Write P95 < 500ms" SLA criterion (and similarly the read P95 / CPU RAM P95 criteria). The check doesn't fail — it simply isn't evaluated, so a submission can omit the most important storage-latency gate without any signal.
#408 / #420 fixed the byte counters (tier_*_kv_bytes_written/read, and the throughput-profile bandwidth derived from them). This issue tracks the analogous latency gap, which #420 was intentionally kept minimal to exclude.
Root cause
_demote_entry() computes read_timing and write_timing from the backend calls and returns their sum, but the caller discards it (success, _ = self._demote_entry(...)), and the method never appends to the latency lists that _evaluate_storage_performance() consumes:
data, read_timing = self.backends[from_tier].read(key)
write_timing = self.backends[to_tier].write(key, data)
self.backends[from_tier].delete(key)
...
with self.stats_lock:
self.stats['evictions'] += 1
# bytes are now tracked (PR #420), but NOT latencies:
# MISSING: append read_timing to tier_{from}_*_latencies
# MISSING: append write_timing to tier_{to}_*_latencies
By contrast, the direct-allocation path (_allocate_cache_inner) and the read path (access_cache) both record latencies, e.g.:
# _allocate_cache_inner (nvme branch)
self.stats['storage_write_latencies'].append(timing.total)
self.stats['storage_write_device_latencies'].append(timing.device)
self.stats['storage_write_host_latencies'].append(timing.host)
Impact
Suggested fix
Inside the existing with self.stats_lock: block in _demote_entry(), append the timings symmetric to the byte counters added in #420 (mirroring _allocate_cache_inner / access_cache):
# read out of from_tier
if from_tier == 'gpu':
self.stats['gpu_read_latencies'].append(read_timing.total)
elif from_tier == 'cpu':
self.stats['cpu_read_latencies'].append(read_timing.total)
elif from_tier == 'nvme':
self.stats['storage_read_latencies'].append(read_timing.total)
self.stats['storage_read_device_latencies'].append(read_timing.device)
self.stats['storage_read_host_latencies'].append(read_timing.host)
# write into to_tier
if to_tier == 'cpu':
self.stats['cpu_write_latencies'].append(write_timing.total)
elif to_tier == 'nvme':
self.stats['storage_write_latencies'].append(write_timing.total)
self.stats['storage_write_device_latencies'].append(write_timing.device)
self.stats['storage_write_host_latencies'].append(write_timing.host)
A regression test analogous to the one in #420 (in TestThreeTierEvictionCascade) should assert that, after an eviction-driven cascade, storage_write_latencies / cpu_*_latencies are non-empty and that _evaluate_storage_performance() actually evaluates the Storage Write/Read P95 criteria.
Notes
Found while reviewing the #408 fix (#420) with a multi-model code-review panel (GPT-5.5, Claude Opus 4.8, Gemini 3.1 Pro). Filed separately to keep #420 scoped to the byte-counter fix the WG already approved.
Environment
- Bench:
kv_cache_benchmark pyproject.toml version 3.0.0 (current main)
Summary
This is a sibling of #408 (fixed by #420) on the latency side. When the KV cache evicts data from a higher tier into a lower one,
kv_cache/cache.py::_demote_entry()performs real backend reads and writes but never appends the resulting timings to the per-tier latency lists (storage_write_latencies,storage_read_latencies,cpu_write_latencies,cpu_read_latencies, and thestorage_*_device/host_latenciesvariants).As a result, on the typical multi-tier setup where a tier is populated via eviction rather than direct allocation,
_evaluate_storage_performance()(defaultperformance_profile='latency') finds an emptystorage_write_basisand silently skips the "Storage Tier Write P95 < 500ms" SLA criterion (and similarly the read P95 / CPU RAM P95 criteria). The check doesn't fail — it simply isn't evaluated, so a submission can omit the most important storage-latency gate without any signal.#408 / #420 fixed the byte counters (
tier_*_kv_bytes_written/read, and the throughput-profile bandwidth derived from them). This issue tracks the analogous latency gap, which #420 was intentionally kept minimal to exclude.Root cause
_demote_entry()computesread_timingandwrite_timingfrom the backend calls and returns their sum, but the caller discards it (success, _ = self._demote_entry(...)), and the method never appends to the latency lists that_evaluate_storage_performance()consumes:By contrast, the direct-allocation path (
_allocate_cache_inner) and the read path (access_cache) both record latencies, e.g.:Impact
latency) profile: the Storage Tier Write/Read P95 SLA criteria are skipped entirely on eviction-driven multi-tier runs — the same configurations that motivated KVCache: tier_storage_kv_bytes_written_gb and tier_cpu_kv_bytes_written_gb always 0 when data reaches those tiers via eviction #408.storage_healthreportspassed_count/total_countover only the criteria that happened to have data.Suggested fix
Inside the existing
with self.stats_lock:block in_demote_entry(), append the timings symmetric to the byte counters added in #420 (mirroring_allocate_cache_inner/access_cache):A regression test analogous to the one in #420 (in
TestThreeTierEvictionCascade) should assert that, after an eviction-driven cascade,storage_write_latencies/cpu_*_latenciesare non-empty and that_evaluate_storage_performance()actually evaluates the Storage Write/Read P95 criteria.Notes
Found while reviewing the #408 fix (#420) with a multi-model code-review panel (GPT-5.5, Claude Opus 4.8, Gemini 3.1 Pro). Filed separately to keep #420 scoped to the byte-counter fix the WG already approved.
Environment
kv_cache_benchmarkpyproject.tomlversion3.0.0(currentmain)