[jwrosewell]

Problem

Three things on rc stop the file system watcher running on its own setting.

1. The data update service tied the watcher to automatic updates. Make the data update follow the specification (#79), merged into rc on 11 November as 9c17f01, added aspectDataFile.getAutomaticUpdatesEnabled() && to the watcher's condition (DataUpdateServiceDefault.java lines 554 to 558 on rc at ef34a03d). The published code on main creates the watcher on the watcher's own setting (DataUpdateServiceDefault.java lines 456 to 460 on main at f69da52e).
2. The builder never registers a Data File for the watcher alone. OnPremiseAspectEngineBuilderBase.preCreateEngine registers a Data File with the data update service only with automatic updates or the update on start-up on (OnPremiseAspectEngineBuilderBase.java lines 203 to 206 on rc). An application with no License Key has to turn automatic updates off to build at all on rc (see "A deployment with no License Key" below), so its Data File is never registered and never watched, even with item 1 put right. On main the same application builds with automatic updates on, which registers its Data File (lines 190 to 201 on main), and the service then watches it. So rc would release without the watcher that main gave applications with no key.
3. A watch service is never closed. Each watch service runs a thread of its own. Unregistering a Data File, as closing its Engine does, cancels the watch key but leaves its watch service open (DataUpdateServiceDefault.java lines 633 to 640 on rc), and every download pauses the watch and resumes it on a new watch service while the old one stays open (lines 1114 to 1117 and 1141 to 1148). Once item 2 is put right, every Engine built from a file on disk with a data update service, which the pipeline builders give it by default, has a watcher by default, so each Engine closed would leave a thread running. While the watch is paused for a download the watch key is null, and the watcher's task fails on it (line 592). The task repeats through scheduleAtFixedRate, and a repeating task that fails is never run again, so a download at the wrong moment stops the watcher for good.

The rule

The rule, set on 12 November, is that the file system watcher runs on its own setting and needs no License Key. The License Key is needed only for the two things that download, which are polling and the update on start-up. The specification states it first, then Java, .NET, Node.js and Rust follow.

What the specification says

specifications#143 put the rule into the data update page, and this draft merges after it. #143 merged into the specification's rc at 23:43 UTC on 12 November as 77dc1f62, and the page there is the same, line for line, as at #143's head d48add76, which was approved. The quotes are from 77dc1f62.

The Automatic update master switch section, lines 99 to 109, says

The autoUpdate option is the master switch for Automatic update via HTTP (polling). When it is disabled, polling does not take place, however it is configured.

Automatic update from File (file system watching) does not depend on autoUpdate. The file system watcher runs on its own setting, whether autoUpdate is enabled or disabled, because watching a file downloads nothing.

A License Key is needed only for the two mechanisms that download, which are polling and the update on start-up. The file system watcher needs no License Key.

The Automatic update from File section, lines 182 to 186, says

The file system watcher runs on its own setting, whether the autoUpdate master switch is enabled or disabled. Watching a file downloads nothing, so the watcher needs no License Key. An application with no License Key, such as one running the free Lite data file, can put a new file in place and the Aspect Engine refreshes from it.

The Other start-up steps, lines 300 to 304, configure the watcher outside the autoUpdate step

• check operational data configuration consistency
• load data from memory or file
• if disk data source, configure file watcher, if requested
• if autoUpdate is enabled (see master switch):
  • configure HTTP update polling, if requested

and the Defaults section, lines 387 and 394 to 396, has the watcher on by default and says

An application that cannot supply a license key, such as one running the free Lite data file, turns autoUpdate off before it builds the Engine. Its file system watcher still runs, because the watcher needs no License Key.

51degrees-go#110 made the same change to the watcher's condition in Go at e53aa8fc, and 51degrees-node#90 in Node.js.

What changed

Seven commits on the branch, made from rc at 0b9257f.

1. Tests for the service, 25c01d03.
   • DataUpdateServiceWatchedFileTests.WatcherRunsOnItsOwnSetting is new, with the three cases of Go's TestTheWatcherRunsOnItsOwnSetting, being the watcher on with automatic updates on, the watcher on with automatic updates off, and the watcher off with automatic updates on. Each case registers a Data File and checks that a watch key exists and the watcher's task was scheduled only with the watcher on, and that a polling timer was set only with automatic updates on. Off macOS it then replaces the file and runs the watcher's task, and checks that the Engine was refreshed only with the watcher on.
   • Three tests had automatic updates on only to get a watcher, and now turn them off. They are DataUpdateServiceLoggingTests.FileSystemWatcher_LogsCreatingFileSystemWatcher, DataUpdateServiceTests.DataUpdateService_UpdateFromWatcher and DataUpdateServiceTests.DataUpdateService_Register_FileSystemWatcher, the last of which Make the data update follow the specification #79 had changed to expect no watch key with automatic updates off and now expects one in both of its cases again.
2. Fix for the service's condition, 7f96e53b. registerDataFile creates the watcher on getFileSystemWatcherEnabled() alone, and the comment says the watcher runs on its own setting and needs neither automatic updates nor a license key.
3. Merge of rc, fab2feb7, bringing in #116 and #117, which change none of this draft's files.
4. Tests for the builder and the watch service, 8fdd85d3.
   • OnPremiseAspectEngineBuilderBaseTest.WatcherAlone_RegistersTheDataFile builds an Engine with a real data update service, from a builder whose update source needs a License Key it does not have, with automatic updates and the update on start-up off. With the watcher on it checks the Data File is registered and a watch key exists, and with the watcher off that neither is so.
   • DataUpdateServiceWatchedFileTests.Unregistering_ClosesTheWatchService finds the watch service threads by WatchService$Poller in each thread's stack, and checks that registering a watched Data File starts one and unregistering it ends that thread.
   • DownloadedFile_WatchResumesOnTheSameWatchService registers a watched Data File with automatic updates on, runs its polling timer to download a new file from a mocked HTTP client, and checks that the Engine was refreshed, the watch resumed, there is still one watch service thread started by the test, and unregistering ends it.
   • WatchPaused_WatcherTaskCarriesOn runs the watcher's task with the watch key null, as it is during a download, and again with the key back, and checks that unregistering while the key is null still cancels the task.
   • The two thread tests are skipped on macOS, where the watch service polls from a thread with no WatchService$Poller in its stack.
5. Fix for the builder and the watch service, 5293783d.
   • preCreateEngine also registers a Data File that is to be watched, meaning the watcher is on, there is a data update service, and the Data File is on disk. A builder with no data update service still builds without one, as before, and the License Key check is unchanged, as the watcher needs no key.
   • The data update service keeps each Data File's watch service (DataUpdateServiceDefault.java lines 95 to 101), creates it once (lines 577 to 578 and 678 to 699), and closes it when the Data File is unregistered (line 671 and 701 to 720). After a download the watch resumes on the same watch service, and not at all if the Data File was unregistered during the download (lines 1208 to 1222). Both are done under the lock that guards the registered Data Files, so an unregister cannot slip between them.
   • Unregistering cancels the watcher's task whether or not the watch key is set (lines 654 to 659), and the task does nothing on a run that finds no watch key (lines 603 to 611).
   • The notes on setDataFileSystemWatcher in DataFileConfigurationBuilderBase.java and SingleFileAspectEngineBuilderBase.java said automatic updates "must also be enabled in order for the file system watcher to work". They now say the watcher runs on its own setting, whether automatic updates are on or off, and needs no license key.
6. Tests for the watcher's task, 324505b4. A read of every exit of registerDataFile after commit 5 found that the service scheduled the watcher's task even when no watch key was created, which on rc failed on its first run and stopped, and with commit 5 would run on doing nothing. It also found that registering a Data File again with no watch key replaced the task without cancelling the earlier one.
   • NoWatchKey_NoWatcherTask registers a watched Data File whose folder does not exist, and checks there is no watch key and no watcher's task.
   • RegisteredAgain_WatcherTaskReplaced registers a watched Data File, takes its watch key away, registers it again, and checks that the watch key is back and the first task was cancelled.
   • The two thread tests now follow the threads started during each test (lines 669 to 704), so a thread from an earlier test that ends late makes no difference to their count.
7. Fix for the watcher's task, ba99c5da. The watcher's task is scheduled only when a watch key exists, and any task scheduled before is cancelled first (lines 587 to 594).

Nothing about polling, the update on start-up or the License Key check changes.

A deployment with no License Key

With no License Key the defaults still do not build. Automatic updates and the watcher default to on and the update on start-up to off (Constants.java lines 30 to 35 on rc), the default update URL is the Distributor's, and the builder refuses that combination with a PipelineConfigurationException that names setAutoUpdate(false) (OnPremiseAspectEngineBuilderBase.java lines 235 to 247 on rc), as the specification's Defaults section asks. This draft does not change that.

Once it turns automatic updates off, the deployment builds and, with this draft, its Data File is registered and watched, as the specification's lines 394 to 396 say. Before this draft it built with no watcher. The new builder test is that case.

This also means an Engine built from a file on disk with automatic updates off, with or without a key, now has a watcher unless setDataFileSystemWatcher(false) is called, because the watcher defaults to on. main did not watch such an Engine either, as its builder registered a Data File only with automatic updates on.

Red and green

• Red, service. Full build 43298416126 on 25c01d0, the service tests alone. The Ubuntu Java 8 job built Setting 'PullRequestSha' to '25c01d039ca9bf6c1740f91527518e68c43173ad' and failed with 8 failures in 208 tests of the Pipeline Engines module, all of them watcher tests, among them WatcherRunsOnItsOwnSetting with "watcher on, automatic updates off, a file system watcher was created expected: but was:". The matrix then cancelled the other Build and Test jobs.

• Red, builder and watch service. Full build 43336990737 on 8fdd85d, the merge of rc and the new tests, with the fix for the service's condition but not the builder or watch service fix. The Ubuntu Java 8 job built Setting 'PullRequestSha' to '8fdd85d37c7139ba368d7fcdbe4de85e5b2ceaef' and ran 212 tests of the Pipeline Engines module with 3 failures and 1 error, being the four new tests.

  • WatcherAlone_RegistersTheDataFile failed with "watcher on, automatic updates off, no license key, the data file is registered expected: but was:".
  • Unregistering_ClosesTheWatchService passed "Registering starts a watch service thread" and failed with "Unregistering ends the watch service thread" after its 10 second wait.
  • DownloadedFile_WatchResumesOnTheSameWatchService passed the refresh and the resumed watch, and failed with "The watch resumes on the same watch service", as the download had left a second watch service thread running.
  • WatchPaused_WatcherTaskCarriesOn ended in a NullPointerException at line 362 of the test at 8fdd85d, the first run of the watcher's task with the watch key null.

  The matrix then cancelled the other eight Build and Test jobs, so this run is the red and not a full build.

• Red, watcher's task. Full build 43344105874 on 324505b, the tests of commit 6 on top of commit 5. The Ubuntu Java 8 job built Setting 'PullRequestSha' to '324505b4bef6d050742d947139cbb2b1f862b393' and ran 214 tests of the Pipeline Engines module with 2 failures, being the two new tests, and the thread tests passed as rewritten.

  • NoWatchKey_NoWatcherTask failed with "No watcher task without a watch key expected null, but was:fiftyone.pipeline.engines.services.DataUpdateServiceDefault$3@15974f4e".
  • RegisteredAgain_WatcherTaskReplaced failed with "Wanted but not invoked: scheduledFuture.cancel(false);" at line 437.

  The matrix then cancelled the other eight Build and Test jobs.

• Green. In the full build below, on head ba99c5d, the Ubuntu Java 8 job ran DataUpdateServiceWatchedFileTests (9 tests), OnPremiseAspectEngineBuilderBaseTest (6), DataUpdateServiceTests (132) and DataUpdateServiceLoggingTests (25) with no failures and none skipped, so every test that failed in the three red runs passes. In the Windows Java 8 job DataUpdateServiceWatchedFileTests also ran 9 with none skipped, so the thread tests ran on Windows too, and OnPremiseAspectEngineBuilderBaseTest ran 6 with its 3 Unix-only folder permission tests skipped.

Full build

Full build 43346033469 on head ba99c5d succeeded at 02:48 UTC on 13 November. Configure printed "Merging in any changes from rc" and "Left out 6 macOS entries", and all nine Build and Test jobs (Ubuntu Java 8, 11, 17 and 21, Ubuntu ARM Java 17, and Windows Java 8, 11, 17 and 21) printed Setting 'PullRequestSha' to 'ba99c5da6f7f31a5bbcbc02d500037e341d6edcd' and passed, ten such lines in all with Configure's.

An earlier full build, 43339435784, on 5293783 (commit 5, before commits 6 and 7) also succeeded on all nine jobs, at 01:51 UTC. Together they show the builder change passes every module, the Device Detection and IP Intelligence ones included, where an Engine built from a file on disk with automatic updates off now has a watcher by default.

rc has moved to 551d916 since the build started, with #120, which changes four usage sharing files in the FiftyOne Pipeline Engines module and none of this draft's, and the draft is MERGEABLE and CLEAN. The draft's three checks on the head (the environment variable name check, Only rc into main and utm-link-lint) are green. The tmp/pr-matrix-119c, 119d, 119e and 119f branches are deleted.

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 draft. The release draft #112 could say under "Before you upgrade" that an Engine built from a file on disk with automatic updates off now watches its Data File unless setDataFileSystemWatcher(false) is called.