Skip to content

fix(messagequeue): remainder-aware fair share and orphan sweep - #526

Merged
mnoah1 merged 1 commit into
mainfrom
messagequeue
Aug 6, 2026
Merged

fix(messagequeue): remainder-aware fair share and orphan sweep#526
mnoah1 merged 1 commit into
mainfrom
messagequeue

Conversation

@behinddwalls

@behinddwalls behinddwalls commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Summary

Why?

The MySQL queue subscriber capped every node at ceil(P/N) partitions, computed independently per node. The caps sum to more than P, so uneven splits can settle into stable starvation: with 12 partitions and 5 subscribers every cap is 3 (sum 15), and 3/3/3/3/0 is a legal end state where every owner sits "at cap" and nothing ever obliges anyone to shed for the empty subscriber. Separately, nothing guaranteed an unleased partition would eventually be picked up if the share arithmetic misfired (divergent heartbeat views during the staleness window, or a subscriber that heartbeats without ever acquiring), and every shedding rebalance tick logged a spurious lease renewal failed: ErrLeaseExpired because rebalance sorted the shared slice in place and renewal then ran over the just-released tail.

What?

  • fairShareCap is now remainder-aware: each subscriber ranks itself in the sorted ActiveSubscribers list; the first P mod N ranks get floor(P/N)+1 and the rest floor(P/N), so per-rank caps sum to exactly P. A starved subscriber or an unclaimed partition now implies a peer over/under its cap that rebalance and discovery resolve — neither is a stable state. The signature, the 0 = unlimited contract, and the minimum-1 floor are unchanged; a subscriber missing from its own active view falls back to ceil over N+1 contenders (never unlimited, never zero).
  • Orphan sweep: every 2 x LeaseDurationMs the discovery tick runs one uncapped acquisition pass. TryAcquireLease cannot steal a valid lease, so the sweep is a no-op in a healthy group, but a partition left unleased for any reason is picked up by whichever subscriber sweeps first; the next rebalance sheds any over-cap grab once a peer has capacity.
  • rebalance sheds from a sorted copy instead of reordering the caller's slice, and returns the released partitions; the lease tick renews only the kept set, eliminating the spurious ErrLeaseExpired error log on every shedding tick.

Test Plan

  • make test — new TestSubscriber_FairShareCap covers the 12/5 starvation case (caps 3,3,2,2,2), P<N flooring, the missing-heartbeat fallback, and a property subtest asserting per-rank caps sum to exactly P for all N≤6, P≤13; TestSubscriber_Rebalance* cover the released-tail contract and that the input slice is not reordered.
  • bazel test //test/integration/extension/messagequeue/... — full Docker suite, including new TestRebalance_NoStarvation_UnevenSplit (12 partitions / 5 subscribers converge with every subscriber owning 2–3, total 12) and TestRebalance_OrphanSweep (phantom heartbeats cap the real subscriber at 1 of 3 partitions; all 3 messages are still delivered and acked via the sweep).

## Summary

### Why?

The MySQL queue subscriber capped every node at ceil(P/N) partitions, computed independently per node. The caps sum to more than P, so uneven splits can settle into stable starvation: with 12 partitions and 5 subscribers every cap is 3 (sum 15), and 3/3/3/3/0 is a legal end state where every owner sits "at cap" and nothing ever obliges anyone to shed for the empty subscriber. Separately, nothing guaranteed an unleased partition would eventually be picked up if the share arithmetic misfired (divergent heartbeat views during the staleness window, or a subscriber that heartbeats without ever acquiring), and every shedding rebalance tick logged a spurious `lease renewal failed: ErrLeaseExpired` because rebalance sorted the shared slice in place and renewal then ran over the just-released tail.

### What?

- `fairShareCap` is now remainder-aware: each subscriber ranks itself in the sorted `ActiveSubscribers` list; the first `P mod N` ranks get `floor(P/N)+1` and the rest `floor(P/N)`, so per-rank caps sum to exactly P. A starved subscriber or an unclaimed partition now implies a peer over/under its cap that rebalance and discovery resolve — neither is a stable state. The signature, the `0 = unlimited` contract, and the minimum-1 floor are unchanged; a subscriber missing from its own active view falls back to `ceil` over N+1 contenders (never unlimited, never zero).
- Orphan sweep: every `2 x LeaseDurationMs` the discovery tick runs one uncapped acquisition pass. `TryAcquireLease` cannot steal a valid lease, so the sweep is a no-op in a healthy group, but a partition left unleased for any reason is picked up by whichever subscriber sweeps first; the next rebalance sheds any over-cap grab once a peer has capacity.
- `rebalance` sheds from a sorted copy instead of reordering the caller's slice, and returns the released partitions; the lease tick renews only the kept set, eliminating the spurious `ErrLeaseExpired` error log on every shedding tick.

## Test Plan

- ✅ `make test` — new `TestSubscriber_FairShareCap` covers the 12/5 starvation case (caps 3,3,2,2,2), P<N flooring, the missing-heartbeat fallback, and a property subtest asserting per-rank caps sum to exactly P for all N≤6, P≤13; `TestSubscriber_Rebalance*` cover the released-tail contract and that the input slice is not reordered.
- ✅ `bazel test //test/integration/extension/messagequeue/...` — full Docker suite, including new `TestRebalance_NoStarvation_UnevenSplit` (12 partitions / 5 subscribers converge with every subscriber owning 2–3, total 12) and `TestRebalance_OrphanSweep` (phantom heartbeats cap the real subscriber at 1 of 3 partitions; all 3 messages are still delivered and acked via the sweep).
@behinddwalls behinddwalls changed the title refactor(speculation): drop snapshot validation from bestfirst fix(messagequeue): remainder-aware fair share and orphan sweep Aug 6, 2026
@behinddwalls
behinddwalls marked this pull request as ready for review August 6, 2026 14:41
@behinddwalls
behinddwalls requested review from a team and sbalabanov as code owners August 6, 2026 14:41
@mnoah1
mnoah1 added this pull request to the merge queue Aug 6, 2026
Merged via the queue into main with commit 917b3d4 Aug 6, 2026
27 checks passed
@behinddwalls
behinddwalls deleted the messagequeue branch August 6, 2026 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants