fix: commit only partitions still assigned to the consumer - #86
Merged
Merged
Conversation
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
During deploys (rolling restarts → rebalances) services log, at ERROR with a traceback:
Tasks for a revoked partition can still reach the committer after the rebalance listener's flush:
flush_timeout_sec, so its task stays pending and is committed after revocation;max_uncommitted_tasks) and dispatches it after the flush;AIOKafkaConsumer.commitvalidates every partition in the offsets dict and raisesIllegalStateErroron the first unassigned one before sending anything. Because_call_committersends one commit per consumer, a single revoked partition discarded the offsets of every partition the consumer still owned. Those only advanced again once newer messages on the same partitions were committed, which on quiet partitions means extra redelivery on the next restart or rebalance. The ERROR-level log also turned a routine rebalance into an error-reporter event on every deploy.Fix
_call_committerfilters the offsets byconsumer.assignment()right beforecommit. The filter and aiokafka's own validation run in the same synchronous step before the firstawait, so the assignment cannot change between them. Revoked partitions are logged once at WARNING; if none remain,commitis not called.CommitFailedError/IllegalStateErrorare still caught for the races the filter cannot see (e.g. a generation change on the broker), now at WARNING without a traceback._call_committerreturnsFalsewhenever any partition was skipped, matching the previous "batch not fully committed" meaning.Skipped offsets are redelivered to the partition's new owner, so at-least-once is unchanged. If the same partition is reassigned back to this consumer, committing offsets whose work already finished is still correct.
Not changed: on a transient
KafkaErrorthe whole batch is re-queued, including tasks for revoked partitions; the next round skips those with a warning.Tests
CommitFailedErrorandIllegalStateErrorlogged at WARNING. All failed before the fix.MockAIOKafkaConsumergainsassignment(), defaulting to every partition so existing tests keep their meaning.just test): 199 passed. ruff and ty clean.