Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(sequencer): provide storage and snapshot types #1801

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Fraser999
Copy link
Contributor

Summary

This adds new storage types to sequencer and replaces bare usage of cnidarium equivalent types.

Background

We want to become less coupled to the raw cnidarium APIs, and we also want to provide a cache of recently-read values.

Changes

  • Provided new storage::Storage type, wrapping the cnidarium::Storage type.
  • Provided new storage::Snapshot type, wrapping the cnidarium::Snapshot type and including a cache of fetched values.
  • When the sequencer main loop has finished, we now await the server handle and then call storage.release().await; which should probably have been the case before this PR.
  • Applied consistent naming to variables of type StateDelta<Snapshot> and StateDelta<StateDelta<Snapshot>>.

Testing

Added unit tests for the new types covering the non-trivial methods (many new methods simply call directly through to the equivalent inner method and don't need independent unit tests).

Changelogs

Changelogs updated.

Metrics

  • Added astria_sequencer_verifiable_cache_hit counter
  • Added astria_sequencer_verifiable_cache_miss counter
  • Added astria_sequencer_verifiable_cache_item_total histogram
  • Added astria_sequencer_non_verifiable_cache_hit counter
  • Added astria_sequencer_non_verifiable_cache_miss counter
  • Added astria_sequencer_non_verifiable_cache_item_total histogram

Related Issues

Closes #1435.

@Fraser999 Fraser999 requested a review from a team as a code owner November 11, 2024 18:44
@Fraser999 Fraser999 requested a review from noot November 11, 2024 18:44
@github-actions github-actions bot added the sequencer pertaining to the astria-sequencer crate label Nov 11, 2024
@@ -32,12 +32,14 @@ telemetry = { package = "astria-telemetry", path = "../astria-telemetry", featur
"display",
] }

anyhow = "1.0.89"
Copy link
Member

Choose a reason for hiding this comment

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

There was a big effort to not have anyhow be a direct dependency of sequencer. Can we avoid reintroducing it here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we might be able to do that as part of #1436, but for now it's required since we're implementing cnidarium::StateRead on the new Snapshot type and that requires anyhow (cnidarium doesn't re-export it either).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sequencer pertaining to the astria-sequencer crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace cnidarium Storage and Snapshot types
2 participants