feat(queue): add lease-owner metrics and processing logs - #736
Merged
Merged
Conversation
## Summary ### Why? Lease ownership was only visible in the DB and the admin CLI. Renewals were a silent batch update, steals had no event, and process logs carried message_id and partition_key but not which replica held the lease — so "who owns this key" and "which node processed this message" were hard to answer from metrics or logs. ### What? Subscriber metrics tag leased_by, topic, tenant, and consumer_group, and deliberately never partition_key: acquire/steal/release counters, renew row counts, partitions_owned on every discovery, and active_subscribers plus fair_share_cap only after a successful lease-tick heartbeat query. A cap of 0 means unlimited internally; the gauge instead reports the size of the owned-plus-discovered partition set, so `partitions_owned > fair_share_cap` cannot fire trivially on a lone replica. Info logs cover steals (previous_owner from the pre-acquire snapshot), the release reason, and a per-tick summary even when renew fails; a first claim stays at Debug. Release counters report effect rather than intent. `ReleaseLease` returns rows deleted alongside its error — it already computed the count and discarded it — so the rebalance and idle counters skip a release whose DELETE matched nothing, which happens whenever the lease was stolen or purged between the tick's read and the release. Shutdown releases one tenant at a time so the count carries a tenant tag and a failure names the tenant it belongs to. The idle path records the release as soon as the lease row is gone, before offset cleanup, so a DeleteOffset failure no longer suppresses a counter for a lease that genuinely was released. Deliveries carry leased_by and consumer_group in metadata. The consumer snapshots those fields once before handing the delivery to the controller, because Metadata() exposes a map the controller can write to, and reuses that snapshot on the process, hold-ignored, reject, and nack logs so every line agrees on the owner. ## Test Plan ✅ `go test -race ./platform/extension/messagequeue/mysql/ ./platform/consumer/` ✅ `make fmt`, `make gazelle`, `make mocks`, `make check-gazelle`
behinddwalls
force-pushed
the
preetam/messagequeue-metrics
branch
from
September 22, 2026 01:24
17e5a7e to
6b3aecf
Compare
behinddwalls
marked this pull request as ready for review
September 22, 2026 18:19
mnoah1
reviewed
Sep 22, 2026
## Summary ### Why? PR #736 made `processDelivery` always call `delivery.Metadata()` for lease-owner log fields. `TestPrimaryConsumer_GitFailureDisposition` did not expect that call, so gomock failed on the consumer goroutine while the test blocked on `<-done` until Bazel's 300s timeout. ### What? Add `Metadata().Return(nil).AnyTimes()` to the runway mock delivery, matching the consumer package's `setupDelivery` helper. ## Test Plan ✅ `go test ./service/runway/server/ -count=1 -run TestPrimaryConsumer_GitFailureDisposition`
mnoah1
approved these changes
Sep 22, 2026
mnoah1
left a comment
mnoah1
left a comment
Contributor
There was a problem hiding this comment.
Approve pending updates to remove pid from the tags
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Why?
Lease ownership was only visible in the DB and the admin CLI. Renewals were a silent batch update, steals had no event, and process logs carried message_id and partition_key but not which replica held the lease — so "who owns this key" and "which node processed this message" were hard to answer from metrics or logs.
What?
Subscriber metrics tag leased_by, topic, tenant, and consumer_group, and deliberately never partition_key: acquire/steal/release counters, renew row counts, partitions_owned on every discovery, and active_subscribers plus fair_share_cap only after a successful lease-tick heartbeat query. A cap of 0 means unlimited internally; the gauge instead reports the size of the owned-plus-discovered partition set, so
partitions_owned > fair_share_capcannot fire trivially on a lone replica. Info logs cover steals (previous_owner from the pre-acquire snapshot), the release reason, and a per-tick summary even when renew fails; a first claim stays at Debug.Release counters report effect rather than intent.
ReleaseLeasereturns rows deleted alongside its error — it already computed the count and discarded it — so the rebalance and idle counters skip a release whose DELETE matched nothing, which happens whenever the lease was stolen or purged between the tick's read and the release. Shutdown releases one tenant at a time so the count carries a tenant tag and a failure names the tenant it belongs to. The idle path records the release as soon as the lease row is gone, before offset cleanup, so a DeleteOffset failure no longer suppresses a counter for a lease that genuinely was released.Deliveries carry leased_by and consumer_group in metadata. The consumer snapshots those fields once before handing the delivery to the controller, because Metadata() exposes a map the controller can write to, and reuses that snapshot on the process, hold-ignored, reject, and nack logs so every line agrees on the owner.
Test Plan
✅
go test -race ./platform/extension/messagequeue/mysql/ ./platform/consumer/✅
make fmt,make gazelle,make mocks,make check-gazelle