snapshots: parallel snapin/wr by mjain-jump · Pull Request #11537 · firedancer-io/firedancer

mjain-jump · 2026-09-16T01:04:41Z

No description provided.

github-actions · 2026-09-16T01:05:08Z

 ┌─ ⚡ PERF · bf2a006 vs main@ce1399b ─────────────────────────────────
 │ SUITE                               BASELINE          NEW         Δ
 │ replay tps, mainnet               29,508 tps   29,476 tps  ·  -0.11%
 │ bench tps, localnet              830,810 tps  830,583 tps  ·  -0.03%
 │ snapshot load, testnet               15.11 s      15.05 s  ·  -0.39%
!│ mem total, mainnet                158.40 GiB   158.68 GiB  ▲  +0.18%
!│ mem total, testnet                 89.40 GiB    89.68 GiB  ▲  +0.32%
!│ mem total, ag mainnet             189.91 GiB   190.19 GiB  ▲  +0.15%
!│ mem total, ag testnet             120.91 GiB   121.19 GiB  ▲  +0.23%
 │ clean compile, firedancer             2.78 s       2.84 s  ·  +2.34%
 │ binary size, firedancer             56.20 MB     56.14 MB  ·  -0.10%
 ├─────────────────────────────────────────────────────────────────────
@@ 0 REGRESSIONS · 4 WARNINGS · 0 IMPROVED · 5 NOISE @@
 └─────────────────────────────────────────────────────────────────────

history · 11 pushes

 ┌─ HISTORY · Δ vs main, per push, newest first ───────────────────────────────────────────
 │ HEAD         TPS    BENCH     SNAP    MEM·M    MEM·T     AG·M     AG·T  COMPILE   BINARY
!│ bf2a006   -0.11%   -0.03%   -0.39%   +0.18%   +0.32%   +0.15%   +0.23%   +2.34%   -0.10%
!│ 4245193   +0.28%   +0.22%  -30.04%   +0.17%   +0.30%   +0.14%   +0.22%   +0.40%   -0.10%
!│ 01f9314   -0.66%   -0.04%   -4.91%   +0.17%   +0.30%   +0.14%   +0.22%   -0.32%   -0.10%
!│ feaf379   -2.22%   +0.27%   -5.97%   +0.17%   +0.29%   +0.14%   +0.22%   -0.52%   -0.10%
!│ 698337d   -0.04%   +0.14%  -31.57%   +0.17%   +0.29%   +0.14%   +0.22%   +2.11%   -0.11%
!│ 14f5f12   +0.22%   +0.07%   -8.06%   +0.17%   +0.29%        …        …   +0.27%   -0.11%
!│ dd2ae43   +0.23%        …        …   +0.17%   +0.29%        …        …   -0.81%   -0.11%
!│ a18c325   -0.10%   -0.38%   -4.34%   +0.17%   +0.29%        …        …   +2.09%   -0.11%
!│ 586c0b1        …        …  -14.44%   +0.17%   +0.29%        …        …   +1.13%   -0.11%
!│ ddd7767   -0.13%   +0.16%   -3.67%   +0.17%   +0.29%        …        …   -0.29%   -0.11%
!│ 00a09db   +0.13%   -1.22%   -2.33%   +0.17%   +0.29%        …        …   -2.98%   -0.11%
 └─────────────────────────────────────────────────────────────────────────────────────────

Copilot

🟡 Changes recommended

An unresolved critical incremental-load lifecycle issue and moderate correctness and validation issues remain.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR parallelizes snapshot ingestion and account-database writes across snapin workers, replacing the dedicated snapwr writer.

Changes:

Adds batched pool and accdb snapshot-write APIs.
Updates parser, topology, configuration, metrics, tooling, tests, and seccomp policies.
Adds worker coordination and aggregated progress reporting.

File summaries

