-
Notifications
You must be signed in to change notification settings - Fork 77
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
base: main
Are you sure you want to change the base?
Conversation
@@ -32,12 +32,14 @@ telemetry = { package = "astria-telemetry", path = "../astria-telemetry", featur | |||
"display", | |||
] } | |||
|
|||
anyhow = "1.0.89" |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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).
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
storage::Storage
type, wrapping thecnidarium::Storage
type.storage::Snapshot
type, wrapping thecnidarium::Snapshot
type and including a cache of fetched values.storage.release().await;
which should probably have been the case before this PR.StateDelta<Snapshot>
andStateDelta<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
astria_sequencer_verifiable_cache_hit
counterastria_sequencer_verifiable_cache_miss
counterastria_sequencer_verifiable_cache_item_total
histogramastria_sequencer_non_verifiable_cache_hit
counterastria_sequencer_non_verifiable_cache_miss
counterastria_sequencer_non_verifiable_cache_item_total
histogramRelated Issues
Closes #1435.