[jwrosewell]

The problem

This draft fixes two faults in the file watcher path of the data update service.

1. After the file watcher finds a new data file and the engine is refreshed from it, polling for updates stops. No later check is scheduled, so a data file that is also set to update from a URL is never updated again by polling. There is no issue for this fault.
2. Before that refresh, the watcher waits for the new file to open for reading, with no limit and no pause. A file that never opens keeps that thread trying at full speed for ever, logging a warning on every try, while it holds the data file's update lock, and the fix for the first fault never runs. A review of this draft found it, and a second review found that the first bound reported a timeout when the wait was interrupted.

The cause

At rc d39b6af, updatedFileAvailable cancels the polling timer and sets it to null (lines 304 to 310) before it refreshes the engine. Two methods call it.

1. checkForUpdate, the polling path, calls registerDataFile afterwards, and registerDataFile sets a new timer because the old one is null (lines 458 to 495).
2. dataFileUpdated, which the file watcher calls, does not. The timer stays null, so polling ends.

The wait is the while (fileLockable == false) loop in dataFileUpdated (lines 374 to 385 at d39b6af), which only ends when the file opens.

What the specification says

specifications#60 merged into specifications rc at 07:37 UTC today. At rc e89679e, the newFileAvailable steps of the data update page cover a file from the watcher as well as a download.

The file will be in dataFilePath from watcher or from download, if the downloaded file is newer than the current file.

• pause file watcher, if any
• pause update polling, if any
• load file
• resume polling, if any, whether or not the file loaded
• resume file watcher, if any, whether or not the file loaded
• trigger onUpdateCallbacks with success, if the file loaded

The Update polling section adds "Once the refresh is complete, the data update service can start checking for updates using the newly refreshed next expected update date/time."

The Errors table has this row, following .NET's messages, levels and statuses.

| A watched file could not be opened for reading within 10 minutes | Not logged | AUTO_UPDATE_TIMEOUT | None |

Reporting each check gives AUTO_UPDATE_IN_PROGRESS "for a file watcher event that found nothing left to do". The page does not cover a wait that is interrupted, because .NET's wait cannot be.

What .NET does

On 51degrees-dotnet rc at b3e90b4, UpdatedDataAvailable refreshes the engine and leaves the timer alone. Only CheckForUpdate clears the timer and registers the data file again, after a successful refresh. So a refresh from the watched file leaves .NET's polling running. Java cancels the timer on both paths, so it has to register again on both.

In DataFileUpdatedInternal, .NET tries to open the file until DateTime.UtcNow.AddMinutes(10), waits 500 ms after each failed try, logs nothing, and then reports AUTO_UPDATE_TIMEOUT without refreshing the engine. Its status reads "An operation could not be completed within the expected time."

What changed

Head 3439e5e. Links are at that commit.

1. dataFileUpdated registers a registered data file again after the refresh and before the callbacks, the order the specification gives. Registering a file that is already registered only sets the timer, because registerDataFile sets up the watcher only while there is no watch key, and runs the start-up check only for a file not yet registered. The new timer uses the next expected update the refresh set, as the Update polling section says. The file is registered again after a refresh that failed as well, as checkForUpdate does for a polled check, so polling carries on either way.
2. The same lines bound the wait as .NET does. The file is tried every 500 ms for up to 10 minutes, nothing is logged while it waits, and if it never opens the update ends with the new AUTO_UPDATE_TIMEOUT status. The engine is not refreshed then, and the polling timer is left as it was, because only a refresh cancels it, so the file is registered again only when the refresh ran. The same file is not tried again, because its time was already recorded, and a file written later is. The exception the loop catches is named ignored, as nothing reads it, which is as near as Java comes to .NET's catch (IOException).
3. An interrupt, as unregistering the data file gives the watcher's task with cancel(true), ends the wait at once. The file was not tried for the whole wait, so the update does not report a timeout. It keeps the status it started with, AUTO_UPDATE_IN_PROGRESS, the status for a watcher event that did not finish an update, and the engine and the polling timer are left as they were. The specification leaves this case open, as above.
4. AUTO_UPDATE_TIMEOUT is added at the end of AutoUpdateStatus, with .NET's text, so no existing value changes its place.
5. The ten minutes and the 500 ms pause are named constants. A package private field holds the wait so the test can shorten it to one second.
6. The comment on updatedFileAvailable gives its steps in order and says the caller registers a registered data file again. It used to say the method itself did that, which it does not.

How it was checked

DataUpdateServiceWatchedFileTests holds three tests. Each records the polling timers the service schedules and runs the watcher's task itself, on a real watch of a temporary folder. Like the existing watcher test, they do not run on macOS, whose file watcher is not reliable.

