RSDK-13995: Clean up old .prog files on start - #6326
Merged
Katie Peters (katiepeters) merged 1 commit intoAug 31, 2026
Conversation
Angela Predolac (angelapredolac)
added
the
safe to test
viambot
added
safe to test
Angela Predolac (angelapredolac)
requested review from
gloriacai01 and
Katie Peters (katiepeters)
August 4, 2026 16:25
Member
|
please link to previous attempt's postmortem doc in the PR description, for future reviewers :) |
Sagie Maoz (n0nick)
left a comment
Sagie Maoz (n0nick)
left a comment
Member
There was a problem hiding this comment.
also, PR title should start with RSDK-13995: ..., not the branch name :)
Angela Predolac (angelapredolac)
changed the title
viambot
added
safe to test
|
Hey Angela Predolac (@angelapredolac) — this PR has been approved and CI has been green for 4+ business days. Ready to merge? Auto-comment from overwatch. Will not re-nudge for 7 days. |
|
|
viambot
added
safe to test
gloriacai01
reviewed
Aug 14, 2026
gloriacai01
left a comment
gloriacai01
left a comment
Member
There was a problem hiding this comment.
Changes:
- builtin.go —
renamedOrphanProgFiles boolreplaced withrenameOrphanProgFilesOnce sync.Once, and the orphan rename is now launched as separate goroutine outsideb.muand async, so a large capture directory doesn't stall Reconfigure. - builtin_capture_test.go — the integration test's immediate post-startup assertions are wrapped in
testutils.WaitForAssertionWithSleepto
account for the rename now happening in a background goroutine.
Manually tested:
- Seeded 50 orphan .prog files using the seed script, backdated to before processStartTime so the mtime guard wouldn't skip them.
- Added a 5-second time.Sleep to
renameOrphanedProgFilesToCaptureto make the async gap obvious. - Restarted viam-server and pulled the logs. The key sequence at 18:38:13:
- New END at 18:38:13.637
- finished renaming orphaned .prog files to .capture at 18:38:18.617 — 5 seconds later
Proving Reconfigure returned immediately and the rename ran in the background.
4. Removed the sleep before committing.
Katie Peters (katiepeters)
approved these changes
Aug 14, 2026
Katie Peters (katiepeters)
left a comment
Katie Peters (katiepeters)
left a comment
Member
There was a problem hiding this comment.
Left one very tiny comment. Other than that, we're 100% sure that sync.Once will do what we want/survive reconfigures?
| // a large capture directory does not stall Reconfigure; the mtime guard | ||
| // makes this safe — any .prog file written after process start belongs to a | ||
| // live collector and is skipped. | ||
| captureDir := captureConfig.CaptureDir |
There was a problem hiding this comment.
is there a reason this is pulled out into a separate variable?
viambot
added
safe to test
Member
Katie Peters (@katiepeters) the mtime guard will ensure we survive reconfigures |
viambot
added
safe to test
gloriacai01
added
safe to test
viambot
added
safe to test
viambot
added
safe to test
gloriacai01
added
go
gloriacai01
force-pushed
the
RSDK-13995
branch
from
August 18, 2026 19:12
48d7bcc to
9fa019a
Compare
viambot
added
safe to test
|
Hey Angela Predolac (@angelapredolac) — this PR has been approved and CI has been green for 3+ business days. Ready to merge? Auto-comment from overwatch. Will not re-nudge for 7 days. |
|
Hey Angela Predolac (@angelapredolac) — this PR has been approved and CI has been green for 8+ business days. Ready to merge? Auto-comment from overwatch. Will not re-nudge for 7 days. |
Katie Peters (katiepeters)
merged commit Aug 31, 2026
86af76a
into
viamrobotics:main
22 checks passed
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
6 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.
Summary
On startup, the data manager now renames orphaned .prog files (left behind when a previous viam-server process died mid-capture) to .capture so sync uploads them instead of skipping them forever. Unlike #5996 (reverted in #6036 / APP-16267 — see the postmortem for full background), the pass runs only on an instance's first Reconfigure before any collectors exist, and an mtime guard skips any .prog file modified after process start, so live collector files are never touched.
Testing
Unit Testing: Added tests covering orphan renaming, the mtime guard, non-.prog files being left untouched, and that a second Reconfigure never re-runs the pass or disturbs a live collector's open .prog file.
Manual Testing: On a real machine, SIGKILLed viam-server mid-capture and verified on restart that the orphan was renamed and its readings synced to app, and that both in-place reconfigures and full service rebuilds leave live .prog files untouched with capture uninterrupted.