Skip to content

Implement online index backfill - #7408

Merged
sudo-shashank merged 17 commits into
mainfrom
shashank/index-backfill
Jul 30, 2026
Merged

Implement online index backfill#7408
sudo-shashank merged 17 commits into
mainfrom
shashank/index-backfill

Conversation

@sudo-shashank

@sudo-shashanksudo-shashank commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Summary of changes

Changes introduced in this pull request:

  • Add Forest.IndexBackfill / IndexBackfillStatus / IndexBackfillCancel RPC methods and matching forest-cli index backfill / backfill-status / backfill-cancel commands to back-fill the chain index (Ethereum mappings, events, block blooms) through the running daemon — no node shutdown required, with progress polling and cancellation.
  • Support flexible runs via --from / --to / --n-tipsets, --recompute, --allow-near-head, --no-wait, and resumable runs (--resume) backed by a persisted checkpoint in the settings store.

Reference issue to close (if applicable)

Closes#7340

Other information and links

Change checklist

  • I have performed a self-review of my own code,
  • I have made corresponding changes to the documentation. All new code adheres to the team's documentation standards,
  • I have added tests that prove my fix is effective or that my feature works (if possible),
  • I have made sure the CHANGELOG is up-to-date. All user-facing changes should be reflected in this document.

Outside contributions

  • This pull request is based on an issue that a maintainer has accepted (see Before Opening a Pull Request).
  • I have read and agree to the CONTRIBUTING document.
  • I have read and agree to the AI Policy document. I understand that failure to comply with the guidelines will lead to rejection of the pull request.

Summary by CodeRabbit

Summary of Release Notes

  • New Features

    • Added index backfill support via forest-cli index backfill, with index backfill-status and index backfill-cancel, including resumable runs (--resume), progress reporting, and cancellation.
    • Added RPC methods Forest.IndexBackfill, Forest.IndexBackfillStatus, and Forest.IndexBackfillCancel (documented in the OpenRPC specs).
  • Bug Fixes

    • Timestamp-aware mapping backfills now avoid overwriting newer (or equal) mappings with older data.
  • Documentation

    • Updated CLI and OpenRPC references, including clarified near-head/epoch indexing behavior.
  • Tests

    • Added/updated tests covering checkpoint/resume, cancellation, and cache behavior for uncached executed-tipset loading.

@coderabbitai

coderabbitaiBot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • ✅ Review completed - (🔄 Check again to review again)

Walkthrough

Forest adds daemon-hosted index backfilling with resumable checkpoints, cancellation, progress reporting, RPC methods, CLI commands, shared workload coordination, timestamp-aware mappings, and updated specifications and documentation.

Changes

Index backfill

Layer / File(s)Summary
Indexing primitives
src/chain/store/chain_store.rs, src/state_manager/..., src/daemon/mod.rs
Adds timestamp-aware mapping writes, uncached executed-tipset loading, and explicit timestamp-comparison behavior for backfill versus head indexing.
Backfill engine and workload coordination
src/daemon/db_util.rs, src/db/gc/snapshot.rs, src/ipld/export_status.rs
Adds cancellable, resumable backfill execution with checkpoints, progress state, batching, head-change handling, and shared exclusion with snapshot operations.
Backfill RPC surface
src/rpc/..., docs/openrpc-specs/...
Adds start, status, and cancellation RPCs with range validation, resume selection, status models, registration, and OpenRPC schemas.
CLI integration and reference
src/cli/..., docs/docs/users/reference/..., CHANGELOG.md, scripts/tests/api_compare/docker-compose.yml
Adds forest-cli index commands, polling and progress output, generated CLI reference entries, changelog documentation, and API comparison setup that waits for backfill completion.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
participant CLI
participant Forest.IndexBackfill
participant BACKFILL_STATUS
participant run_backfill
participant ChainStore
CLI->>Forest.IndexBackfill: submit range and options
Forest.IndexBackfill->>run_backfill: spawn guarded backfill
run_backfill->>ChainStore: index messages and persist checkpoint
CLI->>BACKFILL_STATUS: poll progress
CLI->>Forest.IndexBackfillCancel: request cancellation
BACKFILL_STATUS-->>CLI: terminal state and counters
Loading

