Skip to content

fix(messagequeue): run queue garbage collection on busy partitions - #622

Open
Jal-Bafana wants to merge 2 commits into
uber:mainfrom
Jal-Bafana:fix/busy-partition-gc
Open

fix(messagequeue): run queue garbage collection on busy partitions#622
Jal-Bafana wants to merge 2 commits into
uber:mainfrom
Jal-Bafana:fix/busy-partition-gc

Conversation

@Jal-Bafana

Copy link
Copy Markdown

Why?

Garbage collection of acknowledged queue_messages rows currently runs only on idle poll ticks.

A continuously busy partition resets the GC counter whenever it delivers a message, so it can never reach the GC threshold. Under sustained traffic, acknowledged message rows can therefore accumulate indefinitely.

This also means the message deduplication horizon can remain unnecessarily large on busy partitions.

What?

Run the existing garbage collection cadence on every poll tick rather than only idle ticks.

The existing 100-tick throttle is preserved, so the per-partition GC frequency is not increased. The GC safety mechanism is also unchanged: garbage collection still uses the minimum acknowledged offset across consumer groups.

Added:

  • unit regression coverage proving GC runs on busy ticks
  • MySQL integration coverage proving acknowledged rows are reclaimed under continuous traffic

Testing

  • go test ./platform/extension/messagequeue/... — PASS
  • go test ./platform/base/... — PASS
  • go vet on the messagequeue and integration packages — PASS
  • gofmt and git diff --check — clean

Not run locally due to environment limitations:

  • MySQL integration test TestGCReclaimsAckedRowsUnderContinuousTraffic — Docker daemon unavailable
  • make gazelle, make fmt, make lint, make check-tidy, make check-gazelle and Bazel tests — make unavailable and Bazel's Windows C++ toolchain is not configured

Copilot AI lite review requested due to automatic review settings August 19, 2026 19:18
@Jal-Bafana
Jal-Bafana requested review from a team, behinddwalls and sbalabanov as code owners August 19, 2026 19:18
@CLAassistant

CLAassistant commented Aug 19, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes MySQL-backed message queue garbage collection (GC) starvation on continuously busy partitions by running the existing GC cadence on every poll tick (still throttled), and adds regression tests to prevent reintroduction.

Changes:

  • Run per-partition GC every N poll ticks regardless of whether messages were delivered.
  • Add a unit test ensuring GC triggers even when every tick delivers a message.
  • Add a MySQL integration test verifying acked rows are reclaimed under sustained traffic.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
test/integration/extension/messagequeue/mysql/queue_test.go Adds an integration regression test validating GC reclaims acked rows during continuous traffic.
platform/extension/messagequeue/mysql/subscriber.go Changes GC scheduling from “idle ticks only” to “every N poll ticks” while preserving throttling.
platform/extension/messagequeue/mysql/subscriber_test.go Adds a unit test proving GC runs on busy ticks (no idle periods).

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread test/integration/extension/messagequeue/mysql/queue_test.go

@behinddwalls behinddwalls left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Two notes on the diff — the core change looks right to me. Removing the messageCount == 0 gate genuinely fixes the starvation (the old else branch reset the counter to 0 forever on a continuously busy partition), GC load stays bounded the same way it was, and run() only logs pollAndDeliver errors so a GC failure can't kill a worker or cause re-delivery.

Comment thread platform/extension/messagequeue/mysql/subscriber.go
Comment thread test/integration/extension/messagequeue/mysql/queue_test.go
- Record messages_delivered before the GC block so a GC failure can no
  longer drop the counter for already-delivered messages on busy ticks.
- Seed the integration backlog before subscribing so the GC counter
  starts at zero when polling begins and cannot fire mid-drain.
- Drain hook signals inside the traffic loop so the worker's blocking
  signal send cannot fill the buffer and stall the partition.
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.

4 participants