Skip to content

fix(replays): stop bulk delete progress stalling and rewinding - #120772

Open
JoshuaKGoldberg wants to merge 4 commits into
masterfrom
joshgoldberg/replay-951-bulk-delete-counter-backend
Open

fix(replays): stop bulk delete progress stalling and rewinding#120772
JoshuaKGoldberg wants to merge 4 commits into
masterfrom
joshgoldberg/replay-951-bulk-delete-counter-backend

Conversation

@JoshuaKGoldberg

@JoshuaKGoldberg JoshuaKGoldberg commented Jul 28, 2026

Copy link
Copy Markdown
Member

Bulk replay delete jobs showed two counter symptoms in production:

  • Count Deleted frozen at a value for hours while the job still reported in-progress: fixed by pooling recording deletes once per batch rather than per replay, and by marking the job as 'failed' on the final attempt.
  • Rotating between values while trending upward: fixed by switching progress and status saving to be independent, so writing one won't revert the other.

Closes #120413.

Task delivery is at-least-once, so a job can have more than one live call chain: a redelivered activation re-enters at the offset it was given while the successor it already enqueued keeps going. Both chains wrote the whole row via save(), so the reported count rotated between their two positions and a stale chain could revert a terminal status. Progress and status are now written per-column, the offset only moves forward, and a chain that fell behind the checkpoint exits instead of re-deleting rows.

A blown processing deadline raises a BaseException, so it bypassed the task's exception handler, and the broker discards the activation once retries run out. That left the job reporting in-progress forever with nothing to advance it. It now reaches a terminal failed status on the last attempt.

Deleting a batch's recordings used a fresh thread pool per replay, serializing the batch behind whichever replay had the most segments. Locally a 100-replay batch took 5.4s where one shared pool takes 0.07s, which is what pushed batches past the 600s deadline. The batch now shares one pool.

Refs #120413
@linear-code

linear-code Bot commented Jul 28, 2026

Copy link
Copy Markdown

REPLAY-951

@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Jul 28, 2026
@JoshuaKGoldberg

Copy link
Copy Markdown
Member Author

@cursor review

Comment thread src/sentry/replays/tasks.py Outdated
The guard exited early when an activation arrived behind the checkpoint, on the assumption that another chain was already ahead of it. But the checkpoint is written before the successor is enqueued, so an activation killed in that gap came back, saw itself behind, and returned without enqueueing anything or moving the status. That stranded the job at in-progress, which is the failure this PR is meant to remove.

The guard was not load-bearing: the monotonic offset write already makes the counter immune to a lagging chain, and reproducing the two-chain interleaving without the guard yields the identical sequence. A duplicate chain now just redoes idempotent work instead of being suppressed.

Reported by Cursor Bugbot on #120772.
@JoshuaKGoldberg JoshuaKGoldberg changed the title fix(replays): Stop bulk delete progress stalling and rewinding fix(replays): stop bulk delete progress stalling and rewinding Jul 28, 2026
@JoshuaKGoldberg
JoshuaKGoldberg marked this pull request as ready for review July 28, 2026 20:28
@JoshuaKGoldberg
JoshuaKGoldberg requested a review from a team as a code owner July 28, 2026 20:28
Comment thread src/sentry/replays/tasks.py
Comment thread src/sentry/replays/tasks.py

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 04e2dba. Configure here.

# namespace="replays") continue to resolve and execute.
alias_namespace=replays_tasks,
retry=Retry(times=5),
retry=Retry(times=5, on=(ProcessingDeadlineExceeded,)),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Retry policy drops exception recovery

Medium Severity

Narrowing retry to only ProcessingDeadlineExceeded drops retries for ordinary exceptions, but _advance_offset, status completion, and the next .delay() still sit outside the try/except that marks the job failed. A transient DB or enqueue error after a successful batch can leave the job stuck in-progress with nothing left to advance it.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 04e2dba. Configure here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Errm. I think this is already the case on master? Someone please tell me if I'm wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bulk replay delete jobs showing abnormal counter behavior — stuck (android) and flickering (iOS)

1 participant