Suggested reviewers:akaladarshi, eclesiomelojunior

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title matches the main change: implementing online index backfill through daemon RPCs and CLI support.
Linked Issues check✅ PassedThe changes implement daemon-based online index backfilling with CLI access, progress, cancellation, and resumable runs, satisfying #7340.
Out of Scope Changes check✅ PassedThe additional docs, tests, and compose updates support the index backfill feature and do not appear unrelated.
Docstring Coverage✅ PassedDocstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch shashank/index-backfill
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch shashank/index-backfill

Comment @coderabbitai help to get the list of available commands.

@sudo-shashanksudo-shashank changed the title Added online index backfillImplement online index backfillJul 24, 2026
@sudo-shashanksudo-shashank added the RPC requires calibnet RPC checks to run on CI label Jul 24, 2026

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (6)
src/state_manager/tests.rs (1)

384-394: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Assert the expected error, and consider a success-path case.

Both calls are expected to fail, but let _ = hides that: if load_executed_tipset_uncached ever started succeeding here, the test would still pass while no longer covering what its name claims. The cache-untouched invariant is most interesting on the successful load path, which isn't exercised.

♻️ Make the premise explicit
- let _ = sm.load_executed_tipset_uncached(&ts, false).await;+ assert!(sm.load_executed_tipset_uncached(&ts, false).await.is_err());
assert!(sm.cache.get(ts.key()).is_none());
🤖 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/state_manager/tests.rs` around lines 384 - 394, Update the test around
load_executed_tipset_uncached to assert that both current calls return the
expected error instead of discarding their results. Add a successful
uncached-load case with valid executable state, assert it returns successfully,
and verify the tipset-state cache remains untouched after that load.
src/cli/subcommands/index_cmd.rs (1)

41-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Prefer letting clap reject --from --resume rather than silently ignoring --resume.

The RPC handler gives from precedence, so --resume becomes a no-op. #[arg(long, conflicts_with = "from")] surfaces that at parse time instead of in a doc string.

🤖 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/cli/subcommands/index_cmd.rs` around lines 41 - 44, Update the `resume`
argument definition in the CLI command to declare a Clap conflict with the
`from` argument using `conflicts_with = "from"`, and remove the documentation
stating that `--resume` is ignored when `--from` is provided.
src/daemon/db_util.rs (2)

760-804: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy lift

Long-running blocking DB work now runs on the runtime thread inside the daemon.

start_ts.chain(&db) is a synchronous iterator and process_signed_messages performs a whole batch of serialization + settings/eth-mapping writes inline. Offline this was a dedicated process; online it occupies a Tokio worker for the duration of each batch, competing with sync and RPC. Consider wrapping the batch commit (and ideally the tipset load) in tokio::task::spawn_blocking, or at minimum tokio::task::yield_now().await per iteration.

As per coding guidelines: "Use tokio::task::spawn_blocking for CPU-intensive work such as VM execution and cryptography".

🤖 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/daemon/db_util.rs` around lines 760 - 804, Move the synchronous backfill
work in the loop around `start_ts.chain(&state_manager.chain_store().db())` and
`process_signed_messages` off the Tokio worker, using
`tokio::task::spawn_blocking` for each batch commit and, where feasible, tipset
loading/processing. Await the blocking tasks while preserving batch boundaries,
checkpoint writes, cancellation handling, and report updates.

Source: Coding guidelines


861-965: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Good guard/status coverage; run_backfill itself is untested.

The checkpoint/cancel interplay in run_backfill (resume epoch written on cancel, checkpoint cleared on success, batch flush boundary) is the part most likely to regress. A small test over a synthetic chain with RangeSpec::NumTipsets and a pre-cancelled token would cover it.

🤖 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/daemon/db_util.rs` around lines 861 - 965, Extend the test module with a
focused `run_backfill` test using a synthetic chain, `RangeSpec::NumTipsets`,
and a pre-cancelled cancellation token. Verify cancellation preserves or writes
the resume checkpoint, successful execution clears the checkpoint, and
processing honors the batch flush boundary; reuse existing chain/state-manager
helpers and `run_backfill` APIs rather than adding unrelated coverage.
src/rpc/methods/chain.rs (1)

