squah-confluent commented on code in PR #23155:
URL: https://github.com/apache/kafka/pull/23155#discussion_r3778682129
##########
group-coordinator/src/test/java/org/apache/kafka/coordinator/group/assignor/CommonAssignorTests.java:
##########
@@ -130,13 +141,136 @@ public static void testAssignmentReuse(PartitionAssignor
assignor, SubscriptionT
}
/**
- * Tests that an assignor produces the same assignment when the members
are iterated in
- * different orders.
+ * Tests that an assignor maintains stickiness when the iteration order of
members changes.
+ * This does not test that the assignor is deterministic, only that an
assignment is sticky once
+ * computed.
+ *
* @param assignor The assignor.
* @param subscriptionType The subscription type.
* @param rackAware Whether to test with rack awareness.
*/
- public static void testReassignmentStickiness(PartitionAssignor assignor,
SubscriptionType subscriptionType, boolean rackAware) {
+ public static void testIterationOrderStickiness(
+ PartitionAssignor assignor,
+ SubscriptionType subscriptionType,
+ boolean rackAware
+ ) {
+ testStickiness(
+ assignor,
+ subscriptionType,
+ (permutation, memberIds, assignments) -> {
+ // Return the same (memberId, rack, assignment) tuples in
different orders.
+ Map<String, MemberSubscriptionAndAssignmentImpl> members = new
LinkedHashMap<>();
+ for (int index : permutation) {
+ members.put(memberIds.get(index), new
MemberSubscriptionAndAssignmentImpl(
+ // We want there to be multiple valid assignments,
otherwise we aren't
+ // really testing stickiness. Only give a single
member a rack, so that the
+ // other members are interchangeable.
+ rackAware && index == 1 ? Optional.of("rack1") :
Optional.empty(),
+ Optional.empty(),
+ Set.of(TOPIC_1_UUID, TOPIC_2_UUID, TOPIC_3_UUID),
+ assignments.get(index)
+ ));
+ }
+ return members;
+ }
+ );
+ }
+
+ /**
+ * Tests that an assignor maintains stickiness when static members are
replaced,
+ * ie. members change their member id while keeping the same instance id.
+ *
+ * @param assignor The assignor.
+ * @param subscriptionType The subscription type.
+ * @param rackAware Whether to test with rack awareness.
+ */
+ public static void testStaticMemberReplacementStickiness(
+ PartitionAssignor assignor,
+ SubscriptionType subscriptionType,
+ boolean rackAware
+ ) {
+ List<String> instanceIds = List.of("instance1", "instance2",
"instance3");
+
+ testStickiness(
+ assignor,
+ subscriptionType,
+ (permutation, memberIds, assignments) -> {
+ // Return the same (instanceId, rack, assignment) tuples with
different member ids.
+ // When appending members, the member ids follow the
permutation order while
+ // everything else is in a fixed order.
+ Map<String, MemberSubscriptionAndAssignmentImpl> members = new
LinkedHashMap<>();
+ for (int i = 0; i < permutation.size(); i++) {
+ int index = permutation.get(i);
+ members.put(memberIds.get(index), new
MemberSubscriptionAndAssignmentImpl(
+ // We want there to be multiple valid assignments,
otherwise we aren't
+ // really testing stickiness. Only give a single
instance id a rack, so that
+ // the other members are interchangeable.
+ rackAware && i == 1 ? Optional.of("rack1") :
Optional.empty(),
+ Optional.of(instanceIds.get(i)),
+ Set.of(TOPIC_1_UUID, TOPIC_2_UUID, TOPIC_3_UUID),
+ assignments.get(i)
+ ));
+ }
+ return members;
+ }
+ );
+ }
+
+ /**
+ * Tests that an assignor maintains stickiness when member ids are swapped
around.
+ * An assignor that passes this test will likely maintain stickiness when
a single member is
+ * removed and replaced with a new member and generally during scale up
and scale down, as
+ * members are added and removed.
+ *
+ * @param assignor The assignor.
+ * @param subscriptionType The subscription type.
+ * @param rackAware Whether to test with rack awareness.
+ */
+ public static void testMemberReplacementStickiness(
+ PartitionAssignor assignor,
+ SubscriptionType subscriptionType,
+ boolean rackAware
+ ) {
Review Comment:
I thought about adding a re-usable stickiness test for new members but it's
not easy to make a generic test. The minimal set of partitions that can be
taken from existing members depends on the assignor. For the range assignor,
the re-assigned partitions must have the same indices for example.
--
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]
