Skip to content

Refresh the engine once when a data update is applied in memory - #92

Merged
jwrosewell merged 9 commits into
rcfrom
fix/in-memory-update-refreshes-once
Nov 10, 2026
Merged

jwrosewell merged 9 commits into
rcfrom
fix/in-memory-update-refreshes-once

Conversation

@jwrosewell

@jwrosewell jwrosewell commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

The problem

When a data file has an update URL and no file path, the data update service applies a downloaded update in memory. On rc at b92cfcf, checkForUpdateFromUrl calls refreshData(identifier, data) on the engine with the downloaded data, or puts the data in the configuration where there is no engine yet. It then returns memoryDownloadResult, which is always AUTO_UPDATE_IN_PROGRESS at that point.

checkForUpdate reads AUTO_UPDATE_IN_PROGRESS as "an update is available but the engine has not been refreshed". It therefore calls updatedFileAvailable, which calls refreshData(identifier) on the same engine. That is a second refresh, through the method that reloads from a data file, when the update was applied in memory because there is no data file path. The interface documentation of refreshData(String) says it reloads from the file at getDataFilePath(), and that an engine built from a byte[] should be refreshed through the byte[] overload instead. If the second refresh throws, updatedFileAvailable retries it up to 10 times and the check reports AUTO_UPDATE_REFRESH_FAILED, although the first refresh had applied the update.

The existing test DataUpdateService_Register_UpdateOnStartup_InMemory checks only the call with the downloaded data, so it did not catch the second call.

What .NET does

On the same path, .NET's CheckForUpdateFromUrl sets the result to AUTO_UPDATE_SUCCESS once RefreshData(identifier, stream) succeeds (line 961), or once the stream is put in the configuration where there is no engine (line 978). CheckForUpdate calls UpdatedDataAvailable only for AUTO_UPDATE_IN_PROGRESS (line 855). On AUTO_UPDATE_SUCCESS it clears the timer and registers the data file again (lines 862 and 865). So .NET refreshes the engine once.

What changed

  • The in-memory path of checkForUpdateFromUrl now returns AUTO_UPDATE_SUCCESS once the engine, or the configuration, holds the new data, as .NET does.
  • checkForUpdate has a new branch for AUTO_UPDATE_SUCCESS, which clears the timer the same way updatedFileAvailable did on this path. registerDataFile then registers the next check.
  • checkForUpdateFromUrl never returned AUTO_UPDATE_SUCCESS before, so the new branch is only reached from the in-memory path.

Polling after a manual update

The data updates page on rc says the following in three places.

  • Under Automatic update master switch:

    This master switch only governs automatic activity - it does not disable Programmatic update, which remains available for triggering an update manually

  • Under Programmatic update, a newer file is processed "as per newFile above".
  • Under newFileAvailable, the steps include "pause update polling, if any", "load file" and "resume polling, if any".

Before, checkForUpdate registered the data file again only after a check the timer started. A manual update cleared the timer, through updatedFileAvailable on the file path or the in-memory success branch, and set no new one, so polling stopped after it.

  • checkForUpdate now also registers the data file again after a manual check that applied an update or whose refresh failed, where the data file is registered with the service. So registerDataFile sets the next timer when automatic updates are on, as it does after an automatic check. This covers both the file path and the in-memory path.
  • A manual update never leaves two timers. The pending timer is cancelled and cleared under futureLock, by updatedFileAvailable (line 296) or by the in-memory success branch (line 232), before registerDataFile runs. registerDataFile schedules a timer only while the future is null, under the same lock (lines 450 to 452). A manual check whose refresh failed in memory clears no timer, so registerDataFile keeps the timer the data file already had.
  • A manual check that finds no update leaves the existing timer alone, as before.
  • A data file that was never registered is not registered by a manual check. That keeps DataUpdateService_CheckForUpdate_FileUpdated, which makes a manual check on an unregistered data file and expects no timer, as it was.
  • unregisterDataFile now sets the data file's update service to null, so getIsRegistered is false after it, and a later manual update does not register the data file again.
  • Java never turns automatic updates off on a manual check. .NET's manual CheckForUpdate does (DataUpdateService.cs line 356 at 5b4d06e), and that is being changed to match the master switch rule above.

Overlap with other drafts

Two other changes also edit DataUpdateServiceDefault.java. #85 has merged into rc as fc55341, and #79 is an open draft. This change stays off their lines, and git merge-tree shows that this branch's head, 763f6a4, merges cleanly with rc at 24c6f25, which holds #85, and with #79's head 986b81c. The old line numbers below are those of rc at b92cfcf.

  • #79 (head 986b81c) changes the checkForUpdateFromUrl call in checkForUpdate (old line 218) and the retry loop in updatedFileAvailable (old lines 295 to 311). In checkForUpdateFromUrl it changes the method's signature, both downloadFile calls, the log lines in the in-memory path and the start of the file path (old lines 785, 792, 799, 810 to 812, 817 and 824).
    • This change adds a branch after old line 228, changes the re-registration condition at old line 252 and adds to the comment below it, adds three lines at the end of unregisterDataFile after old line 567, and edits old line 808. Make the data update follow the specification #79 edits none of those lines. Between them sit unchanged lines (old line 809, and old lines 219 to 228).
    • In DataUpdateServiceTests.java, Make the data update follow the specification #79 edits old lines 374, 887 to 890 and 1394 to 1395. This change adds three imports, edits the watcher test at old lines 418 to 425, and adds its new tests after old line 1231.
  • #85 (merged as fc55341) added two imports (after old lines 38 and 46), a constant after old line 66 and the licenseKeysToMask helper after old line 603, and edited downloadFile (old lines 618 to 668). This change edits none of those lines.
  • The change that makes the watcher test wait for the watcher's update (#95) carried d99c9b7 as its own draft, with the AUTO_UPDATE_SUCCESS import on the same line as this branch has it. Wait for the watcher's update in the watcher test, not the polling timer's check #95 has merged into rc as 30b38b9. This branch merges cleanly with rc at 24c6f25, which holds that merge, and the merged test file is the same as this branch's.

For the release notes

  • When a data file has no file path, an update downloaded from the update URL now refreshes the engine once, with the downloaded data. Before, the engine was refreshed a second time through refreshData(String), and a failure of that second refresh was reported as a failed update.
  • A manual update of a data file registered for automatic updates now sets the next update timer, so polling resumes after it. Polling also resumes after a manual update whose refresh failed. Before, polling stopped after a manual update. A manual check that finds no update leaves the timer as it was.
  • After unregisterDataFile, the data file's getIsRegistered is now false, so a later manual update does not register it again.

How to verify

mvn -pl pipeline/pipeline.engines -am test -Dtest='DataUpdateServiceTests' -Dsurefire.failIfNoSpecifiedTests=false
Class Test methods at rc (b92cfcf) This branch (763f6a4)
DataUpdateServiceTests 26 32, each run for the class's 4 parameter sets
  • 0b500e7 holds the test alone, on rc's code. DataUpdateService_CheckForUpdate_InMemory_RefreshesOnce makes a manual check for a data file with an update URL and no file path, and counts every refreshData call on the mocked engine. It is written to fail there in all 4 parameter sets, finding two calls, refreshData(String, byte[]) and then refreshData(String).
  • 66a5704 holds the fix. DataUpdateService_Register_UpdateOnStartup_InMemory should still pass, as it still sees one call with the downloaded data and one timer scheduled.
  • d99c9b7 makes the watcher test wait for the watcher's update rather than the polling timer's check.
  • 6ff3192 holds three manual check tests alone, on d99c9b7's code. Each runs for both settings of automatic updates on a data file registered with the service.
  • 399afbb holds the fix. The commit body quotes the specification lines above.
  • 4215118 adds to both TimerSetAgain tests a check that the timer pending before the manual update was cancelled and replaced. With the count of scheduled timers, that shows one live timer is left.
  • 7d0b6e2 holds two more tests alone, on 4215118's code. Each runs in all 4 parameter sets.
  • 3e51394 holds the fix for both.
  • 763f6a4, this branch's head, corrects the comment above the re-registration to say that a refresh from the data file clears the timer, and changes no code.

Red runs

Each red run builds one commit of tests on the code before its fix, on Ubuntu with Java 17, and runs only that commit's tests.

  • 0b500e7 in run 42727979529. DataUpdateService_CheckForUpdate_InMemory_RefreshesOnce failed in all 4 parameter sets with "The engine was not refreshed exactly once", listing refreshData(java.lang.String,byte[]) and then refreshData(java.lang.String).
  • 6ff3192 in run 42727994734. Of 12 cases, both TimerSetAgain tests failed in the 2 parameter sets where automatic updates are on, with futureFactory.schedule wanted 2 times but called 1 time. DataUpdateService_CheckForUpdate_NoUpdate_TimerKept passed.
  • 7d0b6e2 in run 42728021546. Of 8 cases, DataUpdateService_CheckForUpdate_FileRefreshFails_TimerSetAgain failed in the 2 parameter sets where automatic updates are on, with futureFactory.schedule wanted 2 times but called 1 time, and DataUpdateService_Unregister_NoLongerRegistered failed in all 4 with an AssertionError at its getIsRegistered check.

Full build

The full build and test matrix passed every job other than macOS on head 763f6a4 in run 42797664071 on 10 November 2026, 11 jobs succeeded, run as a dry run so nothing merged. 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.