File	Reviewed change / final note
`src/util/tmpl/test_pool_para.c`	Batch-pool acquisition and contention tests.
`src/util/tmpl/fd_pool_para.c`	Batch pool acquisition support.
`src/flamenco/accdb/fd_accdb.h`	Snapshot-loading lifecycle and batched-write API contracts.
`src/flamenco/accdb/fd_accdb.c`	Batched writes and concurrent reservations. Moderate (2 votes): reserves `cnt` entries before determining which accounts need new entries, potentially aborting valid near-capacity loads.
`src/flamenco/accdb/fd_accdb_shmem.c`	Shared-memory compaction coordination.
`src/flamenco/accdb/fd_accdb_private.h`	Internal account-database state.
`src/discof/restore/utils/test_ssparse.c`	Snapshot parser batch tests.
`src/discof/restore/utils/fuzz_snapshot_parser.c`	Parser fuzzer updates.
`src/discof/restore/utils/fd_ssparse.h`	Batched parser API definitions.
`src/discof/restore/utils/fd_ssparse.c`	Batched parsing and appendvec ownership.
`src/discof/restore/utils/fd_ssctrl.h`	Snapshot control definitions.
`src/discof/restore/utils/fd_snapin_shmem.h`	Shared snapin worker coordination.
`src/discof/restore/test_snapwr_tile.c`	Snapwr tile test updates.
`src/discof/restore/test_snapin_accdb.c`	Multi-worker accdb integration tests.
`src/discof/restore/Local.mk`	Restore build integration.
`src/discof/restore/generated/fd_snapwr_tile_seccomp.h`	Generated policy for the removed snapwr tile.
`src/discof/restore/generated/fd_snapin_tile_seccomp.h`	Generated snapin worker seccomp filter.
`src/discof/restore/fd_snapwr_tile.seccomppolicy`	Removed snapwr syscall policy.
`src/discof/restore/fd_snapwr_tile.c`	Removed dedicated snapwr writer tile.
`src/discof/restore/fd_snapin_tile.seccomppolicy`	Snapin worker syscall policy. Critical (2 votes; comment on the corresponding tile implementation): incremental loads do not call `fd_accdb_snapshot_load_begin`, leaving lifecycle state and compaction handling incorrect.
`src/discof/restore/fd_snapct_tile.c`	Snapshot control tile updates.
`src/disco/topo/fd_topob.c`	Topology builder updates.
`src/disco/topo/fd_topo.h`	Topology declarations.
`src/disco/metrics/metrics.xml`	Snapshot metric definitions. Nit (1 vote): `accounts_loaded` counts newly inserted accounts only, not duplicates; update the summary or metric behavior.
`src/disco/metrics/generated/fd_metrics_snapwr.h`	Removed snapwr metric identifiers.
`src/disco/metrics/generated/fd_metrics_snapwr.c`	Removed snapwr metric metadata.
`src/disco/metrics/generated/fd_metrics_snapin.h`	Snapin metric identifiers.
`src/disco/metrics/generated/fd_metrics_snapin.c`	Snapin metric metadata.
`src/disco/metrics/generated/fd_metrics_all.h`	Aggregate metric declarations.
`src/disco/metrics/generated/fd_metrics_all.c`	Aggregate metric metadata.
`src/disco/gui/fd_gui.h`	GUI metric interfaces.
`src/disco/gui/fd_gui.c`	Aggregated snapshot progress reporting.
`src/disco/gui/fd_gui_printf.c`	GUI metric formatting.
`src/app/shared/test_config_parse.c`	Configuration parsing tests.
`src/app/shared/fd_config.h`	Worker-count configuration definitions.
`src/app/shared/fd_config.c`	Common configuration validation. Moderate (1 vote): worker counts above `FD_TOPO_MAX_TILE_IN_LINKS` are not rejected early and can abort during snapin initialization.
`src/app/shared/fd_config_parse.c`	Configuration parsing updates.
`src/app/shared/commands/watch/watch.c`	Aggregated snapshot monitoring metrics.
`src/app/shared/commands/run/run.c`	Run-command integration.
`src/app/firedancer/topology.c`	Production topology wiring.
`src/app/firedancer/main.c`	Tile and object registration.
`src/app/firedancer/config/default.toml`	Default parallel-loader settings.
`src/app/firedancer/callbacks.c`	Shared-memory callbacks.
`src/app/firedancer-dev/main.h`	Development tile and object registration.
`src/app/firedancer-dev/commands/snapshot_load.c`	Standalone snapshot-loader topology and monitoring.
`src/app/firedancer-dev/commands/forktest/forktest.c`	Forktest topology updates.
`src/app/firedancer-dev/commands/backtest.c`	Backtest topology updates.
`book/api/websocket.md`	WebSocket API documentation updates.
`book/api/metrics-generated.md`	Generated metrics documentation. Nit (1 vote): repeats the inaccurate duplicate-account claim; regenerate after correcting `metrics.xml`.

Review details

Suppressed comments (3)

book/api/metrics-generated.md:134

This generated metric table repeats the inaccurate claim that snapin_account_loaded includes duplicates, although the implementation increments it only for newly inserted accounts. Regenerate this file after correcting the authoritative summary in src/disco/metrics/metrics.xml.

| <span class="metrics-name">snapin_&#8203;account_&#8203;loaded</span> | gauge | Accounts seen during snapshot loading. Includes duplicates. Resets if snapshot load restarts |

src/app/shared/fd_config.c:483

The shared snapin state and fd_snapin_tile reject worker_cnt > FD_TOPO_MAX_TILE_IN_LINKS, but this validation only checks that the configured count is nonzero. A value above 128 passes config validation and topology construction, then aborts during snapin initialization; reject the upper bound during common config validation so all topology entry points fail early and consistently.

  CFG_HAS_NON_ZERO( layout.snapin_tile_count );

