michaeljmarshall commented on code in PR #20026:
URL: https://github.com/apache/pulsar/pull/20026#discussion_r1162017346
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/HashRangeExclusiveStickyKeyConsumerSelector.java:
##########
@@ -52,8 +54,23 @@ public HashRangeExclusiveStickyKeyConsumerSelector(int
rangeSize) {
}
@Override
- public void addConsumer(Consumer consumer) throws
BrokerServiceException.ConsumerAssignException {
- validateKeySharedMeta(consumer);
+ public synchronized CompletableFuture<Void> addConsumer(Consumer consumer)
{
+ return validateKeySharedMeta(consumer).thenRun(() -> {
Review Comment:
Nit: a minor optimization could assign `validateKeySharedMeta` to a local
variable, and then check if that future is already completed. When it is, we
know we had the `synchronized` lock when the validation was done, which would
allow us to skip the secondary call to `findConflictingConsumer`. I am not sure
how expensive that call is, but I assume it has some cost that adds up with
many key shared consumers.
Since it is an optimization, I don't think we should hold up this PR for
that.
--
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]