DataUpdateService_CheckForUpdate_InMemory_RefreshesOnce runs a check for
a data file with an update URL and no file path, so the downloaded data
is applied in memory. It counts every refreshData call on the engine and
expects exactly one, the call with the downloaded data.
When a data file has no file path, checkForUpdateFromUrl applies the
downloaded data to the engine in memory, then returned
AUTO_UPDATE_IN_PROGRESS. checkForUpdate took that to mean the engine had
not been refreshed and called updatedFileAvailable, which refreshed the
engine a second time through refreshData(String), the method that reloads
from a data file.

The in-memory path now returns AUTO_UPDATE_SUCCESS once the engine, or
the configuration where there is no engine yet, holds the new data, as
.NET's DataUpdateService does. checkForUpdate then clears the timer as
updatedFileAvailable did, so the next check is registered as before.
…mer's check

With a one second polling interval the timer's check can complete first, finding no update because the watcher is enabled, and release the test before the watcher has refreshed the engine. The failed run took 1.008 seconds. The flag is now released only by a completion whose status is AUTO_UPDATE_SUCCESS, which only the watcher's update gives.
The data updates page of the specification on rc says the automatic
update master switch "only governs *automatic* activity - it does not
disable *Programmatic update*". A programmatic update processes a newer
file "as per newFile above", and the steps of newFileAvailable are:

- pause file watcher, if any
- pause update polling, if any
- load file
- resume polling, if any
- resume file watcher, if any
- trigger onUpdateCallbacks

Three tests cover a manual check on a data file registered with the
service, for each setting of automatic updates.

DataUpdateService_CheckForUpdate_FileUpdated_TimerSetAgain applies an
update from the data file, and
DataUpdateService_CheckForUpdate_InMemory_TimerSetAgain applies one
downloaded in memory. Each expects a new timer
where automatic updates are on. Both are written to fail where automatic
updates are on, because the manual update clears the timer and only a
check the timer started registers the data file again.

DataUpdateService_CheckForUpdate_NoUpdate_TimerKept checks that a manual
check finding no update leaves the existing timer as it was. It passes
before and after the change.
… says

The data updates page of the specification on rc says the automatic
update master switch "only governs *automatic* activity - it does not
disable *Programmatic update*". Under Programmatic update, a newer file
is processed "as per newFile above", and the steps of newFileAvailable
include "resume polling, if any" and "resume file watcher, if any".

checkForUpdate registered the data file again only after a check the
timer started. A manual update cleared the timer, through
updatedFileAvailable or the in-memory success branch, and set no new one,
so polling stopped. checkForUpdate now also registers the data file
again after a manual check that applied an update, where the data file
is registered with the service, so registerDataFile sets the next timer
when automatic updates are on, as it does after an automatic check. A
manual check that finds no update still leaves the timer as it was, and
a data file that was never registered is not registered by a manual
check.
Both TimerSetAgain tests now keep the timer pending before the manual
update and check, where automatic updates are on, that it was cancelled
and that the data file holds a different one. With the existing check
that exactly one more timer was scheduled, this shows a manual update
with a timer pending leaves one live timer. The pending timer is
cancelled and cleared under futureLock, by updatedFileAvailable for the
file path or by the in-memory success branch, before registerDataFile
schedules a new one, and registerDataFile schedules only while the
future is null under the same lock.
…ng unregisters

DataUpdateService_CheckForUpdate_FileRefreshFails_TimerSetAgain makes a
manual check whose refresh from the data file fails, on a data file
registered for automatic updates, and expects the next update timer to
be set where automatic updates are on. The newFileAvailable steps in the
data updates page of the specification say "resume polling, if any",
with no exception for a failed load.

DataUpdateService_Unregister_NoLongerRegistered expects a data file to be
no longer registered after unregisterDataFile, so a manual update of it
sets no timer.

Both are written to fail on the previous commit. The first fails where
automatic updates are on.
…ion on unregister

updatedFileAvailable clears the update timer before it refreshes the
engine, so after a manual check whose refresh failed polling stopped,
where an automatic check registers the data file again. checkForUpdate
now also registers a registered data file again after a manual check
that returned AUTO_UPDATE_REFRESH_FAILED. A manual failure that did not
clear the timer, such as one in memory, keeps the timer it had, because
registerDataFile schedules only while there is none. So a manual check
leaves one timer whether it applied an update, failed or found none.

unregisterDataFile now sets the data file's update service to null, so
getIsRegistered is false after it and a later manual update does not
register the data file again.
A comment-only change. The comment above the re-registration after a
manual update said that the refresh clears the timer, which is true only
of a refresh from the data file, because a failed refresh in memory
leaves the timer as it was. No code changes.
@jwrosewell
jwrosewell marked this pull request as ready for review November 10, 2026 13:57
@jwrosewell
jwrosewell merged commit 6870059 into rc Nov 10, 2026
2 checks passed
@jwrosewell
jwrosewell deleted the fix/in-memory-update-refreshes-once branch November 10, 2026 13:57
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