KAFKA-14841 Handle callbacks to ConsumerRebalanceListener in MockConsumer #13455
Conversation
|
@philipnee i'd be super grateful for a review if you get the chance |
| // prepare for rebalance callback | ||
| Set<TopicPartition> oldAssignmentSet = this.subscriptions.assignedPartitions(); | ||
| Set<TopicPartition> newAssignmentSet = new HashSet<>(newAssignment); | ||
| List<TopicPartition> added = new ArrayList<>(newAssignment.size()); |
There was a problem hiding this comment.
I'm pretty sure you can do set.removeAll(stuff) - saves some code there.
There was a problem hiding this comment.
i changed it to streams and reduced it to one line per list
| public synchronized void rebalance(Collection<TopicPartition> newAssignment) { | ||
| // TODO: Rebalance callbacks | ||
| // prepare for rebalance callback |
There was a problem hiding this comment.
This is more like, computing the partitions to be added and removed, right?
|
Hey I left some comments, but it looks good afterall. @showuon @guozhangwang - Would you guys be interested in reviewing this PR? |
|
@kirktrue - do you want to take a look? |
…e logic for added/removed lists more terse
Thank you so much, addressed! |
|
There were a lot of test failures. I just triggered it again. It doesn't seem implausible that changing the MockConsumer could cause other tests to fail, but I didn't look at whether the failing tests actually use it. |
|
Thanks @vvcephei - Think this is the failure. ZK related so I think the PR is good to go.
|
Thanks @vvcephei @philipnee! The ci failures for the various builds have all seemed independent of this change |
|
in past few builds, at least one of the builds passed (in this one it was jdk 17 and scala 2.13). https://ci-builds.apache.org/blue/organizations/jenkins/Kafka%2Fkafka-pr/detail/PR-13455/9/pipeline/11/ Test failures look unrelated, and I'm unable to replicate locally (for instance, on jdk 8, scala 2.12 on latest build, failures were: In the previous build (only change was I pulled the latest trunk changes in, jdk 8 and scala 2.12 passed, and other configs failed https://ci-builds.apache.org/blue/organizations/jenkins/Kafka%2Fkafka-pr/detail/PR-13455/8/pipeline It's likely safe to say that these are intermittent failures not related to the changes proposed in this PR. @vvcephei @philipnee what are the next steps? |
More detailed description of your change,
if necessary. The PR title and PR message become
the squashed commit message, so use a separate
comment to ping reviewers.
Ensure the
ConsumerRebalanceListenerpassed to theMockConsumeris notified when a rebalance happens. Addresses a TODO comment left in the code. Computes which partitions were revoked and which partitions were assigned, and makes the appropriate callbacks.Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.
Unit test covers the cases where a consumer is assigned partitions, revoked all partitions, and partitions are switched.
Committer Checklist (excluded from commit message)