Skip to content

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

Closed
vaind wants to merge 2 commits into
ivan/fix-webhook-stale-claim-abortfrom
ivan/webhook-drain-required-claim
Closed

ref(hybridcloud): Require the claim's deadline and mailbox on drains#123265
vaind wants to merge 2 commits into
ivan/fix-webhook-stale-claim-abortfrom
ivan/webhook-drain-required-claim

Conversation

@vaind

@vaind vaind commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Stacked on #122893. Must not reach production in the same deploy — see Rolling deploy.

Change

The transitional fallbacks from #122893 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 stopped enqueueing it when 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 #122893 and let its pre-existing drains clear first. A drain enqueued before the merge carries neither argument and raises TypeError here. That failure is noisy rather than lossy: the rows keep the schedule_for their claim wrote, and the next dispatcher re-claims them once it passes — delivery is 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 0faf56c to 06267c6 Compare August 31, 2026 22:41
A strict provider's mailbox burns one claim per scheduler round trip, and under a wide backlog the mailbox competes with every other due head for each cycle's dispatch budget — the re-discovery gap, not delivery, dominates its burn rate. A drain that ends healthy with due work behind it (consumed its full claim, or released a tail it had been delivering toward) now dispatches the mailbox's next claim itself, while its lineage is within hybridcloud.webhookpayload.max_chain_depth links. The ordinary dispatch counts as the first link, so the default of 1 means no drain chains until the option is raised — the depth ceiling is also the rollout dial.

Strict providers only: their absolute-head gate admits one claim at a time, so a chain stays a single lineage per mailbox, where a due-head provider would fork a new pipeline every scheduler cycle. A drain that delivered nothing before its soft-stop spent its window queued — saturation — and does not chain; nor does one that stopped on a failure or lapsed. The chain claims under the same drain lock and due-gate as every dispatcher, so losing the race to the scheduler continues the lineage under its dispatch.

Dispatch also starts sending the full claim shape (valid_until, mailbox, and chain_depth on chained links) — every worker has bound these since the previous deploy.
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 06267c6 to 99bdea8 Compare August 31, 2026 22:53
@vaind

vaind commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

Recreated with the stack reordered (removals now land last): see the replacement PR.

@vaind vaind closed this Aug 31, 2026
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