Skip to content

Keep one polling timer for each data file - #63

Merged
jwrosewell merged 5 commits into
rcfrom
fix/polling-timer
Nov 13, 2026
Merged

jwrosewell merged 5 commits into
rcfrom
fix/polling-timer

Conversation

@jwrosewell

@jwrosewell jwrosewell commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Each update of a data file, whether the update service scheduled it or it was started by hand with updateDataFile, ends in checkNextUpdate, which calls scheduleUpdate (on rc at 24592ac, fail at line 350 and the end of a download at line 691 of dataFileUpdateService.js). scheduleUpdate, lines 156 to 165, starts a new setTimeout every time and keeps no handle to the one already waiting. So every update started by hand adds another polling schedule for the same data file, and the data file is then checked once for each of them in every polling interval.

What the specification says

Data updates at 77dc1f62. A data file has one polling schedule, which an update pauses and resumes rather than adds to (lines 326 and 332):

  • pause update polling, if any

  • resume polling, if any, whether or not the file loaded

A programmatic update is one immediate check, not a schedule of its own (lines 204 to 206 and 340 to 344):

The effect of a programmatic update is to initiate immediate polling of the remote HTTP server, where this option is enabled.

What changed

scheduleUpdate keeps the pending timer on the data file as pollingTimer, clears it before it starts the next one, and clears the field when the timer fires. So a data file has one polling timer however many updates are started. The timer still holds the update service and the data file weakly, as before, and still passes true for an update the service started, which rc added in #69.

b9d2928e merged rc at 24592ac. Its one conflict was the call in the timer, where rc had added the true and this branch had added the line that clears pollingTimer. Both are kept. 40f20cf8 merges rc at 7cbf8bc, which added #94's change to the same file in other lines, with no conflict.

Tests

tests/pollingTimer.test.js fakes the timers only, so the update's request really fails to connect, and checks four things. An update started by hand leaves one timer pending, several leave one, polling checks once in an interval after updates by hand, and polling carries on after a check the timer started.

18c5bd7f makes "polling checks once in an interval" wait for the check its timer starts. Without that wait the test ended with the check's request still open, and the request's failure then scheduled the next check after the test, on a later test's fake clock or on the real one. The first full build of this draft (run 43479170078) failed on Ubuntu ARM in "polling carries on" with two timers where one was expected, and a separate run of the pipeline tests on x64 passed every test and then never ended, until its time limit.

  • Red, the four tests on rc at 7cbf8bc without the fix: run 43490122037, 3 of 1,147 failed, being "an update started by hand leaves one timer pending" (2 timers where 1 is expected), "several updates started by hand still leave one timer pending" and "polling checks once in an interval after updates by hand". The run then went on to its time limit and was cancelled, because the third test fails before it waits for its check.
  • Green, the pipeline tests and lint at 18c5bd7: run 43485500314, 1,146 passed, and jest ended three minutes after it started. The run's conclusion is failure, from its last step, which regenerates the types and compares them with the committed ones. That comparison fails on rc without this change too, because the committed types are older than the code (run 43485072238 found 23 type files that tsc writes differently on rc at 24592ac), so it is not a test of this change.
  • Full build on the head 40f20cf: run 43490135324 passed. All seven Setting 'PullRequestSha' lines are 40f20cf, and each of the six Ubuntu, Ubuntu ARM and Windows jobs on Node 22 and 24 ran 1,360 unit tests passed with 2 skipped, and 132 integration tests passed with 4 skipped. macOS was left out of this draft's full build under the 9 November decision. The rc head is built with macOS before it merges into main.

What remains

Nothing in this change.

The Programmatic update part of the Data update page starts an immediate
check and no new schedule. Each way out of an update schedules the next check,
so an update started by hand added a second chain of checks. The data file now
keeps its pending timer and the next one replaces it.
The test that counts the checks in one interval ended with the check's
request still open. Its failure then scheduled the next check after the
test, either on a later test's fake clock, which counted two timers on
Ubuntu ARM, or on the real clock, which kept the test run from ending.
@jwrosewell
jwrosewell marked this pull request as ready for review November 13, 2026 23:06
@jwrosewell
jwrosewell merged commit 5ec96e3 into rc Nov 13, 2026
2 checks passed
@jwrosewell
jwrosewell deleted the fix/polling-timer branch November 13, 2026 23:06
@jwrosewell jwrosewell mentioned this pull request Nov 15, 2026
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