fix: delete taskiq stream entries on ack to bound Redis memory - #329
Open
Felipe Alvarado (falvaradorodriguez) wants to merge 1 commit into
Open
fix: delete taskiq stream entries on ack to bound Redis memory#329Felipe Alvarado (falvaradorodriguez) wants to merge 1 commit into
Felipe Alvarado (falvaradorodriguez) wants to merge 1 commit into
Conversation
Felipe Alvarado (falvaradorodriguez)
requested a review
from a team
as a code owner
August 18, 2026 15:14
Felipe Alvarado (falvaradorodriguez)
force-pushed
the
fix/trim-taskiq-stream-on-ack
branch
from
August 18, 2026 15:42
7aba8f9 to
2d79e83
Compare
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.
Redis memory grew without bound.
RedisStreamBrokeracknowledges messages withXACK, which only clears the consumer group's pending entries list — the entry itself stays in the append-only stream forever. Every task ever enqueued was still there.Measured on staging:
taskiqstream, ~262 bytes per entrypending: 0,lag: 0— workers fully caught up, so 100% of those entries were already acknowledged, pure garbagedb0:keys=3213,expires=3212— the stream was the only key in the whole instance without a TTLrecorded-first-entry-id→last-generated-id), which means ~201,000 entries/day ≈ 53 MB/day, with no ceilingProduction has not been measured yet, but it is likely worse rather than better: both cron tasks enqueue one task per row of the
contracttable, and production holds far more contracts than staging.The leak dates back to the dramatiq → taskiq migration (#307, June 22nd). The dramatiq broker deleted the message on ack (
HDELin itsdispatch.lua), so memory was proportional to messages in flight; streams do not do that, and retention silently became our responsibility.How was it fixed? 🎯
DeleteOnAckRedisStreamBrokeroverrides the ack callback to delete the entry alongside acknowledging it, keeping the stream size proportional to the messages in flight. Both commands run in a single transaction so an acknowledged entry is never left behind.Why not
maxlen, the parameter the library offers for this (added in taskiq-redis#81, and requested for the cluster broker in #92): it trims by length without checking whether an entry was processed. Sizing it means choosing how many enqueued tasks you are willing to lose in the worst case, and losing tasks is not acceptable here.Why not
XACKDEL, which does this atomically in one command: it requires Redis 8.2 or newer and production runs 7.4.10. Same trade-offmalvex/sheppymade, whose PR #20 moved fromXACKDELback toXACK+XDELpurely to lower its minimum Redis version from 8.2 to 6.2.References
This is a well-known problem, not something specific to us:
maxlen=1_500by handXACKDELin 8.2, and their own team treats it as the standard path: redis-benchmarks-specification#467 adds coverage for "stream delete strategies (XDELEX / XACKDEL, DELREF and ACKED)"Compatibility
XACKandXDELare available since Redis 5.0, so this works on both production (7.4.10) and staging (8.10). Verified by running the full test suite against a 7.4.10 server. - Works on cluster: both are single-key commands on the same stream key, so the transact#### Verification
- Full test suite green against Redis 7.4.10: 87 passed
AssertionError: 1 != 0), confirming it catches the regression - Ack on the success path:xlen1 → 0,pending0,lag0AcknowledgeType.WHEN_SAVEDis the default inreceiver.py:77andcli/worker/args.py:53, and we do not pass--ack-typeindocker/web/taskiq/worker/run.sh), and thatSmartRetryMiddlewarere-enqueues **befores insiderun_task,receiver.py:349-359). A task that always fails produces its 5attempts and leaves the stream empty: no task is lost.idle_timeout, re-executed and only then deleted. At-least-once delivery is preserved.