dajac commented on code in PR #21652:
URL: https://github.com/apache/kafka/pull/21652#discussion_r2896575845
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/modern/ModernGroup.java:
##########
@@ -45,6 +45,18 @@
*/
public abstract class ModernGroup<T extends ModernGroupMember> implements
Group {
+ /**
+ * The target assignment metadata.
+ *
+ * @param assignmentEpoch The target assignment epoch. An assignment
epoch smaller than the
+ * group epoch means that a new assignment is
required. The
+ * assignment epoch is updated when a new
assignment is installed.
+ * @param assignmentTimestamp The time at which the target assignment
calculation finished.
+ */
+ protected static record TargetAssignmentMetadata(int assignmentEpoch, long
assignmentTimestamp) {
+ private static final TargetAssignmentMetadata NONE = new
TargetAssignmentMetadata(0, 0L);
+ }
Review Comment:
Should we define it once and reuse it for all the group types? It does not
seem necessary to duplicate it in StreamsGroup.
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/modern/ModernGroup.java:
##########
@@ -45,6 +45,18 @@
*/
public abstract class ModernGroup<T extends ModernGroupMember> implements
Group {
+ /**
+ * The target assignment metadata.
+ *
+ * @param assignmentEpoch The target assignment epoch. An assignment
epoch smaller than the
+ * group epoch means that a new assignment is
required. The
+ * assignment epoch is updated when a new
assignment is installed.
+ * @param assignmentTimestamp The time at which the target assignment
calculation finished.
+ */
+ protected static record TargetAssignmentMetadata(int assignmentEpoch, long
assignmentTimestamp) {
+ private static final TargetAssignmentMetadata NONE = new
TargetAssignmentMetadata(0, 0L);
Review Comment:
Should we use -1 and 0?
##########
group-coordinator/src/test/java/org/apache/kafka/coordinator/group/GroupMetadataManagerTest.java:
##########
@@ -1170,7 +1170,7 @@ public void
testStreamsGroupMemberCanRejoinWithEpochZero() {
TaskAssignmentTestUtil.mkTasksTuple(TaskRole.ACTIVE,
TaskAssignmentTestUtil.mkTasks(subtopology1, 0, 1, 2)
)));
-
context.replay(StreamsCoordinatorRecordHelpers.newStreamsGroupTargetAssignmentEpochRecord(groupId,
100));
+
context.replay(StreamsCoordinatorRecordHelpers.newStreamsGroupTargetAssignmentEpochRecord(groupId,
100, 12345L));
Review Comment:
Why do you use `12345L` here and then `context.time.milliseconds()` in the
following ones?
--
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]