src/disco/metrics/metrics.xml:1738

accounts_loaded is incremented from the accdb loaded result, which counts only newly inserted entries; replacements and ignored stale duplicates are reported in separate counters. The current summary therefore incorrectly says this gauge includes duplicates. Describe it as newly inserted accounts (or change the metric update to count every record).

    <gauge name="AccountLoaded" summary="Accounts seen during snapshot loading. Includes duplicates. Resets if snapshot load restarts" />

Files reviewed: 43/52 changed files
Comments generated: 2
Review effort level: Lite (auto)

Note

Copilot is running an experiment and ran this review at Lite.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot

🟡 Changes recommended

Four unresolved findings remain, including two critical snapshot-loading and account-database correctness issues.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

src/discof/restore/fd_snapin_tile.c:1193

fd_accdb_snapshot_write_batch reports only newly inserted entries through accounts_loaded; replacements and stale duplicates are returned separately. Adding only this value makes the new SNAPIN ACCOUNT_LOADED gauge undercount the accounts seen, contrary to its metric definition and the progress/rate displays that consume it. Include accounts_replaced and accounts_ignored in this gauge (while keeping worker.loaded as the count of new entries).
src/flamenco/accdb/fd_accdb.c:4024
This reserves cnt accmeta objects before Phase 2 determines whether any record needs a new pool entry. In an incremental load, a batch can consist entirely of stale records (or in-place replacements) and therefore require zero new objects, but this call still returns NULL and the FD_TEST aborts when fewer than cnt pool entries are free. Allocate only when the per-record path takes the new-account branch, or otherwise reserve based on the actual number of new records.

Files reviewed: 43/52 changed files
Comments generated: 2
Review effort level: Lite (auto)

Note

Copilot is running an experiment and ran this review at Lite.

Copilot

🟡 Changes recommended

Two unresolved fd_accdb.c issues remain, including a critical rollback defect.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

src/flamenco/accdb/fd_accdb.c:4024

This pre-acquires cnt account-pool elements before the stripe walk, even though many records in the batch may be replacements or ignored records and therefore need no new element. Near the account-capacity limit (and especially with multiple workers), a batch of eight existing accounts can fail this assertion despite requiring zero free account slots, aborting a valid snapshot load. Acquire slots only for records proven to be new (or otherwise size the reservation to the actual new-record count) and handle genuine exhaustion without asserting.

Files reviewed: 43/52 changed files
Comments generated: 1
Review effort level: Lite (auto)

Note

Copilot is running an experiment and ran this review at Lite.

Copilot

🔵 Needs a closer look

Address the incremental-load coordination issue and avoid reserving pool entries for non-inserted records.

Review details

Suppressed comments (2)

src/discof/restore/fd_snapin_tile.c:1654

The incremental branch creates the child fork without calling fd_accdb_snapshot_load_begin, but the shared snapshot_loading flag is what suppresses compaction while snapshot writers update partitions. Since fd_accdb_snapshot_load_end is still called after the load, incremental writes can be compacted concurrently and the load-end tiering assumptions are not established. Start snapshot-load mode in this branch before the worker writers can begin (before or immediately around the child-fork setup).
src/flamenco/accdb/fd_accdb.c:4024
This unconditionally reserves cnt account-pool entries before the code determines whether each record is an insertion, replacement, or ignored duplicate. A valid incremental batch containing only replacements can therefore abort when fewer than cnt pool entries are free, even though it needs no new entries; the same can happen for a mixed batch when the number of inserts is smaller than cnt. Determine the new-entry count under the stripe locks and acquire only that many entries (or otherwise handle pool exhaustion without requiring one slot per input record).

Files reviewed: 43/52 changed files
Comments generated: 0 new
Review effort level: Lite (auto)

Note

Copilot is running an experiment and ran this review at Lite.

Copilot

🟡 Changes recommended

Unresolved critical and moderate findings remain in snapshot lifecycle, stake-cache ordering, concurrent test coverage, and account-pool reservation.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (3)

src/discof/restore/fd_snapin_tile.c:1674

The incremental INIT creates the fork but never calls fd_accdb_snapshot_load_begin, while the DONE path still calls fd_accdb_snapshot_load_end. Consequently incremental workers write with snapshot_loading==0: compaction is not suspended and their layer-0 partitions are not tagged Cold, contrary to the new begin/end contract. Start snapshot-load mode for incremental attempts before any worker can write.

        ctx->lead.accdb_incr_fork_id = fd_accdb_attach_child( ctx->accdb, ctx->lead.accdb_root_fork_id );

