ShivsundarR commented on code in PR #20855:
URL: https://github.com/apache/kafka/pull/20855#discussion_r2537128682


##########
clients/src/test/java/org/apache/kafka/clients/consumer/internals/ShareConsumeRequestManagerTest.java:
##########
@@ -2509,6 +2394,90 @@ void testWhenLeadershipChangedAfterDisconnected() {
         assertEquals(1, fetchedRecords.size());
     }
 
+    @Test
+    public void testFetchOneNodeAtATimeForRecordLimitMode() {
+        // We will simulate two nodes, each with one partition. The first node 
will have more records
+        buildRequestManager(ShareAcquireMode.RECORD_LIMIT);
+
+        subscriptions.subscribeToShareGroup(Collections.singleton(topicName));
+        Set<TopicPartition> partitions = new HashSet<>();
+        partitions.add(tp0);
+        partitions.add(tp1);
+        subscriptions.assignFromSubscribed(partitions);

Review Comment:
   Right, I looked into this and there are 2 ways to solve this I thought.
   1. Change the test to be order-agnostic, this makes the test a bit bigger, 
but we can make it work.
   2. Use `LinkedHashMap` in `SubscriptionState` to maintain insertion order, 
so that we can be deterministic in the test and in any future tests that other 
classes may use.
   
   I have updated the PR with option 2 as this was just a temporary map used in 
`assignFromSubscibed` before updating the actual **assignment**. Does this work?



-- 
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]

Reply via email to