showuon commented on code in PR #12748:
URL: https://github.com/apache/kafka/pull/12748#discussion_r1015224638
##########
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractStickyAssignor.java:
##########
@@ -117,30 +117,41 @@ private boolean allSubscriptionsEqual(Set<String>
allTopics,
isAllSubscriptionsEqual = false;
}
- MemberData memberData = memberData(subscription);
+ Optional<Integer> generation;
+ List<TopicPartition> ownedPartitionsInMetadata;
+ if (!subscription.ownedPartitions().isEmpty() &&
subscription.generationId() != DEFAULT_GENERATION) {
+ // In ConsumerProtocolSubscription v2 or higher, we don't need
to deserialize the byte buffer
+ // and take from fields directly
+ ownedPartitionsInMetadata = subscription.ownedPartitions();
+ generation = Optional.of(subscription.generationId());
+ } else {
+ MemberData memberData = memberData(subscription);
+ ownedPartitionsInMetadata = memberData.partitions;
+ generation = memberData.generation;
+ }
Review Comment:
> It seems that we also rely on the generation in
prepopulateCurrentAssignments so I suppose that we need a similar logic over
there in order to be consistent. Do we?
Good point! Will update it.
> Knowing this, I wonder if we should update memberData to get the correct
generation from the subscription. That would ensure that all usages of
memberData get the correct view. What do you think?
Do you mean in L130, we should use the generationId from subscription? I
don't think that's correct because the generationId is bound to the owned
partition in memberData.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