src/discof/restore/test_snapin_accdb.c:306

The 9-worker case does not run snapin workers concurrently: dispatch_snapshot invokes process_account_batch synchronously for one selected worker at a time. Thus the new shared appendvec claiming, concurrent account writes, and shared stake updates are not exercised under the scheduling that production uses, so races and ordering bugs can pass this test. Add an actual concurrent worker/racesan test or equivalent interleaving that covers the writer path.
src/flamenco/accdb/fd_accdb.c:4024
acc_pool_acquire_batch reserves cnt entries before this loop determines whether each account is a new insert, replacement, or ignored duplicate. Near the configured account limit, a valid batch containing only replacements/ignored records (or fewer than cnt new records) can have fewer than cnt free pool entries, so this assertion aborts the validator even though no additional entries are required. Classify the records while holding the appropriate stripes and acquire only the entries needed for actual inserts, or otherwise avoid requiring cnt free entries.

Files reviewed: 41/50 changed files
Comments generated: 1
Review effort level: Lite (auto)

Note

Copilot is running an experiment and ran this review at Lite.

Copilot

🔵 Needs a closer look

Three moderate findings and one documentation nit remain unresolved.

Review details

Suppressed comments (4)

src/disco/metrics/metrics.xml:1738

accounts_loaded is incremented from the batch writer's loaded result, which counts only newly inserted records; replacements and ignored duplicates are reported separately. The summary claiming this gauge includes duplicates is therefore inaccurate and will mislead users interpreting snapshot progress.

    <gauge name="AccountLoaded" summary="Accounts seen during snapshot loading. Includes duplicates. Resets if snapshot load restarts" />

src/discof/restore/fd_snapin_tile.c:1675

The incremental path creates snapshot writers but never calls fd_accdb_snapshot_load_begin; only the full path sets snapshot_loading. As a result, compaction remains enabled while incremental fd_accdb_snapshot_write_batch calls run, contrary to the lifecycle contract and the protection used for full loads, so compaction can race bulk writes. Begin the load for incremental attempts as well and keep it paired with the existing snapshot_load_end (including retry cleanup).

        ctx->lead.accdb_incr_fork_id = fd_accdb_attach_child( ctx->accdb, ctx->lead.accdb_root_fork_id );
      }

src/discof/restore/fd_snapin_tile.c:1200

This updates the shared stake cache before the account-database commit decides whether the record is accepted. With parallel workers, records for the same stake account at different slots can complete out of order, so an older record that fd_accdb_snapshot_write_batch later ignores can overwrite the newer delegation; the final cache then depends on worker scheduling. Apply the update only after the record is known to win, or reconcile it from committed records using slot ordering.

      if( FD_UNLIKELY( batch.lamports[ i ] &&
                       !memcmp( batch.owners[ i ], fd_solana_stake_program_id.uc, 32UL ) ) ) {
        snoop_stake_delegation( ctx, (fd_pubkey_t const *)batch.pubkeys[ i ],
                               batch.lamports[ i ], batch.data_lens[ i ],
                               batch.datas[ i ], batch.data_lens[ i ] );
      }

src/flamenco/accdb/fd_accdb.c:4024

acquired is reserved for every input before the code determines whether each account is new. In an incremental snapshot, replacements and higher-slot duplicates reuse an existing entry or are ignored, so a batch with (for example) 8 existing accounts can fail when fewer than 8 pool entries are free even though it needs no new entries; FD_TEST then aborts the loader instead of returning a normal result. Determine the required new entries while holding the relevant stripe locks, or use a reservation/rollback scheme that only consumes entries for new accounts.

Files reviewed: 41/50 changed files
Comments generated: 0 new
Review effort level: Lite (auto)

Note

Copilot is running an experiment and ran this review at Lite.

Copilot

🔵 Needs a closer look

One or more issues must be addressed before approval.

Review details

Suppressed comments (5)

book/api/websocket.md:2057

The GUI still emits live snapin rows and classifies them as RW (fd_gui.c:359 and fd_gui.c:1100), with rows removed only after shutdown. Omitting snapin from both the tile-name examples and the RW list makes this public schema incomplete for the new parallel loader.

| name                        | `string` | Tile kind name, e.g. `execle`, `execrp`, `replay`, `tower`, `rpc`, `resolv`, or `accdb` |
| kind_id                     | `number` | Instance index within this tile kind |
| joiner_type                 | `string` | `RW` if the tile reads and writes accounts (`execle`, `execrp`, `replay`, `tower`, `accdb`), `RO` if it only reads (`rpc`, `resolv`) |

src/disco/metrics/metrics.xml:1738

