Sweep job workspaces after 30 days - #27
Merged
Merged
Conversation
data/jobs grew without bound: nothing ever deleted a job's workspace once created, regardless of outcome. Add a daily age-based sweep mirroring the existing Drive-file retention sweep in internal/mailgun/retention.go — same shape, same crash-safety properties, applied to local job directories instead of published Sheets. A completed job (jobs.completed_at IS NOT NULL) past 30 days has its data/jobs/<id> workspace removed. The row itself is kept, with a new files_deleted_at marker set instead of deleting it, matching how email_deliveries.deleted_at already works: `jobs get <id>` keeps answering after the files are gone, and a swept row is never offered to the sweep again. Only completed_at gates the query, so a queued or running job can never be swept no matter how old. Verified the migration against a copy of the real production database (135 job rows): row count preserved, every existing row correctly gets files_deleted_at=NULL, a second Open() is a no-op, and ExpiredJobs correctly identifies the 51 jobs that would be swept on first deploy. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
Summary
data/jobsgrew without bound — nothing ever deleted a job's workspace once created. Add a daily age-based sweep mirroring the existing Drive-file retention sweep (internal/mailgun/retention.go): same shape, same crash-safety, applied to local job directories instead.jobs.completed_at IS NOT NULL) past 30 days has itsdata/jobs/<id>workspace removed viaos.RemoveAll.jobs.files_deleted_atcolumn +jobs_sweepindex, migrated in-place for existing databases (plainALTER TABLE, no rebuild needed). The row is kept —files_deleted_atis set instead of deleting it, mirroringemail_deliveries.deleted_at— sojobs get <id>keeps answering after the files are gone, and a swept row is never re-offered to the sweep.completed_atgates the sweep query, so aqueued/runningjob can never be swept regardless of age.cmd/filemill/main.gounconditionally in continuous mode (!once), outside the mailgun-configured branch, since job accumulation happens whether or not email is set up.Test plan
go build ./...,go vet ./...,go test ./... -count=1created_atageSubmitgets deleted past the horizon while a fresh one is untouched; the row survives with status intact; a previously-removed workspace doesn't error the sweep (RemoveAll is idempotent)files_deleted_at=NULL, a secondOpen()is a no-op, andExpiredJobscorrectly identifies the 51 jobs that would be swept on first deploy🤖 Generated with Claude Code