Repository navigation
Relay installer strips control characters from secrets - #3
Closed
ChakraFusion wants to merge 75 commits into
Closed
ChakraFusion wants to merge 75 commits into
ChakraFusion wants to merge 75 commits into
Conversation
On a save of ~240k small files (Project Zomboid map chunks) a repeat manifest build took 13-19s in 2.4.0; now ~1.3s, with an identical hash. - Walk with filepath.WalkDir instead of filepath.Walk. Walk's extra Lstat per entry opens every file on Windows and dominated a warm build. - Reuse block read buffers (sync.Pool) instead of allocating 64KB+ per file: a cold build allocated ~15GB of garbage, now ~0.6GB. - Sort with sort.Slice when evicting from the hash cache. The insertion sort was quadratic in the entry count (map order is random). - Raise the cache budget floor to 256MB and scale it at 1MB per game, ceiling 512MB. 64MB was filled by one large save on its own, so the cache evicted and re-read that save on every pass. A budget, not an allocation: small libraries never come near it. - Share the whole-file hash string with the block hash for single-block files (most save files). - FileEntryForInfo: a cache lookup with the FileInfo a walk already has.
…compressing them again Every snapshot stays a complete zip that restores on its own. What changes is how it is written: a file whose content (SHA-256, as already recorded per snapshot and held by the hash cache) matches the previous snapshot of the same branch is copied in as its already-compressed entry (zip.Writer.Copy). Only new and changed files are compressed. On a 204k-file save: a full snapshot 80.5s, a follow-up 8.9s, same size. The archive walk also moves to filepath.WalkDir for the same reason as the manifest walk.
…ll files, skip unchanged manifests Data safety - "Keep mine" (and "Keep both", which ends the same way) recorded every local file as shared before the peer had pulled any of it. When that pull did not finish, the next sync read the peer's missing files as its deletions and deleted them on the device whose version had just been kept - tens of thousands of files on a large save, and the folders differing again re-raised the conflict. Now only what both sides verifiably hold is recorded as shared; if the peer cannot be asked, nothing is. Same fix for extra save locations. TestKeepLocal_PeerThatNeverPulled_DeletesNothingHere fails on 2.4.0. - A sync that would delete more than 100 files and more than 10% of a game, on either side, is held as a conflict instead of applied. A deletion confirmed on purpose (emptied-folder flow) is not held. - Tests that an interrupted or partial transfer never becomes deletions, per file and batched. - The conflict carries uncapped counts of what differs, so the dialog can say what each answer deletes. Performance - Batched pulls (protocol revision 2): small files are fetched up to 256 per request (4MB), 4 requests in flight, over a new /api/p2p/files route. Each file is still written through the PatchWriter (verified, fsynced, renamed). One request per file cost ~200ms on a real LAN; on loopback 5000 files went from 42s to 12s. Peers below revision 2 and relay connections keep the per-file path. - Unchanged games: when this device still holds the agreed base, the manifest request carries ifHash and a peer that still holds it too answers "unchanged" instead of sending the manifest. Older peers ignore the parameter; their full answer is used, not fetched twice. - Manifests are gzipped for clients that accept it (every Go client does, so older versions read them too): ~5.6x smaller with real hashes. - Manifests and batches get a 10-minute client timeout instead of 30s; a large manifest over a VPN never arrived inside 30s, so those games never synced. - An in-sync pass also mirrors the peer's latest snapshot, which only a pull did before; games identical everywhere from the start showed history on one device only. Robustness - Presence: paired LAN peers are probed every 10s on their own ticker. Probes used to ride along with the minute-long reconcile, which waits for a whole sync-all pass; peers reached over a VPN (no UDP broadcast) showed online/offline minutes late. Probe rounds are serialized. - A peer coming back online triggers one sync-all, not one per concurrent request (re-read under a lock). - Games whose save folder is not on this device are skipped by sync-all and dropped from the retry queue instead of failing every 20s. - "syncing X with Y" is logged only when there is something to do; most passes produced only "already in sync" lines, which rotated everything else out of the log within hours.
- A missing save folder is looked for at the identical path on every other drive letter (a Steam library on D: on one PC and E: on another). Exactly one match is adopted for this device and logged; none or several are logged once. Searched at most once an hour. - A save folder on a drive this device does not have is shown as "Not on this device" (muted) instead of a warning, kept out of the warning headline and the notification bell. A folder gone from a drive that exists is still a warning. - The conflict dialog shows exact counts (not the first 100) and says what each answer does: "Keep mine"/"Keep both" delete nothing; "Keep theirs" deletes N files only this device has, in red when that is many.
…ler memory footprint - DaemonAddr waits for startup to finish. The window could ask while the daemon was still starting (8-18s with many watched folders), got an empty address and showed "can't reach ... at http://" until Retry. - GOGC 50 and a 1GiB soft memory limit, unless GOGC/GOMEMLIMIT are set.
Every peer started each run with whatever status the database held, and a peer counts as offline only after three failed probes. For a PC switched off overnight that meant being shown online after every start until three probes - the first ten seconds after the background loops started - had failed. - The presence loop probes at once on start, then every 10s. - The three-miss tolerance is for devices heard from during this run (answered a probe, or seen by discovery, a request or the relay since start). A peer with no sign of life since start goes offline on its first unanswered probe. Adds an opt-in e2e measurement (OPENSAVE_PRESENCE_TIMING) of how long a stopped device is shown online: 30s, by design, for one seen this run.
New package internal/owntouch records the paths OpenSave itself writes, removes or re-dates (pulled files, a peer's deletions, the mtimes "keep mine" touches). The watcher classifies every event by it: a burst made only of OpenSave's own changes moves the recorded hash and nothing else - no auto-snapshot, and no OnChanged sending the sync straight back out. Before, every change a sync applied looked exactly like the game saving. A peer deleting files one request at a time produced an auto-snapshot every couple of seconds, each of a save that was neither the old one nor the new one; they filled the retention budget and pushed out the snapshots that mattered. - a file counts as OpenSave's only if nothing wrote it after the mark (its mtime is not later), so a game saving straight after a sync is still the game's change. - the rescan after new folders come under watch is OpenSave's own when it created those folders (a pull of a save with subfolders), and someone else's otherwise.
Every device now records, per game, which version of the save it holds as a version vector: one counter per device, moved only when the save really changes there (never by a sync; see internal/owntouch). The version travels in the manifest response, and two devices compare versions before anything else: - one older than the other: that device is behind. It takes the newer save whole - every file and every deletion - and only from a device holding it completely; only then does it adopt that version. Until then it is a source for nobody: nothing is taken from it, pushed by it, or deleted on its word. A pull that stops part-way is remembered across restarts, so the half-copy is never mistaken for a change made there. - a device remembers the newest version it has heard of (its target). Two devices that both know a newer one exists do nothing with each other until a device holding it is back. This is what stopped a half-finished copy being passed on, with everything it lacked read as deletions. - an empty folder holds no version, so it fills itself and never raises a conflict. - neither newer: both changed independently. Only this asks. Resolution stays mutual: "keep mine" on one device does not make the other's save simply older - the other device is asked in turn, and the first is not asked again (the answer travels with the version). If both keep their own, the later answer stands. Every sync and every manifest served first checks the save against the hash recorded with its version, so a change the watcher has not reported yet (it waits for the game to finish writing) is a version before anyone compares - otherwise a device with unrecorded work would look merely behind and take the other's save over it. Changes to excluded files are not versions, and neither is writing an exclusion rule: the recorded hash is tagged with the rules it was taken under and simply re-taken when they change. A device fetching back a save it was asked to put back after it was emptied (hold.go) follows the hold's rules. Saves from before versions get one named after their content, so devices that already agreed agree at once; ones that did not are compared as before until they agree. Games with automatic sync off are not watched, so they keep the old file comparison; so do peers that predate this. Also: - the mass-deletion guard is removed. It held syncs that would delete many files for a decision - a stopgap for deletions read from a half-finished copy, which versions now rule out at the source. - the sync engine marks what it writes and removes (owntouch), and a burst of deletions a peer asks for takes one safety snapshot, at most every ten minutes, instead of one auto-snapshot per file.
…is save everywhere"
Saves that existed before version tracking get a starting version named
after their content, and nothing recorded says which device's is newer.
Between two of those, the old file-by-file comparison used to decide - and
it either asked, or merged the two file by file into a save no game wrote.
Now, between saves from before versions:
- if the two last agreed on a state and one still holds exactly it, the
other is newer;
- otherwise if one is newer in every file that differs (newer, or missing on
the other side) and the other holds no file written after its newest,
that one is newer;
- the older device takes the newer save whole, never file by file. The side
that gives way was older in every differing file, so it loses nothing
newer than what it takes.
- anything else is asked about.
"Use this save everywhere" (game header, POST /api/games/{id}/use-everywhere)
is for states nobody can rank - half-arrived copies, mixtures. The device's
version then covers every save from before versions (entry "g:*"), so every
such device takes it whole, keeping a snapshot first. A change made on
another device since versions existed is not overruled: that is a conflict.
An open conflict no longer blocks a game for good once a version newer than
both sides of it arrives: it was settled elsewhere, and is cleared.
…amage
The transition to save versions happens one device at a time, and a device
still on an older build decides by the old file comparison: whatever its copy
lacks it reads as deleted, so a copy that arrived half-way spread its gaps as
deletions to the devices that had the whole save. That is how a restored save
lost 6,927 files again to a device not yet updated.
- Nothing is taken from a peer that does not keep save versions (status
peer_outdated_app); it can still take from this device.
- Deletions it asks for are refused, on the LAN and over the relay. Requests
from builds that keep versions say so ("versioned": true).
- The device list marks such a peer "needs update".
- The conflict dialog offers "Use this device's save everywhere" for when
neither side is the right save.
Peers signal that they keep versions with "versions": true in the manifest
answer (and LAN protocol revision 3). The relay path now also carries the
save's version: until now, relay syncs never got one and always fell back
to the old file comparison.
… not asked about Before versions, devices that never ran a game held copies relayed from the others at different times, and syncs file by file had left some holding mixtures. Two such copies, neither newer in every file, were asked about - a question about states nobody made, whose right answer is always the copy with the newer work. Now, after the two existing rules (unchanged since the last agreed state; newer in every differing file), the save holding the most recent work wins: the latest-written of the files where the two differ, then the one newer in more of them. The other takes it whole, never file by file, keeping a snapshot of its own first. Only an exact tie is asked about. Changes made since versions existed are not decided this way: they are versions of their own, and a real conflict between them is still asked.
The responder limits a batch to 16 MB counted as every requested block at the file's full block size. 256 files of 4 KB claim exactly 16 MB that way, so any batch of small files with a few larger ones among it was refused as "batch too large" - and the whole pull with it, again on every retry. A save of many small files (Project Zomboid) could not be pulled at all. - The requester also bounds a batch by what the responder counts, with room to spare, so its batches fit any responder of this protocol. - The responder counts each file at most at its real size. - A batch refused as too large anyway is fetched again as two halves rather than failing the pull. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tches Network-aware order: - Each device records how fast its connection to every other one is: from the pulls it makes, and from a short speed test (/api/p2p/speedtest, 2 MB, at most every 6 hours) when nothing has measured it. Before anything is measured, the address kind stands in: home network fast, VPN (Tailscale's range) slower, the internet relay slowest. - A sync goes to the fastest devices first. A device that is behind takes the newer save from the fastest device holding it. - Having asked a close device to take a newer save, a sync waits for it to finish (it reports sync-complete) before asking a much slower one, bounded by how long the transfer should take. The save reaches the close devices at full speed, and the slow ones can then take it from whichever device is nearest to them. - The device list shows each connection's kind and measured speed. Block fetches on direct addresses had a fixed 30-second limit. "Direct" includes VPN addresses, and a few MB of blocks at a VPN relay's hundred-odd KB a second outlasted it: the request failed, and with it the whole pull, over and over. The limit now grows with what is asked for, as it already did over the internet relay. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…said once A speed test that failed - a peer not yet on a build that answers one - counted as done for six hours, so every link stayed unmeasured and the order fell back to address guesses all that time. A link never measured is now tested again ten minutes after a failed test. "<peer> has an older version - asking it to take this one" was logged on every pass while the peer was still taking it, every 20 seconds during a long pull. Logged once per state now. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… end A pull of a large save over a slow link takes an hour, and the link's speed was recorded once, when it ended: all that time the device list said "not measured" and the sync order fell back to address guesses. The link is now measured over each 30-second window while the pull runs, and the rest when it ends. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A save kept as a single file is keyed by its name, so the same save named differently on two devices (an emulator's <profile>.sav) shares no path with the other. The rule settling saves from before versions by their most recent work then made the newer one the save, and the other device would have removed its own file for one with a different name. Saves with no file in common are not two versions of the same files: they are asked about. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Putting Project Zomboid back after its folder was emptied spent most of an hour in the database, the save untouched meanwhile: every file's deletion record was cleared with a statement of its own, a quarter of a million of them, and with SQLite's default rollback journal every one of them created, flushed and deleted a journal file. - Deletion records are cleared in one transaction per save location, on putting a save back and when files come back in a snapshot (ClearDeletedFiles): 50,000 take a third of a second. - The database runs in WAL mode with synchronous NORMAL: writes are appended to one log instead of each making a journal file of its own. The database stays whole whatever happens; a power cut can cost only the last moments' writes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Putting a large save back - a quarter of a million files - wrote them one after another, looking up each one's folder again, while the drive sat at 90% active time and 15 MB/s, and the window showed a spinner for many minutes with nothing to say what it was doing. - internal/iopar runs many small file operations as many at a time as suits the drive they are on: one at a time on a hard disk, starting at 8 on a SATA SSD and 32 on NVMe (Windows: the volume's seek penalty and bus type; Linux: rotational and nvme). On an SSD the number then moves every 750 ms the way that finishes more - on while it helps, back when it hurts, and now and then the other way once settled, so a drive that slows as it goes is followed. - A restore makes every folder once, then writes the files through it; checking the archive reads its entries through it too. On a 4-thread machine restoring Project Zomboid (238,410 files) the processor, not the SSD, is the limit: writing went from 2m29s to 1m42s, settling at 3-8 at once; machines with more cores go further by themselves. - A restore says how far it is - rebuilding a compacted snapshot, checking it, writing the files - with percent and, once it can be told, the time left, in the game's header and while a save is being put back. - An emptied save, or one part-way through being put back, is not snapshotted: the empty folder and a restore stopped half-way were kept as snapshots, the half one as "Latest". It is asked about at once all the same, and kept as "Put back" once every file is back. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Crimson Desert, killed, left two processes behind that kept their memory and used no processor time at all, and its session stayed open: idle was only what held under 32 MB as well. Each process's processor time is followed from poll to poll; one that has used none for a minute is not the game being played, however much memory it holds. A game being played uses some all the time - a paused one still draws its menu. One that cannot be measured still counts. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… any system Linux CI found two things the Windows runs could not: - relativeUnder split paths only on the system's own separator, so on Linux a pattern written Windows-style (Ludusavi's data, a Proton prefix) never matched the folder it names, and path.Match read its backslashes as escapes. Both separators are split on, on every system. - TestRunning_ByInstallFolderName gave processes Windows paths; a process's program is a native path, so the test now builds them for the system it runs on. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
One statement per path, even inside one transaction, took 10s for 50,000 records under the race detector on CI's runners, and the test's limit failed. 500 paths to a statement: 50,000 records in about 0.3s here. The test's limit is generous now (30s): what it guards against took minutes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
owntouch took any marked path found gone for one OpenSave had removed. A file pulled from a peer and then deleted within two minutes - by the game, or by a person - was therefore taken for OpenSave's own change: the watcher recorded the save's new state as already known and raised no change, no snapshot was taken, and with save versions the next sync saw this device still holding the peer's version and did nothing. The deletion never reached the other device (CI: TestReverseDeletionPropagation, Windows). Removals are marked as such now (MarkRemoved, at each place OpenSave removes a file or folder in a save), and only a path marked that way counts as OpenSave's when it is gone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
owntouch took a file for OpenSave's own while it was dated no later than OpenSave's mark plus two seconds of slack. A game saving over a file a pull had just written, inside those two seconds, was therefore taken for the pull: no snapshot of it, no change raised. Once OpenSave is done with a file - renamed into place, re-dated to the peer's time, or re-dated by "keep mine" - it records the time and size it left it with (Settled), and from then on the file is OpenSave's only while it has exactly those. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nished Taking a peer's version checks, once its files are written, that the save is now exactly the peer's; if not, the pull counts as stopped part-way and is resumed on the next sync. A game saving in that moment - after the pull wrote its file, before the check - made the save differ, and the resumed pull then replaced the new save with the peer's. Only the snapshot taken before replacing it still held it, and both devices called themselves in sync. (CI: TestSwitch_TheSameGameUnderDifferentIDsSyncs, about one run in fifteen locally.) When every difference is a file the pull wrote or removed and someone other than OpenSave has written it since (owntouch), the pull did finish: the device takes the peer's version, records the change as a version of its own made from it, and asks the peer to take that. Anything else is still a pull stopped part-way. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A pull renames a file into place (Settled), then re-dates it to the peer's time and settles it again. A watcher event looked at in between compared the file's new time with the one the rename had left, and took OpenSave's own pull for someone else's change: an automatic snapshot, a sync sent back out, and on a save without versions a file the other device had just deleted pulled back (CI: TestWatchAutoSyncChain). Marking the file again before it is re-dated puts it back on the plain time comparison until it settles. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Its simulated drive is timed with sub-millisecond sleeps; on a slow CPU busy with the rest of the test run, one run's windows looked alike and it settled far off (seen: SATA at 2 of 12). One of three runs has to settle near the best now; a broken adaptation misses in all three. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On ChakraFusion/OpenSave only: a release made from this fork says plainly that it is not a Liquid-co release, what it contains and what was tested, instead of upstream's text and Discord invite. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…hange here The check after a pull counted only files the pull had written as possibly changed since. A file the pull did not touch - the same on both sides when the pull was planned - deleted here the moment the pull was done made the save differ from the peer's, the pull counted as stopped part-way, and the resumed pull fetched the deleted file back (CI: TestFullSyncFlow, Windows). A file the pull did not touch that differs now was changed here meanwhile: it counts like a file the pull wrote that someone else has written since. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A session ended and its "After playing" snapshot was taken; the game's last write, settling in the watcher a moment later, made an "Auto backup" of the same save, and duplicate pruning then removed the older one - the session's name with it (CI: TestSession_EndingOneKeepsTheSaveAndSyncsIt, Windows). When a newer snapshot holding exactly the same files takes an older one's place, it takes over the older one's reason if it has only the default "Auto backup", and its note if it has none. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The migrations this fork added are numbered as in the upstream pull requests (0039_game_versions .. 0043_settings_files, after upstream's 0037_game_root_mapped and 0038_peer_links). A database made by an earlier build of the fork recorded them under the old numbers; at open, those are renamed once, in one transaction, so nothing is applied a second time and a later official build containing them finds them applied. Tried on a copy of a real database from one of the four PCs: 94 games, opened normally, names renamed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…eport
The report that a peer took a location is also what records which files
both sides hold there. When it was lost - it times out on a loaded machine
("could not tell ... that ... is sync-complete: context deadline
exceeded") - a file deleted here afterwards read as one the peer had and
this device never did, and the next sync fetched it back: the deletion
undone. Seen on CI as TestMultiRootScenario_DeletionInAnExtraLocationPropagates
failing once on Windows (Liquid-co#23); reproduced deterministically
by TestMultiRootScenario_ADeletionSurvivesALostReport.
- The existing repair - a push is proven once the peer holds exactly what
was handed over - now runs before anything is classified, and restores
the lineage too: both held exactly the pushed state, so its files are
what was shared.
- Extra locations use the deletions this device recorded, as the main
folder does (ComputeWithDeletions): a recorded deletion does not depend
on the lineage, and is acted on only while the peer's copy is still
exactly what was deleted.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…in folder's does The refresh a peer's sync-complete report starts runs in the background, and rebuilt the record of which files both sides hold in an extra location from this device's files as they were when it ran. A file deleted here in the moment before - a loaded machine is enough - dropped out of the record, read as one the peer had and this device never did, and the next sync fetched it back. The second way TestMultiRootScenario_DeletionInAnExtraLocationPropagates failed on Windows (Liquid-co#23, and again with the previous commit, without a lost report); reproduced deterministically by TestMultiRootScenario_ADeletionSurvivesALateRefresh. persistRootLineage now does what persistLineage does for the main folder: keeps previously recorded paths that one side still holds (deletions in flight, keepPendingDeletions), and keeps excluded paths out. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
exchange waited for the larger of wanRequestTimeout and the caller's own deadline. Every sync runs under a 30-minute per-peer deadline, so a manifest request or reply the relay dropped (it drops frames for a client that can't keep up, relay/server.go) held the sync for up to half an hour instead of failing it to be retried. It also made FetchBlocks' payload-sized deadline dead code: it applied only to a context without a deadline, which a sync never is. A request now waits wanRequestTimeout, or longer only when its caller asks with withReplyWait: block fetches for their payload as before, manifest requests for ServeSettleWait more, the time a peer may hold its answer while a sync writes the save. The caller's context still ends it early. Found as an e2e shard hitting the 30-minute limit on Windows CI, with TestThreeDevices_PerPeerStateStaysSeparate waiting 7m47s in one manifest fetch over the relay. Also: presence is written when it changes, not on every frame. trackPresence wrote the peer row (a synced database commit) for every frame a paired peer sent, block replies and heartbeats alike, on the one goroutine that reads the relay. A slow disk held that reading up, and the relay drops frames for a client that falls behind. The last-seen time is now written when something changed and otherwise every 5 seconds, as the LAN path does every 10; peers expire after 20. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…versions follow an exclusion change at once Port of upstream PR Liquid-co#56: the initial snapshot and snapshots taken by hand record the save as it was just before them (daemon.SnapshotWithBaseline), and an exclusion change restates the recorded hash under the new rules when that provably claims nothing new (daemon.LastSnapshotHashUnder). Here in the terms the watcher hashes in: the game's rules with the device settings kept out of sync (IgnoreText). That exposed a gap in save versions. A change to a game's own exclusions re-took the version's hash only at the next start (the device-settings check), never when it was made. Until then the version kept a hash under the old rules, which any later hash reads as the same save measured differently (sameSaveAs): a change made after the rule change was taken into the version's hash by the next sync instead of becoming a new version, and never reached the other device. The automatic snapshot that followed every rule change used to hide it by recording a version first. TestJourney_DeviceSpecificConfigNeverTravels failed 5/5 that way once that snapshot was gone. Now the app and `opensave exclude` re-take it at once (daemon.ExclusionsChanged, via AdoptExclusionView), when the save is unchanged under the old rules. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…files; Stop waits for a catch-up; a baseline never replaces a newer one Port of upstream Liquid-co#56's second and third commits: - Adding or removing a save location restates the last-snapshot hash when the save is unchanged and an added location holds no file yet (daemon.LocationsChanged), here in the watcher's terms (IgnoreText). - The watcher's Stop waits, bounded, for a catch-up already writing a snapshot. - A hash taken before a snapshot is recorded only while no newer one was recorded meanwhile (Store.SetLastManifestHashIf). Without it, a snapshot taken by hand while a newly tracked game's first one ran could have its value replaced by the older one, and the watcher snapshotted the save again. Tests: as upstream; here the location test looks for a new snapshot rather than counting, since an older one a newer holds entirely is removed. And TestCloud_SyncLocalRepairsATruncatedRemoteCopy changes the save only after tracking's snapshot: made before it, the change was in that one too, which was then removed for the identical newer one, leaving the cloud copy the test truncates without a local snapshot to repair it (failed 4-8 in 10). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A new database applied each of its 37 migrations in a transaction of its own, and every commit flushes the rollback journal and the database to disk: 156 fsyncs before store.Open returned. On a busy Windows CI runner that took over thirty seconds, so a freshly started daemon could miss the 30s the CLI e2e harness gives it. CI failed with "daemon never became ready" and empty output; a goroutine dump of the daemon at the deadline shows it still in store.Open -> migrate -> tx.Commit -> FlushFileBuffers, and its log says it started 1.5s after the test gave up. Starts of 5 to 20 seconds, all spent before the address was published, were common on the same runners. Pending migrations now go in one transaction: one commit, 8 fsyncs for a new database (counted with strace), none for one already up to date. A failed step now leaves the schema as it was instead of half upgraded. TestOpeningANewDatabaseCommitsOnce counts commits with the driver's commit hook: 38 before this change, 1 after. Fork adaptation: the fork's openDB keeps WAL with synchronous NORMAL, where a commit does not flush, so the start-up time is mostly saved already here; the gain is 45 commits down to 1 and all-or-nothing migration. The fork's renaming of its old migration names now runs inside the same transaction, and only writes when there is a name to rename (an up-to-date database made one commit on every open before). compactOnOpen is kept. TestOpen_MigratesADatabaseFromAnEarlierForkBuild opens a database as an earlier fork build left it. On fixes-2.4.1 the commit tests fail: 45 commits for a new database, 1 for an up-to-date one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Five tests made their t.TempDir after starting their daemons. Cleanups run last first, so the folder was deleted while the daemons still watched or scanned it, and on Windows the removal fails the test: CI saw TestSwitch_TheSameGameUnderDifferentIDsSyncs fail with "TempDir RemoveAll cleanup: ... The directory is not empty", its daemon logging in the same second that every save file had been deleted. The harness already avoids t.TempDir for the daemons' own homes for this reason. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
RefreshLineage, run when a peer reports it finished pulling or that the two are in sync, stamped "last synced" once it had read both sides and written the lineage. A location with no merge base yet counts only edits made after that stamp (plus the 2 s clock-skew allowance) as new, so an edit made while the refresh ran - seconds on a slow disk - could count as made before the devices were compared. The stamp is now the time the comparison began, before either side was read. A stamp that ends up earlier than another written meanwhile only makes more edits count as new, which is the safe direction. Hardening, not the cause of the CI failure of TestMultiRootScenario_ALateReportDoesNotHideADivergence: that was the test editing inside the skew allowance, fixed in the next commit. TestRefreshLineage_StampsWhenTheComparisonBegan: fails 3/3 without this (stamp ~100 ms after the peer's side was read), passes with it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TestMultiRootScenario_ALateReportDoesNotHideADivergence clears the location's merge base and has both devices edit it. With no base, edits are judged against when the two were last confirmed the same, and only one made more than DetectConflict's 2 s clock-skew allowance after it counts as new. The refresh started by the setup's own reports can stamp that time after SettleSync returns, and the edits then came within it: neither side read as changed, B's version replaced A's, and the test reported "no conflict was raised: A=\"B-version\" B=\"B-version\"" - on Windows CI about one shard run in ten. Instrumented runs show the stamp 1.7-1.9 s before the edits each time; a stamp 1.5 s before them reproduces the failure every time, and 3 s does not. The test now waits until that time has stopped moving and is more than the allowance in the past. It still fails as it should when a report stamps the two as synced while the location differs (the bug it was written for). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A sync this device starts checks first whether every save file was deleted here, and if so holds the game back until someone says whether it was meant. That check decides nothing while anything is writing the save, and for a sync "nothing decided" read as "not held": the sync went on, waited for the write to end, read the folder empty and sent every deletion to the other device - the one thing the hold exists to stop. Something writes the save right after it is emptied as a matter of course: the automatic snapshot of that very change takes the write side of the gate. Windows CI, TestEmptiedFolder_PutBackRestoresThem: "the other device, while the emptied save is held back: slot1.sav is \"\"" for all three files, then the two devices pushing at each other for minutes. A sync now waits for a write in progress to end (up to 30 s) and judges the folder as it then stands; still being written after that, it does not run (ErrSettling) and is tried again. The watcher's check keeps not waiting. TestEmptiedFolder_ASyncStartedDuringAWriteIsStillHeld holds the gate as such a write does, empties the folder and syncs: without this the sync answers 200 and the other device loses all three files, every run; with it the sync is refused (409) and the other device keeps them. Fork adaptation: applied as is; the test file kept the fork's own emptied tests alongside the new one. On fixes-2.4.1 without the fix the test fails 3/3 (the sync runs, 200, instead of being held back, 409); with it, it passes. There the other device kept its files even without the fix - the fork does not take an emptying as a version until it is meant - so on the fork this restores the hold's behaviour rather than stopping a data loss. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
When a sync-complete report is lost, the push-proof repair banks the pushed state as the shared base, but the record of which files both devices hold stayed as it was. A file handed over in that push was missing from it, so if it was deleted before the watcher noted the deletion, the next sync read it as a file the peer had and this device never did, and fetched it back. The repair now records the pushed state's files as shared too, as extra locations already do. Not while the game is held as emptied: that record is then kept on purpose. "Put them back" with no snapshot takes the files out of it so the next sync fetches them from the peer; written back by the repair, they read as deleted here and the deletions went to the peer, and both devices lost the save (TestEmptiedFolder_PutBackWithoutASnapshotFetchesThem on Linux CI, every run together with the emptied-hold fix). Seen on Windows CI as TestWatchAutoSyncChain's "deletion never auto-synced to B"; TestDeletion_SurvivesALostReport reproduces it deterministically, and TestEmptiedFolder_PutBackSurvivesALostReport the put-back case (fails 3/3 without the hold check). Fork adaptation: code applied as is. On this fork an automatic sync is decided by versions, where the device that deleted the file holds the newer one, so there the deletion went through without the fix too. A game synced by hand is still decided file by file: TestDeletion_SurvivesALostReport gains a by-hand case, which fails 3/3 on fixes-2.4.1 (the file comes back) and passes with the fix. TestEmptiedFolder_PutBackSurvivesALostReport passes on fixes-2.4.1 and fails 3/3 with the repair but without its held-as-emptied exception, so the exception is needed here as well. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The push-proof repair for extra locations (first commit here) records the pushed state's files as shared. While the game is held as emptied that record is kept on purpose: "put them back" with no snapshot takes the location's files out of it so the next sync fetches them from the peer. Written back by the repair, they read as deleted on this device instead, the deletions went to the peer, and both devices lost the location's files. The repair now leaves the lineage alone while the game is held as emptied. Found while checking the same repair for the main folder, which had the same flaw. Test: TestMultiRootScenario_PutBackOfAnEmptiedLocationSurvivesALostReport (fails 3/3 without this: the location empty on both devices). Fork adaptation: applied as is; the fork already has the location repair from Liquid-co#49's first two commits. On fixes-2.4.1 the test fails 3/3: both devices lose the location's files. With the fix it passes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Owner
Author
|
Opened automatically by a cloud session; its title doesn't describe the branch. port/fixes-8 is merged into fixes-2.4.1 and released as v2.4.1-fixes.8. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.