Skip to content

chore: pace the duplicate prune sweep by measured block time - #1427

Merged
MicBun merged 1 commit into
mainfrom
chore/prune-scan-cost-and-broadcast-timeout
Sep 8, 2026
Merged

chore: pace the duplicate prune sweep by measured block time#1427
MicBun merged 1 commit into
mainfrom
chore/prune-scan-cost-and-broadcast-timeout

Conversation

@MicBun

@MicBun MicBun commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Sizes the duplicate prune sweep against what a prune transaction actually costs on mainnet, and stops a broadcast timeout from re-running one.

Why

Pruning was enabled on mainnet for one firing on 08-Sep and turned off again. In that window the gateway returned 1,619 × 503 and 28 × 500 between 00:00 and 00:19 UTC, with no 5xx before or after. Block execution for the 22 blocks carrying an auto_prune_duplicates transaction, against their neighbours:

n median max
blocks with a prune tx 22 16,922 ms 18,613 ms
neighbouring blocks 77 50 ms 4,880 ms

A prune transaction held the consensus path for about 17 seconds. The gateway's read timeout is 20 s, so reads queued behind it and timed out — the 503s carry rpc_code -32001 (ErrorTimeout) at a duration of exactly 20,000 ms. Nothing was down; the node was busy.

The knob that was tuned was the wrong one. Run 1 deleted 794 event times in 14.4 s; every later run deleted essentially nothing and still cost 16-18 s. The work is the per-stream history scan across stream_batch_size streams, and PruneDeleteCap bounds deletions only, so it cannot bound the scan.

What changes

A whole pass costs what it costs — roughly 8.6 hours of execution for 182k streams however it is sliced — so the shape is many cheap transactions at a low duty cycle rather than a few expensive ones.

  • PruneStreamBatchSize 100 to 5, putting a transaction near 0.85 s instead of 85% of the read budget.
  • PruneDrainRunDelay 60 s to 10 s and PruneDrainMaxRuns 100 to 1000: an ~8% duty cycle, 5,000 streams a firing, a full pass in about 37 firings.
  • PruneDeleteCap 5,000 to 1,000. Not the bottleneck, but at five streams a run the old cap could never bind.
  • A broadcast timeout no longer retries.

That last one is the defect rather than a tuning change. ktypes.ErrTxTimeout means the wait for inclusion elapsed, not that the transaction was rejected: it stays in the mempool and still executes. Retrying it with a fresh nonce therefore duplicates a 17-second scan instead of replacing it, and the two transactions race the same nonce sequence. On mainnet auto_prune_duplicates ran 22 times on chain while the scheduler recorded one success and five failures. The retry loop now returns ErrBroadcastPending, the drain ends that firing, and the cyclic cursor resumes next time. BroadcastAutoDigestWithArgsAndRetry has the identical shape and left the same nonce gaps, so it gets the same treatment. Detection is errors.Is against the exported sentinel rather than string matching.

Also corrects the DigestDeleteCap comment, which attributed the testnet crash loop to this cap being too large. That was wrong: the loop was log volume — debug level on the awslogs driver, one line per decoded row, blowing the hardcoded 30-second precommit window — and 40 hours across six firings on the old 100,000 cap confirmed it after the log level changed.

Tests

Three regression tests on the pending-broadcast path: a bare timeout, a wrapped one, and an ordinary network error that must still be retried. Each was checked by breaking the fix and watching it fail — without it both timeout cases retry until the context deadline, 30 s each, which is the amplification itself. All three tn_digest packages pass.

What this does not fix

The per-stream scan is still unbounded, because the deletable set is a whole-stream property: one stream holding 1.3 M rows costs the same in a batch of 5 as in a batch of 100. This fixes the average and not that tail. Bounding scan work per transaction needs a design change — a resumable cursor within a stream, or a maintained duplicate index — and does not belong in a constants change.

Re-enabling on mainnet should not be gated on testnet, which holds 212k rows at 1.8% duplicates and never reaches this path. The next attempt wants a mainnet-shaped fixture through internal/benchmark/digest, with per-transaction block time as the pass criterion.

Context

Summary by CodeRabbit

  • Bug Fixes

    • Improved handling of transactions that remain pending, preventing unnecessary retries and avoiding false failure reporting.
    • Digest and duplicate cleanup processes now stop gracefully when a transaction is still pending and resume during a later run.
    • Improved retry behavior for temporary broadcast failures.
  • Performance

    • Tuned duplicate cleanup batch sizes and timing to reduce network and logging pressure.
    • Increased cleanup drain capacity while shortening delays between active runs.

@MicBun MicBun self-assigned this Sep 8, 2026
@holdex

holdex Bot commented Sep 8, 2026

Copy link
Copy Markdown

Time Submission Status

Member # Time Running Total Status Last Update
MicBun 4h ✅ Submitted Sep 8, 2026, 4:21 AM

Submit or update total time with:

@holdex pr submit-time 2h

Add time on top of previous submission with:

@holdex pr add-time 1h30m

See available commands to help comply with our Guidelines.

@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 21081dac-9d96-40ee-8a59-760c1ac50a08

📥 Commits

Reviewing files that changed from the base of the PR and between d876ffc and 5221106.

📒 Files selected for processing (4)
  • extensions/tn_digest/internal/engine_ops.go
  • extensions/tn_digest/internal/prune_ops_test.go
  • extensions/tn_digest/scheduler/constants.go
  • extensions/tn_digest/scheduler/scheduler.go

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

Changes

Broadcast Pending Handling

Layer / File(s) Summary
Broadcast timeout classification and retry behavior
extensions/tn_digest/internal/engine_ops.go, extensions/tn_digest/internal/prune_ops_test.go
Timeout errors, including wrapped errors, return ErrBroadcastPending without retrying. Ordinary failures keep the existing retry behavior. Tests cover both paths.
Scheduler handling of pending broadcasts
extensions/tn_digest/scheduler/scheduler.go
Digest and duplicate-prune drain loops stop the current firing when they receive ErrBroadcastPending.
Duplicate-pruning pacing configuration
extensions/tn_digest/scheduler/constants.go
Pruning uses smaller batches and delete caps, more drain runs, and a shorter active-run delay. Documentation describes the updated timing behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 52211

Broadcast timeouts now stop duplicate retries while scheduled digest and pruning work ends the current firing cleanly; smaller prune batches pace work across transactions. No concrete current-head merge-blocking risk remains.

Sequence Diagram(s)

sequenceDiagram
  participant Scheduler
  participant BroadcastAutoPruneDuplicatesWithRetry
  participant Mempool
  Scheduler->>BroadcastAutoPruneDuplicatesWithRetry: start broadcast
  BroadcastAutoPruneDuplicatesWithRetry->>Mempool: submit transaction
  Mempool-->>BroadcastAutoPruneDuplicatesWithRetry: timeout while transaction remains pending
  BroadcastAutoPruneDuplicatesWithRetry-->>Scheduler: ErrBroadcastPending
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary change: pacing the duplicate-pruning sweep based on measured block time.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 4 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/prune-scan-cost-and-broadcast-timeout

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@MicBun

MicBun commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

@holdex pr submit-time 4h

@MicBun
MicBun merged commit 5bf031b into main Sep 8, 2026
8 checks passed
@MicBun
MicBun deleted the chore/prune-scan-cost-and-broadcast-timeout branch September 8, 2026 04:21
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