[ 
https://issues.apache.org/jira/browse/DRILL-6410?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16549958#comment-16549958
 ] 

ASF GitHub Bot commented on DRILL-6410:
---------------------------------------

vrozov commented on a change in pull request #1333: DRILL-6410: Memory leak in 
Parquet Reader during cancellation
URL: https://github.com/apache/drill/pull/1333#discussion_r203893104
 
 

 ##########
 File path: 
exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/columnreaders/AsyncPageReader.java
 ##########
 @@ -284,30 +278,9 @@ protected void nextInternal() throws IOException {
 
   }
 
-  private void waitForExecutionResult() throws InterruptedException, 
ExecutionException {
-    // Get the execution result but don't remove the Future object from the 
"asyncPageRead" queue yet;
-    // this will ensure that cleanup will happen properly in case of an 
exception being thrown
-    asyncPageRead.peek().get(); // get the result of execution
-    // Alright now remove the Future object
-    asyncPageRead.poll();
-  }
-
   @Override public void clear() {
     //Cancelling all existing AsyncPageReaderTasks
-    while (asyncPageRead != null && !asyncPageRead.isEmpty()) {
-      try {
-        Future<Void> f = asyncPageRead.poll();
-        if(!f.isDone() && !f.isCancelled()){
-          f.cancel(true);
-        } else {
-          f.get(1, TimeUnit.MILLISECONDS);
-        }
-      } catch (RuntimeException e) {
-        // Do Nothing
-      } catch (Exception e) {
-        // Do nothing.
-      }
-    }
+    executableTasksLatch.await(() -> true);
 
 Review comment:
   I know and saw that post before the fix for the same problem was implemented 
only for `PartitionerDecorator`. I decided to go with a similar approach 
outlined in the post, but that does not use `FutureTask`to support additional 
functionality that I outlined in my response to @ilooner. This implementation 
supports asynchronous cancellation (call to `cancel()` does not block waiting 
for a task to complete allowing a faster cancellation) that the solution that 
extends `FutureTask` does not provide.

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


> Memory leak in Parquet Reader during cancellation
> -------------------------------------------------
>
>                 Key: DRILL-6410
>                 URL: https://issues.apache.org/jira/browse/DRILL-6410
>             Project: Apache Drill
>          Issue Type: Bug
>          Components: Storage - Parquet
>            Reporter: salim achouche
>            Assignee: Vlad Rozov
>            Priority: Major
>             Fix For: 1.14.0
>
>
> Occasionally, a memory leak is observed within the flat Parquet reader when 
> query cancellation is invoked.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to