squah-confluent commented on code in PR #20907:
URL: https://github.com/apache/kafka/pull/20907#discussion_r2547611134
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/modern/consumer/ConsumerGroup.java:
##########
@@ -1121,6 +1128,7 @@ public ConsumerGroupDescribeResponseData.DescribedGroup
asDescribedGroup(
/**
* Create a new consumer group according to the given classic group.
*
+ * @param logContext The LogContext.
Review Comment:
```suggestion
* @param logContext The log context.
```
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/modern/consumer/ConsumerGroup.java:
##########
@@ -1089,7 +1096,7 @@ void addPartitionEpochs(
for (Integer partitionId : assignedPartitions) {
Integer prevValue = partitionsOrNull.put(partitionId,
epoch);
Review Comment:
I think compaction only allows records to be dropped and not reordered (they
need to keep their offsets!). So we should only replace a non-null epoch with a
larger one here and it makes sense to add a check to throw an
IllegalStateException otherwise.
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/streams/StreamsGroup.java:
##########
@@ -918,7 +918,7 @@ void removeTaskProcessIds(
*
* @param assignment The assignment.
* @param expectedProcessId The expected process ID.
- * @throws IllegalStateException if the process ID does not match the
expected one. package-private for testing.
+ * @throws IllegalStateException if the process ID does not exist.
package-private for testing.
Review Comment:
```suggestion
* package-private for testing.
```
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/modern/consumer/ConsumerGroup.java:
##########
@@ -103,6 +105,8 @@ public String toLowerCaseString() {
}
}
+ private final Logger log;
Review Comment:
```suggestion
/**
* The logger.
*/
private final Logger log;
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
