fix: Keep overrides through an in-place rewrite of their files by kinyoklion · Pull Request #479 · launchdarkly/ruby-server-sdk

kinyoklion · 2026-11-03T00:58:54Z

Summary

Applications and editors rewrite override 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 this stack, that reload applied the empty file as a document with no entries, so the override source briefly removed every override, and the client served LaunchDarkly data for every overridden flag until the next reload. A slow rewrite (truncate, pause 300 ms, write) made a client serve override-a -> ld-value -> override-b, for JSON and YAML files, in polling and watching modes.

The OVERRIDE spec's retry requirement exists to keep "a partially written or briefly invalid file from emptying the layer" (section 1.3). This change keeps the overrides through such a rewrite. It changes only the shared file data code under FileOverrideSource (Impl::FileData); the file data sources (FileData.data_source and FileData.data_source_v2) do not use it and are unchanged.

The format comes from the file name, and an empty JSON file is invalid

Every override file was parsed with the YAML parser, which also reads JSON, so an empty file was always a YAML document with no entries. The parser is now chosen by the file name, as the spec recommends (sdk-specs #272): a name that ends in .yaml or .yml, in any case, is YAML, and any other file, including one with no extension, is JSON. Whether an empty file is valid is then up to the format, like the rest of the content:

An empty or white-space JSON file fails to parse, with the message the file has no content, which is not a JSON document, and the last good overrides stay, as for any other content that cannot be parsed. To remove every override, write {} or delete the file.
An empty YAML file, or one with only comments or a null, is still a document with no entries.

Other effects of parsing JSON files as JSON, for a file named .json or any other name that is not YAML, including one with no extension:

A YAML document no longer loads, and neither does YAML syntax in a JSON document: a # comment, or a trailing comma.
JSON numbers read as JSON: 1e3 is 1000.0, where the YAML parser read it as the string "1e3".
Content after the end of the JSON document is an error; the YAML parser ignored it.
On JRuby, JSON indented with tabs now loads; JRuby's YAML parser rejected tabs.
A duplicate key inside one JSON object, and a JavaScript comment, follow the application's json gem: 3.x rejects both, 2.x accepts them (2.13 and later warn about a duplicate key and keep the last value). The SDK parses all of its other JSON the same way, and the override_source docs say so.

For both formats, files are read as bytes and taken as UTF-8, without a UTF-8 byte order mark, whatever the process's default encoding. Content that is not valid UTF-8 fails to parse, like any other invalid content: the failure is logged once and the data stays. On MRI the YAML parser accepted a Latin-1 file, and the strings it produced later made event serialization fail. A file with a UTF-16 or UTF-32 byte order mark, which Windows PowerShell 5.1 writes with >, is reported as encoded in UTF-16 or UTF-32; data files must be UTF-8; it is not decoded.

Since the format comes from the name, the same bytes can parse differently in another file, so the skip-unchanged digest now covers each configured path, whether its file is present, and its content; before, it covered only the content of the files that were present, and the same bytes moved from a JSON file to a YAML file were skipped as unchanged.

I checked the rest of the file data code for other rules that decide by content where the name should, and found none. A missing file is handled before parsing, so it never reaches the JSON rule. The skip_missing_paths documentation now says a missing file "contributes no data" instead of calling it "a file with no content".

A file that is emptied is held back before it is applied

With the JSON rule alone, a JSON file that is empty for a moment fails to parse and is retried, and that failure is logged at error level. A YAML file can legitimately be empty, so its parser cannot tell an emptied file from one in the middle of a rewrite. So the reloader now holds back a file that a load finds with no content while the data currently applied has content from that file. No content means empty, white space only, or YAML that the parser reads as a null: a read in the middle of a rewrite can see only a leading comment or ---. A YAML document such as {} is content and is applied at once; so is flagValues: alone, which the parser reads as a mapping (see "Not changed" below). Content with no JSON document is only parsed when the file is not held back, and then fails.

A load that holds a file back keeps the current overrides, logs at debug level that it will read the file again, and arms a retry, whatever the format. It reports no failure: it does not log at error level, call on_error, or change which failure the reloader last reported. reload_now returns false for it, as for a failure.

For each file, the reloader keeps whether it contributed content to the data currently applied (updated when a load is applied or skipped as unchanged), and, once a load has found the file with no content: since when it has been so without being written, its state then (modification time and size, through any symbolic link, with the poller's helper), and when a load first held it back.

A file is held back until it has stayed so, without being written, for exactly the retry delay (1 second), counted from the clock read after that file's own read, so a slow read of another file cannot shorten it. A change of its state means it was written since, for example rewritten and truncated again between two loads, so its delay starts over; so does a read that finds it with content, even content that fails to parse, or finds it missing. Modification times are only compared with each other.
Each file is held back for at most five retry delays from the first load that held it back. That bound goes on across reads that find the file with content and across loads that hold a file back, so a file that keeps being rewritten so that every load finds it empty, files rewritten in turn, and files emptied in staggered turns are bounded. It starts over only at a load that holds nothing back, whether that load applies, is skipped as unchanged, or fails, for each file that load read, including a file whose read or parse failed and a missing file. One file reaching its bound does not cut short another file's rewrite.
A load reads and classifies every configured file, even after one fails to read or parse, so files emptied together start their delays in the same load and are applied together, and a file after a failing file is read too. Then the load fails with the first failure, if there is one, which is reported; otherwise it holds files back, if any; otherwise it merges the files.
The retry that a hold arms comes when the delay or the bound of the first due held file is over, plus a tenth of the retry delay, from the clock read after the reads. A later load within the delay re-arms it for the rest of the delay, not for a whole delay. A failure still arms the retry for the retry delay. The retry logs at debug level that it reads the file data again because a file was found empty.
After its delay, a load parses the file: a YAML file with no content is a document with no entries, and a JSON file fails to parse and keeps the data. The reloader keeps the file's entry, so later loads keep parsing it while it stays unchanged; a later rewrite of a JSON file that stayed empty for long is held back again with a fresh bound.
A file that contributed no content to the data in effect is parsed at once: a placeholder YAML file that has been empty from the start, a file that was missing (so a file deleted and then created empty), and a file whose emptying has been applied.
The rule applies to every load, including a retry left over from an earlier failure, and a load for a change to another file while this one is held back. Changes to the other files wait while a file is held back, because a reload applies all files or none.
With retries off (a retry delay of zero), nothing would read the file again, so a file with no content is parsed right away. FileOverrideSource always retries.
Times come from Process.clock_gettime(Process::CLOCK_MONOTONIC), in floating-point seconds, which does not overflow. Tests inject a controlled clock for the hold; scheduling always uses the system's clock.

A rewrite is covered when the gap between truncating and writing is shorter than the retry delay. A writer that pauses longer gets the empty file applied (YAML) or reported (JSON), as before. On a file system that records modification times in whole seconds, a second truncation within the same second is not seen as a new rewrite.

Errors that a load does not expect

An error that a load does not expect is logged as Unexpected error in file data reloader and retried like a failed load, and the worker thread rescues errors inside its loop, so that no error stops reloading for good. An earlier version of this branch read files in a way that raised on a UTF-16 byte order mark, and that error ended the worker at runtime, or made the data system stop the source at startup.

Documentation

FileData.override_source now says how a file's format is chosen and how it differs from the file data source, 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 with no content (empty, or YAML with only a comment or ---) is held back for about a second, at most about five while files keep being rewritten, that changes to the other files wait too because a reload applies all files or none, and that after the wait only YAML is applied empty while JSON keeps the data. It recommends replacing a YAML file in one operation (see below). The Reloader and Document.parse comments describe the rules above.

Not changed: YAML that is cut off

A read that overlaps a write can see the first part of a YAML file. A part with only a comment or --- is held back (above), but most other parts are valid YAML documents: a cut between two entries gives fewer entries, a cut inside a value gives the value cut short (on-value becomes on-v, or on, which is true), and a cut right after flagValues: gives a mapping whose flagValues is null, a document with no entries. The part is applied until the next reload. JSON that is cut off fails to parse. 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

Document and FileData tests: the format of each file name (.yaml, .yml, any case; JSON for .json, no extension, .txt and .yaml.json); an empty or white-space JSON document fails with the message above, including for a file with no extension; empty YAML (empty, white space, a comment, ---, ~) is a document with no entries; a JSON null and a YAML false are not objects; JSON numbers and trailing content; a YAML document in a JSON file fails; a file with a byte order mark reads, and one with only a byte order mark is empty.
Reloader tests on a controlled clock, with an hour's retry delay on that clock so that no real retry fires, and synchronous loads (holding back a file, on a controlled clock):
- A file is held back at 95 % of the delay and applied at exactly the delay.
- A rewrite and a new truncation between two loads starts the delay over; so does one of the file a symbolic link leads to.
- One file truncated before every load is held back for five retry delays and then applied; so are two files rewritten in turn.
- A slow read of an earlier file (the clock moves 0.9 delays during it) does not shorten a later file's delay.
- A file kept written while empty is still bounded when another file fails every other load.
- Two files kept empty and truncated again in turn, so that one of them is always within its delay, are applied after the bound.
- A file held back during a long failure of another file gets a fresh bound for a later rewrite once a load has read it again, so the load that fixes the other file does not apply it empty.
- When a read after the held file takes longer than the rest of its delay, the retry asks for only the margin.
- A file saved in place with an error and fixed in place six delays later has the fix's gap held back, for YAML and JSON, with no further error; also when the fix's truncation looks exactly like the first one.
- A file whose read fails, and a file found missing, at a load that holds nothing back get a fresh bound, and so does a file configured after a file that fails, which is now read too.
- A load that holds a file back and then fails starts no bound over.
- A file deleted and created empty again with its first emptying's modification time starts a new delay.
- A file that reaches its bound does not cut short another file's ordinary rewrite.
- The bound starts over once data is applied, and once a rewrite with the same content is skipped as unchanged.
- A read with content starts a file's delay again even when its state looks the same (a stubbed state, and a modification time set back to the first emptying's).
- The retry each hold asks for: the delay plus a tenth after the first hold; the rest of the delay after a later one; the bound when it comes first; the first due of several held files; the same through a change signal; the retry delay after a failure.
- Files emptied together are applied together after one delay.
- YAML with only a comment or --- is held back, and {} is applied at once, in YAML and in JSON.
- A JSON file that stays empty keeps failing without being held back again; when it is rewritten more than five delays later, it is held back again, and the rewrite is applied with no further error.
- A file emptied again after its content was read, and a file created empty after it was missing, are held back even when their state looks the same (a stubbed state, as on a file system with coarse modification times).
- A placeholder and a file that was missing are not held back.
Reloader tests on the real clock (a file that is found empty), with a 1.5 s retry delay where a test must finish a rewrite before the retry, waiting on the debug line:
- An emptied YAML file is held back, and the rewritten content is applied with no empty application in between; the debug line is logged.
- A file with only white space counts as empty.
- A YAML file that stays empty is not applied within the delay, and is applied by the retry. Once that is applied, a new truncation is applied at once.
- An emptied JSON file is not reported by the load that finds it empty; the retry then reports it, and the data stays.
- A further change signal within the delay does not apply a file that stayed empty.
- Two quick rewrites, where the second truncation comes before a load reads the first rewrite's content, are held back again, through a change signal and through the retry alone.
- A change to another file within the delay starts a load that holds the emptied file back again.
- A file that cannot be parsed is reported by the load that holds another, emptied file back.
- When the same content moves to another file, the move is applied, not skipped as unchanged, and emptying the other file is held back. The same bytes moved from a JSON file to a YAML file are applied as YAML.
- A load with two failing files reports the first.
- A load halfway through the delay re-arms the retry for the rest of it: with a 2 s retry delay, the file is applied about 2.2 s after it was first found empty, not 3 s.
- The retry after a hold logs that it reads the file data again because a file was found empty; the retry after a failure logs that it retries after a failure.
- An empty placeholder YAML file does not delay a change to the other file; a file deleted and then created empty is applied right away; a retry left over from a failure holds an emptied file back; reload_now returns false for a hold; a reloader stopped during the read neither logs nor arms a retry; with retries off an emptied file is applied right away; at the initial load an empty YAML file has no entries and an empty JSON file fails.
Poller.observe reads the state of the file a symbolic link leads to.
End-to-end, with real in-place rewrites (truncate, pause 300 ms, write; InPlaceWrites.rewrite in spec/in_place_writes.rb):
- The override source never supplies a snapshot without the override, for JSON and YAML, with polling and watching change detection, over two rewrites each; and for a YAML file whose first line, a comment, is written before the pause.
- A client with the override source, sampling evaluations, serves override-a -> override-b and never the LaunchDarkly value, for JSON and YAML.
- With the default timing (polling once a second, 1 s retry delay), the override source never supplies a snapshot without the override while a YAML file is rewritten about once a second, five times.
- A JSON file that stays empty keeps the overrides in effect and logs the parse failure; a YAML file that stays empty removes them after the retry delay.
The reloader and override source tests now rewrite their files in place with a 30 ms pause after truncating, so they exercise this case deliberately instead of by chance. The reloader test retries a failed reload after the retry delay and recovers when the file is fixed no longer accepts an empty application as the recovery.
On the base of this branch, 96 of the new and changed tests fail: 14 override source tests (for example expected: ["override-a", "override-b"], got: ["override-a", "ld-value", "override-b"]), 60 reloader tests (the new ones that pass cover behavior the base already has, such as a placeholder, a file deleted and created empty, retries off, a stop during the read, an empty YAML file at the initial load, and a parse failure of another file), 21 document tests (many of them because Document.parse now takes the format), and the new poller test.
Mutation check: each part of the fix was reverted on its own (65 mutants, in a separate worktree), and the four changed spec files were run against each. 58 fail at least one test, each in the test written for it. That includes the timing mutants (a whole delay, no margin, no clamp, the bound ignored, the last held file deciding, either arming path ignoring the computed delay, the clock read once per load or at the start of the load, the worker keeping an earlier retry), the bound mutants (a bound that starts over when a file alone stays empty for its delay, at every load, at no load, only for files found empty, not for a file whose read or parse failed or that was missing, or that is set for a file not held back), the file state ignoring the size, a failure that stops the load, the last failure reported, a hold that comes before a failure, a digest without both the path and the presence, and the encoding and error mutants (no UTF-8 check, no UTF-16 message, no byte order mark removal, YAML white space parsed, an unexpected error not rescued, not retried, or rescued outside the worker loop). Seven are equivalent here: the retries-off guard, since with a retry delay of zero the delay check is never true anyway; remembering and clearing the records when data is applied or skipped as unchanged (three mutants), since such a load holds nothing back, so the bounds start over anyway, a skip now means the same paths, presence and content, and a file parsed empty is not held back again until a load applies it with content; starting over every configured file instead of the files read, since a load now reads every file; and leaving the path, or the presence, out of the digest, since either one tells the files apart in a fixed order (leaving out both is killed). The guard and the clearing are kept: the guard states the retries-off rule, and the clearing drops records that no longer matter.
Loops: the four changed spec files ran 10 times in a row on Ruby 3.2 and 10 times on JRuby 9.4 with no failures (184 examples each run).
Full suite: bundle exec rspec spec on Ruby 3.2.3: 1502 examples, 1 failure; on JRuby 9.4.6 with --tag ~flaky, as CI runs it: 1469 examples, 1 failure, 1 pending. The failure is can be automatically loaded by Bundler.require, which needs write access to the system gem directory on this machine and fails the same way on the base of this branch. Earlier JRuby runs also failed one of two FDv2 data system timing tests now and then; this PR does not touch them. bundle exec rubocop: no offenses.
Contract tests: the flag overrides suite of sdk-test-harness (3.2.0-alpha.13, built from #515, whose override files are rewritten in place and have no extension) passes against this branch's test service: 79 tests ran, all passed.

Place in the stack

This PR is based on #454, the top of the Ruby overrides stack, and changes code that #449 (the parser and the reloader) and #453 (the override source docs) add. It can be merged on top of the stack or folded into those PRs.

Revisions

Second commit: YAML that the parser reads as a null (a leading comment or --- seen mid-write) is held back; the five-delay cap applies to the run of holding loads, which any load that holds nothing back ends, instead of to each file; the delay check is exact and the retry of a hold is armed for the rest of the delay plus a tenth; the file's state is read through symbolic links with the poller's helper; the delay tests run on a controlled clock. With the first commit alone, the new end-to-end test with a leading YAML comment supplies empty snapshots in both modes, and a load halfway through the delay pushes the application from about 2.2 s to 3 s.
Third commit: each held file has its own bound instead of one for the run of holding loads, so a failing other file no longer restarts it and one file's bound no longer cuts short another file's rewrite; applying data or skipping it as unchanged starts the bounds over; the clock is read after each file's own read; the retry delays a hold asks for are pinned by tests; the retry after a hold logs why it reads again. With the second commit alone, the tests for the slow read, the failing other file, another file's bound and the log fail.
Fourth commit: a held file's bound starts over only at a load that holds nothing back, for every file that load read, whatever its outcome: no longer when the file alone stayed empty for its whole delay, and now also at a failed load. With the third commit alone, two files emptied in staggered turns are held back without end, and a file held back during another file's long failure keeps a stale bound and is applied empty by the load that fixes the other file.
Fifth commit: a load records every file it gets to, including a file whose read or parse fails and a missing file, and a load that holds nothing back starts the bound over for all of them; content that fails to parse ends a file's empty state. With the fourth commit alone, a file saved in place with an error and fixed in place more than five delays later has the fix applied empty, for YAML and JSON.
Sixth commit, after the multi-agent review: files are read as bytes, taken as UTF-8 and checked, so a UTF-16 or UTF-32 file and content that is not valid UTF-8 fail to load like other invalid content instead of raising out of the load; an error a load does not expect is retried, and the worker survives errors; YAML with only white space, including a tab, is a document with no entries; the tests record the delay of the retry that is armed, so both arming paths are checked; comments fixed.
Seventh commit: a load reads and classifies every file even after one fails to read or parse, and fails with the first failure after them, so a file configured after a failing file gets a fresh bound once it is read; the skip-unchanged digest covers each path and whether its file is present. With the sixth commit alone, a file after a failing file kept a stale bound and its next in-place rewrite was applied empty, and the same bytes moved from a JSON file to a YAML file were skipped.

_{Opened from termag}

Note

Overview
Fixes a gap where in-place file rewrites (truncate then write) could briefly load an empty override file and drop all overrides, so clients served LaunchDarkly values until the next reload.

Parsing by filename: Override files ending in .yaml/.yml use YAML; everything else uses JSON (no longer YAML-for-all). Empty JSON fails to parse and keeps last good data; empty YAML is still a valid empty document. Files are read as UTF-8 (BOM stripped), with explicit errors for invalid UTF-8 and UTF-16/UTF-32. Skip-unchanged digests now include path and presence, not just bytes.

Hold-back in Reloader: When a file that currently contributes overrides is read as empty (whitespace, or YAML that parses as null), the reload does not apply—it keeps existing overrides, logs at debug, and schedules a retry (~1s delay, capped at ~5 delays per file, coordinated across multi-file reloads). JSON {} and similar real content still apply immediately; placeholders and never-had-content files are not held back.

Supporting changes: Poller.observe for symlink-aware mtime/size; worker loop survives unexpected errors and retries them.

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

Applications and editors rewrite files in place: they truncate the file, then write the new content. A reload in between found the override file empty and applied it as a document with no entries, so the client briefly served LaunchDarkly data for every overridden flag. The parser is now chosen by the file name: a name that ends in .yaml or .yml, in any case, is YAML, and any other file is JSON. An empty or white space JSON file is not a JSON document and fails to parse, so the last good overrides stay. Files are read as UTF-8 without a byte order mark. A load that finds a file empty while the applied data has content from it puts the file off: it keeps the current data and arms the retry without reporting a failure. The file is put off for one retry delay from the first load that found it empty; a write while it is empty (a new rewrite, seen through the modification time) starts the delay again, up to five retry delays in all. A file that contributed no content to the applied data, such as an empty placeholder, is parsed at once.

kinyoklion · 2026-11-03T00:59:02Z

bugbot review

cursor