fix(test): make the disk-txmap throughput floor an order-of-magnitude gate - #1707
Open
icellan wants to merge 1 commit into
Open
fix(test): make the disk-txmap throughput floor an order-of-magnitude gate#1707icellan wants to merge 1 commit into
icellan wants to merge 1 commit into
Conversation
… gate Backport of the assertion change in 25c0f2c (#1051 / #1058), which release/v0.15 never received. TestDiskTxMap_ExistenceLayerThroughput gates on >1M ops/sec under -race. The CI runners land below that: 970482 and 910122 ops/sec on two consecutive runs of the same commit, so the job fails on every PR targeting this branch. The 1M figure becomes an informational log and the hard assertion drops to a 100k floor, which still fails on an order-of-magnitude regression such as a lock serialising the existence layer, without gating on runner speed. The file is now byte-identical to main. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
|
🤖 Claude Code Review Status: Complete Current Review:
|
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The change is isolated to a test threshold adjustment; the only remaining feedback is a minor in-comment numeric inconsistency.
Pull request overview
This PR backports a test-only change on release/v0.15 to deflake TestDiskTxMap_ExistenceLayerThroughput by converting the previous hard >1M ops/sec assertion (under -race) into an informational target plus a lower hard floor that still detects order-of-magnitude regressions.
Changes:
- Replace the hard
>1M ops/secgate with an informational 1M target log and a hard 100k ops/sec floor. - Improve the inline rationale in the test to document CI runner variability under
-race.
Changed:
- services/blockassembly/subtreeprocessor/disk_tx_map_benchmark_test.go: relax throughput assertion to avoid CI flakes while keeping a regression-detection floor.
Tested:
- Not run here (code review only).
Risks / Notes:
- One comment’s numeric rationale (“~35x below…”) appears inconsistent with the values cited in the same block and should be adjusted for accuracy.
File summaries
| File | Description |
|---|---|
| services/blockassembly/subtreeprocessor/disk_tx_map_benchmark_test.go | Updates throughput gating logic to prevent -race CI flakiness while preserving detection of major performance regressions. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+296
to
+298
| // package. A generous 100k floor (10x below target, ~35x below a measured slow CI | ||
| // run) still fails the run on an order-of-magnitude regression — e.g. an accidental | ||
| // lock serialising the existence layer — without reintroducing runner-speed flakes. |
Contributor
Benchmark Comparison ReportBaseline: Current: Summary
All benchmark results (sec/op)
Threshold: >10% with p < 0.05 | Generated: 2026-09-09 08:21 UTC |
oskarszoon
approved these changes
Sep 9, 2026
oskarszoon
left a comment
oskarszoon
left a comment
Contributor
There was a problem hiding this comment.
The trade is right and it's disclosed in the comment rather than buried, so this should land. #1051 checks out — ~953k on a 16-core runner against a 1M floor, and the 32-core runner it currently passes on is meant to be temporary, so the flake is imminent rather than hypothetical.
Measured the remaining sensitivity, since an order-of-magnitude floor can easily end up gating nothing. M3 Max, -count=1:
- without
-race: 23.1M–29.6M ops/sec — 231x–296x above the 100k floor - with
-race: 3.49M–6.03M ops/sec — 35x–60x above it
So it catches a collapse (an accidental lock serialising the existence layer) and nothing smaller. That matches what the title and comment claim. No flakes in 8 runs, none close to the floor. Timing mechanics are unaffected by the diff and correct — setup outside the timed region, fixed op count over wall-clock.
One nit, in the justification rather than the code: disk_tx_map_benchmark_test.go:296-298 says the floor is "10x below target, ~35x below a measured slow CI run". The 10x is right, but 953k/100k is 9.5x, not 35x — 35x is the race-mode local figure (3.49M/100k). Two different measurement conditions read as one. Worth fixing so the reasoning behind the constant stays auditable.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of the assertion change in
25c0f2c43(#1051 / #1058), whichrelease/v0.15never received.TestDiskTxMap_ExistenceLayerThroughputgates on >1M ops/sec under-race. The CI runners land below that — 970482 then 910122 ops/sec on two consecutive runs of the same commit — so thetestjob fails on every PR targeting this branch, #1705 included.The 1M figure becomes an informational log; the hard assertion drops to a 100k floor. That still fails on an order-of-magnitude regression (an accidental lock serialising the existence layer, say) without gating on runner speed.
services/blockassembly/subtreeprocessor/disk_tx_map_benchmark_test.gois now byte-identical tomain.Local:
go test -race -run TestDiskTxMap_ExistenceLayerThroughputpasses, 6249184 ops/sec.🤖 Generated with Claude Code