Repository navigation
Run the resync whenever its timer fires - #202
Merged
thrawn01 merged 1 commit intoOct 30, 2026
Merged
Conversation
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.
Purpose
With
resync = "5m"the running dispatch often waited 10 minutes between resyncs (25 gaps of ~600 s against 49 of ~300 s in the Oct 29 to 30 history). Dispatch learns that a PR opened or a ticket moved to In Review only on a resync, so ENG-591's PR sat unseen for 8.5 minutes and was merged before dispatch announced it.The cause: when the resync timer fired, the loop compared the wall clock to
nextResyncand skipped the resync if the wall clock was a few milliseconds early. It then armed a new timer a full interval later. The timer counts monotonic time andnextResynccomes from the wall clock, so a lagging wall clock made every fire a skip.After this change a fired timer always runs the resync:
The missing ready-for-review notice at ENG-591's 02:51:16 read had a different cause, and it is not fixed by code in this PR:
~/Development/dispatch/rules(the[dispatch].rulesdefault), which is the main checkout and was updated after a8ef148 (Oct 24).notify-ready-for-reviewon main requirespr.waitingOn === "you" || "merge". The Oct 22 binary has nowaitingOnfact (the recorded world has none), so the condition is always false and the rule never fires.dispatch replay --from 21:51:16 --to 21:51:17on a build of de489f6 matches the recorded cycle (no notice). Replaying the same history with the main checkout's rules and that one condition replaced bypr.readymakes the cycle wantnotify ENG-591 once [ready 32fc462...]. Nonotify-ready-for-reviewreceipt exists in any saved history segment.Implementation
internal/loop/loop.go: the timer case callsonResyncdirectly instead of checkingNow()againstnextResync. The timer is rearmed whenever any resync runs, so a fire means one is due.wakeIfLatestill covers an event that finds the clock pastnextResyncafter a sleep.harness_test.go:AdvanceLaggingmoves the fake wall clocklagless than the timers.loop_test.go:TestAResyncRunsEveryIntervalWhenTheWallClockLagsTheTimeradvances four intervals with the wall clock 3 ms behind and expects a resync each time. It failed on the old code (the harness waited 20 s for a resync that never came) and passes now.Verification
make testandgo vet ./...pass, andgo test -racepasses for the resync, sleep and idle-hour tests. I did not runmake ciormake test-drift. No sub-agent review ran (test review, correctness, fit, next reader), so the only perspectives are the failing-then-passing test, the existing suite and the history replays above.Changes from build
None. The ticket's two clock-skew candidates were not measured on the user's machine; the test models the lag instead.
Fixes ENG-593