Skip to content

store: two-scan SH extract (unique keys, then fuse-hit postings) - #668

Merged
reardencode merged 7 commits into
masterfrom
store/sh-bucket-extract
Sep 22, 2026
Merged

reardencode merged 7 commits into
masterfrom
store/sh-bucket-extract

Conversation

@rearden-grok

@rearden-grok rearden-grok Bot commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Summary

Replaces the one-pass 24 B unsorted
shards (DONE / SHUNSRT3) with two Class A txout scans. No
SCHEMA_VERSION bump.

  • Pass 1. Each worker owns a contiguous create-fk span and unsized
    identity maps (key16 → pack8, 0 = multi), capped at 1.5 GiB
    (64 B/key estimate). After each 64 k-fk batch, spill the largest shard
    while over budget. Spills are SHKSP01 under keys/NN/ (one writer,
    1-slot queue, tmp+rename, no sync_all). Merge folds those files into
    one map, one walk to scripthash.head/NN (singles inline_one, multis
    Empty) and multi/NN.fuse8, then unlinks keys/NN/. DONE.keys is
    SHKEYS02. Progress is scanned= finished fks.
  • Pass 2. Fuse8 only. Same spans. Fuse-hit creates append to
    key16 → Vec<fk> (80n+8f) and spill as SHPST01 under post/NN/.
    Pack folds one shard, then slot_for_key16 and 2+ bodies. One fk after
    fold is fp_singles. DONE.post is SHPOST02.
  • Resume. No valid DONE.keys (including the previous DONE / 24 B
    NN layout, or keys/NN / post/NN as a file) deletes unsorted and
    restarts pass 1. A spill whose magic is not SHKSP01 / SHPST01 is
    Corrupt — wipe scripthash.unsorted and rematerialize. MphfHead::exists
    after pass 1 is not pack-done.
  • Workers: sh_extract_workers() = min(CPUs, max(1, free RAM / 1.5 GiB)).
    RBITCOIN_SH_MERGE_WORKERS overrides.

Test plan

  • cargo test -p rbitcoin-store --lib -- scripthash
  • cargo clippy -p rbitcoin-store --all-targets -- -D warnings
  • ./scripts/ast-grep.sh
  • Required CI on this PR

Performance

Five commits on top of the extract:

  • .val is patched as one image (1 MiB windows above 512 MiB). A bad slot does not tear the file.
  • Posting fk lists stay strictly increasing. Spill encode, fold, and pack do not sort them again.
  • The folded per-shard identity map is consumed and dropped before fuse8 and BDZ. mix_key16 xors the two key halves.
  • Collect counters flush once per 64 k-fk batch.
  • Pass 2 groups body writes by MPHF slot. The fabricated 32-byte key is gone.

Lookup-path atomics that are still per event are Q-70 in docs/quality.md (add_hit_rank, and the once-per-block / getdata / page counters named there).

@Hero-Gamer

Hero-Gamer commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Hi @reardencode was observing the CI cargon mutants in-diffs test and saw it was running workspace or something, and was talking to my clanker about it, had some tiered idea, but idk maybe he found some other approach which may help. I will post the response here for you and your clanker to see if we come up good appropriate and improved approach!

I hope it is not too confusing! The test seemed to be cancelled after 30 minutes according to the #609 implementation. That why I started asking my clanker what's up.

Below are some convo dump sorry! For the purpose of hoping to brainstorm with you & your AI to find a good improvement forward that fits rbitcoin's testing regime


Convo dump 1:

Observation from mutants-pr run on this PR

I was watching the mutants / mutants-pr (pull_request) job I added in #609 run on this PR.

Log from #668:

Run cargo mutants --workspace --in-diff git.diff -j 2
Found 313 mutants to test
ok Unmutated baseline in 12s build + 2s test
INFO Auto-set test timeout to 20s
...
MISSED ... seal_mphf_from_keys in 15s build + 3s test
Error: The operation was canceled.

