Skip to content

Run the resync whenever its timer fires - #202

Merged
thrawn01 merged 1 commit into
mainfrom
thrawn01/eng-593-resync-every-5-minutes-not-every-10
Oct 30, 2026
Merged

thrawn01 merged 1 commit into
mainfrom
thrawn01/eng-593-resync-every-5-minutes-not-every-10

Conversation

@thrawn01

@thrawn01 thrawn01 commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

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 nextResync and 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 and nextResync comes from the wall clock, so a lagging wall clock made every fire a skip.

After this change a fired timer always runs the resync:

resync = "5m"  ->  dispatch.resync at least every 5 minutes, wall clock lag or not

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:

  • The running binary was built Oct 22 and the rules load from ~/Development/dispatch/rules (the [dispatch].rules default), which is the main checkout and was updated after a8ef148 (Oct 24).
  • notify-ready-for-review on main requires pr.waitingOn === "you" || "merge". The Oct 22 binary has no waitingOn fact (the recorded world has none), so the condition is always false and the rule never fires.
  • Evidence: dispatch replay --from 21:51:16 --to 21:51:17 on 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 by pr.ready makes the cycle want notify ENG-591 once [ready 32fc462...]. No notify-ready-for-review receipt exists in any saved history segment.
  • Reinstalling the binary from main fixes it. I did not restart the user's dispatch.

Implementation

  • internal/loop/loop.go: the timer case calls onResync directly instead of checking Now() against nextResync. The timer is rearmed whenever any resync runs, so a fire means one is due. wakeIfLate still covers an event that finds the clock past nextResync after a sleep.
  • harness_test.go: AdvanceLagging moves the fake wall clock lag less than the timers.
  • loop_test.go: TestAResyncRunsEveryIntervalWhenTheWallClockLagsTheTimer advances 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 test and go vet ./... pass, and go test -race passes for the resync, sleep and idle-hour tests. I did not run make ci or make 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

@thrawn01
thrawn01 merged commit e59e1ad into main Oct 30, 2026
2 checks passed
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