Skip to content

chore: Improved StateView and StateSnapshot lifetime logging - #2452

Open
sergerad wants to merge 8 commits into
nextfrom
sergerad-lock-followup
Open

chore: Improved StateView and StateSnapshot lifetime logging#2452
sergerad wants to merge 8 commits into
nextfrom
sergerad-lock-followup

Conversation

@sergerad

@sergerad sergerad commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

Closes #2451.

Previously, snapshot-lifetime observability was conditional logging with thresholds baked into the code:

  • A single warning when a long-held snapshot was finally released;
  • A per-block warning when snapshot lag crossed a threshold;
  • A warning when too many generations were live; and
  • A StateView::drop warning attributing long-held views. Conditional logs make poor metrics — the signal only exists once a hardcoded threshold trips, and alerting can't be tuned without a code change.

This PR replaces all of that with unconditional span fields recorded on the write_block span for every applied block:

  • snapshots.lag_blocks — distance in blocks between the chain tip and the oldest still-pinned snapshot generation.
  • snapshots.oldest_superseded_for_ms — how long the oldest pinned generation has been superseded (0 while the oldest pinned generation is still the latest). This fires while the offending reader is still alive, instead of once at release.
  • snapshots.live — number of live snapshot generations.

Alerts on slow or leaked readers can now be defined on these fields outside the codebase, with no thresholds in code.

How:

  • Supersession tracking moved into PublishedGenerations: record stamps the previous latest generation as superseded when its successor is published, and advance reports the oldest pinned generation's time-since-supersession. The clock still starts at supersession, not publication, so idle chains do not inflate the metric.
  • SnapshotGuard is removed entirely. Its only jobs were maintaining the shared live-generation counter and emitting a release log; the live count is now derived from PublishedGenerations (the Weak refcounts are the same ground truth the counter proxied), so the Arc<AtomicUsize> plumbing through the writer and lifecycle is gone.
  • StateView loses its Drop impl, caller, and created_at. Detection of pinned generations comes from the per-block fields above; attribution comes from the instrumented request spans that acquire views, whose durations bound the hold time and identify the endpoint. With #[track_caller] gone, with_view collapses back to a plain async fn.
  • The tracing field-name registry is updated accordingly: snapshots.oldest_superseded_for_ms added; snapshot.superseded_for_ms, snapshot.lifetime_ms, view.lifetime_ms, and caller removed (no emitters remain).

Tradeoff: per-view and per-generation release events are gone, so exact view hold time is no longer distinguishable from overall handler duration. In practice views are request-scoped and one-per-handler, so span durations carry the same signal.

Changelog

changelog = "none"
reason    = "Internal change only."

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this is where we would want to do this because there is a lack of actionable information.

What do we now do once we receive this? We need to identify which query this is, but we have no way of doing so..

Perhaps we could explore a timer within the actual snapshot itself, and each snapshot taken automatically gets the caller LoC information embedded?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added a LoC WARN log on long-held StateViews (StateSnapshots are not instantiated per-query, StateViews are).

The per-block log is still important in case we ever get snapshots or views that never end.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those two things are essentially synonyms. We should try improve our naming here. I assume its a snapshot because rocksdb calls them snapshots? Perhaps SmtView?

Is there a downside to having just a single one, instead of separate types? I can't imagine a snapshot is expensive to hold temporarily.

@sergerad sergerad Aug 10, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StateViews and StateSnapshots are not synonyms - any number of views (one per API request) can map to a single snapshot. The view is there to enforce the invariants / API appropriate for accessing snapshot + SQL data consistently (block scoped requests). The snapshot is the non-SQL data itself.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that's what they are in our code. I'm saying the word view and the word snapshot mean almost exactly the same thing, and are therefore not good names for us to use.

If I say StateSnapshot everyone would assume that means a snapshot of our state at a moment in time. If I say StateView everyone would assume that means a view of our state at a moment in time.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood. However, I find the terms view and snapshot to be quite appropriate here (differentiating between the data itself [snapshot] and how it is accessed [view]).

Snapshot (computing):

  • A saved state of files or data used for quick backups and recovery.

View (database):

  • Virtual table: A saved query that displays data from one or more base tables without duplicating the physical storage.
  • Security: Hides confidential columns or rows by only exposing safe subsets of data to specific users.
  • Simplicity: Wraps complex multi-table joins into a single, easy-to-query object.

SmtView is not appropriate because StateView is a view into both the trees and the data in SQLite.

Alternatives you might prefer, LMK:

  • StateGeneration
  • StateReader, ReadTxn

@sergerad sergerad changed the title Add warning per block for snapshot lag chore: Improved StateView and StateSnapshot lifetime logging Aug 9, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commenting here so we can thread on this discussion. File isn't relevant.

Could you provide an overview of how things work wrt state views and snapshots, and pruning history. I've lost a lot of context here.

What I was hoping we would have is something like the following.

We have a ReadOnlyState type struct. Holders (i.e. gRPC requests) use this to obtain a singlular StateView. This view comprises a RocksDb snapshot, a SQLite transaction, and the proof + chain tips (as numbers, not channels).

The data provided within the view is internally consistent i.e. it represents the complete state at a specific block height (chain tip). This StateView implements the query methods aka it becomes our "state transaction" type.

In order to guarantee the above, we cannot prune data out from under it while that object is being constructed. So we have to be careful about deleting or invalidating old references to it.

When we delete old "views", we have to first check that no-one is still holding it. If they are, then we can't delete it.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you provide an overview of how things work wrt state views and snapshots, and pruning history. I've lost a lot of context here.

The answer to that is basically in PublishedGeneration::advance().

The writer keeps one Weak<StateSnapshot> per published generation (PublishedGenerations::record, view/snapshot.rs:100-106). While applying a block — before the DB commit, so the result can feed pruning inside the same transaction — it calls advance(chain_tip) (view/snapshot.rs:118-130, invoked at writer/worker.rs:230), which:

  1. drops entries whose Arc is gone (strong_count() == 0) — nobody holds them, safe to have pruned,
  2. finds the oldest generation whose Weak is still alive — that's oldest_pinned,
  3. uses that as the pruning tip (prune_tip), capped at SNAPSHOT_PRUNE_LAG_CAP blocks of lag so one leaked reader can't stall pruning forever (accepting a historical-read race for that reader past the cap — logged loudly, see below).

The resulting prune_tip is passed into db.apply_block(..), and history pruning runs inside that same DB transaction (writer/worker.rs:245-256).

So the "can't delete out from under a holder" guarantee is that pruning simply doesn't advance past a live generation until that generation's Arc refcount hits zero (last StateView/StateSnapshot reference dropped).

We have a ReadOnlyState type struct. Holders (i.e. gRPC requests) use this to obtain a singlular StateView. This view comprises a RocksDb snapshot, a SQLite transaction, and the proof + chain tips (as numbers, not channels).

What we have today is aligned with this except that we still have the channel based proof and chain tips in State rather than StateView (reminder: you get a view via State::view() or State::with_view()). Those channels are used for long-lived subscriptions and so do not make sense to come through the StateView. We cannot tie the views' lifetimes to a long-lived subscription for obvious reasons.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Lock-free followup

2 participants