682-839: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider adding unit tests for the new RPC handlers.

None of the three new handlers (IndexBackfill, IndexBackfillStatus, IndexBackfillCancel) have accompanying tests in this diff. src/rpc/methods/sync.rs already has a reusable ctx() test-harness pattern for building an RPCState; a similar test could exercise index_backfill_range_spec's validation branches and the cancel/status handlers against BACKFILL_STATUS at low cost.

🤖 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/rpc/methods/chain.rs` around lines 682 - 839, Add unit tests for the new
IndexBackfill, IndexBackfillStatus, and IndexBackfillCancel handlers using the
reusable ctx() RPCState test harness pattern from sync.rs. Cover all validation
branches in index_backfill_range_spec, and exercise status and cancellation
behavior against BACKFILL_STATUS without introducing unrelated test coverage.
docs/docs/users/reference/cli.md (1)

800-874: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Fenced code blocks missing language specifiers (MD040).

markdownlint flags the 4 new fenced blocks (802, 819, 854, 866) for no language. This matches the pre-existing convention across the rest of this auto-generated file (e.g., the snapshot export block above), since cli.sh emits plain ``` fences around --help output. A real fix would need to update the generator for all sections, not just the new ones.

🤖 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 `@docs/docs/users/reference/cli.md` around lines 800 - 874, Update the
generator that emits CLI help documentation, rather than editing only these four
blocks, so every generated help-output fence includes an appropriate language
specifier. Preserve the existing plain-text rendering and apply the change
consistently across all sections produced by the generator, including the
existing `snapshot export` output.

Source: Linters/SAST tools

🤖 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 `@CHANGELOG.md`:
- Line 34: Update the changelog entry for the index backfill RPC and CLI feature
to reference issue `#7340` using its issue URL instead of pull request `#7408`,
while preserving the existing description.
---
Nitpick comments:
In `@docs/docs/users/reference/cli.md`:
- Around line 800-874: Update the generator that emits CLI help documentation,
rather than editing only these four blocks, so every generated help-output fence
includes an appropriate language specifier. Preserve the existing plain-text
rendering and apply the change consistently across all sections produced by the
generator, including the existing `snapshot export` output.
In `@src/cli/subcommands/index_cmd.rs`:
- Around line 41-44: Update the `resume` argument definition in the CLI command
to declare a Clap conflict with the `from` argument using `conflicts_with =
"from"`, and remove the documentation stating that `--resume` is ignored when
`--from` is provided.
In `@src/daemon/db_util.rs`:
- Around line 760-804: Move the synchronous backfill work in the loop around
`start_ts.chain(&state_manager.chain_store().db())` and
`process_signed_messages` off the Tokio worker, using
`tokio::task::spawn_blocking` for each batch commit and, where feasible, tipset
loading/processing. Await the blocking tasks while preserving batch boundaries,
checkpoint writes, cancellation handling, and report updates.
- Around line 861-965: Extend the test module with a focused `run_backfill` test
using a synthetic chain, `RangeSpec::NumTipsets`, and a pre-cancelled
cancellation token. Verify cancellation preserves or writes the resume
checkpoint, successful execution clears the checkpoint, and processing honors
the batch flush boundary; reuse existing chain/state-manager helpers and
`run_backfill` APIs rather than adding unrelated coverage.
In `@src/rpc/methods/chain.rs`:
- Around line 682-839: Add unit tests for the new IndexBackfill,
IndexBackfillStatus, and IndexBackfillCancel handlers using the reusable ctx()
RPCState test harness pattern from sync.rs. Cover all validation branches in
index_backfill_range_spec, and exercise status and cancellation behavior against
BACKFILL_STATUS without introducing unrelated test coverage.
In `@src/state_manager/tests.rs`:
- Around line 384-394: Update the test around load_executed_tipset_uncached to
assert that both current calls return the expected error instead of discarding
their results. Add a successful uncached-load case with valid executable state,
assert it returns successfully, and verify the tipset-state cache remains
untouched after that load.
🪄 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: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 8bea9ac5-23cd-4518-9ba2-53c038bc95a2