Current config in .github/workflows/mutants.yml on my branch (from #609):

mutants-pr:
  timeout-minutes: 30
  - run: cargo mutants --workspace --in-diff git.diff -j 2
    continue-on-error: true
mutants-weekly:
  timeout-minutes: 360
  shard: [0..7]
  - run: cargo mutants --workspace --shard ${{matrix.shard}}/8 -j 2

So two things are happening:

  1. The cancel is a GitHub Actions job timeout, not a cargo-mutants result. Per-mutant timeout is Auto-set test timeout to 20s (5x baseline, min 20s). The 30m cancel is the job ceiling. With 313 mutants * ~19s avg / 2 workers = ~50m of work needed, 30m will always cancel a diff this large before all mutants are evaluated. The MISSED lines before the cancel are real gaps, but the run is incomplete.

  2. It does NOT block the PR. Because mutants-pr is continue-on-error: true, it's advisory. A missed mutant (exit code 2) and even a cancelled job won't make required checks fail. That's intentional from ci: add weekly sharded cargo-mutants (22119) + in-diff PR advisory (follows #593) #609 - PR lane is fast feedback, weekly sharded 360m run is the source of truth.

Recommendation for this repo:

For #668 and similar large store rewrites, 30m is too tight. I don't think we need crate-based hard rules like "consensus = 120m", but workload-based:

  • Bump mutants-pr to timeout-minutes: 60 or 90 - keeps it advisory but lets a 300-mutant store diff finish. Small PRs still finish in 5-10m.
  • Or scope PR job to -p rbitcoin-store instead of --workspace for store-only diffs - cuts build time ~60%.
  • Keep weekly at 360m x 8 shards as is.

Happy to send a small follow-up PR to #609 config if maintainers agree. I'm not an expert on mutation testing policy, just noting that the current 30m ceiling means #668's mutants-pr result should be read as partial, not as pass/fail.


Convo dump 2:

No, the tiers-by-timeout idea we had is not what the best Rust repos do — and your log proves why.

What the industry actually does for cargo mutants is not consensus=120m, store=90m, cli=30m. It's workload-driven sharding.

Here's what I found in reputable setups:

1. PR = --in-diff + advisory + artifact

Upload mutants.out/ as an artifact. Review surviving mutants manually to calibrate expectations. Surviving mutants in changed code fail the PR. Optional: sharding for large PRs
PR: Fast --in-diff testing of only changed code (informational)

2. Sharding combines with --in-diff

Sharding can also combine with --in-diff, again as long as all shards see the same diff.
For example, in GitHub Actions, you could use a matrix job to run multiple shards:

3. The pattern for big diffs like your #668 is not "raise timeout to 90m", it's "shard the PR diff"

Shard pr-diff instead of raising its timeout threaded through to cargo-mutants' own --shard/--sharding round-robin (verified locally: shard 0/4 of this PR's 247-mutant diff finishes in 14 minutes, 0 missed). pr-diff becomes a 4-way matrix

Real world policy from jira-cli:

cargo mutants --in-diff <diff> --shard <k>/8 --sharding slice --jobs 2 --baseline skip --timeout 240
with continue-on-error: true

And for full runs:

With 20 shards, ~23k mutants at ~5.5s each is roughly 1,150 mutants × 5.5s ≈ 1.75 hours per shard — well inside a single GitHub-hosted job's default 6-hour timeout (timeout-minutes: 300 is set explicitly as a backstop).

Plus: Better documentation about -j, with stronger recommendations not to set it too high. So keep -j 2.

Better comment for #668 than our tier-timeout idea:

Keep the tier idea for human triage, but implement it with industry mechanics:

This PR generates 313 mutants in scripthash_materialize.rs. At ~19s each with -j 2 that's ~50m, so timeout-minutes: 30 will always cancel it. That's not a cargo-mutants failure, it's the job cap.

Reputable setups don't solve this by giving consensus 120m and store 90m. They do:

  1. mutants-pr always --in-diff, continue-on-error: true, upload mutants.out
  2. If pre-count > ~120 mutants, escalate PR to a 4-shard matrix --shard k/4 --in-diff instead of raising timeout. That's how reconcile-rs got a 247-mutant diff from 50m+ down to 14m per shard.
  3. Weekly = 8 shards x 360-480m, --baseline skip --timeout 240 --jobs 2

