feat(early-kickout): tune production kickout thresholds#16049
Conversation
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…t-pr-6 # Conflicts: # CHANGELOG.md # chain/chain/src/store/mod.rs # chain/epoch-manager/src/epoch_info_aggregator.rs # chain/epoch-manager/src/lib.rs # test-loop-tests/src/tests/chunk_producers.rs
…sert Follow-up to the master merge, resolving the debated untrustworthy-sampling concern for the blacklist-aware writer. - seed_chunk_producer_rows: debug_assert that the blacklist-aware sampler yields a producer for every shard that has one. The safety valve in compute_chunk_producer_blacklist guarantees this; the assert enforces the coupling (a None only means an empty shard settlement, nothing to seed). - nonempty_blacklist_anchor_always_has_row: proves the missing-row region and the non-empty-blacklist region are disjoint, so the aggregator's lenient reader never height-samples (which would re-credit a blacklisted producer) while a blacklist is active. - seeded_rows_match_blacklist_aware_sampler: the seeded ChunkProducers row equals the plain height sampler while the blacklist is empty, and equals the blacklist-aware sampler (never the down node) once it is non-empty; the strict consensus reader returns that same row. Chose a writer-side assert over a reader-side one: at the aggregator miss site the per-shard stats are not in scope, and the partial aggregator there reflects a different point in the backward walk, so asserting against it would check the wrong time slice. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…t-pr-6 #15841 (PR5) landed on master independently; resolve the duplicate by taking master for the pure-PR5 files (epoch_info.rs adopts master's bounds-safe sampler, adapter.rs) and keeping the branch's PR6 payload (lib.rs seed rewrite, early_kickout.rs tests). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
test_finalize_epoch_large_epoch_length asserts the incremental aggregator visits each block exactly once (epoch_info_aggregator_loop_counter == N). With EarlyKickout active (nightly/spice builds), seed_chunk_producers reads get_epoch_info_aggregator_upto_last once per record_block_info, adding bounded extra walk iterations, so the exact count only holds with the feature compiled out. Gate the assertion with #[cfg(not(feature = "nightly"))] (the complement of the seed body's own cfg); the caching invariant stays covered on the stable build and the kickout seed path is covered by the early_kickout tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Remove stacked-PR references (#15908/PR5/PR6) and narration that just restates the code from the comments this branch added: revert the compute_chunk_producer_blacklist comment churn to master's wording, drop the seed_chunk_producers reader/activation paragraphs (readers and gating are visible in the code), and reword the test module doc. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…up blacklist helper Review response for the mid-epoch chunk-producer kickout writer: - Observability: emit a nightly counter (near_early_kickout_chunk_producer_reassigned_total, by shard) + info log when the seeder reassigns a slot away from a blacklisted producer. The kick detector is "the plain scheduled producer is blacklisted", not "plain != excluded" (the excluding sampler renormalizes, so that would false-fire). - DRY: extract blacklist_for_epoch, shared by the seeder and the get_chunk_producer_blacklist accessor, removing the duplicated epoch-reset check. - Tests: 2-shard per-shard blacklist isolation (+ seeding + metric delta), and a finality-stall regression guard pinning the per-block aggregator-walk cost. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cspell (CI spellcheck) flagged 'unfinalized' in the stall-test comments; it is not in the project dictionary. Reword to 'not-yet-finalized'. Comment-only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #16049 +/- ##
==========================================
+ Coverage 73.27% 73.31% +0.03%
==========================================
Files 857 857
Lines 188902 189206 +304
Branches 188902 189206 +304
==========================================
+ Hits 138412 138707 +295
+ Misses 46075 46070 -5
- Partials 4415 4429 +14
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…t-thresholds # Conflicts: # chain/epoch-manager/src/adapter.rs # chain/epoch-manager/src/lib.rs # chain/epoch-manager/src/metrics.rs # chain/epoch-manager/src/tests/early_kickout.rs # chain/epoch-manager/src/tests/mod.rs
… blacklist_for_epoch)
Strip process references from two test comments (keep the invariants they state). Comments only, no code change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pull request overviewTunes the nightly Changes:
Reviewed changesPer-file summary
FindingsNon-blocking (nits, follow-ups, suggestions):
✅ Approved |
The changelog line will be added on stabilization, not on the nightly-gated feature PRs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Re-separate the gas-keys and promise-yield bullets that the previous changelog-drop commit accidentally merged onto one line. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Trisfald review comments: - drive_targeted_misses: drop the nightly-path contrast from the doc; the helper is not(nightly)-only, so just state why it uses the plain height sampler. - get_chunk_producer_blacklist_resets_on_epoch_boundary and get_chunk_producer_blacklist_respects_epoch_grace: consolidate the heavy inline comments into one top-of-function comment each, keeping the non-obvious setup facts and the consensus-sensitivity note. Comment-only, no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Tunes the early-kickout thresholds to the values settled in the protocol-status meeting: a 1000-block grace window at the start of each epoch, at least 100 missed chunks, and under 80% chunk production. Behind
ProtocolFeature::EarlyKickout(v152, nightly only); off the feature the gate is false and behavior is unchanged.The miss floor rises from 20 to 100. The per-producer sample-size floor (
EARLY_KICKOUT_MINIMUM_OBSERVED_BLOCKS = 50) is removed and replaced by a start-of-epoch grace: no producer is blacklisted until the anchor is at least 1000 blocks into its epoch, so upgrade-restart downtime early in an epoch does not trigger kickout. The grace is a height grace (anchor height minus epoch-start height), enforced in the sharedblacklist_for_epochgate that both the record-block seeder and theget_chunk_producer_blacklistaccessor call, so the two cannot drift.Both callers resolve the epoch start defensively: at genesis the
EpochStartrow does not exist yet, so a missing start is treated as "just started" (grace, empty blacklist); any other lookup error propagates rather than being masked as an empty blacklist.Tests recalibrate the blacklist math for the 100-miss floor, grow the integration drives past the 1000-block grace, add a grace-boundary test that also checks the seeder and accessor agree at the first active anchor, and make the epoch-boundary-reset test non-vacuous under the grace.