AccountLoaded is incremented from accounts_loaded, which only counts newly inserted records, so saying duplicates are included is inaccurate. Correct this description and regenerate src/disco/metrics/generated/fd_metrics_snapin.h and book/api/metrics-generated.md so the compiled/API documentation matches the metric.

    <gauge name="AccountLoaded" summary="Accounts seen during snapshot loading. Includes duplicates. Resets if snapshot load restarts" />

src/discof/restore/fd_snapin_tile.c:1199

This updates the shared stake-delegation root for every input record before the account database decides whether that record is the winner. Since snapin workers flush appendvecs independently, an older duplicate can finish after a newer one, and fd_stake_delegations_root_update overwrites unconditionally; the stake cache can then disagree with the account database's highest-slot record. Apply this update only for the committed winner (or preserve/order per-record outcomes before updating the cache).

      if( FD_UNLIKELY( batch.lamports[ i ] &&
                       !memcmp( batch.owners[ i ], fd_solana_stake_program_id.uc, 32UL ) ) ) {
        snoop_stake_delegation( ctx, (fd_pubkey_t const *)batch.pubkeys[ i ],
                               batch.lamports[ i ], batch.data_lens[ i ],
                               batch.datas[ i ], batch.data_lens[ i ] );

src/discof/restore/fd_snapin_tile.c:1674

fd_accdb_snapshot_load_end runs for both full and incremental loads, but snapshot_loading is enabled only in the full branch. Incremental workers therefore leave compaction enabled while fd_accdb_snapshot_write_batch bulk-updates partitions, defeating the lifecycle's compaction exclusion and allowing the compaction tile to race with the load. Start the load lifecycle for the incremental branch as well, before workers begin writing.

      } else {
        /* Create a child fork for incremental writes.  On failure,
           fd_accdb_purge(child) reverts just the incremental changes.
           On success, fd_accdb_advance_root(child) promotes them. */
        ctx->lead.accdb_incr_fork_id = fd_accdb_attach_child( ctx->accdb, ctx->lead.accdb_root_fork_id );

src/flamenco/accdb/fd_accdb.c:4024

This reserves cnt account-pool entries before the batch determines which records are new. A replacement/ignored-heavy batch can therefore abort when fewer than cnt pool slots are free even though it needs no (or fewer) new entries; for example, two same-fork replacements with one free slot fail here. Acquire slots only for the new-entry path (or otherwise reserve the number of actual inserts), while retaining the partial-commit cleanup.

Files reviewed: 41/50 changed files
Comments generated: 0 new
Review effort level: Lite (auto)

Note

Copilot is running an experiment and ran this review at Lite.

Copilot

🔵 Needs a closer look

The critical compaction-coordination issue and two moderate correctness issues remain; concurrent worker coverage is also incomplete.

Review details

Suppressed comments (4)

src/discof/restore/fd_snapin_tile.c:1675

The incremental path never enters snapshot-loading mode. As a result, partitions created by incremental writers are not tagged Cold and background compaction remains enabled while those writers are active, unlike the full-load path. Call fd_accdb_snapshot_load_begin for this branch so it is symmetric with the existing fd_accdb_snapshot_load_end at completion.

        ctx->lead.accdb_incr_fork_id = fd_accdb_attach_child( ctx->accdb, ctx->lead.accdb_root_fork_id );
      }

src/discof/restore/test_snapin_accdb.c:190

The nine-worker case still dispatches each appendvec synchronously (owner = &env->worker[ appendvec_cnt%env->worker_cnt ]), so it never runs workers concurrently. The new shared appendvec claims, concurrent file reservations, and striped account commits are therefore not exercised end-to-end; add a concurrent worker/tile harness that drives the same stream and checks for missing or duplicated accounts.
src/flamenco/accdb/fd_accdb.c:4024
This pre-acquires cnt account-pool elements before the per-key lookup. Near the configured account limit, a batch can contain only stale/replacement entries (or multiple workers can temporarily hold more elements than they will insert), so the pool can have fewer than cnt free elements even though the batch needs no new entries. FD_TEST then aborts the validator instead of processing the snapshot; acquire pool elements only after determining that an entry is new, or otherwise make acquisition track the actual new-entry count.
src/flamenco/accdb/fd_accdb_private.h:387
The snapshot_loading flag is checked here only to suppress future enqueue operations, but the accdb tile still processes any entries already present on the compaction dlists in fd_accdb_background. Such compaction can update account metadata or recycle partitions concurrently with fd_accdb_snapshot_write_batch; the new snapshot stripe locks do not synchronize with that path. The load transition must stop/drain existing compaction work and keep it paused until all snapshot writers finish, rather than only deferring new enqueues.

Files reviewed: 41/50 changed files
Comments generated: 0 new
Review effort level: Lite (auto)

Note

Copilot is running an experiment and ran this review at Lite.

Copilot

🟡 Changes recommended

