Skip to content

fix(messagequeue): release leases on drained partitions - #533

Merged
behinddwalls merged 1 commit into
mainfrom
preetam/mq-idle-lease-release
Aug 7, 2026
Merged

fix(messagequeue): release leases on drained partitions#533
behinddwalls merged 1 commit into
mainfrom
preetam/mq-idle-lease-release

Conversation

@behinddwalls

@behinddwalls behinddwalls commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Summary

Why?

A lease is held forever once acquired: ReleaseLease is only called on shutdown and fair-share shedding, renewal is unconditional, and workers are reconciled from the lease set — so when a partition drains (every message acked and garbage-collected, hence absent from discovery), its holder keeps a goroutine polling an empty partition at ~4-5 queries per poll tick, permanently, plus a lease row and a queue_offsets row that nothing ever deletes. On topics with bounded partition keys (queue names) this is harmless stickiness; on topics with unbounded short-lived keys it is a leak proportional to cumulative traffic: buildsignal partitions per batch ID, and the gateway log and cancel topics partition per request ID — every request ever processed would leave a ghost worker behind. The crash variant is worse: a stale lease on a drained partition is never even stealable, because acquisition only probes discovered partitions.

What?

  • Idle-lease release: the discovery tick tracks, per owned partition absent from discovery, when it was first observed drained (a partition with any in-flight, postponed, or unacked message still has stored rows and is always discovered — only fully-consumed partitions qualify). After a grace of 2x LeaseDurationMs (60s at defaults) the subscriber deletes its consumer group's offsets row (while still holding the lease, so no concurrent initialization is possible), releases the lease, and reconciliation stops the worker. If a message arrives later, the partition reappears in discovery and is reacquired like any new partition — Initialize recreates the offsets row, and since drained meant zero stored rows there is nothing to replay.
  • Stale-lease purge: the lease tick deletes lease rows not renewed within 10x LeaseDurationMs, covering holders that crashed while owning a drained partition. Deleting a stale row is equivalent to expiry — a concurrent renewal refreshes the row and the age predicate skips it.
  • Rebalance integration tests pin Retry.MaxAttempts high: they publish messages they never ack, and dead-lettering mid-test would now drain the partitions and dissolve the lease distribution their assertions wait on.

Test Plan

  • ✅ New integration test TestIdleLeaseRelease covers the full lifecycle against real MySQL: consume, wait out GC + grace, assert the lease and offsets rows are gone, then republish to the same partition key and assert delivery resumes through normal discovery.

@behinddwalls
behinddwalls force-pushed the preetam/mq-heartbeat-gc branch from c26d89e to 4b4888f Compare August 6, 2026 16:58
@behinddwalls
behinddwalls force-pushed the preetam/mq-idle-lease-release branch from 412313c to ac048a1 Compare August 6, 2026 16:58
@behinddwalls
behinddwalls marked this pull request as ready for review August 6, 2026 16:59
@behinddwalls
behinddwalls requested review from a team and sbalabanov as code owners August 6, 2026 16:59
@behinddwalls
behinddwalls changed the base branch from preetam/mq-heartbeat-gc to main August 6, 2026 20:37
@behinddwalls
behinddwalls force-pushed the preetam/mq-idle-lease-release branch from ac048a1 to 80888ad Compare August 6, 2026 20:40
Comment thread platform/extension/messagequeue/mysql/subscriber.go
## Summary

### Why?

A lease is held forever once acquired: `ReleaseLease` is only called on shutdown and fair-share shedding, renewal is unconditional, and workers are reconciled from the lease set — so when a partition drains (every message acked and garbage-collected, hence absent from discovery), its holder keeps a goroutine polling an empty partition at ~4-5 queries per poll tick, permanently, plus a lease row and a `queue_offsets` row that nothing ever deletes. On topics with bounded partition keys (queue names) this is harmless stickiness; on topics with unbounded short-lived keys it is a leak proportional to cumulative traffic: buildsignal partitions per batch ID, and the gateway log and cancel topics partition per request ID — every request ever processed would leave a ghost worker behind. The crash variant is worse: a stale lease on a drained partition is never even stealable, because acquisition only probes discovered partitions.

### What?

- Idle-lease release: the discovery tick tracks, per owned partition absent from discovery, when it was first observed drained (a partition with any in-flight, postponed, or unacked message still has stored rows and is always discovered — only fully-consumed partitions qualify). After a grace of 2x `LeaseDurationMs` (60s at defaults) the subscriber deletes its consumer group's offsets row (while still holding the lease, so no concurrent initialization is possible), releases the lease, and reconciliation stops the worker. If a message arrives later, the partition reappears in discovery and is reacquired like any new partition — `Initialize` recreates the offsets row, and since drained meant zero stored rows there is nothing to replay.
- Stale-lease purge: the lease tick deletes lease rows not renewed within 10x `LeaseDurationMs`, covering holders that crashed while owning a drained partition. Deleting a stale row is equivalent to expiry — a concurrent renewal refreshes the row and the age predicate skips it.
- Rebalance integration tests pin `Retry.MaxAttempts` high: they publish messages they never ack, and dead-lettering mid-test would now drain the partitions and dissolve the lease distribution their assertions wait on.

## Test Plan

- ✅ New integration test `TestIdleLeaseRelease` covers the full lifecycle against real MySQL: consume, wait out GC + grace, assert the lease and offsets rows are gone, then republish to the same partition key and assert delivery resumes through normal discovery.
@behinddwalls
behinddwalls force-pushed the preetam/mq-idle-lease-release branch from 80888ad to 7436364 Compare August 7, 2026 03:39
@behinddwalls
behinddwalls merged commit 79b5252 into main Aug 7, 2026
15 checks passed
@behinddwalls
behinddwalls deployed to stack-rebase August 7, 2026 03:55 — with GitHub Actions Active
@behinddwalls
behinddwalls deleted the preetam/mq-idle-lease-release branch August 7, 2026 03:56
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