Tiers are still useful, but for priority of review, not timeout:
Tier 1 consensus-breaking and Tier 2 data-corruption like this store PR = survivors must be triaged, Tier 3 CLI = informational.

Proposal: keep PR advisory for now, change mutants-pr to matrix 4 when mutant_count > 150, and keep --in-diff + --baseline skip. That fixes #668 without burning 90m on every small PR.

That is both closer to what cargo-mutants docs recommend and to what you actually observed.

@Hero-Gamer

Hero-Gamer commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Fix for 30m cancel on this PR

mutants-pr found 313 mutants in scripthash_materialize.rs and got cancelled:

Found 313 mutants
MISSED ... in 15s build + 3s test
Error: The operation was canceled.  # 30m job timeout

Root cause: 313 * ~19s / 2 workers = ~50m of work needed, but mutants-pr has timeout-minutes: 30 in .github/workflows/mutants.yml. The continue-on-error: true means it doesn't block the PR, but we get incomplete results.

Since rbitcoin is public, standard runners are free - sharding doesn't cost Rearden, it just trades wall time for parallelism.

Proposed fix - shard the PR diff instead of raising timeout:

Change mutants-pr from single 30m job to 4-way matrix, same as weekly is already 8-way:

mutants-pr:
  strategy:
    matrix:
      shard: [0,1,2,3]
    fail-fast: false
  timeout-minutes: 30
  steps:
    - run: cargo mutants --workspace --in-diff git.diff --shard ${{ matrix.shard }}/4 --sharding slice --jobs 2 --baseline skip --timeout 240
      continue-on-error: true
    - uses: actions/upload-artifact@v4
      with:
        name: mutants-pr-${{ matrix.shard }}
        path: mutants.out
  • Small PR (10 mutants) still finishes in 2-3m per shard
  • This PR (313 mutants) goes from 50m cancelled to ~14m per shard, total compute ~same
  • Keeps -j 2 as docs recommend not to set too high
  • Keeps advisory continue-on-error: true - weekly 8x360m stays source of truth

If you want minimal change, just bump timeout-minutes: 30 to 60 on mutants-pr - that also fixes #668, but sharding is the pattern other Rust repos use for 200+ mutant diffs.


Within Bitcoin specifically, no reputable project shards the PR diff yet - they do the first half of what you're doing.

  • rust-bitcoin/rust-bitcoin - the reference:

This adds a new CI workflow in the rust.yml workflow file to be run alongside tests on PRs and pushes. This CI utilizes the --in-diff flag on cargo-mutants to check for mutations in the source code within the PR diff. This does not replace the current weekly cargo-mutants CI

  • That weekly job was introduced as:

This PR introduces cargo-mutants via a Github weekly workflow, similar to how the formatter job runs

  • rust-bitcoin/rust-bech32 same pattern:

Integrated cargo mutants and a workflow for raising issues weekly for top 10 mutations found

  • payjoin/rust-payjoin copied rust-bitcoin:

This heavily copies the rust-bitcoin implementation of their mutants workflow with tweaks

So Bitcoin Rust projects today are: PR = single --in-diff job, weekly = single full job that files an issue. They don't hit your 313-mutant cancel because their PRs are usually smaller, and they accept weekly as source of truth.

The sharding to fix the 30m cancel is from the general Rust ecosystem - reconcile-rs, jira-cli, fortress-rollback etc doing --shard k/4 --in-diff to turn 50m cancelled into 14m per shard.

If you do 4-way sharded mutants-pr on rbitcoin, you'd be the first Bitcoin project to use that pattern - you'd be ahead of rust-bitcoin itself.

@rearden-grok
rearden-grok Bot force-pushed the store/sh-bucket-extract branch from f46a3f1 to 1aa095b Compare September 21, 2026 06:34
@rearden-grok
rearden-grok Bot force-pushed the store/sh-bucket-extract branch from 1aa095b to 156de27 Compare September 21, 2026 06:36
@Hero-Gamer