Unresolved critical and moderate findings affect delegation state, snapshot load lifecycle, compaction, and account-pool reservation safety.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (4)

src/disco/metrics/metrics.xml:1738

AccountLoaded is incremented from the accdb accounts_loaded result, while replacements and ignored duplicates are counted separately. The current summary says this gauge includes duplicates, so dashboards will interpret the metric incorrectly; describe it as inserted accounts (excluding duplicates), or use AccountProcessed for the documented behavior.

    <gauge name="AccountLoaded" summary="Accounts seen during snapshot loading. Includes duplicates. Resets if snapshot load restarts" />

src/discof/restore/fd_snapin_tile.c:1645

This new load flag is not cleared on an aborted attempt: CTRL_FAIL flushes and resets the worker state, but fd_accdb_snapshot_load_end is only called from the successful DONE path. A failed full load therefore suppresses compaction until a later full reset (and indefinitely if no retry), while any deferred threshold sweep is skipped. Add an explicit abort/cleanup lifecycle that runs after all writers have stopped and clears the flag without promoting the failed snapshot.

        fd_accdb_snapshot_load_begin( ctx->accdb );

src/discof/restore/fd_snapin_tile.c:1645

fd_accdb_snapshot_load_begin is only called in the full-snapshot branch, but incremental workers start writing immediately after the else branch. That leaves snapshot_loading clear during incremental loading, so fd_accdb_shmem_try_enqueue_compaction can enqueue layer-0 partitions while concurrent batch writes are filling them; fd_accdb_advance_root is submitted asynchronously before snapshot_load_end runs. Start the load lifecycle for both full and incremental attempts after the full reset/child-fork setup, before publishing the attempt.

        fd_accdb_snapshot_load_begin( ctx->accdb );

src/flamenco/accdb/fd_accdb.c:4023

This reserves cnt account-pool elements before determining which inputs are new. A valid batch can contain existing/repeated/ignored keys and therefore need fewer than cnt new accmeta entries; when the pool has fewer than cnt free elements, acc_pool_acquire_batch returns NULL and this FD_TEST aborts even though the writes could fit. Acquire pool elements only after the stripe lookup confirms a new entry, or otherwise reserve only the number of genuinely new keys.

Files reviewed: 41/50 changed files
Comments generated: 1
Review effort level: Lite (auto)

Note

Copilot is running an experiment and ran this review at Lite.

mmcgee-jump · 2026-09-17T20:02:42Z

+  fd_accdb_t * accdb;
+
+  /* Shared stake data updated by every tile. */
+  fd_stake_delegations_t * stake_delegations;


For later follow up, fold fd_stake_delegations_refresh from replay tile into this snooper

mmcgee-jump · 2026-09-17T20:10:03Z

+  /* Prevent snapshot workers from updating the same hash chain at once.
+     Each lock covers many hash chains to save memory while preserving
+     concurrency. */
+  int snapshot_stripe_locks[ FD_ACCDB_SNAPSHOT_STRIPE_CNT ] __attribute__((aligned(64)));


Shouldn't exist, just CAS on the acc_map

For removal, you need to coordinate with the epoch counters to do RCU

Copilot

🔵 Needs a closer look

Address the accdb pool-exhaustion failure and add the missing snapin WebSocket entries.

Review details

Suppressed comments (2)

book/api/websocket.md:2057

The public API table now has snapin rows, but these examples omit snapin from both the tile-name list and the RW joiner-type list. This contradicts the topology/GUI behavior introduced here, where every snapin tile is registered as an accdb RW consumer; please include snapin in both descriptions.

| name                        | `string` | Tile kind name, e.g. `execle`, `execrp`, `replay`, `tower`, `rpc`, `resolv`, or `accdb` |
| kind_id                     | `number` | Instance index within this tile kind |
| joiner_type                 | `string` | `RW` if the tile reads and writes accounts (`execle`, `execrp`, `replay`, `tower`, `accdb`), `RO` if it only reads (`rpc`, `resolv`) |

src/flamenco/accdb/fd_accdb.c:4023

This pre-acquires cnt pool entries before checking whether any inputs are new. A valid snapshot batch can contain replacements or stale duplicates while fewer than cnt account slots are free (including a full account pool), so acc_pool_acquire_batch returns NULL and FD_TEST aborts instead of processing the batch. Determine the number of entries that actually need allocation while holding the stripe, or acquire entries on demand and return a normal pool-exhaustion error.

Files reviewed: 41/48 changed files
Comments generated: 0 new
Review effort level: Lite (auto)

Note

Copilot is running an experiment and ran this review at Lite.

Copilot

🔵 Needs a closer look

Two moderate account-database issues remain unresolved.

Review details

Suppressed comments (3)

Previously missed (1) — in code that hasn't changed since the last review.

