shibd commented on code in PR #24730:
URL: https://github.com/apache/pulsar/pull/24730#discussion_r2343291872


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/HashRangeExclusiveStickyKeyConsumerSelector.java:
##########
@@ -73,47 +74,41 @@ private synchronized Optional<ImpactedConsumersResult> 
internalAddConsumer(Consu
                     + conflictingConsumer);
         }
         for (IntRange intRange : 
consumer.getKeySharedMeta().getHashRangesList()) {
-            rangeMap.put(intRange.getStart(), consumer);
-            rangeMap.put(intRange.getEnd(), consumer);
+            rangeMap.put(intRange.getStart(), 
Pair.of(Range.of(intRange.getStart(), intRange.getEnd()), consumer));
         }
         return Optional.empty();
     }
 
     @Override
     public synchronized Optional<ImpactedConsumersResult> 
removeConsumer(Consumer consumer) {
-        rangeMap.entrySet().removeIf(entry -> 
entry.getValue().equals(consumer));
+        rangeMap.entrySet().removeIf(entry -> 
entry.getValue().getRight().equals(consumer));
         return Optional.empty();
     }
 
     @Override
     public synchronized ConsumerHashAssignmentsSnapshot 
getConsumerHashAssignmentsSnapshot() {
         List<HashRangeAssignment> result = new ArrayList<>();
-        Map.Entry<Integer, Consumer> prev = null;
-        for (Map.Entry<Integer, Consumer> entry: rangeMap.entrySet()) {
-            if (prev == null) {
-                prev = entry;
-            } else {
-                if (prev.getValue().equals(entry.getValue())) {
-                    result.add(new HashRangeAssignment(Range.of(prev.getKey(), 
entry.getKey()), entry.getValue()));
-                }
-                prev = null;
-            }
+        for (Map.Entry<Integer, Pair<Range, Consumer>> entry : 
rangeMap.entrySet()) {
+            Range assignedRange = entry.getValue().getLeft();
+            Consumer assignedConsumer = entry.getValue().getRight();
+            result.add(new HashRangeAssignment(assignedRange, 
assignedConsumer));

Review Comment:
   But we still need to implement the `getConsumerHashAssignmentsSnapshot` 
method, right?
   
   If we were to remove it from the interface, we would have to modify all 
three implementing classes. I don't think it's necessary to do that in this PR.
   
   



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