diff --git a/crates/storage/src/metrics.rs b/crates/storage/src/metrics.rs index 3270eeb9..99093a8f 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 0a329ebe..51e5f6b0 100644 --- a/crates/storage/src/state_writer.rs +++ b/crates/storage/src/state_writer.rs @@ -66,6 +66,7 @@ use tracing::error; use crate::api::{StorageBackend, StorageReadView, 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, beacon_block_slot, block_slot, block_slot_in, encode_slot_root_key}; @@ -139,6 +140,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 @@ -167,7 +180,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 16d94961..5827e047 100644 --- a/crates/storage/src/store.rs +++ b/crates/storage/src/store.rs @@ -44,10 +44,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}; @@ -3228,7 +3229,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`. @@ -3273,15 +3274,10 @@ 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`). A cached state can be one whose rows @@ -3290,13 +3286,20 @@ impl Store { // `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); } // Both tables are keyed by block slot then root; no block row, no state. diff --git a/docs/metrics.md b/docs/metrics.md index 662e4f18..7a1ba2b1 100644 --- a/docs/metrics.md +++ b/docs/metrics.md @@ -327,6 +327,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` @@ -346,6 +347,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