Skip to content

Commit ea4e622

Browse files
sbalabanov-zzclaude
andcommitted
feat(consumergate): runtime stop/start of queue controllers
Implement the consumer-gate RFC with a file-backed gate shared by gateway, orchestrator, and runway consumers. - clear deliveries through a consumer-group/partition gate before controller processing while extending visibility for blocked deliveries - expose caller-owned delivery descriptors and let gate implementations stamp gate-owned parked-record fields - keep parked payload files only while deliveries are actively blocked and remove them on every terminal watch path - fail open on gate or visibility-extension failures and leave blocked deliveries unacked during shutdown for normal redelivery - run service containers with the host test UID/GID under rootful Docker while preserving rootless Docker ownership mapping - make the cancellation E2E scenario deterministic by parking the runway merge-conflict-check delivery until cancellation reaches a terminal state - consolidate consumer white-box and behavioral unit tests in the consumer package Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 39d61fe commit ea4e622

40 files changed

Lines changed: 2302 additions & 206 deletions

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,7 @@ local-stovepipe-stop: ## Stop the Stovepipe service
364364

365365
mocks: ## Generate mock files using mockgen
366366
@echo "Generating mocks..."
367-
@$(BAZEL) run @rules_go//go -- generate ./submitqueue/extension/storage/... ./submitqueue/extension/buildrunner/... ./submitqueue/extension/changeprovider/... ./platform/extension/counter/... ./platform/extension/messagequeue/... ./submitqueue/extension/queueconfig/... ./submitqueue/extension/mergechecker/... ./submitqueue/extension/pusher/... ./submitqueue/extension/scorer/... ./submitqueue/extension/conflict/... ./submitqueue/extension/validator/... ./platform/consumer/... ./stovepipe/extension/storage/... ./stovepipe/extension/sourcecontrol/...
367+
@$(BAZEL) run @rules_go//go -- generate ./submitqueue/extension/storage/... ./submitqueue/extension/buildrunner/... ./submitqueue/extension/changeprovider/... ./platform/extension/counter/... ./platform/extension/consumergate/... ./platform/extension/messagequeue/... ./submitqueue/extension/queueconfig/... ./submitqueue/extension/mergechecker/... ./submitqueue/extension/pusher/... ./submitqueue/extension/scorer/... ./submitqueue/extension/conflict/... ./submitqueue/extension/validator/... ./platform/consumer/... ./stovepipe/extension/storage/... ./stovepipe/extension/sourcecontrol/...
368368
@echo "Mocks generated successfully!"
369369

370370
proto: ## Generate protobuf files from .proto definitions

