fix(consumer): add recovery from no leader partitions#3101
Conversation
When some topic partitions have no leader due to Kafka broker failures, the Sarama consumer group should be able to continue consuming partitions that do have leaders and resume consuming the partitions that previously had no leader once they return to normal. Signed-off-by: liutao366 <[email protected]>
puellanivis
left a comment
puellanivis
left a comment
There was a problem hiding this comment.
👍 nothing significant I can see that should be improved. While there is a suggestion here, I don’t think it’s necessarily any better than the existing approach.
|
Add test case:
|
dnwe
changed the title
dnwe
force-pushed
the
main
branch
3 times, most recently
from
990524c to
5dc4e24
Compare
Signed-off-by: Dominic Evans <[email protected]>
|
@liutao365 thanks for proposing this change, the approach looks good to me – I added a commit to fixup the client locking (the FV was failing for the race condition) and another commit to slightly refactor the consume partition code into a single named func to cover both paths. Can you take a look and confirm you're happy? Also @puellanivis if you wouldn't mind re-reviewing that would be great too. We should probably add a unittest to cover this scenario, but I'm happy for us to do that under a follow-up PR as it is probably worth landing this fix sooner rather than later |
Hi, @dnwe , I've red the changes and its better than my original commit, I'm happy with this improvements. |
Just to avoid some duplication here Signed-off-by: Dominic Evans <[email protected]>
4 participants
When some topic partitions have no leader due to Kafka broker failures, the Sarama consumer group should be able to continue consuming partitions that have leaders and resume consuming the partitions that previously had no leader once they return to normal. This pull request addresses this issue.