Skip to content

Commit 3d0df4c

Browse files
committed
fix: recover gracefully from connection loss in ack and kick
1 parent 176edd3 commit 3d0df4c

4 files changed

Lines changed: 309 additions & 24 deletions

File tree

‎README.md‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,30 @@ broker = AioPikaBroker(
144144

145145
Once a message has been redelivered more than `delivery_limit` times, RabbitMQ dead-letters it to the broker's dead-letter queue instead of redelivering it again — no application code involved. `delivery_limit` is only supported by quorum queues. See the [RabbitMQ docs](https://www.rabbitmq.com/docs/quorum-queues#poison-message-handling) for details.
146146

147+
## Connection loss and message redelivery
148+
149+
RabbitMQ deliveries are acknowledged on the specific channel they were delivered on. If the underlying connection drops (a network blip, a broker restart, etc.), any message that was already delivered but not yet acked cannot be acked anymore, even after the connection reconnects — the delivery tag isn't valid on a new channel. `AioPikaBroker` handles this by logging a warning and letting the message go instead of crashing the worker; RabbitMQ automatically requeues the message once the old channel closes.
150+
151+
The practical consequence is that a task can run **more than once** whenever a connection is lost while the task is in flight — regardless of `ack_time`. This is a property of AMQP itself, not something a broker can paper over, so:
152+
153+
- Write tasks to be idempotent whenever you can (safe to execute twice with the same effect).
154+
- If you can't, prefer `ack_time="when_received"` (the taskiq default) to shrink the window between delivery and ack, at the cost of losing the message outright if the worker crashes mid-task instead of duplicating it. `ack_time="when_executed"`/`"when_saved"` hold the message unacked for longer (through task execution / result saving), which widens the duplicate-delivery window but guarantees the message isn't lost if the worker itself crashes.
155+
156+
Publishing (`kick`) is affected too, but differently: right after a connection recovers, there's a short window (order of a second) where the write channel can still be mid-recovery. `AioPikaBroker` retries `kick` automatically in that case — configurable via the `retries` constructor argument:
157+
158+
```python
159+
from taskiq_aio_pika import AioPikaBroker
160+
161+
broker = AioPikaBroker(
162+
retries={
163+
"kick": {
164+
"max_attempts": 4, # set to 0 to disable retrying
165+
"backoff": 0.2, # doubled after each retry
166+
},
167+
},
168+
)
169+
```
170+
147171
## Custom Queue and Exchange arguments
148172

149173
You can pass custom arguments to the underlying RabbitMQ queues and exchange declaration by using the `Queue`/`Exchange` classes from `taskiq_aio_pika`. If you used `faststream` before you are probably familiar with this concept.

‎taskiq_aio_pika/broker.py‎

Lines changed: 74 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,18 @@
11
import asyncio
22
from collections.abc import AsyncGenerator, Callable
33
from datetime import timedelta
4+
from functools import partial
45
from logging import getLogger
56
from typing import Any, TypeVar
67

78
import aiormq
89
from aio_pika import DeliveryMode, ExchangeType, Message, connect_robust
9-
from aio_pika.abc import AbstractChannel, AbstractQueue, AbstractRobustConnection
10+
from aio_pika.abc import (
11+
AbstractChannel,
12+
AbstractIncomingMessage,
13+
AbstractQueue,
14+
AbstractRobustConnection,
15+
)
1016
from pamqp.common import FieldTable
1117
from taskiq import AckableMessage, AsyncBroker, AsyncResultBackend, BrokerMessage
1218
from typing_extensions import Self
@@ -19,6 +25,7 @@
1925
)
2026
from taskiq_aio_pika.exchange import Exchange
2127
from taskiq_aio_pika.queue import Queue, QueueType
28+
from taskiq_aio_pika.retries import DEFAULT_KICK_RETRIES, RetriesConfig
2229
from taskiq_aio_pika.utils import merge_async_iterables
2330

