Skip to content

Commit 2db191e

Browse files
sbalabanov-zzclaude
andcommitted
feat(consumergate): runtime stop/start of queue controllers + deterministic e2e cancel test
Implement doc/rfc/consumer-gate.md: - platform/extension/consumergate: extension contract — Gate.Enter checks a delivery against its gate synchronously and returns an Entry future: an open gate admits the delivery immediately, a closed gate hands out a blocked Entry whose Wait records the parked delivery and blocks until the gate opens (implementations own the wait mechanism, so a notification-capable store can release instantly, and the parked-delivery observation records). Admin is the write surface for tests and tooling; Config, Factory interface, and gomock mocks round out the contract. - platform/extension/consumergate/file: polling implementation — gate state as plain files in a shared directory (gate file present = closed, rm = open), parked-delivery records as JSON stamped parked/released by the store, per-partition cached verdicts (TTL = poll interval) so an open gate costs no stat per delivery, and temp-file-plus-rename writes with converged error handling. - platform/extension/consumergate/noop: no-op gate for services and tests that do not need runtime gating. - platform/consumer: consumer.New takes the Gate as a required argument and clears every delivery through Enter. Only a blocked delivery pays for queue mechanics: a lazily armed timer keeps it in-flight by periodically extending visibility (no goroutine unless actually blocked, no retry budget burned, partition order preserved), and blocked wait time is recorded on a latency histogram. Shutdown while blocked leaves the delivery for normal redelivery; gate errors and failed visibility extensions cancel the wait and fail open with a log and counter. - Wiring: gateway, orchestrator (primary + DLQ), and runway construct the file-backed gate rooted at CONSUMER_GATE_DIR, defaulting to /var/submitqueue/consumergate — the path the compose stack bind-mounts into every service; stovepipe wires the no-op gate. - test/e2e/submitqueue: replace TestCancel_RecordsIntent with TestCancel_CaughtPreBatch_NeverLands — the stop→observe→start scenario from the RFC. The gate parks runway's merge-conflict check for the test queue's partition before landing, the parked record proves the request is held pre-batch, the cancel drives it terminal cancelled, and after the gate opens a sentinel request landing on the same partitions proves the stale check signal was consumed and dropped — the cancelled change is never batched and never lands. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent c7987c7 commit 2db191e

34 files changed

Lines changed: 2250 additions & 64 deletions

File tree

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/speculation/enumerator/... ./submitqueue/extension/speculation/dependencylimit/... ./submitqueue/extension/speculation/selector/... ./submitqueue/extension/speculation/selectionlimit/... ./submitqueue/extension/speculation/prioritizer/... ./submitqueue/extension/speculation/prioritizationlimit/... ./submitqueue/extension/validator/... ./submitqueue/extension/speculation/pathscorer/... ./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/speculation/enumerator/... ./submitqueue/extension/speculation/dependencylimit/... ./submitqueue/extension/speculation/selector/... ./submitqueue/extension/speculation/selectionlimit/... ./submitqueue/extension/speculation/prioritizer/... ./submitqueue/extension/speculation/prioritizationlimit/... ./submitqueue/extension/validator/... ./submitqueue/extension/speculation/pathscorer/... ./platform/consumer/... ./stovepipe/extension/storage/... ./stovepipe/extension/sourcecontrol/...
368368
@echo "Mocks generated successfully!"
369369

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

platform/consumer/BUILD.bazel

