squah-confluent commented on code in PR #20055:
URL: https://github.com/apache/kafka/pull/20055#discussion_r2200292220
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/modern/consumer/CurrentAssignmentBuilder.java:
##########
@@ -151,14 +210,18 @@ public ConsumerGroupMember build() {
// If the member provides its owned partitions. We verify if
it still
// owns any of the revoked partitions. If it does, we cannot
progress.
if
(ownsRevokedPartitions(member.partitionsPendingRevocation())) {
- return member;
+ if (hasSubscriptionChanged) {
+ return
updateCurrentAssignment(member.assignedPartitions());
+ } else {
+ return member;
+ }
}
// When the member has revoked all the pending partitions, it
can
// transition to the next epoch (current + 1) and we can
reconcile
// its state towards the latest target assignment.
return computeNextAssignment(
- member.memberEpoch() + 1,
+ Math.min(member.memberEpoch() + 1, targetAssignmentEpoch),
Review Comment:
I don't think these transitions are really that new.
* Previously, we could transition from `STABLE` at epoch N to
`UNREVOKED_PARTITIONS` at epoch N.
* and from `UNREVOKED_PARTITIONS` at epoch N to
`STABLE`/`UNRELEASED_PARTITIONS` at epoch N+1.
With this patch, we can now transition from `UNREVOKED_PARTITIONS` at epoch
N to `STABLE` at epoch N (same epoch!). I can't think of a reason that this
would cause issues. @lianetm can probably confirm whether it's okay.
--
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
