DRILL-8479: Merge Join Leak When Depleting Incoming Batches Throw Exception

[shfshihuafeng]

DRILL-8479: Merge Join Leak When Depleting Incoming Batches Throw Exception

Description

when fragment failed, it call close() from MergeJoinBatch. but if leftIterator.close() throw exception, we could not call rightIterator.close() to release memory。

Relates to: (#2876)

Documentation

(Please describe user-visible changes similar to what should appear in the Drill documentation.)

Testing

The test method is the same with link, only one parameter needs to be modified,
set planner.enable_hashjoin =false to ensure use mergejoin operator
link
DRILL-8478: HashPartition memory leak when it allocate memory exception with OutOfMemoryException (#2874)

[cgivre]

Do we know what kind(s) of exceptions to expect here? Also, can we throw a better error message? Specifically, can we tell the user more information about the cause of the crash and how to fix it?

[shfshihuafeng]

add exception info ?

try {
      leftIterator.close();
    } catch (QueryCancelledException qce) {
      throw UserException.executionError(qce)
          .message("Failed when depleting incoming batches, probably because query was cancelled " +
              "by executor had some error")
          .build(logger);
    } catch (Exception e) {
      throw UserException.internalError(e)
          .message("Failed when depleting incoming batches")
          .build(logger);
    } finally {
      // todo catch exception info or By default,the exception is thrown directly ?
      rightIterator.close();
    }

[shfshihuafeng]

stack

Caused by: org.apache.drill.exec.ops.QueryCancelledException: null
        at org.apache.drill.exec.work.fragment.FragmentExecutor$ExecutorStateImpl.checkContinue(FragmentExecutor.java:533)
        at org.apache.drill.exec.record.AbstractRecordBatch.checkContinue(AbstractRecordBatch.java:278)
        at org.apache.drill.exec.record.AbstractRecordBatch.next(AbstractRecordBatch.java:105)
        at org.apache.drill.exec.record.AbstractUnaryRecordBatch.innerNext(AbstractUnaryRecordBatch.java:59)
        at org.apache.drill.exec.record.AbstractRecordBatch.next(AbstractRecordBatch.java:165)
        at org.apache.drill.exec.record.RecordIterator.clearInflightBatches(RecordIterator.java:359)
        at org.apache.drill.exec.record.RecordIterator.close(RecordIterator.java:365)
        at org.apache.drill.exec.physical.impl.join.MergeJoinBatch.close(MergeJoinBatch.java:301)

[cgivre]

What happens if the right iterator doesn't close properly?

[shfshihuafeng]

it throw exception from method clearInflightBatches() , but it has cleared the memory by clear(); so it does not affect memory leaks ,see following code

public void close() { clear(); clearInflightBatches(); }

[cgivre]

LGTM +1