Repository navigation
DM-56095: Stop edition_reconcile publishing editions deleted mid-tick - #887
Open
jonathansick wants to merge 4 commits into
Open
jonathansick wants to merge 4 commits into
jonathansick wants to merge 4 commits into
Conversation
A branch deleted while an edition_reconcile tick runs is soft-deleted by the ref_deleted webhook after the plan has read the edition as live. The delete also removes the CDN key, so the read-back reports a missing pointer and the tick enqueued a publish for a tombstone. The apply-time re-check now reads the edition's date_deleted, and its project's, under the same row lock as current_build_id, and drops the republish with nothing written. The edition is counted in the tick's tombstoned bucket.
A delete landing between the enqueue and the pickup left publish_edition unable to resolve the edition its payload named. It raised NotFoundError into Sentry and left the queue row in_progress until publish_edition_reaper failed it four hours later. The job now completes with deleted_skipped in its progress, as it does for a deleted build, so the row closes at once and a keeper-sync run waiting on it still finalises. A slug that resolves to a different edition id takes the same path.
Give republishes dropped for a soft-deleted edition their own counter on the tick outcome, the queue job progress and the edition_reconcile_completed event, instead of folding them into the standing tombstoned count where the race is invisible. Also correct the docs and docstrings that still said publish_edition raises for a tombstoned project; it now retires as a skip.
A plain read returned the live row while a soft-delete's transaction was still open, so the job could go on to write a pointer for the tombstone that delete was about to commit. Reading FOR UPDATE waits for the delete and then sees it. Pass the project id resolved before the lock into the loader rather than resolving the slug a second time, which also stops a project delete in that gap being recorded as an edition delete. Add store tests for lock_republish_target and the locked read.
This branch has not been deployed
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
Fixes the race in #886: a branch deleted while an
edition_reconciletick is running could get apublish_editionjob enqueued for the edition the delete webhook had just soft-deleted, which then raisedNotFoundErrorinto Sentry (DOCVERSE-2Y) and left its queue rowin_progressfor four hours._recheck_republishnow reads the edition'sdate_deleted, and its project's, under the same row lock ascurrent_build_id(EditionStore.lock_republish_target, replacinglock_current_build_id). A soft-deleted edition raises_StalePlanErrorwith a new_ApplySkip.deleted, so Phase A writes nothing and enqueues nothing. It is logged at info as "Skipped a republish for a soft-deleted edition".deleted_skippedcounter. Those drops are counted on the tick outcome, theedition_reconcilejob'sprogress, and theedition_reconcile_completedmetrics event (a new required integer field), rather than in the standingtombstonedcount.publish_editionretires a missing target as a skip. When the project or edition the payload names is gone at pickup, or the slug resolves to an edition with a different id, the job completes withdeleted_skippedin itsprogressinstead of raising. The queue row closes at once and a keeper-sync run waiting on it still finalises. This covers a delete that lands between enqueue and pickup from any producer.publish_editionlocks the edition row at pickup. The edition is readFOR UPDATE, so a soft-delete still uncommitted at pickup is waited for and seen, rather than the job publishing a pointer for the tombstone.Notes for review:
publish_editionshare thedeleted_skippedprogress flag with the deleted-build skip; themessagesays which one it was.NotFoundError, since no delete removes those.publishing, but before the KV write, can still leave a key for a tombstone. The next reconcile tick removes it.enqueue_publish_for_editionfor every producer, and treating an edition id mismatch as an error rather than a skip. Both are left as they are here.Validation steps
uv run --only-group=nox nox -s testpasses (4,292 tests), as donox -s typingandprek run --all-files.tests/services/edition_reconcile_test.py::test_a_delete_between_plan_and_apply_drops_the_republishsoft-deletes an edition between plan and apply and asserts noqueue_jobsrow is created,deleted_skippedis 1 and nothing reaches Sentry.tests/worker/publish_edition_test.py::test_publish_edition_deleted_target_skipsdelivers a job for a soft-deleted edition, a soft-deleted project and a mismatched edition id, and asserts the row iscompletedwithdeleted_skippedand no Sentry event.tests/storage/edition_store_test.pycoverslock_republish_target(live, deleted edition, deleted project, missing row) and shows aFOR UPDATEget_by_slugwaiting for an open delete and then reading nothing.deleted_skippedshould appear onedition_reconcile_completed.References
🤖 Generated with Claude Code