From 61d925dce7461dbe47321a3d4b2558ac80a627db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Gr=C3=BCner?= <47506558+MegaRedHand@users.noreply.github.com> Date: Mon, 5 Oct 2026 13:08:53 -0300 Subject: [PATCH] feat(storage): count state cache hits and misses Nothing reported whether a state lookup was served from the 32-entry LRU, so slow epoch-boundary head updates could only be blamed on a checkpoint-state miss by inference, and a has_state that fell through to RocksDB was invisible. lean_state_cache_lookups_total{method, kind, result} counts every lookup. `method` separates get_state, has_state and cached_state, because gossip validation's cached_state calls (one per attestation and aggregate) would otherwise drown out the import path's misses. `kind` separates block states from checkpoint states, which is the miss the epoch-tick head recompute pays. has_state now peeks the cache before pending_states, the same order read_state uses. A state in flight is in both, so no answer changes, but with pending first a just-imported parent never reached the cache check and its lookup went uncounted. --- crates/storage/src/metrics.rs | 56 ++++++++++++++++++++++++++++++ crates/storage/src/state_writer.rs | 15 +++++++- crates/storage/src/store.rs | 33 ++++++++++-------- docs/metrics.md | 10 ++++++ 4 files changed, 98 insertions(+), 16 deletions(-) diff --git a/crates/storage/src/metrics.rs b/crates/storage/src/metrics.rs index 3270eeb92..99093a8fb 100644 --- a/crates/storage/src/metrics.rs +++ b/crates/storage/src/metrics.rs @@ -4,6 +4,8 @@ use std::sync::LazyLock; use ethlambda_metrics::*; +use crate::state_writer::CacheKey; + static LEAN_STATE_WRITE_QUEUE_DEPTH: LazyLock = LazyLock::new(|| { register_int_gauge!( "lean_state_write_queue_depth", @@ -45,3 +47,57 @@ pub(crate) fn dec_state_write_queue_depth() { pub(crate) fn time_state_write() -> TimingGuard { TimingGuard::new(&LEAN_STATE_WRITE_SECONDS) } + +static LEAN_STATE_CACHE_LOOKUPS_TOTAL: LazyLock = LazyLock::new(|| { + register_int_counter_vec!( + "lean_state_cache_lookups_total", + "State cache lookups, by the Store method that made them, the kind of state asked \ + for, and whether the cache held it", + &["method", "kind", "result"] + ) + .unwrap() +}); + +/// The `Store` method a state-cache lookup was made for. +/// +/// A label rather than one series, because the callers' rates mean different +/// things: gossip validation calls `cached_state` once per attestation and +/// aggregate, far more often than a block is imported, so a single rate would +/// be dominated by it and hide the import path's misses. +#[derive(Debug, Clone, Copy)] +pub(crate) enum StateCacheMethod { + /// `Store::get_state`, and the writer thread's own parent read, which + /// shares its read path (`state_writer::read_state`). A miss falls back + /// to the write buffer, then to storage. + Get, + /// `Store::has_state`. An existence check, so it peeks rather than + /// promoting the entry; a miss falls back to the write buffer, then to + /// key-existence checks against storage. + Has, + /// `Store::cached_state`, the memoization lookup fork choice's + /// `checkpoint_state` and gossip validation use. A miss is the caller's + /// to handle; nothing here falls back to storage. + Cached, +} + +impl StateCacheMethod { + fn label(self) -> &'static str { + match self { + Self::Get => "get_state", + Self::Has => "has_state", + Self::Cached => "cached_state", + } + } +} + +/// Count one state-cache lookup for `key`, made by `method`. +pub(crate) fn inc_state_cache_lookups(method: StateCacheMethod, key: &CacheKey, hit: bool) { + let kind = match key { + CacheKey::BlockState(_) => "block", + CacheKey::CheckpointState { .. } => "checkpoint", + }; + let result = if hit { "hit" } else { "miss" }; + LEAN_STATE_CACHE_LOOKUPS_TOTAL + .with_label_values(&[method.label(), kind, result]) + .inc(); +} diff --git a/crates/storage/src/state_writer.rs b/crates/storage/src/state_writer.rs index d1103d0a7..06ca3533d 100644 --- a/crates/storage/src/state_writer.rs +++ b/crates/storage/src/state_writer.rs @@ -58,6 +58,7 @@ use tracing::error; use crate::api::{StorageBackend, StorageReadViewExt, Table}; use crate::beacon_state_delta; use crate::error::Error; +use crate::metrics::StateCacheMethod; use crate::state_codec::{decode_lean_state_value, decode_state_value, encode_state_value}; use crate::state_diff::StateDiff; use crate::store::Chain; @@ -130,6 +131,18 @@ pub enum CacheKey { /// The shared state cache, memoizing post-states by block root. pub(crate) type StateCache = Mutex>>; +/// Looks `key` up in `cache`, promoting it to most recently used, and counts +/// the lookup as `method`'s. +pub(crate) fn cache_get( + cache: &StateCache, + key: CacheKey, + method: StateCacheMethod, +) -> Option> { + let state = cache.lock().unwrap().get(&key).cloned(); + crate::metrics::inc_state_cache_lookups(method, &key, state.is_some()); + state +} + /// Reads the post-state for `root`, from wherever it currently lives. /// /// The single state read path, called both by @@ -158,7 +171,7 @@ pub(crate) fn read_state( root: &H256, ) -> Result>, Error> { let key = CacheKey::BlockState(*root); - if let Some(state) = cache.lock().unwrap().get(&key).cloned() { + if let Some(state) = cache_get(cache, key, StateCacheMethod::Get) { return Ok(Some(state)); } if let Some(state) = pending.get(root) { diff --git a/crates/storage/src/store.rs b/crates/storage/src/store.rs index 28b147de7..e2d489a52 100644 --- a/crates/storage/src/store.rs +++ b/crates/storage/src/store.rs @@ -32,10 +32,11 @@ use ethlambda_types::{ }; use libssz::{SszDecode, SszEncode}; +use crate::metrics::StateCacheMethod; use crate::state_codec::encode_state_value; use crate::state_writer::{ CacheKey, PendingStates, STATE_WRITE_QUEUE_CAPACITY, StateCache, StateWriteRequest, - StateWriterHandle, read_state, + StateWriterHandle, cache_get, read_state, }; use thiserror::Error; use tracing::{info, warn}; @@ -2653,7 +2654,7 @@ impl Store { /// The memoized state for `key`, if it is still resident. pub fn cached_state(&self, key: CacheKey) -> Option> { - self.state_cache.lock().unwrap().get(&key).cloned() + cache_get(&self.state_cache, key, StateCacheMethod::Cached) } /// Memoizes `state` under `key`. @@ -2685,28 +2686,30 @@ impl Store { /// Returns whether a state is available for the given block root. /// - /// True if `pending_states` or the state cache holds the state, a snapshot + /// True if the state cache or `pending_states` holds the state, a snapshot /// exists, or the state can be reconstructed from a diff. Never reads a /// value: existence checks only. pub fn has_state(&self, root: &H256) -> Result { - // Same pending-before-backend order as `read_state`; see its doc for - // why the backend never has to consult `pending_states` on its own. - if self.pending_states.get(root).is_some() { - return Ok(true); - } // A cached block state is always one that was handed to the writer // (`insert_state`, so in `pending_states` until committed) or read back // from the backend (`read_state`); nothing deletes a persisted state. // `peek`, not `get`: an existence check must not reorder the LRU. // Without this, a state stored as a snapshot only (a beacon epoch // anchor) costs a full-value read from RocksDB just to be found. - if self - .state_cache - .lock() - .unwrap() - .peek(&CacheKey::BlockState(*root)) - .is_some() - { + // + // Cache first, the same order as `read_state`: a state in flight is in + // both, so the order changes no answer, and cache first is what makes + // every call a counted lookup rather than only the ones + // `pending_states` missed. + let key = CacheKey::BlockState(*root); + let cached = self.state_cache.lock().unwrap().peek(&key).is_some(); + crate::metrics::inc_state_cache_lookups(StateCacheMethod::Has, &key, cached); + if cached { + return Ok(true); + } + // Same pending-before-backend order as `read_state`; see its doc for + // why the backend never has to consult `pending_states` on its own. + if self.pending_states.get(root).is_some() { return Ok(true); } let view = self.backend.begin_read().expect("read view"); diff --git a/docs/metrics.md b/docs/metrics.md index bf215a3c6..c3bc7facc 100644 --- a/docs/metrics.md +++ b/docs/metrics.md @@ -326,6 +326,7 @@ In practice the distribution is bimodal and dominated by production rather than | `lean_table_bytes` | Gauge | Estimated byte size of a storage table (key + value bytes) | After each processed block (one update per table); retains its previous value on empty slots | table=`` | | `lean_state_write_queue_depth` | Gauge | States handed to the background writer but not yet committed | On every hand-off and on every commit | | | `lean_state_write_seconds` | Histogram | Time the background writer spends encoding, diffing and committing one state | Per state written | | +| `lean_state_cache_lookups_total` | Counter | State cache lookups, by the `Store` method that made them, the kind of state asked for, and whether the cache held it | On every `get_state` (and the writer's own parent read), `has_state` and `cached_state` call | method=get_state,has_state,cached_state
kind=block,checkpoint
result=hit,miss | **On a beacon follower, watch `lean_table_bytes{table="data_columns"}`.** Every other table's series either stays flat or is bounded by pruning; `data_columns` @@ -345,6 +346,15 @@ and `lean_state_write_seconds` says whether that time went into the encode or the commit. A depth that stays at zero means the writer has nothing outstanding. +**Read `lean_state_cache_lookups_total` per `method`, not summed.** +`cached_state` with `kind="block"` is gossip validation's lookup of the voted +or parent block's state, made once per attestation and aggregate, so it runs far +more often than the other two and a summed rate is mostly its own. A `get_state` +or `has_state` miss means the state was not resident and came from the writer's +buffer or RocksDB. A `cached_state` miss with `kind="checkpoint"` means fork +choice's `checkpoint_state` rebuilt the state, running `process_slots` up to the +epoch boundary when the checkpoint's block precedes it. + ### Beacon Gossip Validation Every beacon gossip message gets a verdict before gossipsub forwards it