[kinyoklion]

Summary

Applications and editors rewrite data files in place: they truncate the file and then write the new content. A reload that reads the file in between finds it empty. On v7, that reload applied the empty file. The format is chosen by the content (a file that does not start with { is YAML), so an empty file of any name is a YAML document with no entries. A slow rewrite (truncate, pause 300 ms, write) made a client serve a -> default -> b, for JSON and YAML files alike:

• the override source (ldoverrides.FileSource) briefly removed every override, in watching and in polling mode;
• the file data sources (ldfiledata with ldfilewatch.WatchFiles, and ldfiledatav2 as an FDv2 synchronizer) briefly served no flags.

The file data source has behaved this way since before the shared engine: 7.17.1 serves the same sequence for the same rewrite.

The OVERRIDE spec's retry requirement exists to keep "a partially written or briefly invalid file from emptying the layer" (requirement 1.3.10). The three sources share internal/filedata.Reloader, so this change covers all of them.

An emptied file is held back until it has stayed empty for the retry delay

The reloader now remembers, for each configured file, whether the file contributed content to the data in effect (the last load that was applied, or skipped as unchanged; the comparison for skipping covers only the content of the files that are present). A load that finds such a file with no entries holds it back: it keeps the current data, logs at debug level (File <path> is empty, possibly while it is being rewritten; reading it again before applying it), reports nothing, and arms a retry. A file has no entries when it is empty or white space only, or when its content is no YAML document, such as only a comment or ---, which a read can see at the start of a YAML file that is being written. {} is a document and is applied right away.

The hold is bounded by time, not by a count of loads:

• Every load holds the file back until it has stayed so, without being written, for the retry delay (1 second), whatever started the load: the retry, a poll, a touch, a watcher error, or a change to another configured file. A reload applies all of the files or none, so changes to the other files wait too.
• The clock is read after each file's own read, so a slow read of another configured file does not make a file's emptiness look older than it is.
• The reloader records the file's state (modification time and size, from os.Stat, which follows symbolic links on every platform Go supports; the poller uses the same helper) when a load finds it so. If the state differs at a later load, the file was written in between (for example rewritten and truncated again between two loads), so its delay starts over; so does a read with content, even content that fails to parse, or finding the file missing. States are only compared with each other, never with the clock.
• Each file's hold is bounded: a file is held back for at most five retry delays from when a load first held it back, so a file that keeps being written, or files that are rewritten or emptied in turn so that every load finds one of them empty, are applied after about five seconds. The bound is the file's own, so one file reaching its bound does not cut short another file's rewrite. It starts over only at a load that holds nothing back, for every file that load got to: read with content, parsed empty, found missing, or failed to read or parse. That holds whether the load applies, is skipped as unchanged, or fails, so a file saved in place with a typo and then fixed gets a bound of its own for the fix. While any file holds the loads back, nothing restarts a bound: not a read with entries, not a failure, and not a file staying empty for the whole delay. Every load reads every configured file, even after one of them fails to read or parse, so no file's bound is left over from before a failure. Applying data also clears every file's record.
• The hold arms its retry for when the first held file has stayed so for the retry delay, or has reached its bound, plus a tenth of the retry delay, measured from the end of the load. A change signal during the hold does not postpone it. A failure still arms the retry delay. The retry after a hold logs Reading file data again after a file was found empty at debug level, and a retry after a failure keeps Retrying flag data load after earlier failure.
• Once the delay is over, the file is parsed by that load and by every later one until it is written: a file that is still empty is a YAML document with no entries, applied about a second after it was emptied.
• A load reads and classifies every configured file before it decides anything, even after a read or parse failure, so files that are emptied together start their delays in the same load and are applied together, and every file's bound is kept up to date. Then a failure comes first: the load fails with the first file that failed to read or parse, and is reported, even when another file is held back. Then a hold. The merge (duplicate keys) runs only after every file is read.

What is not held back:

• A file whose content is not in effect: a placeholder that has always been empty, an emptied file whose empty state has been applied, and, at the initial load, any file.
• With SkipMissingPaths (the override source), a file that is deleted and then created empty: the deletion was applied, so the file's content is no longer in effect. The file data sources fail a load on a missing file and keep their data, so an empty file created in its place is held back like an emptied one.
• Any empty file when retries are off (a file data source without a reloader loads once), since nothing would read it again. ldfiledatav2's Fetch, used as an initializer, reads once and is unchanged.

Holding back reports nothing: no OnError call (so ldfiledata does not move to Interrupted), no error log, and the record of the last reported failure is left alone, so a failure that comes back afterwards is still a repeat and is not reported twice.

A rewrite is covered when the file stays empty for less than the retry delay after the first load that finds it empty, however many loads read it in that time. A gap shorter than a second between truncating and writing is therefore always covered, unless the file has already been held back for about five seconds without a load in between that held nothing back. The bounds of different files do not overlap: with n files that each keep being written while empty, started about 4.5 retry delays apart, the first load that applies comes after about 4.5 n + 0.5 retry delays, and a failure timed to every expiry of a bound could hold the loads back without end. Both are accepted. A writer that pauses longer gets the empty file applied once the delay is over.

Not changed: the format is chosen by the content

An empty .json file is still a YAML document with no entries, so a JSON file that stays empty removes its overrides (or the data source's flags) once it is no longer held back, about a second after it was emptied. This is released behavior, and sdk-specs #272 allows it: it recommends choosing the parser by extension, and lets an SDK whose file data source already inspects the content keep doing so for override files. The .NET SDK chooses by extension and treats an empty JSON file as invalid, so there the last good data stays. The docs now say how an empty file is read.

Documentation

• ldoverrides.FileSource says that a file can be rewritten in place, how to remove a file's overrides (delete it or write {}), that a file whose content is in effect and that is found empty or with only a YAML comment or --- is held back for about a second, at most about five while it keeps being rewritten, that changes to the other files wait while a file is held back, how the format is chosen, that a file that stays empty, even one named .json, removes its overrides once it is no longer held back, and that JSON cut off fails to parse and keeps the overrides.
• The ldfiledata and ldfiledatav2 package docs say the same for flags, and that the retry and the hold apply only with a reloader; without one the files are loaded once.
• Both recommend replacing a YAML file in one operation (see below). Neither tells applications to replace JSON files atomically.

Not changed: YAML cut off part way

A read that overlaps a write can see the first part of a YAML file. If the cut falls between two entries, the part is valid YAML with fewer entries; if it falls inside an unquoted value, the value is cut short (flag1: fa for flag1: false is the string "fa"). Either is applied until the change signal for the rest of the write reloads the file. So is a read that sees only a first key, such as flagValues: before any entry: it is a YAML document with a member and no entries, so it is not held back like a comment or ---. JSON cut off anywhere fails to parse and is retried. This predates this change. Telling a part from a whole file needs a content rule this change does not add, so the docs recommend replacing a YAML file in one operation.

Testing

• Reloader tests on the real clock, with a 1.5 s retry delay (internal/filedata/reloader_test.go), waiting on the debug line rather than fixed sleeps:
  • An emptied file, and one rewritten to white space with a tab (which YAML cannot parse), is held back, and the rewritten content is applied by the retry with no empty application in between.
  • A file that stays empty is applied by the first retry, not the second, and that retry logs that it reads the files again because a file was found empty; a retry after a failure keeps the failure message.
  • A change signal half way through the delay does not postpone the retry (3 s retry delay).
  • A retry left over from a failure that finds the file emptied holds it back instead of applying it empty.
  • Holding back reports no failure, and a failure that comes back after it is not reported again.
  • An empty placeholder file does not delay a change to the other file; at the initial load an empty file is applied right away; with SkipMissingPaths, a file that is deleted and then created empty is applied right away; with retries off, an emptied file is applied right away.
• Reloader tests on a fake clock, with a retry delay of an hour so that no real retry fires, each load run synchronously, and real in-place writes. Where a test empties a file again, it also moves the file's modification time later than any it set before, because Windows advances file times only every few milliseconds and two truncations in a row would otherwise look the same. A test hook replaces the file reader to make one read slow:
  • A load at 95% of the delay holds the file back; one at exactly the delay applies it.
  • A file with only a YAML comment or --- is held back; {} is applied right away.
  • An emptied file whose empty state has been applied is not held back when it is written again.
  • Two quick rewrites (the second truncation before any load reads the first content) restart the delay, and so does a read with content, even when the file is then emptied with the same modification time and size (two tests: one where another file fails, one where another file is kept written while empty, so that nothing is applied in between).
  • The bound: one file written again before every load, two files rewritten in turn, and two files kept empty and truncated again in staggered turns, are applied after five retry delays; failed loads of another file that still hold the file back do not restart its bound, nor a load that holds a file back and then fails; a file after a failing file gets a new bound once a load that holds nothing back reads it again, and a failing load with two broken files reports the first; a file held back during a failure gets a new bound once a load that holds nothing back reads it again; and one file reaching its bound does not cut short another file's rewrite.
  • A fix saved in place after a typo saved in place is held back (YAML and JSON, and YAML with the modification time of the first emptying), after six retry delays of loads that failed on the typo, with no error logged for the hold; a file that fails to read (deleted, without SkipMissingPaths) for six retry delays is held back when it is created empty again; and with SkipMissingPaths, a file found missing gets a new delay when it is created empty again with the first emptying's modification time.
  • A slow read of the first file, during which the second is emptied, does not make the second file's emptiness look older.
  • The second of two files is emptied while the first changes; neither change is applied until it is rewritten.
  • Two files emptied together are held back by the same load and applied together.
  • A second emptying 4.9 retry delays after the first, once the first was applied, gets a delay and a bound of its own.
  • A file that fails to parse is reported while another file is held back, after the delay the emptied file is parsed by every load, and a file that has been empty for longer than its bound while loads failed is held back again, with a new bound, when it is written.
  • The retry a hold asks for: a whole delay plus a tenth after the first hold, the rest of the delay after a later one, the bound cutting it short, the file that is due first deciding when several are held, the time measured from the end of the load (down to just the margin when the delay ran out during the load), and the retry delay after a failure. Both arming paths, a change signal and ReloadNow, arm what the hold asks for.
  • The content of one configured file is moved to the other (mv second.yaml data.yaml), so the load is skipped as unchanged; a later in-place rewrite of the file it was moved to is held back.
• In-place rewrites, with a new test helper sharedtest.RewriteFileInPlace that truncates the file, pauses, and writes it through the same handle:
  • The override source never applies a snapshot without the override during a 300 ms rewrite, for JSON and YAML, in watching and polling modes (ldoverrides/file_source_test.go; the polling test lowers the interval inside the package so a poll lands in the gap).
  • End to end (ldclient_file_in_place_rewrite_test.go), a client samples StringVariation every 2 ms during a 300 ms rewrite and serves a -> b, never the default, for JSON and YAML, with ldfiledata + ldfilewatch, with ldfiledatav2 as a synchronizer, and with the override source in watching mode.
  • Each of these also checks the debug line, so that it cannot pass because no reload read the file while it was empty.
• On v7 without the fix, 37 of the 43 new tests fail, including every in-place rewrite test. The 6 that pass check the limits of the rule (placeholder, initial load, deleted file, retries off, parsing after the delay, the first failure reported). The earlier versions of this PR fail 27 (one load), 16 (0.9 slack, per-file bound from the first emptying), 12 (shared bound for a run of holding loads), 6 (per-file bound restarted by a file staying empty for the whole delay), 4 (bound not started over for a file that fails) and 1 (loads stop at the first failure, so a later file keeps a stale bound).
• To check that the tests catch what they target, each part of the change was reverted or altered on its own, with a fresh build each time. Every variant fails at least one test, except four that cannot change behavior (below):
  • The delay: no bound by the delay 6, 0.9 slack 2, > instead of >= 4, no restart on a state change 9, a read with content keeping since 2, the record deleted once the delay is over 2.
  • The bound: none 7, restarted by a read with content 2, restarted by a file staying empty for the whole delay 1, not restarted by a load that holds nothing back 5, restarted even by a load that holds files back 4, restarted only by a load that succeeds 5, restarted by a load that holds a file back and then fails 1.
  • What a load records: a file that fails to read 1, a file that fails to parse 1 (and the parse failure not ending the empty state 1), a missing file 1 (and a missing file not ending the empty state 1), a file read with content 2, an empty file that is not held back 1.
  • Failures: the load stopping at a read failure 1, at a parse failure 1, or at white space that cannot be parsed 1; the last failure reported instead of the first 1; a hold decided before a failure 6.
  • The clock: read once per load 1, the retry measured from the start of the load 1.
  • Reading the files: stopping at the first held file 12, only the first file held 9, white space parsed 1, YAML with no document not detected 1, {} treated as no document 1.
  • What is in effect: recorded by every read 2, an empty read recorded as content 2, a missing file not recorded 1, a skipped load not recording it 1, a file never loaded counted as content 2.
  • Reporting: holding back reported as a failure 33, no retry 9, the last reported failure reset 1, the retry after a hold logged as a failure retry 2.
  • The retry: armed for the whole delay 5, without the margin 4, not clamped at zero 1, ignoring the bound 1, decided by the last held file 1, the run loop arming the retry delay 2, ReloadNow arming the retry delay 1.
  • Not detectable: removing the explicit check for retries off (with no retry delay nothing can be held anyway); removing the closed check before holding back (after Close the worker exits and the retry it would arm never runs); keeping the records when data is applied, and starting over the bounds of every recorded file rather than of the files the load read (every load reads every file, so a load that holds nothing back covers them all anyway).
• The 43 new tests passed 12 runs in a row with the race detector (go test -race -count=12, 516 passes) on a loaded machine.
• go test -race ./... passes (31 packages), as do the two proxytest runs, go vet ./..., and golangci-lint v2.14.0 over the SDK, ldotel and ldmiddleware (the make lint targets, 0 issues). The test service builds. One earlier full run failed in testhelpers/ldtestdatav2 (TestSupportsMultipleSynchronizers, a 1 ms channel timeout); that package does not depend on this change, and the same test fails 1 run in 10 on v7.

_{Opened from termag}

---

Note

Overview
Reloads that read a truncated data file (common during in-place saves) no longer apply an empty document and briefly drop flags or overrides. internal/filedata.Reloader now tracks whether each path previously contributed content; when a file with active data reads as having no entries (empty, whitespace-only, or YAML with only a comment/--- via new parseFileContent), it holds back the load—keeps the last good merge, logs at debug, and schedules a retry—until the file stays unchanged for RetryDelay or hits a 5× delay cap, with multi-file loads applying all paths or none.

Retry scheduling now carries variable delays (retryRequestCh sends time.Duration); loads still read every configured file before failing or holding. observe / fileState.equal support mod-time/size detection for restarts. Docs for ldfiledata, ldfiledatav2, and ldoverrides describe the behavior; sharedtest.RewriteFileInPlace plus broad unit and client E2E tests cover JSON/YAML rewrites.

^{Reviewed by Cursor Bugbot for commit 1fd735b. Bugbot is set up for automated code reviews on this repo. Configure here.}

[kinyoklion]

bugbot review

[cursor]

Stale Bugbot comment from a previous run.

[kinyoklion]

bugbot review

[cursor]

Stale Bugbot comment from a previous run.

[kinyoklion]

bugbot review

[cursor]

Stale Bugbot comment from a previous run.

[kinyoklion]

bugbot review

[cursor]

Stale Bugbot comment from a previous run.

[kinyoklion]

bugbot review

[cursor]

Stale Bugbot comment from a previous run.

[kinyoklion]

bugbot review

[cursor]

Stale Bugbot comment from a previous run.

[kinyoklion]

bugbot review

[cursor]

Stale Bugbot comment from a previous run.

[kinyoklion]

bugbot review

[cursor]

Stale Bugbot comment from a previous run.

[kinyoklion]

bugbot review

[cursor]

✅ 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 1fd735b. Configure here.}