Skip to content

fix(outbox): back off counted retries and keep each key's events in order behind a failed one - #64

Merged
AlexeyShalaev merged 1 commit into
masterfrom
fix/60-61-outbox-retry-order
Oct 16, 2026
Merged

AlexeyShalaev merged 1 commit into
masterfrom
fix/60-61-outbox-retry-order

Conversation

@AlexeyShalaev

Copy link
Copy Markdown
Member

Closes #60. Closes #61.

One PR for both. Backing off a counted retry (#61) moves the failed row's scheduled_at minutes 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

Fix

Backoff (HandlerExecutionStep). A counted failure with no next_retry_at of its own is now rescheduled at now + calculate_backoff_with_jitter(attempts_made, base, cap, jitter_mode="equal"). That applies to the outbox, the inbox and the dispatching processors. An explicit next_retry_at still wins. retry_backoff_base_seconds (default 10.0) and retry_backoff_cap_seconds (default 3600.0) are new keyword arguments on HandlerExecutionStep, the three create_*_processor factories and OutboxPublisher, and 0 gives 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 before LIMIT … 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:

  • An event scheduled for later that has never been tried does not hold its key back. Otherwise a "remind in an hour" event would block its aggregate for an hour.
  • A failed row (terminal) does not block either. Its successors go out without it, and requeue_failed sends 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_order on (topic, partition_key, created_at) WHERE status = 'pending' (idx_outbox_events_p_pending_key_order on the partitioned base). migrations.md has the DDL, along with a note to use CREATE INDEX CONCURRENTLY on an existing table.

mark_failed keeps scheduled_at when given no next_retry_at, as bulk_mark_failed already did. Before, the single-row path moved the row to now(), so the commit strategy decided when a deferred row came back.

Rejected

  • Holding back a key only inside the batch, which was the issue's first option. It cannot hold once the failed row is rescheduled for later, because the next cycle fetches the row's successors without it.
  • Taking a run of each key per batch: lock candidates by 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 small batch_size the batch fills with successors that are then dropped every cycle. 286 of 600 events went out in 60 s. Fixing that needs a created_at order for the whole candidate scan, which the existing scheduled_at index cannot serve. Head-only has no such failure mode.

Behaviour changes, who notices

  • Counted retries are spaced out by default. Anyone who relied on a failed row being retried on the next cycle will notice. That includes tests that call publish_batch/process_batch twice and expect a second attempt, and alerts or DLQ flows that expect failed within seconds. The row now stays pending with a future scheduled_at. Pass retry_backoff_base_seconds=0 to 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.
  • One event per key per cycle. A batch carries the heads of up to batch_size keys, 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.
  • A failed row holds back its key until it is published or becomes failed. That is the guarantee, and it is documented as rule 20 in agents.md, under "Event order" in troubleshooting, and in the user guide.
  • Migration. Users own their tables, so they need to add the index. Without it the fetch is still correct, but it does a full-table scan on each call: 12 ms per fetch with 300k completed rows, against 1.1 ms with the index (EXPLAIN ANALYZE, Postgres 17).

Tests

tests/integration/postgres/test_outbox_order.py runs against real Postgres, using the relay from the user guide (OutboxPublisher over PostgresOutboxRepository, one transaction per cycle):

  • The two scenarios from One relay publishes an aggregate's events out of order after a single failed publish #60, a transient and then a counted failure on the first of three events: they go out as created, paid, cancelled, and nothing goes out while the head backs off. The same holds with the backoff off.
  • Another key is not held back. Two keys go out one head each per cycle. A delayed event doesn't block. A terminally failed event doesn't block.
  • Two relays: while relay 1 holds a key's head, relay 2 takes only another key's event.
  • The busy relay from Counted failures are retried with no backoff, so max_attempts is spent in seconds #61: after six busy cycles the rejected row has spent one attempt and its next try is at least 5 s away. With retry_backoff_base_seconds=0 it reaches failed in 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.py check the backoff window per attempt, the cap, a timeout, a bare and a timed handler_retry(), 0, and argument validation.

Fails before. With the omni_box files from master swapped 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 reaches FAILED, the per-cycle test publishes [6, 0, 0], the mark_failed schedule moves, and the index is missing. The other three failures are TypeErrors on the new keyword. The backoff unit tests fail with next_retry_at being None.

Stand test. Four relays and two writers, 600 events over 25 keys, writers serialized per key, random ValueError (5%) and TransientError (3%) from the broker, batch sizes 1, 5 and 20 mixed:

master:  sent=600 out_of_order_keys=16 relay_errors=0
branch:  sent=600 out_of_order_keys=0  duplicate_keys=0 relay_errors=0   (also 0 at 10% / 5%)

Issues' scripts on this branch. #60 (run for more cycles, because the counted case now waits out its backoff):

TransientError cycle 4: sent so far ['order.created', 'order.paid', 'order.cancelled'], pending []
ValueError cycle 7: sent so far [], pending ['order.created', 'order.paid', 'order.cancelled']
ValueError cycle 10: sent so far ['order.created', 'order.paid', 'order.cancelled'], pending []

#61: busy relay cycle 9 at 0.07 s: pending, attempts=1 (on master, failed, attempts=6 at cycle 6).

make check is clean. The full suite gives 897 passed, 3 skipped.

Independent of #62/#63. The files overlap only in docs/agents.md, in different sections.

…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
@AlexeyShalaev
AlexeyShalaev merged commit d9ef2f9 into master Oct 16, 2026
6 checks passed
@AlexeyShalaev
AlexeyShalaev deleted the fix/60-61-outbox-retry-order branch October 16, 2026 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant