Repository navigation
fix(outbox): back off counted retries and keep each key's events in order behind a failed one - #64
Merged
Merged
Conversation
…rder behind a failed one A counted failure (an exception, a timeout, a handler_retry() without next_retry_at) was retried on the very next cycle, so max_attempts measured relay cycles: a busy relay spent the default budget of 6 in a third of a second. HandlerExecutionStep now reschedules it after an exponential backoff with equal jitter (retry_backoff_base_seconds=10.0, retry_backoff_cap_seconds=3600.0, on the three factories and OutboxPublisher; 0 restores the next-cycle retry). mark_failed keeps scheduled_at when given no time, as bulk_mark_failed already did. Backing off a row would let the events written after it overtake it, and a single failed publish already did: the probe moved the failed row behind its successors, and a counted failure let the rest of the batch go ahead. PostgresOutboxRepository.fetch_and_lock_pending now hands out only the head of each (topic, partition_key): a row waits while an earlier pending row of its key is due, held by a relay or waiting for its retry. That keeps order across relays too. A delayed event that was never tried does not hold its key back, and a failed (terminal) row does not either. The outbox bases get the partial index idx_outbox_events_pending_key_order on (topic, partition_key, created_at) WHERE status = 'pending' behind the lookup. Closes #60 Closes #61
This was referenced Oct 16, 2026
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.
Closes #60. Closes #61.
One PR for both. Backing off a counted retry (#61) moves the failed row's
scheduled_atminutes ahead, and with the fetch as it was, that lets every later event of the same key overtake it. #60 already shows the one-cycle version of that. The ordering fix in turn depends on a failed row that waits holding its key back. So this is one contract: a failed row is retried later, and its key waits for it.Problem
handler_retry()withoutnext_retry_at) was due again on the very next cycle. That mademax_attemptsa count of relay cycles. A busy relay used up the default 6 attempts in 0.03 s.FOR UPDATE SKIP LOCKEDlet relay 2 take event 2 while relay 1 still held event 1.Fix
Backoff (
HandlerExecutionStep). A counted failure with nonext_retry_atof its own is now rescheduled atnow + calculate_backoff_with_jitter(attempts_made, base, cap, jitter_mode="equal"). That applies to the outbox, the inbox and the dispatching processors. An explicitnext_retry_atstill wins.retry_backoff_base_seconds(default 10.0) andretry_backoff_cap_seconds(default 3600.0) are new keyword arguments onHandlerExecutionStep, the threecreate_*_processorfactories andOutboxPublisher, and0gives the old next-cycle retry. I chose equal jitter over full jitter because full jitter can draw close to 0 on any attempt, which is the exact problem #61 describes. With equal jitter the default budget runs out between 2.5 and 5 minutes after the first failure.Per-key order (
PostgresOutboxRepository.fetch_and_lock_pending). The fetch now hands out only the head of each(topic, partition_key). A row is skipped while an earlier pending row of its key is in line, meaning that row is due, held by a relay, or has been tried and is waiting for its retry. The check sits in the candidate query beforeLIMIT … FOR UPDATE SKIP LOCKED, so a batch never fills up with rows it then can't use. It also covers several relays: while relay 1 holds a head, the row behind it is not a head for relay 2 either. Two decisions to note:failedrow (terminal) does not block either. Its successors go out without it, andrequeue_failedsends it later, out of order. Blocking instead would let one poison event stall its key forever, with nothing visible but a growing backlog.Index. Both outbox bases get the partial index
idx_outbox_events_pending_key_orderon(topic, partition_key, created_at) WHERE status = 'pending'(idx_outbox_events_p_pending_key_orderon the partitioned base).migrations.mdhas the DDL, along with a note to useCREATE INDEX CONCURRENTLYon an existing table.mark_failedkeepsscheduled_atwhen given nonext_retry_at, asbulk_mark_failedalready did. Before, the single-row path moved the row tonow(), so the commit strategy decided when a deferred row came back.Rejected
scheduled_at, then drop any row whose earlier in-line row isn't in this batch. I built this first, and the stand test below caught it starving. A transient probe moves a key's head one second ahead, so its successors sort first, and with a smallbatch_sizethe batch fills with successors that are then dropped every cycle. 286 of 600 events went out in 60 s. Fixing that needs acreated_atorder for the whole candidate scan, which the existingscheduled_atindex cannot serve. Head-only has no such failure mode.Behaviour changes, who notices
publish_batch/process_batchtwice and expect a second attempt, and alerts or DLQ flows that expectfailedwithin seconds. The row now stayspendingwith a futurescheduled_at. Passretry_backoff_base_seconds=0to get the old timing. This changes a default, so the release notes should mention it. I did not mark it!, because nothing is removed and the new pacing is what the user guide already promised. Please decide.batch_sizekeys, so throughput across keys is unchanged. A backlog on a single key now drains one event per cycle: 1000 events of one key took 1000 cycles (5.3 s locally, broker stubbed) where they used to take 10. Three events written in one transaction now go out over three back-to-back cycles.failed. That is the guarantee, and it is documented as rule 20 inagents.md, under "Event order" in troubleshooting, and in the user guide.Tests
tests/integration/postgres/test_outbox_order.pyruns against real Postgres, using the relay from the user guide (OutboxPublisheroverPostgresOutboxRepository, one transaction per cycle):retry_backoff_base_seconds=0it reachesfailedin six cycles, as before.mark_failed(next_retry_at=None)keeps the schedule, and both outbox tables carry the new index.Unit tests in
tests/unit/core/pipeline/steps/test_handler.pycheck the backoff window per attempt, the cap, a timeout, a bare and a timedhandler_retry(),0, and argument validation.Fails before. With the
omni_boxfiles frommasterswapped back in, 8 of the 13 integration tests fail. The relevant failures: the transient scenario sends['order.paid', …, 'order.created'], the two-relay test hands relay 2 the second event of the held key, the busy relay reachesFAILED, the per-cycle test publishes[6, 0, 0], themark_failedschedule moves, and the index is missing. The other three failures areTypeErrors on the new keyword. The backoff unit tests fail withnext_retry_atbeingNone.Stand test. Four relays and two writers, 600 events over 25 keys, writers serialized per key, random
ValueError(5%) andTransientError(3%) from the broker, batch sizes 1, 5 and 20 mixed:Issues' scripts on this branch. #60 (run for more cycles, because the counted case now waits out its backoff):
#61:
busy relay cycle 9 at 0.07 s: pending, attempts=1(on master,failed, attempts=6at cycle 6).make checkis clean. The full suite gives 897 passed, 3 skipped.Independent of #62/#63. The files overlap only in
docs/agents.md, in different sections.