• WatchedFileRefresh_PollingCarriesOn changes the file, then requires the engine to be refreshed once, a second polling timer to be scheduled, and the data file to hold it.
• WatchedFileCannotBeOpened_GivesUpAfterTheWait uses a file that throws on every open. It requires the update to end within 10 seconds with AUTO_UPDATE_TIMEOUT, no more than four tries in the one second wait, no warning or error logged, no refresh, and the first polling timer kept.
• WatchedFileWaitInterrupted_LeavesTheUpdateUnfinished uses the same file with a 60 second wait, interrupts the watcher's thread once the file has been tried, and requires the update to end within 5 seconds with AUTO_UPDATE_IN_PROGRESS, nothing logged, no refresh and the first polling timer kept.

The last two share one fixture. Its stand-ins for the file and the logger keep no record of their calls, so a loop that never ends cannot fill the memory, and the stand-in file pauses without sleeping, so an interrupt reaches the service.

Each run used a temporary workflow on a tmp/ branch that builds the named modules and runs the named tests. The polling runs were on Ubuntu with Java 17, and the wait runs ran every data update service test, 137 and then 138 of them, on Ubuntu and Windows with Java 8 and 21.

Run	Commit	Result
Red, polling	42895763177 at add0886, the first test alone on rc	Failed as expected, "A new polling timer is set after the refresh expected:<2> but was:<1>"
Green, polling	42896368424 at 20167c3	Passed
Red, wait	42980723161 at 49057d1, the second test with the loop unchanged	Failed as expected on Ubuntu and Windows, Java 8 and 21, "The update gives up on a file it cannot open", 1 of 137 in each of the 4 jobs, with the polling test passing
Green, wait	42980723167 at 03999c5	Passed on Ubuntu and Windows, Java 8 and 21, 0 of 137 failed in each of the 4 jobs, both watched file tests run and none skipped
Red, interrupt	42988780886 at bddd746, the third test with the earlier bound	Failed as expected on Ubuntu and Windows, Java 8 and 21, WatchedFileWaitInterrupted_LeavesTheUpdateUnfinished "expected:<AUTO_UPDATE_IN_PROGRESS> but was:<AUTO_UPDATE_TIMEOUT>", 1 of 138 in each of the 4 jobs
Green, interrupt	42988903317 at 3439e5e, the head	Passed on Ubuntu and Windows, Java 8 and 21, 0 of 138 failed in each of the 4 jobs, all 3 watched file tests run and none skipped
Full build	42989873852 at 3439e5e, the head	Passed, all 9 jobs

The Windows jobs of the green run also cover 0beb384, which leaves to deleteOnExit a data file Windows still holds, because the reader maps the file and Windows keeps a mapped file until the buffer is collected. The three tests share that clean up.

The full build's 9 jobs without macOS all passed, being Ubuntu Java 8, 11, 17 and 21, Ubuntu ARM Java 17, and Windows Java 8, 11, 17 and 21. Every Setting 'PullRequestSha' line, in Configure and in each of the 9 jobs, is 3439e5e, the head, and Configure printed "Left out 6 macOS entries". Two earlier full builds are superseded, 42904274257 at 0beb384, before the bound, and 42981459548 at 03999c5, whose first attempt failed on Ubuntu Java 8 when Maven could not read the descriptor of plexus-compiler-manager 2.17.0, and whose rerun was cancelled when the head moved.

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.

The branch has rc at c77fb05 merged in, which moves cxx to c6d4e4d9.

Other drafts

#79 changes DataUpdateServiceDefault.java too, in checkForUpdate, updatedFileAvailable, updateFromMemory, registerDataFile and the download. This draft changes the comment above updatedFileAvailable, the constants at the start of the class and dataFileUpdated, and #79 changes neither dataFileUpdated nor AutoUpdateStatus. git merge-tree --write-tree of this draft's head with #79 at 10a8a5d reports no conflicts.

What remains

1. An Error thrown by refreshData leaves checkForUpdate before it registers the file again. That is left for later.
2. When a watched file is not newer than the one loaded, Java reports AUTO_UPDATE_IN_PROGRESS where .NET and the newFileAvailable section report AUTO_UPDATE_NOT_NEEDED.
3. Java's AutoUpdateStatus still lacks three statuses .NET has and the Errors table names, being AUTO_UPDATE_NO_CONFIGURATION, AUTO_UPDATE_TEMP_PATH_NOT_SET and AUTO_UPDATE_UNKNOWN_ERROR. Points 2 and 3 belong with the work to align Java's update messages and statuses with .NET.