Batch dataclip retention wiping so it stops timing out on large backlogs - #5115
Merged
Conversation
Adds note() (cyan, no prefix) and hint() (cyan "Hint:") to the shared style.sh helpers. note() marks worktree's own status lines so they're easy to tell apart from bootstrap's and git's interleaved output; hint() replaces the yellow warn() previously used for the --no-clean suggestion, since nothing is actually wrong at that point.
The data_retention Oban job was a single unbatched UPDATE that reliably hit Ecto's 15s timeout once a project's dataclip backlog grew large (confirmed on staging via Postgrex query_canceled errors). Rewrites wipe_dataclips_for/1 to page through eligible ids and apply updates in chunks, adds a partial index matching the new query shape (self-healing against a killed CONCURRENTLY build leaving an invalid index behind), and orders the id fetch by inserted_at so it stays index-backed instead of falling back to an unbounded scan. CON-158
Security Review ✅
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5115 +/- ##
=====================================
Coverage 90.7% 90.7%
=====================================
Files 417 417
Lines 20296 20305 +9
=====================================
+ Hits 18400 18413 +13
+ Misses 1896 1892 -4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
Description
This PR fixes the
data_retentionOban job timing out when a project has a large backlog of dataclips to wipe.wipe_dataclips_for/1was a single unbatchedUPDATE ... WHERE project_id = ? AND inserted_at < ?, which reliably hits Ecto's 15s query timeout once the eligible set gets large enough (confirmed on staging viaPostgrex.Error: query_canceled, after enabling retention on some old QA/test projects that had none set and had accumulated ~1TB of dataclips). Production doesn't have this today, but any customer with tens of thousands of eligible dataclips, or who pulls their retention period back, would hit the same wall.The fix pages through eligible dataclip ids (ordered by
inserted_at, matching a new partial index) and applies the wipe inactivity_cleanup_chunk_size-sized batches, so no single query does more work than a normal batch job.Closes CON-158
Validation steps
activity_cleanup_chunk_sizebatches) withinserted_atolder than the retention period.dataclip_retention_periodon the project and trigger the retention worker (or callLightning.Projects.wipe_dataclips_for/1directly).test/lightning/projects_test.exsfor the new coverage).dataclips_pending_wipe_idxis created; re-running it after a killedCONCURRENTLYbuild should self-heal rather than error.Additional notes for the reviewer
project_id,inserted_at, filtered on the wipeable types/name/wiped_atconditions) so the id-fetch stays index-backed instead of falling back to a scan.fetch_size(id page size) is deliberately much larger thanbatch_size(update chunk size) - wiping only sets a column rather than deleting the row, so a small per-update batch would re-walk a growing prefix of already-wiped rows on every iteration.AI Usage
Please disclose whether you've used AI anywhere in this PR (it's cool, we just
want to know!):
You can read more details in our
Responsible AI Policy
Pre-submission checklist
/reviewwith Claude Code)
(e.g.,
:owner,:admin,:editor,:viewer)