[GitHub] [druid] clintropolis commented on a diff in pull request #14130: disable broker parallel merge for Java versions greater than 8 and less than 20 due to apparent bug

["clintropolis (via GitHub)" <gi...@apache.org>]

clintropolis commented on code in PR #14130:
URL: https://github.com/apache/druid/pull/14130#discussion_r1174094976


##########
processing/src/main/java/org/apache/druid/java/util/common/guava/ParallelMergeCombiningSequence.java:
##########
@@ -796,6 +796,7 @@ public boolean block() throws InterruptedException
         if (hasTimeout) {
           final long thisTimeoutNanos = timeoutAtNanos - System.nanoTime();
           if (thisTimeoutNanos < 0) {
+            item = null;

Review Comment:
   these changes were just me be cautious to help ensure that `isReleasable` always can return true if an exception is thrown. Im not sure if these are actually problematic or fix any problems, but based on reading the fork join pool code and looking at how it does the managed blocking stuff there were a handful of places where it is checked on a loop which made me a bit nervous, so I wanted to try to minimize the chances of anything getting stuck.



-- 
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: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org