linera-storage: add in-memory caches for immutable DB reads#6389
Draft
ndr-ds wants to merge 1 commit into
Draft
Conversation
acfde26 to
2822615
Compare
2822615 to
27b6bfe
Compare
27b6bfe to
a02af66
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
DbStoragepays a full Scylla/RocksDB round-trip on every call to four read methods that return permanently immutable data. On busy validators this contributes directly to Scylla read pressure (one node sustained ~7-8s p99 read spikes, with cache storms making it worse).The four uncached paths:
read_certificate_hashes_by_heights—(ChainId, BlockHeight) → CryptoHashread_event_block_heights—EventId → BlockHeightread_events_from_index— per-event bytes (keys still require a DB scan, but values now hit the cache)read_network_description— singleNetworkDescriptionvalueAll four are permanently immutable once written. They are safe to cache indefinitely.
Proposal
Add three new caches to
StorageCaches:block_hash_by_height: Arc<ValueCache<(ChainId, BlockHeight), CryptoHash>>— S3-FIFO bounded, consistent with the existing five cachesevent_block_height: Arc<ValueCache<EventId, BlockHeight>>— samenetwork_description: Arc<OnceLock<NetworkDescription>>— single-value, zero overhead after first readRead path: check-then-populate pattern (same as
read_certificates_raw). Misses batch to DB; hits short-circuit. Write path: caches are populated inwrite_blobs_and_certificateat write time so write-then-read (the common path) is immediately cached.read_events_from_indexstill scans DB for the key list (unavoidable), but now checks the existingeventcache before bulk-reading values.New CLI flags with conservative defaults (1 000 entries), overridden to larger values in the validator
values.yaml:--block-hash-by-height-cache-size(validator: 100 000)--event-block-height-cache-size(validator: 50 000)New Prometheus counters:
READ_BLOCK_HASH_BY_HEIGHT_COUNTER,READ_EVENT_BLOCK_HEIGHT_COUNTER(labeled[CACHE]/[DB]).read_network_descriptiongets[CACHE]/[DB]on the existingREAD_NETWORK_DESCRIPTION_COUNTER.Test Plan
cargo clippyclean on--all-targets --all-featuresand--no-default-featuresread_certificate_hashes_by_heightsandread_events_from_indexfor the full round-tripRelease Plan
testnetbranch, then be released in a validator hotfix.Links