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