src/flamenco/accdb/fd_accdb.c:191

These deferred add/subtract operations can be flushed by different snapshot joins in either order. If one worker inserts an account and another worker replaces that newly inserted entry before the first worker flushes its stats, the replacement flush subtracts from disk_used_bytes before the add is published and the unsigned metric wraps to a huge value. Publish a single ordered/signed delta or otherwise synchronize these updates so the metric cannot underflow during parallel loading.

book/api/websocket.md:2071

The new topology exposes snapin rows, but these descriptions no longer list snapin among the kinds whose eviction/commit rates are always zero. The FD_GUI_ACCDB_TILE_KIND_SNAPIN case does not populate either counter, so the API documentation is inaccurate for the new rows.

| evicted_per_sec             | `number` | Recent rate at which this tile's commits evicted lines from the cache (always `0` for `RO` tiles) |
| committed_per_sec           | `number` | Recent rate of account version commits (new + overwrite) by this tile (always `0` for `RO` tiles and `accdb`) |

src/flamenco/accdb/fd_accdb.c:4023

This pre-acquires cnt free account-pool entries before determining which inputs are new. A valid batch containing only replacements/ignored duplicates can therefore fail when fewer than cnt pool entries are free (for example, a full snapshot whose unique-account count reaches max_accounts), and FD_TEST aborts even though no new entry is needed. Classify the entries under the stripe locks and acquire only the number of new records, or otherwise handle this shortage without requiring one free record per input.

Files reviewed: 41/48 changed files
Comments generated: 0 new
Review effort level: Lite (auto)

Note

Copilot is running an experiment and ran this review at Lite.

Copilot

🔵 Needs a closer look

Two moderate findings and three documentation nits remain unresolved.

Review details

Suppressed comments (5)

book/api/websocket.md:2057

The parallel loader is an RW accdb joiner, but this public schema's RW list omits snapin, so consumers can misclassify the new per-tile row. Include snapin in the list.

| joiner_type                 | `string` | `RW` if the tile reads and writes accounts (`execle`, `execrp`, `replay`, `tower`, `accdb`), `RO` if it only reads (`rpc`, `resolv`) |

book/api/websocket.md:2063

snapin direct writers do not publish writable-acquire metrics, so this zero-value qualifier is incomplete after replacing snapwr with snapin. Include snapin in the list.

| acquired_writable_per_sec   | `number` | Recent writable acquire rate for this tile, in accounts per second (always `0` for `RO` tiles and `accdb`) |

book/api/websocket.md:2071

The snapin GUI branch only contributes direct disk bytes and leaves eviction/commit counters at zero, but these updated descriptions no longer mention it. Include snapin in both qualifiers.

| evicted_per_sec             | `number` | Recent rate at which this tile's commits evicted lines from the cache (always `0` for `RO` tiles) |
| committed_per_sec           | `number` | Recent rate of account version commits (new + overwrite) by this tile (always `0` for `RO` tiles and `accdb`) |

src/app/shared/fd_config.h:116

This new effective layout setting is not emitted by fd_config_to_json: the renderer's layout object in src/app/shared/fd_config_json.c:351-368 has no snapin_tile_count. The resolved boot-telemetry configuration therefore silently omits the loader parallelism; add the field to that renderer and its JSON coverage.

    uint snapin_tile_count;

src/flamenco/accdb/fd_accdb.c:4023

This pre-acquires cnt pool entries before determining which inputs are inserts. A batch of replacements or ignored duplicates can therefore abort when fewer than cnt entries are free, even though it needs no new entries (common for incremental loads near max_accounts). Acquire an entry only when the locked lookup proves an insertion is needed, and handle exhaustion at that point.

Files reviewed: 41/48 changed files
Comments generated: 0 new
Review effort level: Lite (auto)

Note

Copilot is running an experiment and ran this review at Lite.

Copilot

Copilot review overview

🟡 Changes recommended

Batch writes can fail despite sufficient effective capacity and do not reliably detect same-slot duplicates across batches.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1

Open (1)

Reject duplicate pubkey-slot occurrences across snapshot batches · New

Copilot

Copilot review overview

🟡 Changes recommended

Batch pool preallocation can abort valid writes, and parallel stake updates can publish stale account state.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 · 1

Open (2)

Reject duplicate pubkey-slot occurrences across snapshot batches
Unordered publication deltas can underflow disk usage · New

Copilot AI balanced review requested due to automatic review settings September 16, 2026 01:04

Copilot started reviewing on behalf of mjain-jump September 16, 2026 01:05 View session

Copilot AI reviewed Sep 16, 2026

View reviewed changes

Comment thread src/discof/restore/fd_snapin_tile.c

Comment thread src/flamenco/accdb/fd_accdb.c

