fix(backups): reclaim only staged bundles old enough to be abandoned - #67
Merged
Conversation
The bundle staging root is a fixed path in the machine's shared temp directory, and startup cleanup deleted the whole tree. Two processes sharing a temp dir therefore clobbered each other's staged bundles — which is exactly how CI flaked: dotnet test runs both test assemblies in parallel on one runner, every Api test host boots the real Program (whose startup runs the cleanup), and it wiped the bundle a parallel Application test had just staged, so BundleExportState.Current saw the file gone and returned null. Startup cleanup now deletes only per-export directories whose last write is older than two hours: age is the only thing distinguishing a dead process's leftovers from a live sibling's staged bundle. A fresh leftover from a crashed process just waits for the next startup, and only costs temp-directory disk in the meantime. (cherry picked from commit 2e96375)
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
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.
Fixes the CI flake that failed the post-merge run on
main(run 33156562598):BackupBundleTests.CarriesTheInstanceArchiveAndEveryStacksNewestOneassertingBundleExportState.Currentis null after a successful export.What was happening
The bundle staging root is a fixed path in the machine's shared temp directory (
$TMPDIR/watchtower-bundle), andCleanStagingDirectory()— run at startup fromProgram— deleted the whole tree.dotnet test Watchtower.slnxruns both test assemblies in parallel processes on one runner, and everyWatchtower.Api.Testsfixture boots the realProgram. So an Api test host starting up wiped the bundle a concurrently running Application test had just staged;BundleExportState.Currentself-nulls when the staged file has vanished, producing theAssert.IsType … Actual: nullfailure. Timing-dependent, hence flaky. The same hazard exists for any two Watchtower processes sharing a temp directory.The fix
Startup cleanup now deletes only per-export staging directories whose last write is older than two hours (
StagingReclaimAge) — age is the only signal distinguishing a dead process's leftovers from a live sibling's freshly staged bundle. A fresh leftover from a crashed process just waits for the next startup and costs only temp-directory disk in the meantime.The logic is extracted as
ReclaimStaleStagingEntries(root, minAge)so it's directly testable; two new tests cover reclaim-old/spare-fresh and tolerance of a missing root.Reviewer notes
main(-xtrailer points at the original).