Lines changed: 5 additions & 1 deletion
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",
@@ -22,14 +23,17 @@ go_library(
2223
go_test(
2324
name = "go_default_test",
2425
srcs = [
26+
"consumer_internal_test.go",
2527
"consumer_test.go",
2628
"registry_test.go",
2729
],
30+
embed = [":go_default_library"],
2831
deps = [
29-
":go_default_library",
3032
"//platform/base/messagequeue:go_default_library",
3133
"//platform/consumer/mock:go_default_library",
3234
"//platform/errs:go_default_library",
35+
"//platform/extension/consumergate:go_default_library",
36+
"//platform/extension/consumergate/noop:go_default_library",
3337
"//platform/extension/messagequeue:go_default_library",
3438
"//platform/extension/messagequeue/mock:go_default_library",
3539
"//submitqueue/core/topickey:go_default_library",

platform/consumer/consumer.go

Lines changed: 178 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,146 @@ 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. Only for a blocked entry does the consumer
527+
// arrange its queue mechanics — a keep-in-flight timer periodically extending
528+
// the delivery's visibility — before blocking on the entry's Wait. Failures
529+
// fail open: if gate state cannot be read or recorded, or the delivery can no
530+
// longer be held safely because a visibility extension failed, the delivery
531+
// proceeds and the failure is surfaced via log and counter. Only consumer
532+
// shutdown drops the delivery.
533+
func (m *consumer) waitGate(ctx context.Context, controller Controller, delivery extqueue.Delivery, scope tally.Scope) bool {
534+
const opName = "gate"
535+
536+
msg := delivery.Message()
537+
consumerGroup := controller.ConsumerGroup()
538+
topic := controller.TopicKey().String()
539+
540+
entry, err := m.gate.Enter(ctx, consumergate.Key{ConsumerGroup: consumerGroup, PartitionKey: msg.PartitionKey})
541+
if err != nil {
542+
// Gate state could not be read: fail open — gating is auxiliary, and
543+
// a broken gate medium must not become a pipeline stall.
544+
metrics.NamedCounter(scope, opName, "enter_errors", 1)
545+
m.logger.Errorw("gate check failed, failing open",
546+
"consumer_group", consumerGroup,
547+
"topic", topic,
548+
"message_id", msg.ID,
549+
"error", err,
550+
)
551+
return true
552+
}
553+
if !entry.Blocked() {
554+
return true
555+
}
556+
557+
// The delivery is blocked; only now is the parked descriptor built and
558+
// the queue mechanics arranged. The wait context is cancellable by the
559+
// keep-in-flight timer: when a visibility extension fails, the delivery
560+
// can no longer be held safely (the queue may already be re-delivering
561+
// it), so the wait is cancelled with the extension failure as the cause.
562+
parked := consumergate.Parked{
563+
Topic: topic,
564+
MessageID: msg.ID,
565+
Payload: msg.Payload,
566+
Attempt: delivery.Attempt(),
567+
}
568+
waitCtx, cancelWait := context.WithCancelCause(ctx)
569+
defer cancelWait(nil)
570+
571+
stopExtending := m.keepInFlight(waitCtx, delivery, cancelWait)
572+
start := time.Now()
573+
err = entry.Wait(waitCtx, parked)
574+
stopExtending()
575+
metrics.NamedLatencyHistogram(scope, opName, "wait_latency", time.Since(start))
576+
577+
if err == nil {
578+
return true
579+
}
580+
581+
if errors.Is(err, context.Canceled) {
582+
// Two cancellation sources share the context error; the cancellation
583+
// cause tells them apart. A keep-in-flight failure carries itself as
584+
// the cause; a consumer shutdown propagates the plain context.Canceled.
585+
if cause := context.Cause(waitCtx); cause != nil && !errors.Is(cause, context.Canceled) {
586+
metrics.NamedCounter(scope, opName, "extend_errors", 1)
587+
m.logger.Errorw("failed to keep gate-blocked delivery in-flight, failing open",
588+
"consumer_group", consumerGroup,
589+
"topic", topic,
590+
"message_id", msg.ID,
591+
"error", cause,
592+
)
593+
return true
594+
}
595+
metrics.NamedCounter(scope, opName, "shutdown_while_blocked", 1)
596+
m.logger.Infow("consumer shutdown while delivery was blocked by gate",
597+
"consumer_group", consumerGroup,
598+
"topic", topic,
599+
"message_id", msg.ID,
600+
)
601+
return false
602+
}
603+
604+
// Gate state could not be re-read or the release could not be recorded:
605+
// fail open, as above.
606+
metrics.NamedCounter(scope, opName, "wait_errors", 1)
607+
m.logger.Errorw("gate wait failed, failing open",
608+
"consumer_group", consumerGroup,
609+
"topic", topic,
610+
"message_id", msg.ID,
611+
"error", err,
612+
)
613+
return true
614+
}
615+
616+
// keepInFlight periodically extends a delivery's visibility while it is
617+
// blocked behind its gate, so the queue does not redeliver a held message.
618+
// It is timer-driven rather than goroutine-driven: nothing runs unless the
619+
// delivery actually stays blocked past the first extend interval, so the
620+
// common case — an open gate — costs one timer arm/disarm and no goroutine.
621+
//
622+
// An extension failure means the delivery can no longer be held safely (its
623+
// visibility may lapse and the queue redeliver it), so the failure propagates
624+
// through cancelWait — unblocking the gate wait immediately — and the timer
625+
// chain stops. The returned stop function disarms the timer and waits out any
626+
// in-flight extension, guaranteeing no extension races the controller.
627+
func (m *consumer) keepInFlight(ctx context.Context, delivery extqueue.Delivery, cancelWait context.CancelCauseFunc) (stop func()) {
628+
var mu sync.Mutex
629+
stopped := false
630+
631+
// The timer variable is assigned under the lock so the callback — which
632+
// may fire before AfterFunc returns — observes the assignment through the
633+
// same lock it takes before Reset.
634+
var timer *time.Timer
635+
mu.Lock()
636+
timer = time.AfterFunc(m.gateExtendInterval, func() {
637+
// The lock is held across the extension so stop() joins an in-flight
638+
// callback before returning.
639+
mu.Lock()
640+
defer mu.Unlock()
641+
if stopped {
642+
return
643+
}
644+
if err := delivery.ExtendVisibilityTimeout(ctx, gateExtensionMs); err != nil {
645+
cancelWait(fmt.Errorf("failed to extend visibility of gate-blocked delivery: %w", err))
646+
return
647+
}
648+
timer.Reset(m.gateExtendInterval)
649+
})
650+
mu.Unlock()
651+
652+
return func() {
653+
mu.Lock()
654+
stopped = true
655+
mu.Unlock()
656+
timer.Stop()
657+
}
658+
}
659+
488660
// Stop gracefully shuts down all handlers with the specified timeout.
489661
// Cancels all subscription contexts and waits for consumption goroutines to finish.
490662
// timeoutMs is the maximum time in milliseconds to wait for graceful shutdown.

0 commit comments

Comments
 (0)