Skip to content

Lead cap: also wait for rounds trained before the next PULL arrives - #333

Merged
michaellchung merged 2 commits into
mainfrom
fix-lead-cap-flaky
Nov 3, 2026
Merged

michaellchung merged 2 commits into
mainfrom
fix-lead-cap-flaky

Conversation

@michaellchung

@michaellchung michaellchung commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Problem

test_real_syncer_fast_island_stays_within_the_cap_and_both_islands_join_every_merge fails intermittently on CI (e.g. assert 40 <= 1 for _max_lead): with max_local_rounds_ahead=1 the 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

  • RolePairing records the latest local round it retained a pair from (retained_round) and when.
  • New lead_unpushed(): true once the island trained K rounds since then. wait_for_lead waits while either condition holds, with the same escape at half the wait timeout (measured from that retain for the no-pair case).

Tests

  • Two new fake-pairing tests: the wait covers rounds trained before the next PULL arrives, and that wait gives up at half the wait timeout. Both fail without the fix.
  • The real-syncer test now checks the cap's rule directly: 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 bound is 2, not 1: when the slow island itself waits under the cap, its push from an older cut can complete the fast island's merge while it has completed one round less. The absolute c_steps <= 2 bound is dropped (it depends on PULL timing).
  • The test waits for the syncer to listen before the clients dial (their connect timeout is 10 s).

Verification: tests/test_sync_lead_cap.py 60/60 passing in 20 parallel runs per batch alongside 16 CPU burners; all 42 test files touching full_parameter_sync pass (810 passed), including test_verl_miles_parity.py.

🤖 Generated with Claude Code

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>
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>
@michaellchung
michaellchung merged commit fa42d84 into main Nov 3, 2026
2 checks passed
@michaellchung
michaellchung deleted the fix-lead-cap-flaky branch November 3, 2026 09:53
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