Skip to content

perf(miner,market): batch settlement of TerminateSectors - #1768

Open
wjmelements wants to merge 4 commits into
masterfrom
fix/terminate-sectors-gas
Open

perf(miner,market): batch settlement of TerminateSectors#1768
wjmelements wants to merge 4 commits into
masterfrom
fix/terminate-sectors-gas

Conversation

@wjmelements

@wjmelements wjmelements commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Reviewer @rvagg
CC @TippyFlitsUK

Context

We're seeing a lot of congestion from TerminateSectors recently.
This is a batch operation but a quick look found it wasn't benefitting as much from the batching as it could.
For example, it would write the balance changes once per sector.

Claude summary

Two optimizations landed in c4c1d387, both aimed at the TerminateSectors gas cost:
1. Miner actor — skip the early-termination queue round trip (actors/miner/src/lib.rs, deadline_state.rs, partition_state.rs)
TerminateSectors used to always: mark sectors terminated → write them into a per-partition early-termination queue → immediately pop that same
queue back out in a second transaction to compute the penalty. For the common case (no pre-existing backlog), that round trip is pure overhead.
Now, when there's no backlog, the sectors are settled immediately in-line using the SectorOnChainInfos already loaded during marking — cutting
out:
- the write-then-immediately-read-and-delete pass through each partition's early_terminated bitfield queue and the deadline's early_terminations
bitfield
- a duplicate load of every terminated sector's on-chain info (previously loaded once to update the expiration queue, then reloaded a second
time to compute the penalty)
The old deferred path is untouched and still used whenever a backlog already exists (matching cron/backlog semantics exactly).
2. Market actor — batch balance-table updates during deal termination (actors/market/src/lib.rs, state.rs)
OnMinerSectorsTerminate slashes/settles every deal in the terminated sectors. Each deal's settlement (transfer_balance, two unlock_balance
calls, slash_balance) was independently loading and re-flushing the escrow_table/locked_table HAMTs — roughly 6 separate HAMT round trips per
deal. Now the tables are loaded once before the loop and flushed once after, so the cost no longer scales per-deal.
Verified impact (test_vm/tests/terminate_gas_bench.rs, before vs. after, terminating N sectors each with a verified deal):
┌─────────────────────────────────┬─────────────────────┬────────────────────┬─────────────────┐
│ sectors (all w/ verified deals) │  blockstore reads   │ blockstore writes  │  bytes written  │
├─────────────────────────────────┼─────────────────────┼────────────────────┼─────────────────┤
│                               1 │            100 → 83 │            34 → 20 │ 15.7KB → 12.8KB │
├─────────────────────────────────┼─────────────────────┼────────────────────┼─────────────────┤
│                              25 │    316 → 155 (-51%) │    226 → 68 (-70%) │ 53.3KB → 44.4KB │
├─────────────────────────────────┼─────────────────────┼────────────────────┼─────────────────┤
│                             100 │   1214 → 600 (-51%) │  1020 → 412 (-60%) │   227KB → 199KB │
├─────────────────────────────────┼─────────────────────┼────────────────────┼─────────────────┤
│                             500 │  5682 → 2656 (-53%) │ 5002 → 1994 (-60%) │ 1.31MB → 1.18MB │
├─────────────────────────────────┼─────────────────────┼────────────────────┼─────────────────┤
│                            1000 │ 11249 → 5205 (-54%) │ 9978 → 3970 (-60%) │ 3.16MB → 2.90MB │
└─────────────────────────────────┴─────────────────────┴────────────────────┴─────────────────┘
Writes drop ~60-70% at scale (market-side batching, scales with deal count); reads roughly halve (miner-side dedup, scales with sector count).
Follow-up cleanup (9a30e0cc):
  - Factored the shared penalty/pledge/debt-repayment logic (apply_penalty, add_initial_pledge,
    repay_partial_debt_in_priority_order) out of settle_terminated_sectors and
    process_early_terminations into one helper, so the two paths can't diverge going forward.
  - Partition::terminate_sectors now returns an empty Vec for terminated sector infos on the
    deferred path (record_termination = true), instead of moving the full SectorOnChainInfo batch
    up through Deadline::terminate_sectors and the top-level handler only to be dropped unused.

@codecov-commenter

codecov-commenter commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.48387% with 45 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.35%. Comparing base (b0a3bdd) to head (9a30e0c).

Files with missing lines Patch % Lines
actors/miner/src/lib.rs 78.75% 41 Missing ⚠️
actors/market/src/state.rs 96.62% 3 Missing ⚠️
actors/miner/src/deadline_state.rs 91.66% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1768      +/-   ##
==========================================
- Coverage   90.55%   90.35%   -0.20%     
==========================================
  Files         140      140              
  Lines       27814    28005     +191     
==========================================
+ Hits        25186    25303     +117     
- Misses       2628     2702      +74     
Files with missing lines Coverage Δ
actors/market/src/lib.rs 91.04% <100.00%> (+0.05%) ⬆️
actors/miner/src/partition_state.rs 91.33% <100.00%> (+0.06%) ⬆️
actors/miner/src/deadline_state.rs 83.09% <91.66%> (-0.04%) ⬇️
actors/market/src/state.rs 92.34% <96.62%> (+0.26%) ⬆️
actors/miner/src/lib.rs 81.92% <78.75%> (-0.63%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: 📌 Triage

Development

Successfully merging this pull request may close these issues.

2 participants