2431
_T = TypeVar("_T")
@@ -64,6 +71,7 @@ def __init__(
6471
delayed_message_exchange: Exchange | None = None,
6572
label_for_routing: str = "queue_name",
6673
label_for_priority: str = "priority",
74+
retries: RetriesConfig | None = None,
6775
**connection_kwargs: Any,
6876
) -> None:
6977
"""
@@ -82,6 +90,7 @@ def __init__(
8290
:param delayed_message_exchange: parameters of exchange that used to send messages with delay.
8391
:param label_for_routing: label name to use for routing key selection.
8492
:param label_for_priority: label name to use for message priority.
93+
:param retries: per-operation retry policies.
8594
:param connection_kwargs: additional keyword arguments, for connect_robust method of aio-pika.
8695
"""
8796
super().__init__(result_backend, task_id_generator)
@@ -96,6 +105,9 @@ def __init__(
96105

97106
self._label_for_routing = label_for_routing
98107
self._label_for_priority = label_for_priority
108+
self._retries: RetriesConfig = {
109+
"kick": {**DEFAULT_KICK_RETRIES, **(retries or {}).get("kick", {})},
110+
}
99111

100112
self._delay_queue = delay_queue
101113

@@ -362,6 +374,7 @@ async def kick(self, message: BrokerMessage) -> None:
362374
"""
363375
if self.write_channel is None:
364376
raise NoStartupError("Please run startup before kicking.")
377+
write_channel = self.write_channel
365378
priority = parse_val(int, message.labels.get(self._label_for_priority))
366379
rmq_message = Message(
367380
body=message.message,
@@ -395,28 +408,65 @@ async def kick(self, message: BrokerMessage) -> None:
395408
f"Check routing keys and queue names in broker queues.",
396409
)
397410

398-
if x_delay is None:
399-
exchange = await self.write_channel.get_exchange(
400-
self._exchange.name,
401-
ensure=False,
402-
)
403-
await exchange.publish(rmq_message, routing_key=routing_key_name)
404-
elif self._delayed_message_exchange_plugin:
405-
rmq_message.headers["x-delay"] = int(x_delay * 1000)
406-
exchange = await self.write_channel.get_exchange(
407-
self._delayed_message_exchange.name,
408-
)
409-
await exchange.publish(rmq_message, routing_key=routing_key_name)
410-
elif self._delay_queue:
411-
rmq_message.expiration = timedelta(seconds=x_delay)
412-
await self.write_channel.default_exchange.publish(
413-
rmq_message,
414-
routing_key=self._delay_queue.routing_key or self._delay_queue.name,
415-
)
416-
else:
417-
raise IncorrectRoutingKeyError(
418-
"Delay requested but no delay queue or delayed-message-exchange "
419-
"is configured in the broker.",
411+
async def _publish() -> None:
412+
if x_delay is None:
413+
exchange = await write_channel.get_exchange(
414+
self._exchange.name,
415+
ensure=False,
416+
)
417+
await exchange.publish(rmq_message, routing_key=routing_key_name)
418+
elif self._delayed_message_exchange_plugin:
419+
rmq_message.headers["x-delay"] = int(x_delay * 1000)
420+
exchange = await write_channel.get_exchange(
421+
self._delayed_message_exchange.name,
422+
)
423+
await exchange.publish(rmq_message, routing_key=routing_key_name)
424+
elif self._delay_queue:
425+
rmq_message.expiration = timedelta(seconds=x_delay)
426+
await write_channel.default_exchange.publish(
427+
rmq_message,
428+
routing_key=self._delay_queue.routing_key or self._delay_queue.name,
429+
)
430+
else:
431+
raise IncorrectRoutingKeyError(
432+
"Delay requested but no delay queue or delayed-message-exchange is configured in the broker.",
433+
)
434+
435+
await self._publish_with_retry(_publish)
436+
437+
async def _publish_with_retry(self, _publish: Callable[[], Any]) -> None:
438+
"""Run a publish callback, retrying on a transient channel-recovery race."""
439+
max_attempts = self._retries["kick"]["max_attempts"]
440+
delay = self._retries["kick"]["backoff"]
441+
for attempt in range(max_attempts + 1):
442+
try:
443+
await _publish()
444+
return
445+
except aiormq.exceptions.ChannelInvalidStateError:
446+
if attempt == max_attempts:
447+
raise
448+
logger.warning(
449+
"Publish failed because the write channel was invalidated by a connection recovery race; "
450+
"retrying in %.2fs (attempt %d/%d).",
451+
delay,
452+
attempt + 1,
453+
max_attempts,
454+
)
455+
await asyncio.sleep(delay)
456+
delay *= 2
457+
458+
@staticmethod
459+
async def _safe_ack(message: AbstractIncomingMessage, queue_name: str) -> None:
460+
"""Ack a message, tolerating a channel invalidated by a connection loss."""
461+
try:
462+
await message.ack()
463+
except aiormq.exceptions.ChannelInvalidStateError:
464+
logger.warning(
465+
"Could not ack message (delivery_tag=%s, redelivered=%s) on queue '%s' - the channel was invalidated by"
466+
" a connection loss.",
467+
message.delivery_tag,
468+
message.redelivered,
469+
queue_name,
420470
)
421471

422472
async def listen(self) -> AsyncGenerator[AckableMessage, None]:
@@ -442,7 +492,7 @@ async def body(
442492
async for message in iterator:
443493
yield AckableMessage(
444494
data=message.body,
445-
ack=message.ack,
495+
ack=partial(self._safe_ack, message, queue.name),
446496
)
447497
except (RuntimeError, asyncio.CancelledError):
448498
# Suppress errors during iterator cleanup if channel is being closed

‎taskiq_aio_pika/retries.py‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
from typing_extensions import TypedDict
2+
3+
4+
class KickRetries(TypedDict, total=False):
5+
"""Retry policy for `AioPikaBroker.kick`."""
6+
7+
max_attempts: int
8+
backoff: float
9+
10+
11+
class RetriesConfig(TypedDict, total=False):
12+
"""Per-operation retry policies for `AioPikaBroker`."""
13+
14+
kick: KickRetries
15+
16+
17+
DEFAULT_KICK_RETRIES: KickRetries = {"max_attempts": 4, "backoff": 0.2}

0 commit comments

Comments
 (0)