Repository navigation
Lead cap: also wait for rounds trained before the next PULL arrives - #333
Merged
Merged
Conversation
The local-round lead cap only blocked on pairs in flight. Between a merge and the syncer's next PULL an island has nothing in flight, so a fast island kept training until the PULL arrived; under load that let it run 10-40 rounds ahead of the slow island and push inflated c_steps. RolePairing now records the latest local round it retained a pair from, and wait_for_lead also waits while the island trained K rounds since then (same half-wait-timeout escape, measured from that retain). The real-syncer test now checks the cap per merge (the fast island's push cut is at most K rounds newer than the slow island's, and its c_steps at most 2K + 1 larger) instead of an absolute c_steps bound that depends on PULL timing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
michaellchung
force-pushed
the
fix-lead-cap-flaky
branch
from
November 3, 2026 08:50
05f2518 to
f8fa17f
Compare
The real-syncer test now checks what the cap guarantees: the fast island starts round r + 1 only once every push it made from a cut <= r has the slow island's push for the same step, and only after it pushed from its round-r cut. The completed-round lead may reach 2 when the slow island itself waits under the cap while its push from an older cut completes the fast island's merge, so that bound is 2. The per-merge cut and c_steps checks are dropped: a slow island can push a step from an older cut than it has completed. The test also waits for the syncer to listen before the clients dial, so a slow syncer start on a loaded machine does not exhaust their 10 s connect timeout. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Problem
test_real_syncer_fast_island_stays_within_the_cap_and_both_islands_join_every_mergefails intermittently on CI (e.g.assert 40 <= 1for_max_lead): withmax_local_rounds_ahead=1the fast island ran tens of rounds ahead of the slow island.Root cause is in
RolePairing: the lead cap only blocked on pairs in flight (lead_blockers). After a merge and before the syncer's next PULL, the island has nothing in flight, so it keeps training until the PULL arrives. Under load the syncer's PULL latency covers many fast rounds; those rounds all land in the next push (c_steps 9-13 observed) and widen the lead over the island the next merge waits for. Reproduced locally in 5 of 16 parallel runs under CPU load.Fix
RolePairingrecords the latest local round it retained a pair from (retained_round) and when.lead_unpushed(): true once the island trained K rounds since then.wait_for_leadwaits while either condition holds, with the same escape at half the wait timeout (measured from that retain for the no-pair case).Tests
c_steps <= 2bound is dropped (it depends on PULL timing).Verification:
tests/test_sync_lead_cap.py60/60 passing in 20 parallel runs per batch alongside 16 CPU burners; all 42 test files touchingfull_parameter_syncpass (810 passed), includingtest_verl_miles_parity.py.🤖 Generated with Claude Code