Skip to content

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

Open
rearden-grok[bot] wants to merge 3 commits into
store/txfixedfrom
store/sh-bucket-extract
Open

rearden-grok[bot] wants to merge 3 commits into
store/txfixedfrom
store/sh-bucket-extract

Conversation

@rearden-grok

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

Copy link
Copy Markdown
Contributor

Summary

  • Pass 1 fills per-shard identity maps (key16 → pack8 word; 0 = multi) and spills at 150 MiB to keys/NN/ (SHKSP01 files, first-fk delta singles). Merge folds those spills into one map, one walk to scripthash.head/NN + multi/NN.fuse8, then unlinks keys/NN/.
  • Leftover in-process keys (keys/NN file, SHKEYU02, framed 18 B, wrong magic) refuse — wipe scripthash.unsorted / rematerialize. DONE.keys stays SHKEYS02 as the last-fk marker. No SCHEMA_VERSION bump.
  • Pass 2 is unchanged: fuse-hit 18 B key16‖offset postings, pack rewrites 2+ bodies, skips 1-fk fuse FPs. Stacked on store/txfixed (store: schema 25 txstat.body 8 B/create ULEB #659).

Test plan

  • cargo test -p rbitcoin-store --lib
  • cargo clippy -p rbitcoin-store --lib -- -D warnings
  • CI store jobs on this PR

@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 Bot and others added 2 commits September 20, 2026 23:36
Pass 1 appends 18 B key16||u16 frames to keys/NN (65535-fk windows).
Merge sort-dedups each shard in RAM, writes scripthash.head/NN and
multi/NN.fuse8, then unlinks keys. Pass 2 writes fuse-hit postings;
pack rewrites 2+ bodies and skips 1-fk fuse FPs.

Co-authored-by: Cursor <cursoragent@cursor.com>
Record the RAM-merge-to-head path, leftover SHKEYS01/16 B/SHUNSRT3
restart, and extract worker cap (1.5 GiB).

Co-authored-by: Cursor <cursoragent@cursor.com>
@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.

BDZ and fuse8 do not need key16 order. Spill 150 MiB/shard as SHKSP01
files under keys/NN/ and walk the map once to head+fuse. Leftover
in-process keys refuse.

Co-authored-by: Cursor <cursoragent@cursor.com>
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