Uh oh!
There was an error while loading. Please reload this page.
fix: RPC cache prefilling may cause data loss in GC - #7124
Conversation
WalkthroughThe PR removes the ChangesExport API Refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/db/gc/snapshot.rs (1)
51-52: ⚡ Quick winAdd phase-specific context to the new GC export failures.
These two new
?sites will now surface without identifying whether GC failed during head-state recomputation or during snapshot export.Proposed change
+use anyhow::Context as _; use crate::interpreter::VMTrace; use crate::prelude::*; @@ self.chain_follower .state_manager .compute_tipset_state( head_ts.shallow_clone(), crate::state_manager::NO_CALLBACK, VMTrace::NotTraced, ) - .await?;+ .await+ .context("failed to recompute head tipset state before GC snapshot export")?; let _ = crate::chain::export::<Sha256, _>( db, &head_ts, self.recent_state_roots, file, @@ - )- .await?;+ )+ .await+ .context("failed to export GC snapshot")?;As per coding guidelines
**/*.rs: Useanyhow::Result<T>for most operations and add context with.context()when errors occur.Also applies to: 246-267
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/db/gc/snapshot.rs` around lines 51 - 52, The two error-propagation sites in the GC snapshot export flow need phase-specific context: locate the call that recomputes the head state (e.g., the call to recompute_head_state or the function that returns the recomputed head state) and wrap its `?` with `.context("GC failed while recomputing head-state")`; likewise locate the call that performs the snapshot export (e.g., export_snapshot/export or the function that writes the snapshot) and wrap its `?` with `.context("GC failed while exporting snapshot")`; apply the same `.context(...)` additions to the analogous calls in the second block flagged (lines ~246-267) so all GC failure sites report whether the error occurred during head-state recomputation or snapshot export.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/db/gc/snapshot.rs`:
- Around line 51-52: The two error-propagation sites in the GC snapshot export
flow need phase-specific context: locate the call that recomputes the head state
(e.g., the call to recompute_head_state or the function that returns the
recomputed head state) and wrap its `?` with `.context("GC failed while
recomputing head-state")`; likewise locate the call that performs the snapshot
export (e.g., export_snapshot/export or the function that writes the snapshot)
and wrap its `?` with `.context("GC failed while exporting snapshot")`; apply
the same `.context(...)` additions to the analogous calls in the second block
flagged (lines ~246-267) so all GC failure sites report whether the error
occurred during head-state recomputation or snapshot export.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d989c043-60e3-47fb-af00-0cb350faf9e7
📒 Files selected for processing (2)
src/chain/mod.rssrc/db/gc/snapshot.rs
Codecov Report❌ Patch coverage is
Additional details and impacted files
... and 7 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Uh oh!
There was an error while loading. Please reload this page.
Summary of changes
To fix an issue that we occasionally see state mismatches after enabling RPC cache prefilling
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes
Other information and links
Change checklist
Outside contributions
Summary by CodeRabbit
Release Notes
Refactor