Copilot AI review requested due to automatic review settings September 16, 2026 17:00

Copilot started reviewing on behalf of mjain-jump September 16, 2026 17:00 View session

Copilot AI reviewed Sep 16, 2026

View reviewed changes

Comment thread src/discof/restore/fd_snapin_tile.c Outdated

Comment thread src/flamenco/accdb/fd_accdb.c

Copilot AI review requested due to automatic review settings September 16, 2026 17:35

Copilot started reviewing on behalf of mjain-jump September 16, 2026 17:36 View session

Copilot AI reviewed Sep 16, 2026

View reviewed changes

Comment thread src/flamenco/accdb/fd_accdb.c

Copilot AI review requested due to automatic review settings September 16, 2026 17:50

Copilot started reviewing on behalf of mjain-jump September 16, 2026 17:51 View session

Copilot AI reviewed Sep 16, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 16, 2026 18:06

Copilot started reviewing on behalf of mjain-jump September 16, 2026 18:07 View session

Copilot AI reviewed Sep 16, 2026

View reviewed changes

Comment thread src/discof/restore/fd_snapin_tile.c Outdated

Copilot AI review requested due to automatic review settings September 16, 2026 18:23

Copilot started reviewing on behalf of mjain-jump September 16, 2026 18:24 View session

Copilot AI reviewed Sep 16, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 16, 2026 18:38

Copilot started reviewing on behalf of mjain-jump September 16, 2026 18:40 View session

Copilot AI reviewed Sep 16, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 16, 2026 18:56

Copilot started reviewing on behalf of mjain-jump September 16, 2026 18:56 View session

Copilot AI reviewed Sep 16, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 16, 2026 20:15

Copilot started reviewing on behalf of mjain-jump September 16, 2026 20:16 View session

Copilot AI reviewed Sep 16, 2026

View reviewed changes

Comment thread src/discof/restore/fd_snapin_tile.c Outdated

Copilot AI review requested due to automatic review settings September 16, 2026 20:39

Copilot started reviewing on behalf of mjain-jump September 16, 2026 20:40 View session

mmcgee-jump reviewed Sep 17, 2026

View reviewed changes

Comment thread src/app/firedancer/topology.c Outdated

mmcgee-jump reviewed Sep 17, 2026

View reviewed changes

Comment thread src/discof/restore/fd_snapct_tile.c Outdated

mmcgee-jump reviewed Sep 17, 2026

View reviewed changes

Comment thread src/discof/restore/fd_snapin_tile.c Outdated

mmcgee-jump reviewed Sep 17, 2026

View reviewed changes

Comment thread src/discof/restore/fd_snapin_tile.c

mmcgee-jump reviewed Sep 17, 2026

View reviewed changes

Comment thread src/discof/restore/fd_snapin_tile.c Outdated

Copilot AI review requested due to automatic review settings September 17, 2026 21:58

Copilot started reviewing on behalf of mjain-jump September 17, 2026 21:58 View session

Copilot AI reviewed Sep 17, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 17, 2026 23:38

Copilot started reviewing on behalf of mjain-jump September 17, 2026 23:39 View session

Copilot AI reviewed Sep 17, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 17, 2026 23:45

Copilot started reviewing on behalf of mjain-jump September 17, 2026 23:46 View session

Copilot AI reviewed Sep 17, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 19, 2026 06:19

Copilot started reviewing on behalf of mjain-jump September 19, 2026 06:20 View session

Copilot AI reviewed Sep 19, 2026

View reviewed changes

Comment thread src/flamenco/accdb/fd_accdb.c

mjain-jump added 7 commits September 19, 2026 06:42

snapshots: parallel snapin/wr

6640c3d

fix comments

1512df1

rename to next_appendvec_ticket

10379e6

update shmem format

34e7a99

nits

7b0f42e

fix name

6949510

remove stripe lock

bf2a006

Copilot AI review requested due to automatic review settings September 19, 2026 06:55

mjain-jump force-pushed the snap-par-poteto branch from 339e1b1 to bf2a006 Compare September 19, 2026 06:55

Copilot started reviewing on behalf of mjain-jump September 19, 2026 06:56 View session

Copilot AI reviewed Sep 19, 2026

View reviewed changes

Comment thread src/flamenco/accdb/fd_accdb.c

Conversation

mjain-jump commented Sep 16, 2026

Uh oh!

github-actions Bot commented Sep 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (3)

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (2)

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (1)

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Suppressed comments (2)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (3)

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Suppressed comments (4)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Suppressed comments (5)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Suppressed comments (4)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (4)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mmcgee-jump Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mmcgee-jump Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

mmcgee-jump Sep 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Suppressed comments (2)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

github-actions Bot commented Sep 16, 2026 •

edited

Loading