[GitHub] jvrao commented on a change in pull request #1372: OrderedScheduler#chooseThread(key) handle null key

[GitBox <gi...@apache.org>]

jvrao commented on a change in pull request #1372: OrderedScheduler#chooseThread(key) handle null key
URL: https://github.com/apache/bookkeeper/pull/1372#discussion_r184918511
 
 

 ##########
 File path: bookkeeper-common/src/main/java/org/apache/bookkeeper/common/util/OrderedExecutor.java
 ##########
 @@ -348,7 +348,11 @@ public ExecutorService chooseThread(Object orderingKey) {
             return threads[0];
         }
 
-        return threads[MathUtils.signSafeMod(orderingKey.hashCode(), threads.length)];
+        if (null == orderingKey) {
+            return threads[rand.nextInt(threads.length)];
+        } else {
+            return threads[MathUtils.signSafeMod(orderingKey.hashCode(), threads.length)];
+        }
 
 Review comment:
   Looks fine, but wondering what is the usecase of this? I worry it might mask bugs as ordered safe executor is an absolute must to protect the correctness (wherever it is needed). So callers need to know what they are doing.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services