doc/rfc/consumer-gate.md

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ Every controller subscribes with a unique consumer group (`orchestrator-batch`,
2929

3030
### Gate state is a separate extension
3131

32-
The consumer gate is a shared extension in its own right, not a feature of any queue backend. The contract lives at `platform/extension/consumergate/`: the behavioral interface the middleware reads (is this group/partition gated? record a parked delivery, record its release), the write surface tests and tooling use (close a gate, open it), and the `Config`. Implementations live in subdirectories, per the standard extension layout. The consumer package takes the read-side interface as a dependency — wiring constructs an implementation and passes it to `consumer.New` via a new option; when no gate is configured, the middleware is absent and the consumer behaves exactly as today. The wiring delta is one option argument at each consumer construction site (gateway, orchestrator primary, orchestrator DLQ, runway); no per-controller wiring, and DLQ consumers are gated uniformly with the rest.
32+
The consumer gate is a shared extension in its own right, not a feature of any queue backend. The contract lives at `platform/extension/consumergate/`: the behavioral interface the middleware reads (is this group/partition gated? record a parked delivery, record its release), the write surface tests and tooling use (close a gate, open it), and the `Config`. `Watch` accepts a caller-owned `DeliveryDescriptor` containing only message data; the implementation combines it with the gate identity captured by `Enter` and its own timestamp to create the observable `Parked` record, so callers cannot supply or overwrite gate-owned fields. Implementations live in subdirectories, per the standard extension layout. The consumer package takes the read-side interface as a dependency — wiring constructs an implementation and passes it to `consumer.New` via a new option; when no gate is configured, the middleware is absent and the consumer behaves exactly as today. The wiring delta is one option argument at each consumer construction site (gateway, orchestrator primary, orchestrator DLQ, runway); no per-controller wiring, and DLQ consumers are gated uniformly with the rest.
3333

3434
Keeping the contract separate from any backend is what lets the storage medium be chosen per deployment: a filesystem directory first (below), a database- or config-service-backed implementation later if fleet-wide coordination demands it — with the middleware, the wiring shape, and every test written against the contract unchanged.
3535

@@ -43,25 +43,25 @@ The first implementation stores gate state as plain files under a configured dir
4343
{dir}/parked/{consumer_group}/{topic}/{urlenc(id)}.json # one parked delivery record
4444
```
4545

46-
Consumer groups and topics are already filesystem-safe by the repo's naming rules; partition keys and message IDs may contain `/` (request IDs like `queue/1`), so they are URL-encoded in file names. Gate files contain human-readable JSON metadata — `reason`, `created_by`, `created_at_ms` — so an operator finding a paused controller can tell why. Parked records carry the payload, attempt, `parked_at_ms`, and a `released_at_ms` stamped when the delivery proceeds; all writes go through temp-file-plus-rename so readers never see partial JSON.
46+
Consumer groups and topics are already filesystem-safe by the repo's naming rules; partition keys and message IDs may contain `/` (request IDs like `queue/1`), so they are URL-encoded in file names. Gate files contain human-readable JSON metadata — `reason`, `created_by`, `created_at_ms` — so an operator finding a paused controller can tell why. Parked records carry the payload, attempt, and `parked_at_ms` while a delivery is blocked; the record is deleted before the wait ends, so payloads are not retained after release, cancellation, or monitoring failure. All writes go through temp-file-plus-rename so readers never see partial JSON.
4747

4848
Files are the simplest medium that satisfies every requirement in this RFC, and simplicity is the point of the first implementation:
4949

5050
- **Operator interface for free.** Pausing a controller is writing a small file; resuming is `rm`. Inspecting a paused stage is `ls` and `cat`. No client, no schema, no query.
5151
- **Trivially reachable out of process.** In the e2e stack, the compose file bind-mounts a host directory into every service container at a fixed path (passed via one environment variable); the test process manipulates gates and reads parked records as local files. In single-host dev the same directory works as-is.
5252
- **Durable and independent.** State survives service restarts — a paused stage stays paused until explicitly opened — and the gate has no dependency on the queue backend or any database being healthy.
5353

54-
The middleware **polls** the directory rather than using filesystem notifications: inotify events do not propagate reliably across bind mounts and overlay filesystems, and the cached-poll posture (below) makes notification latency irrelevant. The known limit of the file medium is multi-replica fleets: a file gates the replicas that see the directory, so a fleet-wide pause needs the deployment platform to distribute the file — or a future store-backed implementation of the same contract. That trade is accepted; the deployments this RFC serves (e2e, single-host dev, per-instance operational pause) are exactly where files excel.
54+
The middleware **polls** the directory rather than using filesystem notifications: inotify is platform-specific, watches can overflow or require re-registration, and event behavior varies across bind mounts, overlay or network filesystems, rootless Docker, and Docker Desktop's host/container filesystem bridge. Polling is the portable convergence mechanism; filesystem events may be added later as an optional wakeup optimization alongside it. The known limit of the file medium is multi-replica fleets: a file gates the replicas that see the directory, so a fleet-wide pause needs the deployment platform to distribute the file — or a future store-backed implementation of the same contract. That trade is accepted; the deployments this RFC serves (e2e, single-host dev, per-instance operational pause) are exactly where files excel.
5555

56-
### Read path: cached poll, bounded effect latency
56+
### Read path: direct reads and bounded release latency
5757

58-
The middleware does not check gate state per message. Gate state is cached per controller and refreshed on a short interval (configurable, ~1s), and a parked delivery re-checks on the same tick. The dormant cost of the feature — the common case, forever — is one directory stat per controller per interval. The price is that closing a gate takes effect within one refresh interval plus the in-flight message's completion; opening one takes effect within one interval.
58+
The middleware checks the applicable gate files for every delivery. A parked delivery re-checks them on a short interval (configurable, ~1s). Closing a gate therefore affects the next delivery check without waiting for a cache refresh; opening one releases already parked deliveries within one poll interval.
5959

6060
Tests do not depend on that latency. The deterministic patterns are two: **arrange first** (close the gate before publishing the message that must be caught — exact by construction), or **await the observed effect** (the parked record, below) instead of assuming timing.
6161

6262
### Observation: parked deliveries are recorded
6363

64-
Parking writes the parked record before blocking. This record is the "observe" half of stop/observe/start: a test awaits the record to *know* the stop caught its message (there is otherwise no signal distinguishing "gated and parked" from "not arrived yet"), can assert on the recorded payload, and can decide what to do next while the controller is provably stopped. For an operator, the same records answer "what is this paused controller holding?". Records are bounded by parked messages, which are bounded by gate usage; the directory is empty whenever no gate is in use.
64+
Parking writes the parked record before blocking. This record is the "observe" half of stop/observe/start: a test awaits the record to *know* the stop caught its message (there is otherwise no signal distinguishing "gated and parked" from "not arrived yet"), can assert on the recorded payload, and can decide what to do next while the controller is provably stopped. For an operator, the same records answer "what is this paused controller holding?". The record is removed before the wait reports release, cancellation, or failure, so records are bounded by currently parked messages and the directory is empty whenever no delivery is held behind a gate.
6565

6666
### Failure posture: fail open
6767

platform/consumer/BUILD.bazel

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ go_library(
1212
deps = [
1313
"//platform/base/messagequeue:go_default_library",
1414
"//platform/errs:go_default_library",
15+
"//platform/extension/consumergate:go_default_library",
1516
"//platform/extension/messagequeue:go_default_library",
1617
"//platform/metrics:go_default_library",
1718
"@com_github_uber_go_tally//:go_default_library",
@@ -25,11 +26,12 @@ go_test(
2526
"consumer_test.go",
2627
"registry_test.go",
2728
],
29+
embed = [":go_default_library"],
2830
deps = [
29-
":go_default_library",
3031
"//platform/base/messagequeue:go_default_library",
31-
"//platform/consumer/mock:go_default_library",
3232
"//platform/errs:go_default_library",
33+
"//platform/extension/consumergate:go_default_library",
34+
"//platform/extension/consumergate/noop:go_default_library",
3335
"//platform/extension/messagequeue:go_default_library",
3436
"//platform/extension/messagequeue/mock:go_default_library",
3537
"//submitqueue/core/topickey:go_default_library",

platform/consumer/consumer.go

Lines changed: 160 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323

2424
"github.com/uber-go/tally"
2525
"github.com/uber/submitqueue/platform/errs"
26+
"github.com/uber/submitqueue/platform/extension/consumergate"
2627
extqueue "github.com/uber/submitqueue/platform/extension/messagequeue"
2728
"github.com/uber/submitqueue/platform/metrics"
2829
"go.uber.org/zap"
@@ -32,6 +33,16 @@ const (
3233
// startupCleanupTimeoutMs is the timeout for cleaning up subscriptions when
3334
// a controller fails to start during Start().
3435
startupCleanupTimeoutMs = 30000
36+
37+
// gateExtensionMs is the visibility extension applied to a delivery blocked
38+
// behind its consumer gate on each keep-in-flight tick, keeping it in-flight
39+
// without burning retry budget (milliseconds). Must comfortably exceed
40+
// defaultGateExtendInterval.
41+
gateExtensionMs = int64(30000)
42+
43+
// defaultGateExtendInterval is how often a gate-blocked delivery's
44+
// visibility is extended.
45+
defaultGateExtendInterval = 10 * time.Second
3546
)
3647

3748
// Consumer orchestrates multiple queue consumers. It handles subscription lifecycle,
@@ -61,6 +72,12 @@ type consumer struct {
6172
metricsScope tally.Scope
6273
registry TopicRegistry
6374
processor errs.ErrorProcessor
75+
gate consumergate.Gate
76+
77+
// gateExtendInterval is how often a gate-blocked delivery's visibility is
78+
// extended. Fixed to defaultGateExtendInterval by New; a field (not the
79+
// const) so in-package tests can exercise the keep-in-flight path quickly.
80+
gateExtendInterval time.Duration
6481

6582
mu sync.Mutex
6683
stopped bool
@@ -85,13 +102,20 @@ type activeSubscription struct {
85102
// consumers such as DLQ reconciliation that must redeliver on any failure.
86103
// processor must not be nil; callers that genuinely want no transformation
87104
// can pass errs.NewClassifierProcessor() with no classifiers.
88-
func New(logger *zap.SugaredLogger, scope tally.Scope, registry TopicRegistry, processor errs.ErrorProcessor) Consumer {
105+
//
106+
// gate is the consumer-gate implementation consulted before each delivery
107+
// reaches its controller. Pass noop.New() (from
108+
// platform/extension/consumergate/noop) for services that do not need runtime
109+
// gating. gate must not be nil.
110+
func New(logger *zap.SugaredLogger, scope tally.Scope, registry TopicRegistry, processor errs.ErrorProcessor, gate consumergate.Gate) Consumer {
89111
return &consumer{
90-
logger: logger,
91-
metricsScope: scope.SubScope("consumer"),
92-
registry: registry,
93-
processor: processor,
94-
subscriptions: make(map[TopicKey]*activeSubscription),
112+
logger: logger,
113+
metricsScope: scope.SubScope("consumer"),
114+
registry: registry,
115+
processor: processor,
116+
gate: gate,
117+
gateExtendInterval: defaultGateExtendInterval,
118+
subscriptions: make(map[TopicKey]*activeSubscription),
95119
}
96120
}
97121

@@ -341,6 +365,14 @@ func (m *consumer) processPartition(ctx context.Context, controller Controller,
341365
func (m *consumer) processDelivery(ctx context.Context, controller Controller, delivery extqueue.Delivery, controllerScope tally.Scope) {
342366
const opName = "process"
343367

368+
// Consumer gate: block the delivery while the controller's gate is closed.
369+
// A false return means the consumer is shutting down while blocked — leave
370+
// the delivery in-flight (no process, no ack/nack) so its visibility lapses
371+
// into a normal redelivery. Gate errors fail open inside waitGate.
372+
if !m.waitGate(ctx, controller, delivery, controllerScope) {
373+
return
374+
}
375+
344376
start := time.Now()
345377
metrics.NamedCounter(controllerScope, opName, "messages_received", 1)
346378

@@ -485,6 +517,128 @@ func (m *consumer) processDelivery(ctx context.Context, controller Controller, d
485517
)
486518
}
487519

520+
// waitGate clears a delivery through the consumer gate before it reaches the
521+
// controller. It returns true when the delivery may proceed, false when it
522+
// must be dropped without processing or ack/nack (the visibility timeout then
523+
// lapses into a normal redelivery).
524+
//
525+
// Gate.Enter checks the gate synchronously; an unblocked entry is the common
526+
// path and costs nothing further. For a blocked entry the gate hands back a
527+
// watch channel (its own monitoring goroutine behind it), and this routine
528+
// multiplexes three events in a single select loop — the watch channel, a
529+
// visibility-extension ticker that keeps the blocked delivery in-flight, and
530+
// parent-context cancellation — with no extra goroutine on the consumer side.
531+
// The watch context is a child of ctx cancelled on every return path, so the
532+
// gate's goroutine always exits and nothing is left dangling. Failures fail
533+
// open: if gate state cannot be read or recorded, or the delivery can no longer
534+
// be held safely because a visibility extension failed, the delivery proceeds
535+
// and the failure is surfaced via log and counter. Only consumer shutdown drops
536+
// the delivery.
537+
func (m *consumer) waitGate(ctx context.Context, controller Controller, delivery extqueue.Delivery, scope tally.Scope) bool {
538+
const opName = "gate"
539+
540+
msg := delivery.Message()
541+
consumerGroup := controller.ConsumerGroup()
542+
topic := controller.TopicKey().String()
543+
544+
entry, err := m.gate.Enter(ctx, consumergate.Key{ConsumerGroup: consumerGroup, PartitionKey: msg.PartitionKey})
545+
if err != nil {
546+
if errors.Is(err, context.Canceled) {
547+
// Cancellation is in progress; return false per the contract.
548+
return false
549+
}
550+
// Gate state could not be read: fail open — gating is auxiliary, and
551+
// a broken gate medium must not become a pipeline stall.
552+
metrics.NamedCounter(scope, opName, "enter_errors", 1)
553+
m.logger.Errorw("gate check failed, failing open",
554+
"consumer_group", consumerGroup,
555+
"topic", topic,
556+
"message_id", msg.ID,
557+
"error", err,
558+
)
559+
return true
560+
}
561+
if !entry.Blocked() {
562+
return true
563+
}
564+
565+
start := time.Now()
566+
defer func() {
567+
metrics.NamedHistogram(scope, opName, "wait_latency", metrics.DefaultLatencyBuckets).RecordDuration(time.Since(start))
568+
}()
569+
570+
// The delivery is blocked. Build the caller-owned descriptor and start
571+
// watching the gate; watchCtx is cancelled on every return path so the gate's
572+
// monitoring goroutine exits even when we fail open with the consumer still
573+
// running.
574+
descriptor := consumergate.DeliveryDescriptor{
575+
Topic: topic,
576+
MessageID: msg.ID,
577+
Payload: msg.Payload,
578+
Attempt: delivery.Attempt(),
579+
}
580+
581+
// Start a child context for the watch and cancel it on every return path,
582+
// so the gate's monitoring goroutine always exits.
583+
watchCtx, cancelWatch := context.WithCancel(ctx)
584+
defer cancelWatch()
585+
watchCh := entry.Watch(watchCtx, descriptor)
586+
587+
// Keep the blocked delivery in-flight on a ticker while multiplexing the
588+
// gate watch and parent cancellation.
589+
ticker := time.NewTicker(m.gateExtendInterval)
590+
defer ticker.Stop()
591+
592+
loop:
593+
for {
594+
select {
595+
case e := <-watchCh:
596+
if e == nil {
597+
// The gate opened; the delivery proceeds. Returning is safe:
598+
// the deferred cancelWatch stops the gate's monitoring
599+
// goroutine and the deferred ticker.Stop halts the extender.
600+
return true
601+
}
602+
if err == nil {
603+
// This includes a cancellation error propagated from the
604+
// parent context.
605+
err = e
606+
}
607+
break loop
608+
609+
case <-ticker.C:
610+
if err != nil {
611+
// The error is already set, so this tick is bogus and can be
612+
// skipped; wait for the gate watch to report back and end the
613+
// loop.
614+
continue
615+
}
616+
if e := delivery.ExtendVisibilityTimeout(ctx, gateExtensionMs); e != nil {
617+
// The delivery can no longer be held safely, so cancel the gate
618+
// watch; it reports back on watchCh and ends the loop.
619+
err = e
620+
cancelWatch()
621+
}
622+
}
623+
}
624+
625+
// The loop only breaks with a non-nil err.
626+
if errors.Is(err, context.Canceled) {
627+
// Cancellation is in progress; return false per the contract.
628+
return false
629+
}
630+
// Gate state could not be re-read or the record could not be
631+
// written: fail open, as above.
632+
metrics.NamedCounter(scope, opName, "wait_errors", 1)
633+
m.logger.Errorw("gate wait failed, failing open",
634+
"consumer_group", consumerGroup,
635+
"topic", topic,
636+
"message_id", msg.ID,
637+
"error", err,
638+
)
639+
return true
640+
}
641+
488642
// Stop gracefully shuts down all handlers with the specified timeout.
489643
// Cancels all subscription contexts and waits for consumption goroutines to finish.
490644
// timeoutMs is the maximum time in milliseconds to wait for graceful shutdown.

0 commit comments

Comments
 (0)