sohami commented on a change in pull request #1334: DRILL-6385: Support JPPD feature URL: https://github.com/apache/drill/pull/1334#discussion_r208680330
########## File path: exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java ########## @@ -408,6 +424,9 @@ private void runPhysicalPlan(final PhysicalPlan plan) throws ExecutionSetupExcep fragmentsRunner.setFragmentsInfo(work.getFragments(), work.getRootFragment(), work.getRootOperator()); startQueryProcessing(); + if (enableRuntimeFilter) { + runtimeFilterManager.waitForComplete(); Review comment: `SendingAccountor` on which this Foreman thread waits for completion is local to the `RunTimeFilterManager`. The job of `SendingAccountor` is to wait the caller thread iff there is any pending message to be sent unless the send is completed successfully or failed. The buffer release is handled in the failure path of the mesaage not in sending accountor itself. The only time this `SendingAccountor` is incremented is when there is aggregated filter to be broadcasted. By the time this `Foreman` thread is exited there is no guarantee that remote minor fragments may have completed sending all their respective `Bloomfilters` and that `aggregated filter` is also broadcasted, hence it will be no-op in those cases. And more over if there is any failure case as you mentioned above there are chances of memory leak. To avoid that it will make more sense to put this `runtimeFilterManager.waitForComplete` inside `ForemanResult::close` method right before `closeFuture.removeListener(closeListener)` ---------------------------------------------------------------- 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: us...@infra.apache.org With regards, Apache Git Services