Repository navigation
feat: Add a shared file data reloader, poller, and watcher - #361
kinyoklion wants to merge 15 commits into
Conversation
Adds the Internal/FileLoading namespace with the file reading code that the flag overrides feature needs: a document parser and merger that keep each entry's own version and report per-document counts, a reloader that serializes reloads, debounces change signals, retains the last good result on failure, retries after a bounded delay, reports an identical repeated failure once, and skips unchanged content, a stat-based poller, and a directory watcher that handles files that do not exist yet. The existing file data source keeps its current behavior and does not use the shared code. Tests that pin that behavior are added so later changes to the shared code cannot alter it.
883fd4b to
353e748
Compare
… change detection The poller and the watcher observe a configured file through any symbolic link, so a replaced link target or a swapped ConfigMap data directory is a change. Frameworks without link resolution compare the content of a link. The watcher retries a directory it cannot watch every second while it watches the others, drops the watch on a directory that no longer exists and sets it up again when it appears, compares the configured files on a notification for another entry, guards its handlers, computes each full path once, and accepts a null logger.
… failures Requests that arrive while a reload runs are served by one follow-up reload instead of each waiting for the lock. An exception from Apply or from the flag value expander is a load failure that is reported and retried, and the result is remembered as applied only after Apply returns. An exception from OnError is logged and does not stop the retry. The immediate reload path is guarded like the timer paths.
…ets a live watch The platform reports neither the deletion of a watched directory nor a new directory of the same name, so a watch on a replaced directory delivers nothing. The watcher now checks every second that each directory exists and that each configured file's state matches its last notification. A change without a notification is reported and the directory's watch is set up again.
Link resolution returns no target for a reparse point that is not a link, such as a file kept by a cloud storage provider. Such a file was reported as absent. The link resolver is now a parameter, so the case is tested.
|
bugbot review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…a notification The periodic check compares the configured files' states before it examines whether the directory exists, so the absence of the files of a directory that was removed or moved away is reported, and the reload clears the data they supplied. The watch is still dropped and set up again when the directory appears.
On Windows, deleting a watched directory raises the file system watcher's Error event with an access-denied exception. The error handler dropped the watch and retried its setup, so the periodic check skipped the directory and the warning that the directory no longer exists was never logged, while the first retry logged the deletion as an error. The outcome depended on whether the check or the error event ran first, which made DeletedDirectoryIsWatchedAgainWhenItAppears fail intermittently on Windows. The error handler now checks whether the directory still exists. When it does not, it handles the directory as the periodic check does: the deletion is logged once as a warning, the files' absence is reported when it was not already, and the setup failures that follow are logged at debug level until the directory exists again.
|
bugbot review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit e737bd5. Configure here.
…ence On macOS, a notification for a write made shortly before the watch was set up can be delivered after it, tens of milliseconds later. A notification that names a configured file is reported without comparing its state, so the tests that assert that nothing is reported saw a change from the write that created their file. Those tests now wait out and discard such notifications first.
On Windows, a file that is open for reading with the default sharing mode cannot be written, replaced by a rename, or deleted until the read ends. The reloader reads a file right after it changed, so an editor that saves again, or a tool that replaces the file, could fail while a reload was in progress. The reloader test that fixes a file during a 20 ms retry loop hit this on windows-latest: its own write was refused while the retry was reading. FileDataReader opens files so that others may read, write, rename, and delete them, and otherwise behaves like the file data source's reader, including its wait when a writer denies reading. The reloader uses it by default. The file data source's reader is unchanged.
…t never settles The settle window moved out with every change notification, so a file that was written more often than the window never reloaded until the writing stopped. The window is now bounded: a reload runs no later than MaxDebounceDelay, one second by default, after the first notification of a stream, and the next notification starts a new stream. The Node engine has the same bound.
…le, with an ordinary write A plain rewrite is not atomic: the write and the modification-time update are separate steps, and a poll that lands between them legitimately reports an extra change. The poller promises generous detection, not one callback per logical change; the reloader coalesces and skips the excess. The test that wrote through a temporary file and a rename hid that behavior. The helper writes in the ordinary way again, and the test drains the callbacks of the write and then requires silence, which still fails for a poller that does not update its baseline.
|
The file data poller test that expected exactly one callback per rewrite is replaced by one that checks the poller goes quiet once the file is stable, following launchdarkly/go-server-sdk#473. A plain rewrite is not atomic (the write and the modification-time update are separate steps), so a poll that lands between them legitimately reports an extra change; the poller promises generous detection and the reloader absorbs the excess. The test writes in the ordinary way, drains the callbacks of the write, and then requires silence, which still fails for a poller that does not update its baseline. Written by Claude Code on behalf of @kinyoklion. |
…te before requiring silence Two reloader tests fix a failing file while the automatic retry is running and then require that nothing further is reported. The write is ordinary, not atomic, so a retry can read the file while it is empty or half written and report that failure; the report is correct for what the retry saw. The tests now discard the reports that arrived before the recovery was applied and require silence only for what follows it.
|
The ubuntu Reproduced locally with a deliberately slow write (truncate, sleep 60 ms, write): both tests failed 3/3 before the change and passed 3/3 after it. The tests now discard the reports that arrived before the recovery was applied and require silence only for what follows (90159a0). Verified with the local project references ( Written by Claude Code on behalf of @kinyoklion. |
This branch has not been deployed
1 participant
Summary
This is the first of the file-based flag overrides changes for the server SDK, as defined by the OVERRIDE specification. The override source that a later change adds needs file reading code with the reliability the specification requires of a source that operators edit during an incident: a reload that keeps the last good data when a file is unreadable or malformed, a retry after such a failure that does not wait for another change notification, coalescing of the burst of notifications that one edit produces, a file that does not exist yet contributing no data, and a change detector that does not depend on notification delivery.
The new
Internal/FileLoadingnamespace holds that code. It is purely additive: nothing in the SDK uses it yet, and the existing file data source keeps its current behavior.FileDataParserparses the file data document format (flags,flagValues,segments) into aFileDataDocument, keeping each entry's own version, with the same optional alternate parser for non-JSON content as the file data source.MakeFallthroughFlagWithValueexpands aflagValuesentry into a flag that is on and serves its single value by fallthrough, which is the form the override source uses.FileDataMergercombines documents in configured order withFailorIgnoreduplicate keys handling and reports how many entries it kept from each document.FileDataReloaderowns the reload cycle: it serializes reloads, debounces change signals with a settle window that each signal extends, bounded so that a stream of signals which never settles still reloads within a second, retains the last good result on failure by not applying anything, retries a failed reload after a bounded delay, reports an identical repeated failure once, skips a reload whose raw contents are unchanged, and optionally treats a missing file as a file with no content. Dispose never waits for an in-flight reload. Requests that arrive while a reload runs coalesce into one follow-up reload. An exception from Apply, OnError, or the value expander is reported like any other failure and retried, and nothing from that load is remembered.FileDataReaderreads a file while allowing others to write, rename, or delete it, so that a reload in progress never fails the editor or tool that is updating the file on Windows. The file data source's reader is unchanged.FileDataPollerdetects changes by examining each file's modification time, size, and existence on a fixed interval. A configured path that is a symbolic link is observed through its final target; on frameworks without link resolution a link is observed by its content.FileDataWatcherdetects changes throughFileSystemWatcheron each file's directory, so a file that appears later or is deleted is reported. BecauseFileSystemWatcherreports the deletion of its directory as an error on some platforms and not at all on others, and reports nothing when the directory is replaced, the watcher also checks its directories once per second: a missing directory, whether reported or found by the check, is watched again once it exists, a configured file that changed without a notification is reported and its directory watch re-created, and an event for another entry compares the configured files through any link, so a swap of a linked file is detected.FileDataSource,FlagFileParser, and the file data source builder are unchanged, and their existing tests are unchanged.FileDataSourceExistingBehaviorTestadds tests that pin behavior of the file data source that its existing tests did not assert directly, so that later changes to the shared code cannot alter it: every successful load is applied even when the content is unchanged, the 600 ms retry delay and its warning on every failed attempt, the log messages and levels for a failed load, a path in a missing directory being a failure even withSkipMissingPaths, and a deleted file producing no reload.Every new component has unit tests, including the debounce settle window, retry without further signals, last-good retention and recovery, serialized reloads, and non-blocking dispose. No public API changes.
SDK-3248
Note
Overview
Adds a new internal
Internal/FileLoadingstack for reliable file-based flag data loading (ahead of override-source work). Nothing in the SDK wires to it yet;FileDataSourceand its builder stay unchanged.Parsing and merge:
FileDataParser/FileDataDocumentread JSON (or an optional alternate parser) forflags,flagValues, andsegments, withMakeFallthroughFlagWithValuefor simple overrides.FileDataMergermerges multiple documents in path order with fail-or-ignore duplicate keys.Reload pipeline:
FileDataReloaderserializes loads, debounces change signals (with a max delay), retries failures without new notifications, skips unchanged raw content when configured, keeps last-good data by not callingApplyon failure, and supports optional missing-file skip.FileDataReaderreads with share modes that don’t block writers on Windows.Change detection:
FileDataPollerpolls mtime/size/existence;FileDataWatcherusesFileSystemWatcherplus periodic checks for missing dirs, dead watches, and symlink/ConfigMap-style updates viaFileState.Tests: Broad unit coverage for the new types, plus
FileDataSourceExistingBehaviorTestto lock current file data source behavior when shared code is adopted later. Test helpers addTempDirectoryandSymbolicLinks.Reviewed by Cursor Bugbot for commit 90159a0. Bugbot is set up for automated code reviews on this repo. Configure here.