fix(node): implement reconciliation sweep as durability backstop (#218)#221
fix(node): implement reconciliation sweep as durability backstop (#218)#221Gravirei wants to merge 1 commit into
Conversation
github-actions
Bot
added
the
needs-tests
|
Thanks for the contribution. A couple of things will help us review this faster:
See CONTRIBUTING.md. Update the PR and these notes will clear automatically. |
|
Warning Review limit reached
Next review available in: 54 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds an hourly, batched reconciliation worker with cooperative shutdown, startup wiring, and gap metrics. It scans eligible repository objects, fills missing IPFS/Pinata pins, and optionally encrypts path-scoped data and anchors manifests to IRYS. ChangesReconciliation sweep
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ReconciliationWorker
participant Database
participant LocalRepository
participant IPFS
participant IRYS
ReconciliationWorker->>Database: Load repository batch and pinned state
ReconciliationWorker->>LocalRepository: Scan eligible objects
ReconciliationWorker->>IPFS: Pin missing objects and encrypted output
ReconciliationWorker->>IRYS: Anchor encrypted manifest when configured
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
beardthelion
added
crate:node
5c771ec to
b31cf88
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
crates/gitlawb-node/src/reconciliation.rs (1)
25-32: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winFirst reconciliation pass waits a full hour after startup.
The sweep exists as a crash/restart backstop, but the loop always sleeps
SWEEP_INTERVAL_SECS(1h) before the first pass. A crash/restart is precisely when unfilled gaps are most likely, so the backstop is least effective right when it's needed most.♻️ Run one pass immediately, then fall into the interval
tokio::spawn(async move { let node_seed = *node_keypair.to_seed(); let mut cursor = 0usize; + + // Run one pass shortly after startup so a crash/restart doesn't + // wait a full interval before the backstop kicks in. + run_startup_pass(&db, &config, &http_client, &node_seed, &node_did, &mut cursor).await; loop {🤖 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 `@crates/gitlawb-node/src/reconciliation.rs` around lines 25 - 32, Update the reconciliation loop inside the tokio::spawn block so one sweep executes immediately on startup before waiting for SWEEP_INTERVAL_SECS. Preserve the existing periodic interval behavior for all subsequent passes, including the current cursor and node_seed handling.
🤖 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.
Inline comments:
In `@crates/gitlawb-node/src/reconciliation.rs`:
- Around line 151-157: Replace the per-object awaited db.is_pinned calls in the
repo_gaps loop with one batched query for all object_list SHAs, using an array
membership check against pinned_cids, then compute repo_gaps from the returned
pinned set while preserving the existing treatment of lookup errors as gaps.
- Around line 147-259: Decouple the encrypted reseal and Arweave-anchor flow
from the public-object gap checks in the reconciliation loop. Update the early
exits around repo_gaps so zero public gaps skips only public pinning, metrics,
and related reporting, while the has_path_scoped block still runs to repair
withheld copies and anchor manifests. Preserve the existing behavior for
repositories with public gaps and avoid invoking encrypted resealing when its
existing path/configuration guards are not satisfied.
- Around line 68-77: Update the reconciliation sweep around list_all_repos so
quarantined repositories are excluded before batching and cursor advancement.
Reuse the repository’s established quarantined filtering behavior or predicate,
ensuring batch, pagination, and empty-result handling operate only on eligible
repositories.
---
Nitpick comments:
In `@crates/gitlawb-node/src/reconciliation.rs`:
- Around line 25-32: Update the reconciliation loop inside the tokio::spawn
block so one sweep executes immediately on startup before waiting for
SWEEP_INTERVAL_SECS. Preserve the existing periodic interval behavior for all
subsequent passes, including the current cursor and node_seed handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 53b027bc-082b-4c03-98fb-79aedf72c8d0
📒 Files selected for processing (3)
crates/gitlawb-node/src/main.rscrates/gitlawb-node/src/metrics.rscrates/gitlawb-node/src/reconciliation.rs
b31cf88 to
9352318
Compare
Summary
Implements the periodic reconciliation sweep the replication path already assumes as a durability backstop. Previously, every path that drops a pin or recovery copy (mid-drain panic, node crash/seal, client disconnect at the receive-pack tail) resulted in data loss with no safety net.
Motivation & context
Closes #218
The codebase justified tolerating dropped post-push replication work by pointing at a reconciliation sweep that did not exist. This made "lost forever" literal rather than conservative phrasing, violating the project's stated promise that "once code is pushed to the network, it should not disappear because one server went down."
Kind of change
What changed
crates/gitlawb-node/src/reconciliation.rs (new): Periodic sweep that re-derives the set of objects a repo should have pinned/sealed under current visibility rules
crates/gitlawb-node/src/metrics.rs: Added gitlawb_reconciliation_gaps_found_total and gitlawb_reconciliation_gaps_filled_total counters
crates/gitlawb-node/src/main.rs: Registered reconciliation module and spawned the background sweep task
How a reviewer can verify
cargo check -p gitlawb-node cargo clippy -p gitlawb-node -- -D warnings cargo test -p gitlawb-node -- metrics::testsBefore you request review
cargo test --workspacepasses locally (DB-dependent tests require a running Postgres)cargo clippy --workspace --all-targets -- -D warningsis cleanfix(...))Notes for reviewers
The sweep is intentionally conservative per pass (100 repos, hourly) to avoid competing with the push path for resources. The cursor wraps around so every repo is eventually covered. Encrypted pin re-sealing and Arweave manifest anchoring are best-effort (failures are logged and skipped).
Summary by CodeRabbit
New Features
Monitoring