Skip to content

refactor: transaction.rs extraction — PR 1 (module split + FaultInjector) - #76

Merged
Xof merged 4 commits into
mainfrom
feature/transaction-extraction
Jun 23, 2026
Merged

refactor: transaction.rs extraction — PR 1 (module split + FaultInjector)#76
Xof merged 4 commits into
mainfrom
feature/transaction-extraction

Conversation

@Xof

@Xof Xof commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

First PR of the transaction.rs god-module extraction (the last carve-out from the 2026-06-22 fresh-eyes review — SMELL #4). A pure, behavior-preserving refactor: no logic, signature, or on-disk format change. The existing test suite is the oracle.

What's here

  1. Design spec + implementation plan (docs/specs/, docs/plans/) — the 6-step extraction roadmap (file-split → FaultInjector → SlotPacker → FreemapRecycle → CommitProtocol → StagingTxn), each step green, each its own PR.
  2. Task 1 — file-split (224d66c): src/transaction.rs (5,580 lines) → a src/transaction/ directory module by concern (recovery, lifecycle, staging, freemap, packing, savepoints, named_roots, read, mutate, config, stats, tests). mod.rs is now a clean container (struct + Roots + Savepoint + mod decls). Pure code movement — verified by a normalized line-multiset diff showing zero dropped/altered code or comments (every delta is a visibility-prefix bump, the mod tests wrapper, or a cargo fmt reflow). Each concern file gets a //! role header.
  3. Task 2 — FaultInjector (d6618aa): the four #[cfg(test)] Cell fault flags consolidated off the production TransactionManager into one #[cfg(test)] fault: fault::FaultInjector. Module-gated (#[cfg(test)] mod fault;) so it's fully absent in release. Pure access-path change (self.Xself.fault.X); every Cell operation byte-identical.

Verification (the refactor discipline)

The full suite is green at the exact pre-refactor counts — no test behavior changed:

  • cargo test578 passing, 0 failed (273 in the chisel lib units that hold the transaction tests)
  • cargo build --release → clean (proves the cfg(test) fault field/module is absent in release)
  • cargo clippy --workspace --all-targets -- -D warnings → clean
  • cargo fmt --check → clean
  • cd python && maturin develop && pytest119 passing

Not in this PR

Tasks 3–6 (the owned-unit / behavior-unit extractions) each ship as their own PR off updated main, in low-risk → high-risk order.

@Xof
Xof merged commit a459327 into main Jun 23, 2026
18 of 19 checks passed
@Xof
Xof deleted the feature/transaction-extraction branch June 23, 2026 02:18
@github-actions

Copy link
Copy Markdown

🚦 Bench results: PR vs main

✅ No regressions detected

Scenario Mode Δ throughput Worst Δ
document-store chisel-mem -0.6%
document-store chisel-strict -1.0%
document-store redb-strict +0.8%
document-store sqlite-strict +0.1%
mutation-log chisel-mem -1.2%
mutation-log chisel-strict -0.7%
mutation-log redb-strict +0.2%
mutation-log sqlite-strict +0.1%
ycsb-a chisel-mem -0.3%
ycsb-a chisel-strict +0.4%
ycsb-a redb-strict -0.0%
ycsb-a sqlite-strict -1.1%
ycsb-b chisel-mem -0.1%
ycsb-b chisel-strict +0.5%
ycsb-b redb-strict +0.1%
ycsb-b sqlite-strict -1.0%
Per-scenario detail (4 metrics × cells)

document-store

Mode Throughput p50 p95 p99
chisel-mem 24293 ops/s → 24141 ops/s (-0.6%) 6.9 µs → 6.9 µs (-0.1%) 87.2 µs → 87.2 µs (+0.0%) 443.5 µs → 451.9 µs (+1.9%)
chisel-strict 3052 ops/s → 3020 ops/s (-1.0%) 13.0 µs → 13.3 µs (+2.8%) 818.2 µs → 829.9 µs (+1.4%) 1.91 ms → 1.99 ms (+4.0%)
redb-strict 3998 ops/s → 4030 ops/s (+0.8%) 11.9 µs → 11.7 µs (-1.0%) 515.1 µs → 506.0 µs (-1.8%) 1.94 ms → 1.79 ms (-7.6%)
sqlite-strict 5152 ops/s → 5155 ops/s (+0.1%) 14.8 µs → 14.7 µs (-0.6%) 314.0 µs → 313.8 µs (-0.1%) 1.19 ms → 1.21 ms (+1.4%)

mutation-log

Mode Throughput p50 p95 p99
chisel-mem 58952 ops/s → 58273 ops/s (-1.2%) 20.9 µs → 20.9 µs (+0.3%) 24.9 µs → 25.2 µs (+1.3%) 30.2 µs → 32.3 µs (+6.9%)
chisel-strict 1792 ops/s → 1779 ops/s (-0.7%) 297.9 µs → 296.2 µs (-0.6%) 628.6 µs → 628.2 µs (-0.1%) 18.72 ms → 19.85 ms (+6.0%)
redb-strict 1801 ops/s → 1805 ops/s (+0.2%) 156.8 µs → 155.3 µs (-0.9%) 224.6 µs → 222.2 µs (-1.1%) 34.25 ms → 33.96 ms (-0.8%)
sqlite-strict 5123 ops/s → 5126 ops/s (+0.1%) 85.9 µs → 86.5 µs (+0.6%) 248.6 µs → 250.4 µs (+0.7%) 330.2 µs → 335.8 µs (+1.7%)

ycsb-a

Mode Throughput p50 p95 p99
chisel-mem 24128 ops/s → 24065 ops/s (-0.3%) 66.2 µs → 66.5 µs (+0.4%) 86.1 µs → 86.3 µs (+0.2%) 157.7 µs → 157.2 µs (-0.3%)
chisel-strict 2250 ops/s → 2259 ops/s (+0.4%) 284.8 µs → 281.9 µs (-1.0%) 757.0 µs → 736.5 µs (-2.7%) 1.14 ms → 1.03 ms (-9.6%)
redb-strict 2682 ops/s → 2681 ops/s (-0.0%) 123.7 µs → 123.3 µs (-0.3%) 204.8 µs → 205.2 µs (+0.2%) 340.8 µs → 338.5 µs (-0.7%)
sqlite-strict 216435 ops/s → 213999 ops/s (-1.1%) 4.5 µs → 4.6 µs (+1.2%) 6.1 µs → 6.2 µs (+0.5%) 7.2 µs → 7.5 µs (+4.4%)

ycsb-b

Mode Throughput p50 p95 p99
chisel-mem 181978 ops/s → 181880 ops/s (-0.1%) 2.0 µs → 1.8 µs (-7.3%) 67.8 µs → 69.5 µs (+2.5%) 79.2 µs → 80.2 µs (+1.2%)
chisel-strict 22011 ops/s → 22124 ops/s (+0.5%) 3.3 µs → 3.3 µs (-0.1%) 291.2 µs → 286.4 µs (-1.6%) 681.2 µs → 667.1 µs (-2.1%)
redb-strict 27132 ops/s → 27165 ops/s (+0.1%) 2.5 µs → 2.4 µs (-4.0%) 123.2 µs → 121.2 µs (-1.6%) 193.5 µs → 190.7 µs (-1.4%)
sqlite-strict 261105 ops/s → 258579 ops/s (-1.0%) 3.8 µs → 3.8 µs (-0.0%) 5.2 µs → 5.1 µs (-1.7%) 6.2 µs → 6.3 µs (+1.1%)
Generated by chisel-bench-diff at 2026-06-23T02:21:32Z. Compares PR HEAD against main. Never blocks merge — signal, not gate. Thresholds: throughput 5%, p50 5%, p95 10%, p99 10%.

Xof added a commit that referenced this pull request Jun 23, 2026
…ion deferred (I141) (#80)

The 2026-06-22 review's god-module SMELL was worked through four unit
extractions (SlotPacker #77, FreemapRecycle #78, CommitProtocol #79,
FaultInjector #76); the final StagingTxn extraction is deliberately
deferred. The candidate-prepare/install staging vocabulary is shared
across allocate_inner (staging.rs) and update_inner/delete_inner
(mutate.rs), so a context-based extraction cannot be contained to
staging.rs without dragging the delicate mutation paths through a
mechanical wrapper change. Recorded as future work to be done
incrementally if/when those paths are touched, not re-triggered from
the SMELL alone.
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.

1 participant