apollo_storage,apollo_batcher,apollo_reverts: remove state commitment infos from storage - #15068
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
|
This change is |
PR SummaryMedium Risk Overview The batcher still keeps commitment infos in its in-memory LRU (filled by commitment results and optionally warmed from the committer on startup via Types move to Reviewed by Cursor Bugbot for commit 5f01d2f. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
Artifacts upload workflows: |
c7f4b7d to
79f86ce
Compare
381d66f to
ee9fd48
Compare
79f86ce to
6eafd67
Compare
ee9fd48 to
b600bcf
Compare
6eafd67 to
4d9c466
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4d9c466. Configure here.
| error!("Failed to check state commitment infos existence in storage: {err}"); | ||
| BatcherError::InternalError | ||
| }) | ||
| Ok(self.commitment_manager.recent_state_commitment_infos_cache.contains(&block_number)) |
There was a problem hiding this comment.
Cache miss stalls commitment infos backfill
Medium Severity
get_state_commitment_infos and has_state_commitment_infos now return a cache miss as None/false for any height outside the in-memory LRU, including heights the committer still has. Consensus walks from the cende recorder offset and stops at the first missing height, so a recorder that lags past the cache window receives no infos at all, including heights that are still cached. Blob backfill then cannot catch up.
Reviewed by Cursor Bugbot for commit 4d9c466. Configure here.
There was a problem hiding this comment.
It would organize the top 10 elements in the cache in some random order.
Saying block H is in place H-10. We can add up to 10 more elements to the cache before reading H.
So the cache size of 20 solves it.
4d9c466 to
3ae5f21
Compare
b600bcf to
1d4d2db
Compare
yoavGrs
left a comment
yoavGrs
left a comment
There was a problem hiding this comment.
@yoavGrs made 1 comment.
Reviewable status: 0 of 13 files reviewed, 1 unresolved discussion (waiting on itamar-starkware).
| error!("Failed to check state commitment infos existence in storage: {err}"); | ||
| BatcherError::InternalError | ||
| }) | ||
| Ok(self.commitment_manager.recent_state_commitment_infos_cache.contains(&block_number)) |
There was a problem hiding this comment.
It would organize the top 10 elements in the cache in some random order.
Saying block H is in place H-10. We can add up to 10 more elements to the cache before reading H.
So the cache size of 20 solves it.
… infos from storage
3ae5f21 to
5f01d2f
Compare
2 participants
No description provided.