📥 Commits

Reviewing files that changed from the base of the PR and between da5badf and f5d5d8b.

⛔ Files ignored due to path filters (3)
  • src/rpc/snapshots/forest__rpc__tests__rpc__v0.snap is excluded by !**/*.snap
  • src/rpc/snapshots/forest__rpc__tests__rpc__v1.snap is excluded by !**/*.snap
  • src/rpc/snapshots/forest__rpc__tests__rpc__v2.snap is excluded by !**/*.snap
📒 Files selected for processing (17)
  • CHANGELOG.md
  • docs/docs/users/reference/cli.md
  • docs/docs/users/reference/cli.sh
  • docs/openrpc-specs/v0.json
  • docs/openrpc-specs/v1.json
  • docs/openrpc-specs/v2.json
  • src/chain/store/chain_store.rs
  • src/cli/subcommands/index_cmd.rs
  • src/cli/subcommands/mod.rs
  • src/daemon/db_util.rs
  • src/daemon/mod.rs
  • src/db/gc/snapshot.rs
  • src/ipld/export_status.rs
  • src/rpc/methods/chain.rs
  • src/rpc/mod.rs
  • src/state_manager/state_computation.rs
  • src/state_manager/tests.rs
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • filecoin-project/lotus(manual)

Comment threadCHANGELOG.md Outdated
@sudo-shashank
sudo-shashank marked this pull request as ready for review July 27, 2026 10:56
@sudo-shashank
sudo-shashank requested a review from a team as a code ownerJuly 27, 2026 10:56
@sudo-shashank
sudo-shashank requested review from akaladarshi and hanabi1224 and removed request for a teamJuly 27, 2026 10:56

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/db/gc/snapshot.rs (1)

185-190: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Do not use is_running() as the backfill/GC coordination primitive.

This is only a status check: backfill can observe false immediately before gc_once() claims the run, allowing both workloads to overlap. Use a shared guard/lock for admission and retain this accessor for observability only.

🤖 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 185 - 190, Replace uses of
SnapshotGc::is_running() for backfill/GC admission with a shared guard or lock
that atomically coordinates entry between backfill and gc_once(). Retain
is_running() solely as an observability status accessor, and ensure neither
workload can begin while the other holds the coordination guard.
🧹 Nitpick comments (1)
src/db/gc/snapshot.rs (1)

220-230: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add context when starting the export guard.

The new error boundary propagates guard-start failures without contextual information, making coordination failures harder to diagnose.

As per coding guidelines, add .context() when errors occur.

Suggested fix
- let chain_export_guard = ChainExportGuard::try_start_export(ChainExportKind::SnapshotGc)?;+ let chain_export_guard =+ ChainExportGuard::try_start_export(ChainExportKind::SnapshotGc)+ .context("failed to start snapshot GC export")?;
🤖 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 220 - 230, Add contextual error
information to the ChainExportGuard::try_start_export call in export_snapshot,
using anyhow’s .context() before propagating the guard-start failure. Keep the
existing export, cleanup, and error-handling flow unchanged.

Source: Coding guidelines

🤖 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.
Outside diff comments:
In `@src/db/gc/snapshot.rs`:
- Around line 185-190: Replace uses of SnapshotGc::is_running() for backfill/GC
admission with a shared guard or lock that atomically coordinates entry between
backfill and gc_once(). Retain is_running() solely as an observability status
accessor, and ensure neither workload can begin while the other holds the
coordination guard.
---
Nitpick comments:
In `@src/db/gc/snapshot.rs`:
- Around line 220-230: Add contextual error information to the
ChainExportGuard::try_start_export call in export_snapshot, using anyhow’s
.context() before propagating the guard-start failure. Keep the existing export,
cleanup, and error-handling flow unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 1b41c657-f201-4c53-82de-85787a534099

📥 Commits

Reviewing files that changed from the base of the PR and between f5d5d8b and 3b898c5.

📒 Files selected for processing (2)
  • CHANGELOG.md
  • src/db/gc/snapshot.rs
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • filecoin-project/lotus(manual)
🚧 Files skipped from review as they are similar to previous changes (1)
  • CHANGELOG.md

@codecov

codecovBot commented Jul 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 37.11152% with 344 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.16%. Comparing base (e22f29f) to head (d4c79a1).
✅ All tests successful. No failed tests found.

Files with missing linesPatch %Lines
src/daemon/db_util.rs51.29%150 Missing and 1 partial ⚠️
src/rpc/methods/chain.rs0.00%105 Missing ⚠️
src/cli/subcommands/index_cmd.rs0.00%69 Missing ⚠️
src/chain/store/chain_store.rs76.31%8 Missing and 1 partial ⚠️
src/db/gc/snapshot.rs11.11%8 Missing ⚠️
src/daemon/mod.rs0.00%1 Missing ⚠️
src/state_manager/state_computation.rs91.66%0 Missing and 1 partial ⚠️
Additional details and impacted files
Files with missing linesCoverage Δ
src/cli/subcommands/mod.rs8.33% <ø> (ø)
src/ipld/export_status.rs98.28% <100.00%> (+0.02%)⬆️
src/rpc/mod.rs94.62% <ø> (ø)
src/daemon/mod.rs24.28% <0.00%> (ø)
src/state_manager/state_computation.rs78.40% <91.66%> (+3.40%)⬆️
src/db/gc/snapshot.rs31.03% <11.11%> (-2.20%)⬇️
src/chain/store/chain_store.rs73.69% <76.31%> (+1.01%)⬆️
src/cli/subcommands/index_cmd.rs0.00% <0.00%> (ø)
src/rpc/methods/chain.rs48.55% <0.00%> (-4.62%)⬇️
src/daemon/db_util.rs54.88% <51.29%> (+0.44%)⬆️

... and 15 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e22f29f...d4c79a1. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@sudo-shashank
sudo-shashank marked this pull request as draft July 27, 2026 11:19
@sudo-shashank

Copy link
Copy Markdown
ContributorAuthor

@hanabi1224@akaladarshi awaiting your review on this

Comment threadsrc/daemon/db_util.rs Outdated
Comment threadsrc/daemon/db_util.rs Outdated
Comment threadsrc/daemon/db_util.rs Outdated
Comment threadsrc/daemon/db_util.rs Outdated
Comment threadsrc/daemon/db_util.rs Outdated
Comment threadsrc/daemon/db_util.rs Outdated
Comment threadsrc/cli/subcommands/index_cmd.rs

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/daemon/db_util.rs (2)

780-858: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Re-check cancellation before successful cleanup.

Cancellation after Line 773—especially while processing the final tipset—still commits, clears the checkpoint, and is reported as success. Re-check the token after the final flush and before re-indexing/clearing the checkpoint.

Proposed fix
 batch.clear();
+ if cancel.is_cancelled() {+ report.cancelled = true;+ }+
// Re-index tipsets applied during the walk so the canonical mapping wins.
if !report.cancelled {
🤖 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/daemon/db_util.rs` around lines 780 - 858, Re-check the cancellation
token after the final trailing-batch flush and before the re-indexing and
cleanup branches in the backfill flow. Ensure cancellation discovered there
follows the cancelled path, preserves the resume checkpoint, records
cancellation, and avoids clearing the checkpoint or reporting success; only
perform re-indexing and successful cleanup when the refreshed cancellation state
is false.

832-835: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Fail the backfill if the head-change receiver lags.Lagged(n) means some head changes were dropped, and this loop only replays the remaining changes.applies; continuing and clearing the checkpoint can leave older mappings in place. Abort with the checkpoint intact, or rescan the missed range before reporting success.

🤖 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/daemon/db_util.rs` around lines 832 - 835, Update the head-change
listener loop handling in the backfill flow so Err(TryRecvError::Lagged(n))
aborts the backfill instead of logging and continuing. Preserve the checkpoint
when this occurs, ensuring the operation cannot report success or clear progress
after dropped head changes; alternatively, rescan the missed range before
completing successfully.
🧹 Nitpick comments (1)
src/daemon/db_util.rs (1)

735-744: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add context to backfill failures. The ? paths in run_backfill should include the epoch and operation (load_required_tipset_by_height, process_ts, process_signed_messages, and checkpoint writes) so RPC errors are actionable.

🤖 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/daemon/db_util.rs` around lines 735 - 744, Update run_backfill error
propagation so each fallible operation adds contextual information before
returning: include the relevant epoch and operation name for
load_required_tipset_by_height, process_ts, process_signed_messages, and
checkpoint writes. Preserve the existing error flow while ensuring RPC failures
identify which operation and epoch failed.

Source: Coding guidelines

🤖 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 `@scripts/tests/api_compare/docker-compose.yml`:
- Around line 122-123: Update the script containing the FULLNODE_API_INFO export
to prevent xtrace from logging the token: disable xtrace for the entire script
or at least before loading and exporting the token, while preserving errexit and
pipefail behavior.
---
Outside diff comments:
In `@src/daemon/db_util.rs`:
- Around line 780-858: Re-check the cancellation token after the final
trailing-batch flush and before the re-indexing and cleanup branches in the
backfill flow. Ensure cancellation discovered there follows the cancelled path,
preserves the resume checkpoint, records cancellation, and avoids clearing the
checkpoint or reporting success; only perform re-indexing and successful cleanup
when the refreshed cancellation state is false.
- Around line 832-835: Update the head-change listener loop handling in the
backfill flow so Err(TryRecvError::Lagged(n)) aborts the backfill instead of
logging and continuing. Preserve the checkpoint when this occurs, ensuring the
operation cannot report success or clear progress after dropped head changes;
alternatively, rescan the missed range before completing successfully.
---
Nitpick comments:
In `@src/daemon/db_util.rs`:
- Around line 735-744: Update run_backfill error propagation so each fallible
operation adds contextual information before returning: include the relevant
epoch and operation name for load_required_tipset_by_height, process_ts,
process_signed_messages, and checkpoint writes. Preserve the existing error flow
while ensuring RPC failures identify which operation and epoch failed.
🪄 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: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 67b51709-50c5-4bbd-9c0c-b58cfb7f63cc

📥 Commits

Reviewing files that changed from the base of the PR and between 7cf5d3a and 7cdf5c3.

⛔ Files ignored due to path filters (2)
  • src/rpc/snapshots/forest__rpc__tests__rpc__v0.snap is excluded by !**/*.snap
  • src/rpc/snapshots/forest__rpc__tests__rpc__v1.snap is excluded by !**/*.snap
📒 Files selected for processing (7)
  • docs/docs/users/reference/cli.md
  • docs/openrpc-specs/v0.json
  • docs/openrpc-specs/v1.json
  • scripts/tests/api_compare/docker-compose.yml
  • src/cli/subcommands/index_cmd.rs
  • src/daemon/db_util.rs
  • src/rpc/methods/chain.rs
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • filecoin-project/lotus(manual)
🚧 Files skipped from review as they are similar to previous changes (5)
  • src/cli/subcommands/index_cmd.rs
  • docs/docs/users/reference/cli.md
  • docs/openrpc-specs/v1.json
  • docs/openrpc-specs/v0.json
  • src/rpc/methods/chain.rs

Comment threadscripts/tests/api_compare/docker-compose.yml Outdated

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/daemon/db_util.rs (1)

846-857: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Linearize cancellation with successful completion.

A cancellation accepted after the final loop check can leave report.cancelled false, clear the checkpoint, and then report Succeeded. The caller is told cancellation succeeded but cannot resume. Atomically claim completion versus cancellation; if cancellation wins, retain a checkpoint and publish Cancelled.

🤖 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/daemon/db_util.rs` around lines 846 - 857, Update the completion path
around report.cancelled so final completion and cancellation are claimed
atomically, preventing a late cancellation from being reported as successful. If
cancellation wins, persist the backfill checkpoint through
write_backfill_checkpoint, call guard.record_cancelled, and publish Cancelled;
only clear_backfill_checkpoint and publish Succeeded after completion wins.
🧹 Nitpick comments (1)
src/daemon/db_util.rs (1)

780-789: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add the failing tipset epoch to propagated errors.

Wrap this call with .with_context(|| format!("indexing tipset at epoch {}", ts.epoch())) so the persisted failure status identifies the failing tipset. As per coding guidelines, “add context with .context() when errors occur.”

🤖 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/daemon/db_util.rs` around lines 780 - 789, Update the process_ts call in
the tipset indexing flow to attach context with the failing tipset’s epoch using
with_context before awaiting and propagating the error. Preserve the existing
ProcessOutcome handling and ensure the added context identifies the operation as
indexing that tipset.

Source: Coding guidelines

🤖 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.
Outside diff comments:
In `@src/daemon/db_util.rs`:
- Around line 846-857: Update the completion path around report.cancelled so
final completion and cancellation are claimed atomically, preventing a late
cancellation from being reported as successful. If cancellation wins, persist
the backfill checkpoint through write_backfill_checkpoint, call
guard.record_cancelled, and publish Cancelled; only clear_backfill_checkpoint
and publish Succeeded after completion wins.
---
Nitpick comments:
In `@src/daemon/db_util.rs`:
- Around line 780-789: Update the process_ts call in the tipset indexing flow to
attach context with the failing tipset’s epoch using with_context before
awaiting and propagating the error. Preserve the existing ProcessOutcome
handling and ensure the added context identifies the operation as indexing that
tipset.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 50a79d74-bcb5-448f-9624-38ee99835ca3

📥 Commits

Reviewing files that changed from the base of the PR and between 7cf5d3a and d4c79a1.

⛔ Files ignored due to path filters (2)
  • src/rpc/snapshots/forest__rpc__tests__rpc__v0.snap is excluded by !**/*.snap
  • src/rpc/snapshots/forest__rpc__tests__rpc__v1.snap is excluded by !**/*.snap
📒 Files selected for processing (7)
  • docs/docs/users/reference/cli.md
  • docs/openrpc-specs/v0.json
  • docs/openrpc-specs/v1.json
  • scripts/tests/api_compare/docker-compose.yml
  • src/cli/subcommands/index_cmd.rs
  • src/daemon/db_util.rs
  • src/rpc/methods/chain.rs
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • filecoin-project/lotus(manual)
🚧 Files skipped from review as they are similar to previous changes (5)
  • src/cli/subcommands/index_cmd.rs
  • docs/openrpc-specs/v0.json
  • docs/docs/users/reference/cli.md
  • docs/openrpc-specs/v1.json
  • src/rpc/methods/chain.rs

@sudo-shashank
sudo-shashank added this pull request to the merge queueJul 30, 2026
Merged via the queue into main with commit 256a920Jul 30, 2026
53 of 54 checks passed
@sudo-shashank
sudo-shashank deleted the shashank/index-backfill branch July 30, 2026 06:53
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

RPCrequires calibnet RPC checks to run on CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow online index backfilling

2 participants

@sudo-shashank@hanabi1224