Skip to content

Batch dataclip retention wiping so it stops timing out on large backlogs - #5115

Merged
stuartc merged 2 commits into
mainfrom
stuart/con-158
Sep 4, 2026
Merged

Batch dataclip retention wiping so it stops timing out on large backlogs#5115
stuartc merged 2 commits into
mainfrom
stuart/con-158

Conversation

@stuartc

@stuartc stuartc commented Sep 4, 2026

Copy link
Copy Markdown
Member

Description

This PR fixes the data_retention Oban job timing out when a project has a large backlog of dataclips to wipe.

wipe_dataclips_for/1 was a single unbatched UPDATE ... WHERE project_id = ? AND inserted_at < ?, which reliably hits Ecto's 15s query timeout once the eligible set gets large enough (confirmed on staging via Postgrex.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 in activity_cleanup_chunk_size-sized batches, so no single query does more work than a normal batch job.

Closes CON-158

Validation steps

  1. Seed a project with a large number of dataclips (more than a few activity_cleanup_chunk_size batches) with inserted_at older than the retention period.
  2. Set dataclip_retention_period on the project and trigger the retention worker (or call Lightning.Projects.wipe_dataclips_for/1 directly).
  3. Confirm it completes without hitting the Ecto query timeout, and that the same rows get wiped as before (see test/lightning/projects_test.exs for the new coverage).
  4. Run the migration and confirm dataclips_pending_wipe_idx is created; re-running it after a killed CONCURRENTLY build should self-heal rather than error.

Additional notes for the reviewer

  1. The migration adds a partial index shaped to match the new query (project_id, inserted_at, filtered on the wipeable types/name/wiped_at conditions) so the id-fetch stays index-backed instead of falling back to a scan.
  2. fetch_size (id page size) is deliberately much larger than batch_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!):

  • I have used Claude Code
  • I have used another model
  • I have not used AI

You can read more details in our
Responsible AI Policy

Pre-submission checklist

  • I have performed an AI review of my code (we recommend using /review
    with Claude Code)
  • I have implemented and tested all related authorization policies.
    (e.g., :owner, :admin, :editor, :viewer)
  • I have updated the changelog.
  • I have ticked a box in "AI usage" in this PR

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
@github-project-automation github-project-automation Bot moved this to New Issues in Core Sep 4, 2026
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

Security Review ✅

  • S0 (project scoping): New eligible_query in lib/lightning/projects.ex:1685 filters d.project_id == ^project.id; per-chunk update on d.id in ^chunk inherits that scoping since the IDs come from that project-filtered fetch.
  • S1 (authorization): N/A — no new web-layer actions; changes live inside the data_retention Oban worker.
  • S2 (audit trail): N/A — no configuration writes; retention wiping is an automated cleanup that already had no audit entry and this PR only changes batching.

@codecov

codecov Bot commented Sep 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.7%. Comparing base (1342c17) to head (0ddfc41).

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@stuartc
stuartc merged commit 2f41895 into main Sep 4, 2026
8 checks passed
@stuartc
stuartc deleted the stuart/con-158 branch September 4, 2026 13:31
@github-project-automation github-project-automation Bot moved this from New Issues to Done in Core Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant