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

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

jasonk000 commented on PR #14130:
URL: https://github.com/apache/druid/pull/14130#issuecomment-1547183528

   @clintropolis I took a look at this and I think there is a an assumption that is broken by the testing configuration but that could occur in the real world. It's late here so I'll share my notes and wrap up.
   
   In short I think there is something where the two-layer yielders do not propagate cancellation when the exception occurs, so the one started on the fjpool keeps running.
   
   If I extend the timeout and capture a stack trace,
   ```
   
   "ForkJoinPool-1-worker-5" #28 daemon prio=5 os_prio=0 cpu=16.62ms elapsed=8.44s tid=0x00007fe044003800 nid=0x7700 waiting on condition  [0x00007fe08ebe0000]
      java.lang.Thread.State: TIMED_WAITING (sleeping)
   	at java.lang.Thread.sleep(java.base@11.0.18/Native Method)
   	at org.apache.druid.java.util.common.guava.ParallelMergeCombiningSequenceTest$2$1.next(ParallelMergeCombiningSequenceTest.java:903)
   	at org.apache.druid.java.util.common.guava.ParallelMergeCombiningSequenceTest$2$1.next(ParallelMergeCombiningSequenceTest.java:876)
   	at org.apache.druid.java.util.common.guava.BaseSequence.toYielder(BaseSequence.java:82)
   	at org.apache.druid.java.util.common.guava.ParallelMergeCombiningSequence$ResultBatch.fromSequence(ParallelMergeCombiningSequence.java:892)
   	at org.apache.druid.java.util.common.guava.ParallelMergeCombiningSequence$SequenceBatcher.block(ParallelMergeCombiningSequence.java:943)
   	at java.util.concurrent.ForkJoinPool.managedBlock(java.base@11.0.18/ForkJoinPool.java:3118)
   	at org.apache.druid.java.util.common.guava.ParallelMergeCombiningSequence$SequenceBatcher.getBatchYielder(ParallelMergeCombiningSequence.java:931)
   	at org.apache.druid.java.util.common.guava.ParallelMergeCombiningSequence$YielderBatchedResultsCursor.initialize(ParallelMergeCombiningSequence.java:1039)
   	at org.apache.druid.java.util.common.guava.ParallelMergeCombiningSequence$PrepareMergeCombineInputsAction.compute(ParallelMergeCombiningSequence.java:742)
   	at java.util.concurrent.RecursiveAction.exec(java.base@11.0.18/RecursiveAction.java:189)
   	at java.util.concurrent.ForkJoinTask.doExec(java.base@11.0.18/ForkJoinTask.java:290)
   	at java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(java.base@11.0.18/ForkJoinPool.java:1020)
   	at java.util.concurrent.ForkJoinPool.scan(java.base@11.0.18/ForkJoinPool.java:1656)
   	at java.util.concurrent.ForkJoinPool.runWorker(java.base@11.0.18/ForkJoinPool.java:1594)
   	at java.util.concurrent.ForkJoinWorkerThread.run(java.base@11.0.18/ForkJoinWorkerThread.java:183)
   ```
   
   And adding a whole ton of println / statements [I get the stdout](https://gist.github.com/jasonk000/11907d0b44f484474c720dfb26d79b77), in which we can see:
   - yielder 0 at ParallelMergeCombiningSequence.toYielder
   - yielder 1 at ParallelMergeCombiningSequence$ResultBatch.fromSequence
   - during `initialize()`, `make()` is called, which starts loading values
   - yielder 0 sees the `QueryTimeoutException`
   - yielder 1 keeps generating values
   
   
   I haven't quite gotten to the bottom of it, but there's something here, I think it's vaguely in this area:
   
   In the test:
   - we create a number of sequences, including a blocking sequence: https://github.com/apache/druid/blob/f9861808bc7e1ce4d7358260e024540d13015aad/processing/src/test/java/org/apache/druid/java/util/common/guava/ParallelMergeCombiningSequenceTest.java#L512
   - we expect that the blocking sequence will run for longer than the given query timeout, so the query timeout will throw an exception
   - the flow creates a ParallelMergeCombiningSequence, and a timer is set up
   - when `toYielder` is called on the flow it creates a sub-action and yielder, the parent yielder has a timeout / cancellation
   - since the first round of loading values (up to 2048?) is too long in duration, given 1-500ms blocking, we never get enough values to complete a full first batch to complete a yield
   - so the cancellation on `yielder 1` never gets to a point of being checked, it's stuck in initialize, at no point during the initialize call do we have an opportunity to bail out


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