fix: bind durable DLQ consumer so retry drains on workqueue stream - #220
Open
ecv wants to merge 2 commits into
Open
fix: bind durable DLQ consumer so retry drains on workqueue stream#220ecv wants to merge 2 commits into
ecv wants to merge 2 commits into
Conversation
The DLQ retry controller created an ephemeral pull consumer per batch on the ACTIVITY_DEAD_LETTER stream, which uses workqueue retention. Workqueue streams permit only one consumer per non-overlapping subject filter, so concurrent replicas and the per-policy narrow filter collided with "filtered consumer not unique on workqueue stream" and the DLQ never drained (totalSucceeded: 0 every run; prod backlog flat for 42h). Bind a single shared durable consumer instead, provisioned declaratively like every other stream's consumer. Per-policy filtering stays client-side, so no second overlapping consumer is created. Defer redelivery of backed-off events with NakWithDelay so a periodic run reaches processed==0 and stops instead of re-fetching the same events until the run deadline. Key changes: - Add dlq-retry-consumer.yaml durable pull consumer (filter activity.dlq.>) and wire it into the nats-streams kustomization - Bind the durable in processRetryBatch; drop the ephemeral consumer and the overlapping per-policy server-side filter - NakWithDelay backed-off events instead of immediate Nak Fixes #216
Add in-process JetStream tests covering the #216 failure modes: draining a workqueue-retention DLQ stream, two replicas binding the shared durable consumer without a "filtered consumer not unique" collision, and a backed-off event not being redelivered within the same run. Adds nats-server/v2 as a test dependency for the embedded server.
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
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.
Problem
The periodic DLQ retry run never drains anything (
totalSucceeded: 0on every run) and the consumer frequently can't even attach:ACTIVITY_DEAD_LETTERis a workqueue-retention stream, which permits only one consumer per non-overlapping subject filter. ButprocessRetryBatchcreated an ephemeral pull consumer on every batch (PullSubscribe(subject, "", BindStream)):periodicRetryon their own ticker, so two consumers filteractivity.dlq.>and the second is rejected.RetryForPolicyused a narrower filteractivity.dlq.*.<apiGroup>.<kind>that overlapsactivity.dlq.>, so it collides even within one pod.defer UnsubscribeleavesFetchwithno responders available.Net: the consumer usually can't attach, so nothing drains. In prod the backlog has been flat at 177 for 42h with zero drain.
Secondary: when a consumer did attach, backoff-ineligible events were
msg.Nak()'d (immediate redelivery), so the same events were re-fetched every batch,processed == 0was never reached, and the run spun to the 2-minute cap. That is thetotalProcessed: 107000, totalSucceeded: 0signature.Fix
config/components/nats-streams/dlq-retry-consumer.yaml(durable pull consumer, filteractivity.dlq.>), wired into the nats-streams kustomization, following the same declarative pattern as every other stream's consumer.processRetryBatchinstead of creating an ephemeral one, and drop the overlapping per-policy server-side filter. Per-policy narrowing already happens client-side in the batch loop. Both replicas compete for messages on the one consumer; no second overlapping consumer is ever created, and the durable is never unsubscribed or deleted.NakWithDelaybacked-off events instead of an immediateNak, so a periodic run reachesprocessed == 0and terminates instead of re-fetching the same events until the deadline.Tests
New
internal/activityprocessor/dlq_retry_integration_test.goruns an in-process JetStream server and reproduces all three failure modes:TestDLQRetryDrainsWorkqueueStreampublishes 5 events to a workqueue DLQ stream, runs the controller, asserts the DLQ drains to 0 and the events are republished. The old ephemeral consumer failed to attach here.TestDLQRetryTwoReplicasNoConsumerCollisionhas two connections bind the shared durable; both must succeed. This is the exactfiltered consumer not uniquescenario.TestDLQRetryDefersBackedOffEventsfetches a backed-off event once and asserts it is not redelivered within the run, guarding the spin.Adds
nats-io/nats-server/v2as a test dependency for the embedded server.go build ./...andgo test ./internal/activityprocessor/...pass. Root-cause writeup with prod evidence in #216.Post-merge
Once the durable exists, the 177-event backlog drains: project events re-evaluate cleanly (milo CEL fix live in v0.30.1). Connector and billing events keep cycling on proper backoff until their policy fixes land (datum-cloud/network-services-operator#241, milo-os/billing#75), deferred by
NakWithDelayrather than spinning.Fixes #216