Copy link
Copy Markdown
Contributor

Sorry it might depends on how many concurrent jobs running! Idk how many exactly..

But if based on below numbers of concurrent... below might be an idea for improvement. But if concurrent jobs are different to what below says, might have different solution ideas

--

Based on their current 9 concurrent jobs per PR, conditional 4-way, not always 4-way.

Here's why:

  • Today: fmt, deny, clippy, ast-grep, test, windows, macos, coverage = 8 jobs + mutants-pr = 1 = 9 jobs per PR
  • Always 4-way: 8 + 4 = 12 jobs per PR - still under GitHub's 20 free concurrent limit, so 1 PR is fine
  • But 2 PRs at once: 12 x 2 = 24 jobs -> you start queueing. With 3 PRs = 36 jobs queued.

rbitcoin is 38 stars, so you rarely have 2 PRs at once right now, but conditional is still better because:

Best final solution for rbitcoin:

Keep mutants-pr single job but bump timeout to 60m, and auto-escalate to 4-way only when count > 150:

That fixes the cancel, keeps concurrency low for normal work, and matches what rust-bitcoin does today (single --in-diff for PRs, full job weekly) plus the sharding trick from other Rust repos only when needed.

If you want absolute simplest fix to unblock #668 today: just change timeout-minutes: 30 to 60 on mutants-pr and keep it 1 job. That's 9 jobs, no queue impact, and solves #668.

@reardencode

Copy link
Copy Markdown
Owner

Thanks @Hero-Gamer. Yeah, was thinking something along these lines after seeing that some PRs timed out while others didn't. We'll think about what to do with it and possibly revise the overall PR workflow to be a little bit less concurrent to make space for more mutants runners.

@rearden-grok
rearden-grok Bot force-pushed the store/sh-bucket-extract branch from d3a6c81 to 7f8ec53 Compare September 21, 2026 22:41
@rearden-grok rearden-grok Bot changed the title store: two-scan SH extract (unique keys, then windowed postings) store: two-scan SH extract (unique keys, then fuse-hit postings) Sep 21, 2026
Pass 1 keeps a per-worker identity map (key16 to a pack8 word, 0 = multi)
on a contiguous create-fk span, spills SHKSP01 under keys/NN/ at the
1.5 GiB cap, and folds one map into scripthash.head/NN plus multi fuse8.
Pass 2 records fuse hits as SHPST01 fk runs and packs 2+ bodies.
A previous DONE / 24 B NN unsorted tree is wiped and pass 1 restarts.
Record the identity-map spill layout, the 1.5 GiB worker cap, and resume.
Previous SHUNSRT3 unsorted is deleted and pass 1 restarts.
Pass-2 pack rewrote each multi key with its own 8-byte pwrite.
Check every slot first, then patch the val image once (1 MiB windows
above 512 MiB) so a bad slot cannot tear the file.
insert_post_fk maintains a strictly increasing run, so spill encode,
cross-spill fold, and pack no longer clone and sort those vecs.
Seal consumes the per-shard identity map into pack8 records so its
buckets are gone before fuse8 and BDZ. mix_key16 xors the two key
halves directly.
Each extract worker tallies outputs and fuse hits locally and adds them
to the shared atomics after the 64 k-fk batch. Q-70 records the lookup
path counters that are still per event.
Pass 2 no longer invents a 32-byte key to reuse the scripthash bulk
session. The open key is the slot, and the dummy head records are not
kept.
@rearden-grok
rearden-grok Bot force-pushed the store/sh-bucket-extract branch from 0c9bcab to 80f646e Compare September 22, 2026 02:43
@rearden-grok
rearden-grok Bot changed the base branch from store/txfixed to master September 22, 2026 02:43
@reardencode
reardencode merged commit 698177c into master Sep 22, 2026
23 of 41 checks passed
@rearden-grok
rearden-grok Bot deleted the store/sh-bucket-extract branch September 22, 2026 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants