Repository navigation
txncache: spill to disk - #11581
txncache: spill to disk#11581
Conversation
┌─ ⚡ PERF · e104cdb vs main@7695701 ─────────────────────────────────
│ SUITE BASELINE NEW Δ
│ replay tps, mainnet 12,887 tps 12,852 tps · -0.27%
│ bench tps, localnet 663,693 tps 661,031 tps · -0.40%
│ snapshot load, testnet 4.86 s 4.85 s · -0.32%
│ epoch boundary, mainnet 146.9 ms 147.7 ms · +0.60%
+│ mem total, mainnet 147.34 GiB 142.84 GiB ▼ -3.06%
+│ mem total, testnet 84.66 GiB 80.15 GiB ▼ -5.32%
│ clean compile, firedancer 3.13 s 3.17 s · +1.47%
│ binary size, firedancer 57.98 MB 58.05 MB · +0.12%
├─────────────────────────────────────────────────────────────────────
@@ 0 REGRESSIONS · 0 WARNINGS · 2 IMPROVED · 6 NOISE @@
└─────────────────────────────────────────────────────────────────────history · 11 pushes ┌─ HISTORY · Δ vs main, per push, newest first ──────────────────────────────────
│ HEAD TPS BENCH SNAP EPOCH MEM·M MEM·T COMPILE BINARY
+│ e104cdb -0.27% -0.40% -0.32% +0.60% -3.06% -5.32% +1.47% +0.12%
!│ 7180090 … -2.46% … … … … … …
!│ 284c5e2 -0.72% -2.92% +0.60% +0.20% -3.06% -5.32% -0.03% +0.12%
!│ 6229280 +0.14% -3.29% -0.37% +1.08% -3.06% -5.32% +0.68% +0.12%
+│ ff965c3 … … … … -3.06% -5.32% … +0.12%
+│ 15f011c -0.88% -0.91% -3.94% +1.02% -3.06% -5.32% -0.27% +0.12%
+│ 889c219 -0.06% -1.02% -0.37% … -2.99% -5.19% -0.54% +0.05%
+│ 53c8c46 -0.04% -0.90% -0.12% … -2.99% -5.19% +0.48% +0.05%
+│ 0c823a4 +0.54% -0.30% -0.05% … -2.66% -4.61% +0.18% +0.05%
+│ 4b6b34a -0.27% -0.22% +0.11% … -2.66% -4.62% +0.62% +0.05%
+│ 0c823a4 -0.01% -0.08% +0.07% … -2.66% -4.62% -1.29% +0.05%
└──────────────────────────────────────────────────────────────────────────────── |
There was a problem hiding this comment.
🔵 Needs a closer look
The concurrent spill path has unresolved descriptor-collision and snapshot-staging failures.
Pull request overview
Adds disk-backed transaction-cache spilling to bound resident memory across Firedancer runtime tiles.
Changes:
- Implements concurrent page spilling and bounded cache sizing.
- Adds spill-file initialization, inheritance, and seccomp permissions.
- Expands unit, fuzz, snapshot, and concurrency coverage.
File summaries
| File | Description |
|---|---|
| src/flamenco/runtime/tests/fd_svm_mini.c | Updates mini-runtime cache construction. |
| src/flamenco/runtime/test_txncache.c | Adds spill and concurrency tests. |
| src/flamenco/runtime/fuzz_txncache_fork_graph.c | Fuzzes bounded spill behavior. |
| src/flamenco/runtime/fd_txncache.h | Extends the public cache API. |
| src/flamenco/runtime/fd_txncache.c | Implements page residency and spill I/O. |
| src/flamenco/runtime/fd_txncache_shmem.h | Adds resident-memory configuration. |
| src/flamenco/runtime/fd_txncache_shmem.c | Sizes and initializes bounded storage. |
| src/flamenco/runtime/fd_txncache_private.h | Adds shared spill metadata. |
| src/discof/restore/test_snapin_tile.c | Updates snapshot-loader cache tests. |
| src/discof/restore/generated/fd_snapin_tile_seccomp.h | Adds generated spill syscall rules. |
| src/discof/restore/fd_snapin_tile.seccomppolicy | Permits transaction-cache spill I/O. |
| src/discof/restore/fd_snapin_tile.c | Connects snapin to the spill file. |
| src/discof/replay/generated/fd_replay_tile_seccomp.h | Adds generated replay spill rules. |
| src/discof/replay/fd_replay_tile.seccomppolicy | Permits replay spill I/O. |
| src/discof/replay/fd_replay_tile.c | Connects replay to the spill file. |
| src/discof/execrp/test_execrp_tile.c | Updates allowed-descriptor tests. |
| src/discof/execrp/generated/fd_execrp_tile_seccomp.h | Adds generated execrp spill rules. |
| src/discof/execrp/fd_execrp_tile.seccomppolicy | Permits execrp spill I/O. |
| src/discof/execrp/fd_execrp_tile.c | Connects execrp to the spill file. |
| src/discof/execle/test_execle_tile.c | Updates allowed-descriptor tests. |
| src/discof/execle/generated/fd_execle_tile_seccomp.h | Adds generated execle spill rules. |
| src/discof/execle/fd_execle_tile.seccomppolicy | Permits execle spill I/O. |
| src/discof/execle/fd_execle_tile.c | Connects execle to the spill file. |
| src/discof/backup/test_snap_roundtrip.c | Tests snapshots with forced spilling. |
| src/discof/backup/generated/fd_snapmk_tile_seccomp.h | Adds generated snapmk spill rules. |
| src/discof/backup/fd_txncache_writer.c | Reads cache pages through pinned accessors. |
| src/discof/backup/fd_snapmk_tile.seccomppolicy | Permits snapmk spill I/O. |
| src/discof/backup/fd_snapmk_tile.c | Connects snapmk to the spill file. |
| src/app/shared/commands/run/run.h | Declares spill-file initialization. |
| src/app/shared/commands/run/run.c | Creates and distributes the spill descriptor. |
| src/app/shared_dev/commands/dev.c | Initializes spilling for development runs. |
| src/app/shared_dev/commands/bench/bench.c | Initializes spilling for benchmarks. |
| src/app/firedancer/topology.c | Configures the resident-memory limit. |
| src/app/firedancer/callbacks.c | Applies bounded cache sizing. |
| src/app/firedancer-dev/commands/snapshot_load.c | Initializes snapshot-load spilling. |
| src/app/firedancer-dev/commands/forktest/forktest.c | Initializes forktest spilling. |
| src/app/firedancer-dev/commands/backtest.c | Initializes backtest spilling. |
Review details
- Files reviewed: 32/37 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
07eec2f to
80605bb
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
It changes concurrent shared state, persistent I/O, descriptor inheritance, and multiple seccomp policies.
Review details
Suppressed comments (4)
src/app/firedancer/topology.c:221
- The fixed 640 MiB cap is smaller than snapin's required staging scratch for supported raised limits.
fd_txncache_snapin_scratch()now exposes only the resident extent, while snapin requires151 * config->limits.max_txn_per_slot * 40bytes (fd_snapin_tile.c:537-542). The checked-in 540M-CU benchmark configuration yields 529,411 transactions/slot, requiring about 3.2 GB, so snapshot loading aborts with “txncache scratch too small.” Size the resident region for this contract or move snapin staging to separately sized storage.
FD_TEST( fd_pod_insertf_ulong( topo->props, 640UL<<20, "obj.%lu.resident_bytes", obj->id ) );
src/flamenco/runtime/fd_txncache.h:113
123465is inside the XDP descriptor range.fd_topo_install_xdpassigns two descriptors per interface starting at 123462 (src/disco/net/fd_net_tile_topo.c:415-420), and the normal physical-interface-plus-loopback setup therefore already uses 123465.initialize_txncache_fd()subsequentlydup2s onto this number, closing the loopback XDP program-link fd and detaching/breaking XDP networking. Use an fd outside the documented123462+XDP range (the existing reservation is documented insrc/flamenco/stakes/fd_stake_delegations.h:75-76).
src/app/firedancer/topology.c:221- The fixed 640 MiB resident budget is not sufficient for the benchmark limits this code supports. With
development.bench.max_cost_per_block,max_txn_per_slotcan be about 529k, whilefd_snapin_tile.c:539-542stagesFD_TXNCACHE_MAX_SLOT_DELTAS * max_txn_per_slot40-byte groups in this scratch region (over 3 GiB), so snapshot loading aborts with “txncache scratch too small” before it can populate the cache. Size the staging scratch independently or reject/disable snapshot loading for limits that exceed the resident budget.
FD_TEST( fd_pod_insertf_ulong( topo->props, 640UL<<20, "obj.%lu.resident_bytes", obj->id ) );
src/app/firedancer/topology.c:221
- The bounded cache now exposes only its resident frames as snapin scratch, but this hard-codes that extent to 640 MiB. That is enough for the default
max_txn_per_slot(the staging buffer is151 * max_txn_per_slot * 40bytes), but development bench limits can raiseconfig->limits.max_txn_per_slotwell above the roughly 111k-entry budget; with the snapshot loader enabled,txncache_staging_scratchthen aborts with “scratch too small” before loading. Derive the resident/scratch budget from the configured limit or provide snapin a separate staging allocation.
FD_TEST( fd_pod_insertf_ulong( topo->props, 640UL<<20, "obj.%lu.resident_bytes", obj->id ) );
- Files reviewed: 32/37 changed files
- Comments generated: 1
- Review effort level: Balanced
80605bb to
4c4f146
Compare
4c4f146 to
91123fa
Compare
There was a problem hiding this comment.
🟡 Changes recommended
A missing shared test helper header prevents multiple test and fuzz targets from compiling.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (1)
src/app/shared/commands/run/run.c:1147
- The new backing file is opened with
O_TRUNCon every run but is never unlinked, unlike the existing stake-delegation spill file immediately above. After a validator spills pages, the allocated.txncachefile remains on disk after normal shutdown (and after crashes), so repeated runs can leave a potentially very large transient cache file behind. Unlink the path after opening/duplicating the descriptor, or otherwise remove it during cleanup.
int txncache_fd = open( path, O_RDWR|O_CREAT|O_TRUNC, S_IRUSR|S_IWUSR );
if( FD_UNLIKELY( -1==txncache_fd ) ) FD_LOG_ERR(( "failed to open transaction cache backing file `%s` (%i-%s)", path, errno, fd_io_strerror( errno ) ));
- Files reviewed: 32/37 changed files
- Comments generated: 1
- Review effort level: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
This WIP changes concurrent cache eviction, persistent I/O, and sandbox permissions across consensus-critical runtime paths.
Review details
- Files reviewed: 32/37 changed files
- Comments generated: 1
- Review effort level: Balanced
91123fa to
4522fff
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
The WIP spans concurrent eviction, disk I/O, FD lifecycle, and sandbox policy, with unresolved correctness and test-execution gaps.
Review details
Suppressed comments (3)
src/app/shared/commands/run/run.c:1147
- This backing file is transient—the cache metadata is rebuilt and the file is truncated on every run—but it remains named after shutdown or a crash. Since spilled pages can consume many GiB, this leaves persistent disk usage with no cleanup path. Unlink it immediately after opening, as
initialize_stake_delegations_fddoes for its spill file, while keeping the descriptor alive.
int txncache_fd = open( path, O_RDWR|O_CREAT|O_TRUNC, S_IRUSR|S_IWUSR );
if( FD_UNLIKELY( -1==txncache_fd ) ) FD_LOG_ERR(( "failed to open transaction cache backing file `%s` (%i-%s)", path, errno, fd_io_strerror( errno ) ));
src/flamenco/runtime/test_txncache.c:214
- This replaces an exact footprint regression check with only an upper bound, so a layout that silently omits the new resident pages/metadata (or otherwise computes the wrong size) can still pass as long as it remains below 1 GiB. Keep a check tied to the resident-page/layout calculation, rather than only this broad limit.
src/flamenco/runtime/test_txncache.c:224 - This changes the exact bench-footprint assertion to a one-sided lower bound. An accidental over-allocation or a changed metadata/layout size above 3,197,642,440 bytes will now pass, even though this test is the sizing guard for the supported 540M benchmark configuration. Retain an upper/lower range or an exact calculation that checks both capacity and bounded memory use.
- Files reviewed: 32/37 changed files
- Comments generated: 2
- Review effort level: Balanced
| test_spill_prefers_free_frame(); | ||
| test_spill_io_failure( 0 ); | ||
| test_spill_io_failure( 1 ); | ||
| test_spill(); | ||
| test_spill_concurrent(); |
4522fff to
37c1340
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
It changes consensus-critical shared-memory concurrency, disk I/O, descriptor lifecycle, and sandbox policies, with unresolved review findings.
Review details
Suppressed comments (1)
src/flamenco/runtime/test_txncache.c:838
- These new spill regressions are only invoked from
test_txncache, butsrc/flamenco/runtime/Local.mk:102still comments outrun-unit-test,test_txncache, so CI never executes them. Re-enable this target or add these checks to an enabled test target before relying on them.
- Files reviewed: 38/43 changed files
- Comments generated: 2
- Review effort level: Balanced
| ulong shmem_sz = fd_txncache_shmem_footprint( 4UL, 2UL*max_txn_per_slot ); | ||
| ulong local_sz = fd_txncache_footprint( 4UL ); | ||
| ulong sz = shmem_sz+local_sz; | ||
| uchar * mem = mmap( NULL, sz, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0 ); |
37c1340 to
3a145d0
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
The WIP changes cross-process cache concurrency, persistent I/O, startup descriptor handling, and multiple seccomp sandboxes.
Review details
Suppressed comments (2)
src/flamenco/runtime/test_txncache.c:133
- Truncating the file to zero does not make this read fail: the resident page for
BLOCKHASH(2)is dirty, so the query first evicts it and writes it at its nonzero page offset, creating a zero-filled sparse hole at offset 0. The subsequentpreadof page 0 therefore succeeds and returns a zero page; its zeroblockcache_nextmakes the query loop forever, so the parentwaitpidhangs instead of validating the failure path. Make page 0 resident and clean beforeftruncate, then query the page 1 transaction after truncation.
src/discof/restore/test_snapin_tile.c:990 - The first invocation maps roughly 3.2 GiB to test the 540M benchmark staging size, but this anonymous mapping omits
MAP_NORESERVE. On hosts with insufficient commit available,mmapcan fail even though the test touches only a few pages; the existing large lazy mapping intest_txncache.c:244usesMAP_NORESERVEfor this reason.
- Files reviewed: 38/43 changed files
- Comments generated: 1
- Review effort level: Balanced
3a145d0 to
b45d035
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The cancellation stress test can wait indefinitely after its worker completes, hanging the test suite.
8 open findings
Stop waiting when cancellation reaches the terminal count · New Bump shared-memory ABI magic when layout changes Reader preference can indefinitely block spill inserts dev1 path leaves txncache spill descriptor uninitialized Dropping the read lock leaves the requested fork unprotected. Before this thread acquires the write… The 529411 case creates an anonymous mapping of roughly 3.2 GiB. WithoutMAP_NORESERVE, hosts… The new spill, disk-error, and concurrent-eviction regressions are added totest_txncache, but… Document snapshot startup failure when cache staging is undersized
1 resolved since last review
🧠 Review effort: Balanced
| } while( atomic_load_explicit( &m.cancel_cnt, memory_order_acquire )<STRESS_CANCEL_CYCLES ); | ||
| /* The serializer holds the txncache lock almost continuously, so a | ||
| mutator sharing its CPU can starve unless we wait for it here. */ | ||
| while( (cancel_cnt=atomic_load_explicit( &m.cancel_cnt, memory_order_acquire ))==cancel_cnt0 ) FD_YIELD(); |
There was a problem hiding this comment.
🟡 Changes recommended
Non-lead snapshot-loader tiles require a descriptor they do not inherit, blocking default sandboxed startup.
9 open findings
Unconditionally listing txncache FD breaks non-lead snapin tiles · New Stop waiting when cancellation reaches the terminal count Bump shared-memory ABI magic when layout changes Reader preference can indefinitely block spill inserts dev1 path leaves txncache spill descriptor uninitialized Dropping the read lock leaves the requested fork unprotected. Before this thread acquires the write… The 529411 case creates an anonymous mapping of roughly 3.2 GiB. WithoutMAP_NORESERVE, hosts… The new spill, disk-error, and concurrent-eviction regressions are added totest_txncache, but… Document snapshot startup failure when cache staging is undersized
🧠 Review effort: Balanced
| out_fds[ out_cnt++ ] = FD_ACCDB_FD_RW; /* accounts db */ | ||
| out_fds[ out_cnt++ ] = FD_STAKE_DELEGATIONS_FD; /* stake delegation disk spill */ | ||
| out_fds[ out_cnt++ ] = ctx->writer.accdb_direct_fd; /* accounts db, O_DIRECT */ | ||
| out_fds[ out_cnt++ ] = FD_TXNCACHE_FD; /* transaction cache spill */ |
There was a problem hiding this comment.
🔵 Needs a closer look
The cancellation stress test can wait indefinitely after its worker finishes, hanging snapshot roundtrip validation.
9 open findings
Unconditionally listing txncache FD breaks non-lead snapin tiles Stop waiting when cancellation reaches the terminal count Bump shared-memory ABI magic when layout changes Reader preference can indefinitely block spill inserts dev1 path leaves txncache spill descriptor uninitialized Dropping the read lock leaves the requested fork unprotected. Before this thread acquires the write… The 529411 case creates an anonymous mapping of roughly 3.2 GiB. WithoutMAP_NORESERVE, hosts… The new spill, disk-error, and concurrent-eviction regressions are added totest_txncache, but… Document snapshot startup failure when cache staging is undersized
🧠 Review effort: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
Validator-critical paging, concurrency, and snapshot serialization changes require human review and runtime validation.
8 open findings
Unconditionally listing txncache FD breaks non-lead snapin tiles Stop waiting when cancellation reaches the terminal count Bump shared-memory ABI magic when layout changes Reader preference can indefinitely block spill inserts dev1 path leaves txncache spill descriptor uninitialized Dropping the read lock leaves the requested fork unprotected. Before this thread acquires the write… The 529411 case creates an anonymous mapping of roughly 3.2 GiB. WithoutMAP_NORESERVE, hosts… The new spill, disk-error, and concurrent-eviction regressions are added totest_txncache, but…
1 resolved since last review
🧠 Review effort: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
Consensus-critical paging and snapshot-locking changes need human review and runtime validation.
8 open findings
Unconditionally listing txncache FD breaks non-lead snapin tiles Stop waiting when cancellation reaches the terminal count Bump shared-memory ABI magic when layout changes Reader preference can indefinitely block spill inserts dev1 path leaves txncache spill descriptor uninitialized Dropping the read lock leaves the requested fork unprotected. Before this thread acquires the write… The 529411 case creates an anonymous mapping of roughly 3.2 GiB. WithoutMAP_NORESERVE, hosts… The new spill, disk-error, and concurrent-eviction regressions are added totest_txncache, but…
🧠 Review effort: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
Consensus-critical locking, disk paging, and snapshot traversal changes require human review and runtime validation.
8 open findings
Unconditionally listing txncache FD breaks non-lead snapin tiles Stop waiting when cancellation reaches the terminal count Bump shared-memory ABI magic when layout changes Reader preference can indefinitely block spill inserts dev1 path leaves txncache spill descriptor uninitialized Dropping the read lock leaves the requested fork unprotected. Before this thread acquires the write… The 529411 case creates an anonymous mapping of roughly 3.2 GiB. WithoutMAP_NORESERVE, hosts… The new spill, disk-error, and concurrent-eviction regressions are added totest_txncache, but…
🧠 Review effort: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
Validator-critical paging and cross-tile snapshot locking require human review and runtime validation before approval.
8 open issues
Unconditionally listing txncache FD breaks non-lead snapin tiles Stop waiting when cancellation reaches the terminal count Bump shared-memory ABI magic when layout changes Reader preference can indefinitely block spill inserts dev1 path leaves txncache spill descriptor uninitialized Dropping the read lock leaves the requested fork unprotected. Before this thread acquires the write… The 529411 case creates an anonymous mapping of roughly 3.2 GiB. WithoutMAP_NORESERVE, hosts… The new spill, disk-error, and concurrent-eviction regressions are added totest_txncache, but…
Review effort: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
Shared paging, locking, and snapshot traversal changes require human review backed by runtime correctness and performance validation.
8 open issues
Unconditionally listing txncache FD breaks non-lead snapin tiles Stop waiting when cancellation reaches the terminal count Bump shared-memory ABI magic when layout changes Reader preference can indefinitely block spill inserts dev1 path leaves txncache spill descriptor uninitialized Dropping the read lock leaves the requested fork unprotected. Before this thread acquires the write… The 529411 case creates an anonymous mapping of roughly 3.2 GiB. WithoutMAP_NORESERVE, hosts… The new spill, disk-error, and concurrent-eviction regressions are added totest_txncache, but…
Review effort: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
Validator-critical paging, lock transitions, and snapshot serialization need human review and targeted runtime validation before approval.
8 open issues
Unconditionally listing txncache FD breaks non-lead snapin tiles Stop waiting when cancellation reaches the terminal count Bump shared-memory ABI magic when layout changes Reader preference can indefinitely block spill inserts dev1 path leaves txncache spill descriptor uninitialized Dropping the read lock leaves the requested fork unprotected. Before this thread acquires the write… The 529411 case creates an anonymous mapping of roughly 3.2 GiB. WithoutMAP_NORESERVE, hosts… The new spill, disk-error, and concurrent-eviction regressions are added totest_txncache, but…
Review effort: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
The shared-memory paging and snapshot-locking redesign needs human concurrency and performance validation, including resolution of snapshot read amplification.
9 open issues
Avoid repeated spilled-page reads during blockcache traversal · New Unconditionally listing txncache FD breaks non-lead snapin tiles Stop waiting when cancellation reaches the terminal count Bump shared-memory ABI magic when layout changes Reader preference can indefinitely block spill inserts dev1 path leaves txncache spill descriptor uninitialized Dropping the read lock leaves the requested fork unprotected. Before this thread acquires the write… The 529411 case creates an anonymous mapping of roughly 3.2 GiB. WithoutMAP_NORESERVE, hosts… The new spill, disk-error, and concurrent-eviction regressions are added totest_txncache, but…
Review effort: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Missing runtime interfaces and incomplete topology, snapshot, and configuration migrations block compilation and startup.
16 open issues
Update topology helper signature and cache footprint property · New Add transaction-cache default and configuration extraction · New Update config JSON rendering, redaction, and layout guard · New Preserve writer fork argument and failure handling · New Migrate transaction-cache APIs and add spill-capable implementation · New Pass txncache descriptor through tile setup and constructor · New Add row staging and corrected multi-slot group indexing · New Unconditionally listing txncache FD breaks non-lead snapin tiles Stop waiting when cancellation reaches the terminal count Bump shared-memory ABI magic when layout changes Reader preference can indefinitely block spill inserts Extract paths.txncache configuration override · New dev1 path leaves txncache spill descriptor uninitialized Dropping the read lock leaves the requested fork unprotected. Before this thread acquires the write… The 529411 case creates an anonymous mapping of roughly 3.2 GiB. WithoutMAP_NORESERVE, hosts… The new spill, disk-error, and concurrent-eviction regressions are added totest_txncache, but…
1 resolved since last review
Review effort: Balanced
| ulong max_txn_per_slot, | ||
| ulong cache_footprint ); |
| } | ||
|
|
||
| CFG_HAS_NON_ZERO( runtime.program_cache_size_mib ); | ||
| CFG_HAS_NON_ZERO( runtime.transaction_cache_size_mib ); |
| char epoch_credits[ PATH_MAX ]; | ||
| char cost_tracker[ PATH_MAX ]; | ||
| char vote_stakes[ PATH_MAX ]; | ||
| char txncache[ PATH_MAX ]; |
| fd_txncache_metrics_t txncache_metrics[1]; | ||
| fd_txncache_metrics_snap( ctx->txncache, txncache_metrics ); |
| # or purge out of cache to the disk to make forward | ||
| # progress. | ||
| uint logfile_fd, uint accounts_fd, uint accounts_direct_fd, uint stake_spill_fd | ||
| uint logfile_fd, uint accounts_fd, uint accounts_direct_fd, uint stake_spill_fd, uint txncache_fd |
| ulong row_sz = txncache_staging_row_sz( TEST_MAX_GROUPS_PER_SLOT ); | ||
| ctx->lead.blockhash_groups = fd_wksp_alloc_laddr( wksp, FD_TXNCACHE_SNAPIN_STAGE_ALIGN, row_sz, 1UL ); |
| if( FD_UNLIKELY( strcmp( config->paths.txncache, "" ) ) ) { | ||
| replace( config->paths.txncache, "{user}", config->user ); | ||
| replace( config->paths.txncache, "{name}", config->name ); |
There was a problem hiding this comment.
🔵 Needs a closer look
Cross-tile paging and snapshot-locking changes require human review and runtime validation before approval.
17 open issues
Add row staging and corrected multi-slot group indexing Pass txncache descriptor through tile setup and constructor Migrate transaction-cache APIs and add spill-capable implementation Preserve writer fork argument and failure handling Update config JSON rendering, redaction, and layout guard Add transaction-cache default and configuration extraction Update topology helper signature and cache footprint property Unconditionally listing txncache FD breaks non-lead snapin tiles Stop waiting when cancellation reaches the terminal count Bump shared-memory ABI magic when layout changes Reader preference can indefinitely block spill inserts Avoid repeated spilled-page reloads during blockcache snapshots · New Extract paths.txncache configuration override dev1 path leaves txncache spill descriptor uninitialized Dropping the read lock leaves the requested fork unprotected. Before this thread acquires the write… The 529411 case creates an anonymous mapping of roughly 3.2 GiB. WithoutMAP_NORESERVE, hosts… The new spill, disk-error, and concurrent-eviction regressions are added totest_txncache, but…
Review effort: Balanced
| iter->bucket++; | ||
| blockcache_iter_bucket_open( iter ); |
There was a problem hiding this comment.
🔵 Needs a closer look
Shared-cache locking and disk-backed snapshot traversal require human review and targeted runtime validation.
17 open issues
Add row staging and corrected multi-slot group indexing Pass txncache descriptor through tile setup and constructor Migrate transaction-cache APIs and add spill-capable implementation Preserve writer fork argument and failure handling Update config JSON rendering, redaction, and layout guard Add transaction-cache default and configuration extraction Update topology helper signature and cache footprint property Unconditionally listing txncache FD breaks non-lead snapin tiles Stop waiting when cancellation reaches the terminal count Bump shared-memory ABI magic when layout changes Reader preference can indefinitely block spill inserts Avoid repeated spilled-page reloads during blockcache snapshots Extract paths.txncache configuration override dev1 path leaves txncache spill descriptor uninitialized Dropping the read lock leaves the requested fork unprotected. Before this thread acquires the write… The 529411 case creates an anonymous mapping of roughly 3.2 GiB. WithoutMAP_NORESERVE, hosts… The new spill, disk-error, and concurrent-eviction regressions are added totest_txncache, but…
Review effort: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
Shared-cache locking, disk eviction, and snapshot changes require human review backed by concurrent runtime and sandbox validation.
17 open issues
Add row staging and corrected multi-slot group indexing Pass txncache descriptor through tile setup and constructor Migrate transaction-cache APIs and add spill-capable implementation Preserve writer fork argument and failure handling Update config JSON rendering, redaction, and layout guard Add transaction-cache default and configuration extraction Update topology helper signature and cache footprint property Unconditionally listing txncache FD breaks non-lead snapin tiles Stop waiting when cancellation reaches the terminal count Bump shared-memory ABI magic when layout changes Reader preference can indefinitely block spill inserts Avoid repeated spilled-page reloads during blockcache snapshots Extract paths.txncache configuration override dev1 path leaves txncache spill descriptor uninitialized Dropping the read lock leaves the requested fork unprotected. Before this thread acquires the write… The 529411 case creates an anonymous mapping of roughly 3.2 GiB. WithoutMAP_NORESERVE, hosts… The new spill, disk-error, and concurrent-eviction regressions are added totest_txncache, but…
Review effort: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
Cross-tile locking, disk-backed eviction, and snapshot serialization affect validator-critical behavior and need human review backed by runtime validation.
17 open issues
Add row staging and corrected multi-slot group indexing Pass txncache descriptor through tile setup and constructor Migrate transaction-cache APIs and add spill-capable implementation Preserve writer fork argument and failure handling Update config JSON rendering, redaction, and layout guard Add transaction-cache default and configuration extraction Update topology helper signature and cache footprint property Unconditionally listing txncache FD breaks non-lead snapin tiles Stop waiting when cancellation reaches the terminal count Bump shared-memory ABI magic when layout changes Reader preference can indefinitely block spill inserts Avoid repeated spilled-page reloads during blockcache snapshots Extract paths.txncache configuration override dev1 path leaves txncache spill descriptor uninitialized Dropping the read lock leaves the requested fork unprotected. Before this thread acquires the write… The 529411 case creates an anonymous mapping of roughly 3.2 GiB. WithoutMAP_NORESERVE, hosts… The new spill, disk-error, and concurrent-eviction regressions are added totest_txncache, but…
Review effort: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
Consensus-sensitive paging, cross-tile locking, and snapshot traversal changes need human review and integration validation.
16 open issues
Add row staging and corrected multi-slot group indexing Pass txncache descriptor through tile setup and constructor Migrate transaction-cache APIs and add spill-capable implementation Update config JSON rendering, redaction, and layout guard Add transaction-cache default and configuration extraction Update topology helper signature and cache footprint property Unconditionally listing txncache FD breaks non-lead snapin tiles Stop waiting when cancellation reaches the terminal count Bump shared-memory ABI magic when layout changes Reader preference can indefinitely block spill inserts Avoid repeated spilled-page reloads during blockcache snapshots Extract paths.txncache configuration override dev1 path leaves txncache spill descriptor uninitialized Dropping the read lock leaves the requested fork unprotected. Before this thread acquires the write… The 529411 case creates an anonymous mapping of roughly 3.2 GiB. WithoutMAP_NORESERVE, hosts… The new spill, disk-error, and concurrent-eviction regressions are added totest_txncache, but…
1 resolved since last review
Review effort: Balanced
| <gauge name="StoreFecCacheUsed" summary="FEC payload slots in use in the store RAM cache" /> | ||
| <gauge name="StoreFecCacheCapacity" summary="FEC payload capacity of the store RAM cache" /> | ||
| <gauge name="StoreFecCachePinned" ambiguous-kind="true" summary="FEC payload views currently pinned against eviction" /> | ||
| <counter name="TxncachePageMiss" summary="Transaction cache lookups under the read lock that found their page not resident and had to retake the write lock" /> |
There was a problem hiding this comment.
Don't you need this on execle/execrp as well?
There was a problem hiding this comment.
🔵 Needs a closer look
Consensus-critical cross-tile paging and snapshot locking require human concurrency review and targeted validation of concurrent eviction and sandboxed O_DIRECT I/O, but runtime testing was unavailable.
16 open issues
Add row staging and corrected multi-slot group indexing Pass txncache descriptor through tile setup and constructor Migrate transaction-cache APIs and add spill-capable implementation Update config JSON rendering, redaction, and layout guard Add transaction-cache default and configuration extraction Update topology helper signature and cache footprint property Unconditionally listing txncache FD breaks non-lead snapin tiles Stop waiting when cancellation reaches the terminal count Bump shared-memory ABI magic when layout changes Reader preference can indefinitely block spill inserts Avoid repeated spilled-page reloads during blockcache snapshots Extract paths.txncache configuration override dev1 path leaves txncache spill descriptor uninitialized Dropping the read lock leaves the requested fork unprotected. Before this thread acquires the write… The 529411 case creates an anonymous mapping of roughly 3.2 GiB. WithoutMAP_NORESERVE, hosts… The new spill, disk-error, and concurrent-eviction regressions are added totest_txncache, but…
Review effort: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
The metric summary can misidentify exclusive-lock fallbacks as disk activity, requiring consistent wording corrections in the metric definitions and header documentation, followed by regeneration of derived files.
17 open issues
Add row staging and corrected multi-slot group indexing Pass txncache descriptor through tile setup and constructor Migrate transaction-cache APIs and add spill-capable implementation Update config JSON rendering, redaction, and layout guard Add transaction-cache default and configuration extraction Update topology helper signature and cache footprint property Unconditionally listing txncache FD breaks non-lead snapin tiles Stop waiting when cancellation reaches the terminal count Bump shared-memory ABI magic when layout changes Reader preference can indefinitely block spill inserts Avoid repeated spilled-page reloads during blockcache snapshots Extract paths.txncache configuration override dev1 path leaves txncache spill descriptor uninitialized Dropping the read lock leaves the requested fork unprotected. Before this thread acquires the write… The 529411 case creates an anonymous mapping of roughly 3.2 GiB. WithoutMAP_NORESERVE, hosts… The new spill, disk-error, and concurrent-eviction regressions are added totest_txncache, but… Clarify stalled_cnt as exclusive-lock fallbacks, not disk activity · New
Review effort: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
Cross-tile paging changes locking, eviction, and snapshot consistency, requiring concurrency stress and Linux O_DIRECT snapshot/replay validation that was unavailable in this review.
16 open issues
Add row staging and corrected multi-slot group indexing Pass txncache descriptor through tile setup and constructor Migrate transaction-cache APIs and add spill-capable implementation Update config JSON rendering, redaction, and layout guard Add transaction-cache default and configuration extraction Update topology helper signature and cache footprint property Unconditionally listing txncache FD breaks non-lead snapin tiles Stop waiting when cancellation reaches the terminal count Bump shared-memory ABI magic when layout changes Reader preference can indefinitely block spill inserts Avoid repeated spilled-page reloads during blockcache snapshots Extract paths.txncache configuration override dev1 path leaves txncache spill descriptor uninitialized Dropping the read lock leaves the requested fork unprotected. Before this thread acquires the write… The 529411 case creates an anonymous mapping of roughly 3.2 GiB. WithoutMAP_NORESERVE, hosts… The new spill, disk-error, and concurrent-eviction regressions are added totest_txncache, but…
1 resolved since last review
Review effort: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
Snapshot serialization repeatedly reloads spilled cache pages while traversing hash chains, causing severe I/O amplification and exclusive-lock contention that can stall snapshot production and transaction processing.
16 open issues
Add row staging and corrected multi-slot group indexing Pass txncache descriptor through tile setup and constructor Migrate transaction-cache APIs and add spill-capable implementation Update config JSON rendering, redaction, and layout guard Add transaction-cache default and configuration extraction Update topology helper signature and cache footprint property Unconditionally listing txncache FD breaks non-lead snapin tiles Stop waiting when cancellation reaches the terminal count Bump shared-memory ABI magic when layout changes Reader preference can indefinitely block spill inserts Avoid repeated spilled-page reloads during blockcache snapshots Extract paths.txncache configuration override dev1 path leaves txncache spill descriptor uninitialized Dropping the read lock leaves the requested fork unprotected. Before this thread acquires the write… The 529411 case creates an anonymous mapping of roughly 3.2 GiB. WithoutMAP_NORESERVE, hosts… The new spill, disk-error, and concurrent-eviction regressions are added totest_txncache, but…
Review effort: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
Disk-backed duplicate detection changes cross-tile synchronization and snapshot traversal, requiring concurrency stress testing and sandboxed O_DIRECT validation beyond this static review.
16 open issues
Add row staging and corrected multi-slot group indexing Pass txncache descriptor through tile setup and constructor Migrate transaction-cache APIs and add spill-capable implementation Update config JSON rendering, redaction, and layout guard Add transaction-cache default and configuration extraction Update topology helper signature and cache footprint property Unconditionally listing txncache FD breaks non-lead snapin tiles Stop waiting when cancellation reaches the terminal count Bump shared-memory ABI magic when layout changes Reader preference can indefinitely block spill inserts Avoid repeated spilled-page reloads during blockcache snapshots Extract paths.txncache configuration override dev1 path leaves txncache spill descriptor uninitialized Dropping the read lock leaves the requested fork unprotected. Before this thread acquires the write… The 529411 case creates an anonymous mapping of roughly 3.2 GiB. WithoutMAP_NORESERVE, hosts… The new spill, disk-error, and concurrent-eviction regressions are added totest_txncache, but…
Review effort: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
Hash-bucket snapshot traversal repeatedly reloads spilled pages when the cache exceeds resident capacity, amplifying disk I/O under an exclusive lock and potentially stalling transaction execution.
16 open issues
Add row staging and corrected multi-slot group indexing Pass txncache descriptor through tile setup and constructor Migrate transaction-cache APIs and add spill-capable implementation Update config JSON rendering, redaction, and layout guard Add transaction-cache default and configuration extraction Update topology helper signature and cache footprint property Unconditionally listing txncache FD breaks non-lead snapin tiles Stop waiting when cancellation reaches the terminal count Bump shared-memory ABI magic when layout changes Reader preference can indefinitely block spill inserts Avoid repeated spilled-page reloads during blockcache snapshots Extract paths.txncache configuration override dev1 path leaves txncache spill descriptor uninitialized Dropping the read lock leaves the requested fork unprotected. Before this thread acquires the write… The 529411 case creates an anonymous mapping of roughly 3.2 GiB. WithoutMAP_NORESERVE, hosts… The new spill, disk-error, and concurrent-eviction regressions are added totest_txncache, but…
Review effort: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
The concurrency documentation incorrectly guarantees that cache misses do not stall other threads, omitting the exclusive-lock fallback that can block all txncache users during spill reads and dirty-page writeback.
17 open issues
Add row staging and corrected multi-slot group indexing Pass txncache descriptor through tile setup and constructor Migrate transaction-cache APIs and add spill-capable implementation Update config JSON rendering, redaction, and layout guard Add transaction-cache default and configuration extraction Update topology helper signature and cache footprint property Unconditionally listing txncache FD breaks non-lead snapin tiles Stop waiting when cancellation reaches the terminal count Bump shared-memory ABI magic when layout changes Reader preference can indefinitely block spill inserts Avoid repeated spilled-page reloads during blockcache snapshots Extract paths.txncache configuration override dev1 path leaves txncache spill descriptor uninitialized Dropping the read lock leaves the requested fork unprotected. Before this thread acquires the write… The 529411 case creates an anonymous mapping of roughly 3.2 GiB. WithoutMAP_NORESERVE, hosts… The new spill, disk-error, and concurrent-eviction regressions are added totest_txncache, but… Document write-locked fallback and cache-miss stalls · New
Review effort: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
The bucket-wise snapshot traversal causes critical read amplification when pages exceed the resident frame budget, repeatedly loading pages under a global write lock and blocking execution tiles.
16 open issues
Add row staging and corrected multi-slot group indexing Pass txncache descriptor through tile setup and constructor Migrate transaction-cache APIs and add spill-capable implementation Update config JSON rendering, redaction, and layout guard Add transaction-cache default and configuration extraction Update topology helper signature and cache footprint property Unconditionally listing txncache FD breaks non-lead snapin tiles Stop waiting when cancellation reaches the terminal count Bump shared-memory ABI magic when layout changes Reader preference can indefinitely block spill inserts Avoid repeated spilled-page reloads during blockcache snapshots Extract paths.txncache configuration override dev1 path leaves txncache spill descriptor uninitialized Dropping the read lock leaves the requested fork unprotected. Before this thread acquires the write… The 529411 case creates an anonymous mapping of roughly 3.2 GiB. WithoutMAP_NORESERVE, hosts… The new spill, disk-error, and concurrent-eviction regressions are added totest_txncache, but…
1 resolved since last review
Review effort: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
Concurrent spill I/O, shared-frame ownership, and snapshot traversal require human concurrency review and runtime race/spill validation that were not available here.
16 open issues
Add row staging and corrected multi-slot group indexing Pass txncache descriptor through tile setup and constructor Migrate transaction-cache APIs and add spill-capable implementation Update config JSON rendering, redaction, and layout guard Add transaction-cache default and configuration extraction Update topology helper signature and cache footprint property Unconditionally listing txncache FD breaks non-lead snapin tiles Stop waiting when cancellation reaches the terminal count Bump shared-memory ABI magic when layout changes Reader preference can indefinitely block spill inserts Avoid repeated spilled-page reloads during blockcache snapshots Extract paths.txncache configuration override dev1 path leaves txncache spill descriptor uninitialized Dropping the read lock leaves the requested fork unprotected. Before this thread acquires the write… The 529411 case creates an anonymous mapping of roughly 3.2 GiB. WithoutMAP_NORESERVE, hosts… The new spill, disk-error, and concurrent-eviction regressions are added totest_txncache, but…
Review effort: Balanced



No description provided.