Skip to content

ref(hybridcloud): Require the claim's deadline and mailbox on drains - #123275

Open
vaind wants to merge 4 commits into
ivan/webhook-drain-chain-dispatchfrom
ivan/webhook-drain-required-claim
Open

ref(hybridcloud): Require the claim's deadline and mailbox on drains#123275
vaind wants to merge 4 commits into
ivan/webhook-drain-chain-dispatchfrom
ivan/webhook-drain-required-claim

Conversation

@vaind

@vaind vaind commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Last of the drain-claim stack (on the chain-dispatch PR, which stacks on #122893).

Change

The transitional fallbacks come out:

  • valid_until and mailbox are required on drain_mailbox, so _begin_drain reduces to building the claim and asking whether it has lapsed — no head-row read, no nullable mailbox anywhere downstream.
  • drain_mailbox_parallel is deleted. Dispatch has not enqueued it since the drains merged; it only served in-flight tasks from the deploy before that.
  • The tests covering the fallbacks and the shim go with them; every direct drain invocation now states its claim (valid_until, mailbox) explicitly.

⚠️ Rolling deploy

Deploy at least one deploy after the wire flip (the chain-dispatch PR) and let its pre-existing drains clear first. A drain enqueued before dispatch sent the claim carries neither argument and raises TypeError here; the taskbroker discards it, the rows keep the schedule_for their claim wrote, and the next dispatcher re-claims them once it passes — delivery delayed by up to the claim horizon, not dropped.

Follow-up enabled by this

With valid_until always the claim's written value, next_slice filtering on schedule_for = valid_until would become a true ownership fetch and could subsume the first-slice head check — left out here to keep this PR pure removal.

@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Aug 31, 2026
@vaind
vaind force-pushed the ivan/webhook-drain-required-claim branch from 99bdea8 to 66b93b7 Compare August 31, 2026 22:56
The transitional fallbacks from the drain-claim work come out: valid_until and mailbox are now required, so _begin_drain reduces to building the claim and asking whether it lapsed — no head-row read and no nullable mailbox downstream. The drain_mailbox_parallel alias goes with them; dispatch has not enqueued it since the drains merged.

Must deploy at least one deploy after the wire flip: a drain enqueued before dispatch sent the claim carries neither argument and now raises TypeError. That failure is noisy rather than lossy — the rows keep their claim's schedule_for and the next dispatcher re-claims them once it passes.
@vaind
vaind force-pushed the ivan/webhook-drain-required-claim branch from 66b93b7 to 5cfdccd Compare August 31, 2026 23:05
@sentry

sentry Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Sentry Snapshot Testing

Name Added Removed Changed Renamed Unchanged Skipped Status
sentry-frontend
sentry-frontend
0 0 0 0 481 0 ✅ Unchanged

⚙️ sentry-frontend Snapshot Settings

@vaind
vaind marked this pull request as ready for review August 31, 2026 23:08
@vaind
vaind requested a review from a team as a code owner August 31, 2026 23:08
…ain-required-claim

# Conflicts:
#	src/sentry/hybridcloud/tasks/deliver_webhooks.py
#	tests/sentry/hybridcloud/tasks/test_deliver_webhooks.py
@vaind
vaind requested a review from joseph-sentry September 1, 2026 05:39
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.

1 participant