You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@drill.apache.org by Jacques Nadeau <ja...@gmail.com> on 2015/05/02 19:24:05 UTC

Review Request 33782: DRILL-2826

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33782/
-----------------------------------------------------------

Review request for drill, abdelhakim deneche and Steven Phillips.


Repository: drill-git


Description
-------

DRILL-2826: Simplify and centralize Operator Cleanup

- Remove cleanup method from RecordBatch interface
- Make OperatorContext creation and closing the management of FragmentContext
- Make OperatorContext an abstract class and the impl only available to FragmentContext
- Make RecordBatch closing the responsibility of the RootExec
- Make all closes be suppresing closes to maximize memory release in failure
- Add new CloseableRecordBatch interface used by RootExec
- Make RootExec AutoCloseable
- Update RecordBatchCreator to return CloseableRecordBatches so that RootExec can maintain list
- Generate list of operators through change in ImplCreator


Diffs
-----

  common/src/main/java/org/apache/drill/common/StackTrace.java 454c3a8 
  common/src/main/java/org/apache/drill/common/exceptions/UserExceptionContext.java 1054c7f 
  contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseScanBatchCreator.java 9256157 
  contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveScanBatchCreator.java a1273c0 
  contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoScanBatchCreator.java c4597b5 
  exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java d22651e 
  exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java e6d5acd 
  exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorContext.java ccafa67 
  exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorContextImpl.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/BaseRootExec.java 628dcd3 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/BatchCreator.java 1cf7da7 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ImplCreator.java 912dfd7 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/MergingReceiverCreator.java daef44c 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/RootExec.java 8fd68b2 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java ca2a048 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java 2069d35 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SingleSenderCreator.java 18ea71d 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNBatch.java 681c3e3 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNSortBatchCreator.java aa8b611 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/WriterRecordBatch.java b419f71 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java c29fbf2 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatchCreator.java 8c60541 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java 87cd4d6 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java ed5b415 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatchCreator.java 0203b81 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/broadcastsender/BroadcastSenderRootExec.java b3a6a8f 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterBatchCreator.java 7f2fe8e 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterRecordBatch.java 064d5c8 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/flatten/FlattenBatchCreator.java 6f02824 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java 79fe177 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatchCreator.java bfe89c0 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java 1a7e60e 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinCreator.java 7d100af 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/NestedLoopJoinBatch.java 4fb1409 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/NestedLoopJoinBatchCreator.java 12588ac 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitBatchCreator.java e71daba 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitRecordBatch.java 7e66893 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java 40cbc89 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java b26c78a 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java 7e3f4b2 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/producer/ProducerConsumerBatch.java c2d6166 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/producer/ProducerConsumerBatchCreator.java c568ed4 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ComplexToJsonBatchCreator.java 0df9491 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectBatchCreator.java cb1d4f1 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 0a097c1 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatchCreator.java 217acf2 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/RemovingRecordBatch.java 1fa759c 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/SVRemoverCreator.java 455a5f9 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/trace/TraceBatchCreator.java 12afa33 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/trace/TraceRecordBatch.java 8a7d659 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/union/UnionAllBatchCreator.java 7f7e110 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/union/UnionAllRecordBatch.java 52b1794 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unorderedreceiver/UnorderedReceiverBatch.java 094865e 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unorderedreceiver/UnorderedReceiverCreator.java d9864f9 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/validate/IteratorValidatorBatchIterator.java 66ec22f 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/validate/IteratorValidatorCreator.java 5d08afb 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/values/ValuesBatchCreator.java d526a84 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFrameBatchCreator.java 285e2cd 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFrameRecordBatch.java bc86390 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java ca93a72 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatchCreator.java eb5d83b 
  exec/java-exec/src/main/java/org/apache/drill/exec/record/AbstractRecordBatch.java c96cb7c 
  exec/java-exec/src/main/java/org/apache/drill/exec/record/AbstractSingleRecordBatch.java 3cfe177 
  exec/java-exec/src/main/java/org/apache/drill/exec/record/CloseableRecordBatch.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatch.java 0a8ece5 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/avro/AvroRecordReader.java 489a989 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/easy/EasyFormatPlugin.java f1271b1 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/easy/EasyReaderBatchCreator.java ac0d2e7 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/easy/EasyWriterBatchCreator.java c91ceba 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/direct/DirectBatchCreator.java 84587a9 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONRecordReader.java 2cccc64 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaBatchCreator.java b38a33f 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/mock/MockScanBatchCreator.java 0bfd038 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetFormatPlugin.java 7298f53 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetRecordWriter.java 3e35721 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetScanBatchCreator.java 52dccd9 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetWriterBatchCreator.java 10dd26d 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet2/DrillParquetReader.java 4d837c1 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemTableBatchCreator.java 92f676a 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java 3368412 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/UnlimitedRawBatchBuffer.java 85262de 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java edbcfde 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java 0783fee 
  exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java b02051b 
  exec/java-exec/src/test/java/org/apache/drill/exec/RunRootExec.java 4258e60 
  exec/java-exec/src/test/java/org/apache/drill/exec/client/DumpCatTest.java f4f4966 
  exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/ExpressionInterpreterTest.java 4942185 
  exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java 1f0951b 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/SimpleRootExec.java 2536bbb 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestCastFunctions.java e5448ac 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestComparisonFunctions.java 0f6fd43 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/filter/TestSimpleFilter.java a112d92 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/join/TestHashJoin.java ef3a330 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/partitionsender/TestPartitionSender.java 46bcc60 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/trace/TestTraceMultiRecordBatch.java d0d4005 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/trace/TestTraceOutputDump.java f6766b1 

Diff: https://reviews.apache.org/r/33782/diff/


Testing
-------

regression/unit


Thanks,

Jacques Nadeau

Re: Review Request 33782: DRILL-2826: Centralize operator cleanup

Posted by abdelhakim deneche <ad...@gmail.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33782/#review82334
-----------------------------------------------------------



exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java
<https://reviews.apache.org/r/33782/#comment133057>

    [DRILL-2878](https://issues.apache.org/jira/browse/DRILL-2878)
    is still prevalent here. If a fagment executor is never run, closeOutResources will never get called and it's allocator won't release it's reserved memory


- abdelhakim deneche


On May 2, 2015, 5:27 p.m., Jacques Nadeau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33782/
> -----------------------------------------------------------
> 
> (Updated May 2, 2015, 5:27 p.m.)
> 
> 
> Review request for drill, abdelhakim deneche and Steven Phillips.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> DRILL-2826: Simplify and centralize Operator Cleanup
> 
> - Remove cleanup method from RecordBatch interface
> - Make OperatorContext creation and closing the management of FragmentContext
> - Make OperatorContext an abstract class and the impl only available to FragmentContext
> - Make RecordBatch closing the responsibility of the RootExec
> - Make all closes be suppresing closes to maximize memory release in failure
> - Add new CloseableRecordBatch interface used by RootExec
> - Make RootExec AutoCloseable
> - Update RecordBatchCreator to return CloseableRecordBatches so that RootExec can maintain list
> - Generate list of operators through change in ImplCreator
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/drill/common/StackTrace.java 454c3a8 
>   common/src/main/java/org/apache/drill/common/exceptions/UserExceptionContext.java 1054c7f 
>   contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseScanBatchCreator.java 9256157 
>   contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveScanBatchCreator.java a1273c0 
>   contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoScanBatchCreator.java c4597b5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java d22651e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java e6d5acd 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorContext.java ccafa67 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorContextImpl.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/BaseRootExec.java 628dcd3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/BatchCreator.java 1cf7da7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ImplCreator.java 912dfd7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/MergingReceiverCreator.java daef44c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/RootExec.java 8fd68b2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java ca2a048 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java 2069d35 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SingleSenderCreator.java 18ea71d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNBatch.java 681c3e3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNSortBatchCreator.java aa8b611 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/WriterRecordBatch.java b419f71 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java c29fbf2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatchCreator.java 8c60541 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java 87cd4d6 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java ed5b415 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatchCreator.java 0203b81 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/broadcastsender/BroadcastSenderRootExec.java b3a6a8f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterBatchCreator.java 7f2fe8e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterRecordBatch.java 064d5c8 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/flatten/FlattenBatchCreator.java 6f02824 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java 79fe177 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatchCreator.java bfe89c0 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java 1a7e60e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinCreator.java 7d100af 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/NestedLoopJoinBatch.java 4fb1409 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/NestedLoopJoinBatchCreator.java 12588ac 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitBatchCreator.java e71daba 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitRecordBatch.java 7e66893 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java 40cbc89 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java b26c78a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java 7e3f4b2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/producer/ProducerConsumerBatch.java c2d6166 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/producer/ProducerConsumerBatchCreator.java c568ed4 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ComplexToJsonBatchCreator.java 0df9491 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectBatchCreator.java cb1d4f1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 0a097c1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatchCreator.java 217acf2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/RemovingRecordBatch.java 1fa759c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/SVRemoverCreator.java 455a5f9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/trace/TraceBatchCreator.java 12afa33 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/trace/TraceRecordBatch.java 8a7d659 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/union/UnionAllBatchCreator.java 7f7e110 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/union/UnionAllRecordBatch.java 52b1794 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unorderedreceiver/UnorderedReceiverBatch.java 094865e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unorderedreceiver/UnorderedReceiverCreator.java d9864f9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/validate/IteratorValidatorBatchIterator.java 66ec22f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/validate/IteratorValidatorCreator.java 5d08afb 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/values/ValuesBatchCreator.java d526a84 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFrameBatchCreator.java 285e2cd 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFrameRecordBatch.java bc86390 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java ca93a72 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatchCreator.java eb5d83b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/AbstractRecordBatch.java c96cb7c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/AbstractSingleRecordBatch.java 3cfe177 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/CloseableRecordBatch.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatch.java 0a8ece5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/avro/AvroRecordReader.java 489a989 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/easy/EasyFormatPlugin.java f1271b1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/easy/EasyReaderBatchCreator.java ac0d2e7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/easy/EasyWriterBatchCreator.java c91ceba 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/direct/DirectBatchCreator.java 84587a9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONRecordReader.java 2cccc64 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaBatchCreator.java b38a33f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/mock/MockScanBatchCreator.java 0bfd038 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetFormatPlugin.java 7298f53 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetRecordWriter.java 3e35721 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetScanBatchCreator.java 52dccd9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetWriterBatchCreator.java 10dd26d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet2/DrillParquetReader.java 4d837c1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemTableBatchCreator.java 92f676a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java 3368412 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/UnlimitedRawBatchBuffer.java 85262de 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java edbcfde 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java 0783fee 
>   exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java b02051b 
>   exec/java-exec/src/test/java/org/apache/drill/exec/RunRootExec.java 4258e60 
>   exec/java-exec/src/test/java/org/apache/drill/exec/client/DumpCatTest.java f4f4966 
>   exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/ExpressionInterpreterTest.java 4942185 
>   exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java 1f0951b 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/SimpleRootExec.java 2536bbb 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestCastFunctions.java e5448ac 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestComparisonFunctions.java 0f6fd43 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/filter/TestSimpleFilter.java a112d92 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/join/TestHashJoin.java ef3a330 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/partitionsender/TestPartitionSender.java 46bcc60 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/trace/TestTraceMultiRecordBatch.java d0d4005 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/trace/TestTraceOutputDump.java f6766b1 
> 
> Diff: https://reviews.apache.org/r/33782/diff/
> 
> 
> Testing
> -------
> 
> regression/unit
> 
> 
> Thanks,
> 
> Jacques Nadeau
> 
>

Re: Review Request 33782: DRILL-2826: Centralize operator cleanup

Posted by Jacques Nadeau <ja...@gmail.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33782/#review82333
-----------------------------------------------------------



exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java
<https://reviews.apache.org/r/33782/#comment133056>

    Caught it in my manual testing before merge.


- Jacques Nadeau


On May 2, 2015, 5:27 p.m., Jacques Nadeau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33782/
> -----------------------------------------------------------
> 
> (Updated May 2, 2015, 5:27 p.m.)
> 
> 
> Review request for drill, abdelhakim deneche and Steven Phillips.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> DRILL-2826: Simplify and centralize Operator Cleanup
> 
> - Remove cleanup method from RecordBatch interface
> - Make OperatorContext creation and closing the management of FragmentContext
> - Make OperatorContext an abstract class and the impl only available to FragmentContext
> - Make RecordBatch closing the responsibility of the RootExec
> - Make all closes be suppresing closes to maximize memory release in failure
> - Add new CloseableRecordBatch interface used by RootExec
> - Make RootExec AutoCloseable
> - Update RecordBatchCreator to return CloseableRecordBatches so that RootExec can maintain list
> - Generate list of operators through change in ImplCreator
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/drill/common/StackTrace.java 454c3a8 
>   common/src/main/java/org/apache/drill/common/exceptions/UserExceptionContext.java 1054c7f 
>   contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseScanBatchCreator.java 9256157 
>   contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveScanBatchCreator.java a1273c0 
>   contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoScanBatchCreator.java c4597b5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java d22651e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java e6d5acd 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorContext.java ccafa67 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorContextImpl.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/BaseRootExec.java 628dcd3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/BatchCreator.java 1cf7da7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ImplCreator.java 912dfd7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/MergingReceiverCreator.java daef44c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/RootExec.java 8fd68b2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java ca2a048 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java 2069d35 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SingleSenderCreator.java 18ea71d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNBatch.java 681c3e3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNSortBatchCreator.java aa8b611 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/WriterRecordBatch.java b419f71 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java c29fbf2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatchCreator.java 8c60541 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java 87cd4d6 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java ed5b415 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatchCreator.java 0203b81 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/broadcastsender/BroadcastSenderRootExec.java b3a6a8f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterBatchCreator.java 7f2fe8e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterRecordBatch.java 064d5c8 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/flatten/FlattenBatchCreator.java 6f02824 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java 79fe177 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatchCreator.java bfe89c0 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java 1a7e60e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinCreator.java 7d100af 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/NestedLoopJoinBatch.java 4fb1409 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/NestedLoopJoinBatchCreator.java 12588ac 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitBatchCreator.java e71daba 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitRecordBatch.java 7e66893 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java 40cbc89 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java b26c78a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java 7e3f4b2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/producer/ProducerConsumerBatch.java c2d6166 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/producer/ProducerConsumerBatchCreator.java c568ed4 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ComplexToJsonBatchCreator.java 0df9491 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectBatchCreator.java cb1d4f1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 0a097c1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatchCreator.java 217acf2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/RemovingRecordBatch.java 1fa759c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/SVRemoverCreator.java 455a5f9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/trace/TraceBatchCreator.java 12afa33 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/trace/TraceRecordBatch.java 8a7d659 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/union/UnionAllBatchCreator.java 7f7e110 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/union/UnionAllRecordBatch.java 52b1794 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unorderedreceiver/UnorderedReceiverBatch.java 094865e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unorderedreceiver/UnorderedReceiverCreator.java d9864f9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/validate/IteratorValidatorBatchIterator.java 66ec22f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/validate/IteratorValidatorCreator.java 5d08afb 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/values/ValuesBatchCreator.java d526a84 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFrameBatchCreator.java 285e2cd 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFrameRecordBatch.java bc86390 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java ca93a72 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatchCreator.java eb5d83b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/AbstractRecordBatch.java c96cb7c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/AbstractSingleRecordBatch.java 3cfe177 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/CloseableRecordBatch.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatch.java 0a8ece5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/avro/AvroRecordReader.java 489a989 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/easy/EasyFormatPlugin.java f1271b1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/easy/EasyReaderBatchCreator.java ac0d2e7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/easy/EasyWriterBatchCreator.java c91ceba 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/direct/DirectBatchCreator.java 84587a9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONRecordReader.java 2cccc64 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaBatchCreator.java b38a33f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/mock/MockScanBatchCreator.java 0bfd038 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetFormatPlugin.java 7298f53 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetRecordWriter.java 3e35721 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetScanBatchCreator.java 52dccd9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetWriterBatchCreator.java 10dd26d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet2/DrillParquetReader.java 4d837c1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemTableBatchCreator.java 92f676a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java 3368412 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/UnlimitedRawBatchBuffer.java 85262de 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java edbcfde 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java 0783fee 
>   exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java b02051b 
>   exec/java-exec/src/test/java/org/apache/drill/exec/RunRootExec.java 4258e60 
>   exec/java-exec/src/test/java/org/apache/drill/exec/client/DumpCatTest.java f4f4966 
>   exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/ExpressionInterpreterTest.java 4942185 
>   exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java 1f0951b 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/SimpleRootExec.java 2536bbb 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestCastFunctions.java e5448ac 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestComparisonFunctions.java 0f6fd43 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/filter/TestSimpleFilter.java a112d92 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/join/TestHashJoin.java ef3a330 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/partitionsender/TestPartitionSender.java 46bcc60 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/trace/TestTraceMultiRecordBatch.java d0d4005 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/trace/TestTraceOutputDump.java f6766b1 
> 
> Diff: https://reviews.apache.org/r/33782/diff/
> 
> 
> Testing
> -------
> 
> regression/unit
> 
> 
> Thanks,
> 
> Jacques Nadeau
> 
>

Re: Review Request 33782: DRILL-2826: Centralize operator cleanup

Posted by Jacques Nadeau <ja...@gmail.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33782/#review82330
-----------------------------------------------------------



exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java
<https://reviews.apache.org/r/33782/#comment133054>

    Debugging code change that leaked into patch.  Will remove before committing.


- Jacques Nadeau


On May 2, 2015, 5:27 p.m., Jacques Nadeau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33782/
> -----------------------------------------------------------
> 
> (Updated May 2, 2015, 5:27 p.m.)
> 
> 
> Review request for drill, abdelhakim deneche and Steven Phillips.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> DRILL-2826: Simplify and centralize Operator Cleanup
> 
> - Remove cleanup method from RecordBatch interface
> - Make OperatorContext creation and closing the management of FragmentContext
> - Make OperatorContext an abstract class and the impl only available to FragmentContext
> - Make RecordBatch closing the responsibility of the RootExec
> - Make all closes be suppresing closes to maximize memory release in failure
> - Add new CloseableRecordBatch interface used by RootExec
> - Make RootExec AutoCloseable
> - Update RecordBatchCreator to return CloseableRecordBatches so that RootExec can maintain list
> - Generate list of operators through change in ImplCreator
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/drill/common/StackTrace.java 454c3a8 
>   common/src/main/java/org/apache/drill/common/exceptions/UserExceptionContext.java 1054c7f 
>   contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseScanBatchCreator.java 9256157 
>   contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveScanBatchCreator.java a1273c0 
>   contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoScanBatchCreator.java c4597b5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java d22651e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java e6d5acd 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorContext.java ccafa67 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorContextImpl.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/BaseRootExec.java 628dcd3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/BatchCreator.java 1cf7da7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ImplCreator.java 912dfd7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/MergingReceiverCreator.java daef44c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/RootExec.java 8fd68b2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java ca2a048 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java 2069d35 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SingleSenderCreator.java 18ea71d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNBatch.java 681c3e3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNSortBatchCreator.java aa8b611 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/WriterRecordBatch.java b419f71 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java c29fbf2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatchCreator.java 8c60541 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java 87cd4d6 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java ed5b415 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatchCreator.java 0203b81 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/broadcastsender/BroadcastSenderRootExec.java b3a6a8f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterBatchCreator.java 7f2fe8e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterRecordBatch.java 064d5c8 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/flatten/FlattenBatchCreator.java 6f02824 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java 79fe177 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatchCreator.java bfe89c0 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java 1a7e60e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinCreator.java 7d100af 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/NestedLoopJoinBatch.java 4fb1409 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/NestedLoopJoinBatchCreator.java 12588ac 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitBatchCreator.java e71daba 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitRecordBatch.java 7e66893 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java 40cbc89 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java b26c78a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java 7e3f4b2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/producer/ProducerConsumerBatch.java c2d6166 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/producer/ProducerConsumerBatchCreator.java c568ed4 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ComplexToJsonBatchCreator.java 0df9491 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectBatchCreator.java cb1d4f1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 0a097c1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatchCreator.java 217acf2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/RemovingRecordBatch.java 1fa759c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/SVRemoverCreator.java 455a5f9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/trace/TraceBatchCreator.java 12afa33 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/trace/TraceRecordBatch.java 8a7d659 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/union/UnionAllBatchCreator.java 7f7e110 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/union/UnionAllRecordBatch.java 52b1794 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unorderedreceiver/UnorderedReceiverBatch.java 094865e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unorderedreceiver/UnorderedReceiverCreator.java d9864f9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/validate/IteratorValidatorBatchIterator.java 66ec22f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/validate/IteratorValidatorCreator.java 5d08afb 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/values/ValuesBatchCreator.java d526a84 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFrameBatchCreator.java 285e2cd 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFrameRecordBatch.java bc86390 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java ca93a72 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatchCreator.java eb5d83b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/AbstractRecordBatch.java c96cb7c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/AbstractSingleRecordBatch.java 3cfe177 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/CloseableRecordBatch.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatch.java 0a8ece5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/avro/AvroRecordReader.java 489a989 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/easy/EasyFormatPlugin.java f1271b1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/easy/EasyReaderBatchCreator.java ac0d2e7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/easy/EasyWriterBatchCreator.java c91ceba 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/direct/DirectBatchCreator.java 84587a9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONRecordReader.java 2cccc64 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaBatchCreator.java b38a33f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/mock/MockScanBatchCreator.java 0bfd038 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetFormatPlugin.java 7298f53 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetRecordWriter.java 3e35721 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetScanBatchCreator.java 52dccd9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetWriterBatchCreator.java 10dd26d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet2/DrillParquetReader.java 4d837c1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemTableBatchCreator.java 92f676a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java 3368412 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/UnlimitedRawBatchBuffer.java 85262de 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java edbcfde 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java 0783fee 
>   exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java b02051b 
>   exec/java-exec/src/test/java/org/apache/drill/exec/RunRootExec.java 4258e60 
>   exec/java-exec/src/test/java/org/apache/drill/exec/client/DumpCatTest.java f4f4966 
>   exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/ExpressionInterpreterTest.java 4942185 
>   exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java 1f0951b 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/SimpleRootExec.java 2536bbb 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestCastFunctions.java e5448ac 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestComparisonFunctions.java 0f6fd43 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/filter/TestSimpleFilter.java a112d92 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/join/TestHashJoin.java ef3a330 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/partitionsender/TestPartitionSender.java 46bcc60 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/trace/TestTraceMultiRecordBatch.java d0d4005 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/trace/TestTraceOutputDump.java f6766b1 
> 
> Diff: https://reviews.apache.org/r/33782/diff/
> 
> 
> Testing
> -------
> 
> regression/unit
> 
> 
> Thanks,
> 
> Jacques Nadeau
> 
>

Re: Review Request 33782: DRILL-2826: Centralize operator cleanup

Posted by Sudheesh Katkam <sk...@maprtech.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33782/#review82332
-----------------------------------------------------------



exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java
<https://reviews.apache.org/r/33782/#comment133055>

    Check if root is null before invoking close()
    
    Say executor was cancelled before run was invoked. Now when executor runs, **if (shouldContinue())** is false and root is not initialized, but **closeOutResources()** is called in finally block. So root can be null.


- Sudheesh Katkam


On May 2, 2015, 5:27 p.m., Jacques Nadeau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33782/
> -----------------------------------------------------------
> 
> (Updated May 2, 2015, 5:27 p.m.)
> 
> 
> Review request for drill, abdelhakim deneche and Steven Phillips.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> DRILL-2826: Simplify and centralize Operator Cleanup
> 
> - Remove cleanup method from RecordBatch interface
> - Make OperatorContext creation and closing the management of FragmentContext
> - Make OperatorContext an abstract class and the impl only available to FragmentContext
> - Make RecordBatch closing the responsibility of the RootExec
> - Make all closes be suppresing closes to maximize memory release in failure
> - Add new CloseableRecordBatch interface used by RootExec
> - Make RootExec AutoCloseable
> - Update RecordBatchCreator to return CloseableRecordBatches so that RootExec can maintain list
> - Generate list of operators through change in ImplCreator
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/drill/common/StackTrace.java 454c3a8 
>   common/src/main/java/org/apache/drill/common/exceptions/UserExceptionContext.java 1054c7f 
>   contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseScanBatchCreator.java 9256157 
>   contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveScanBatchCreator.java a1273c0 
>   contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoScanBatchCreator.java c4597b5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java d22651e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java e6d5acd 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorContext.java ccafa67 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorContextImpl.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/BaseRootExec.java 628dcd3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/BatchCreator.java 1cf7da7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ImplCreator.java 912dfd7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/MergingReceiverCreator.java daef44c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/RootExec.java 8fd68b2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java ca2a048 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java 2069d35 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SingleSenderCreator.java 18ea71d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNBatch.java 681c3e3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNSortBatchCreator.java aa8b611 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/WriterRecordBatch.java b419f71 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java c29fbf2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatchCreator.java 8c60541 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java 87cd4d6 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java ed5b415 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatchCreator.java 0203b81 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/broadcastsender/BroadcastSenderRootExec.java b3a6a8f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterBatchCreator.java 7f2fe8e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterRecordBatch.java 064d5c8 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/flatten/FlattenBatchCreator.java 6f02824 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java 79fe177 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatchCreator.java bfe89c0 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java 1a7e60e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinCreator.java 7d100af 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/NestedLoopJoinBatch.java 4fb1409 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/NestedLoopJoinBatchCreator.java 12588ac 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitBatchCreator.java e71daba 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitRecordBatch.java 7e66893 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java 40cbc89 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java b26c78a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java 7e3f4b2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/producer/ProducerConsumerBatch.java c2d6166 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/producer/ProducerConsumerBatchCreator.java c568ed4 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ComplexToJsonBatchCreator.java 0df9491 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectBatchCreator.java cb1d4f1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 0a097c1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatchCreator.java 217acf2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/RemovingRecordBatch.java 1fa759c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/SVRemoverCreator.java 455a5f9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/trace/TraceBatchCreator.java 12afa33 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/trace/TraceRecordBatch.java 8a7d659 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/union/UnionAllBatchCreator.java 7f7e110 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/union/UnionAllRecordBatch.java 52b1794 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unorderedreceiver/UnorderedReceiverBatch.java 094865e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unorderedreceiver/UnorderedReceiverCreator.java d9864f9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/validate/IteratorValidatorBatchIterator.java 66ec22f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/validate/IteratorValidatorCreator.java 5d08afb 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/values/ValuesBatchCreator.java d526a84 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFrameBatchCreator.java 285e2cd 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFrameRecordBatch.java bc86390 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java ca93a72 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatchCreator.java eb5d83b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/AbstractRecordBatch.java c96cb7c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/AbstractSingleRecordBatch.java 3cfe177 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/CloseableRecordBatch.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatch.java 0a8ece5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/avro/AvroRecordReader.java 489a989 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/easy/EasyFormatPlugin.java f1271b1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/easy/EasyReaderBatchCreator.java ac0d2e7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/easy/EasyWriterBatchCreator.java c91ceba 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/direct/DirectBatchCreator.java 84587a9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONRecordReader.java 2cccc64 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaBatchCreator.java b38a33f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/mock/MockScanBatchCreator.java 0bfd038 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetFormatPlugin.java 7298f53 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetRecordWriter.java 3e35721 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetScanBatchCreator.java 52dccd9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetWriterBatchCreator.java 10dd26d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet2/DrillParquetReader.java 4d837c1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemTableBatchCreator.java 92f676a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java 3368412 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/UnlimitedRawBatchBuffer.java 85262de 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java edbcfde 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java 0783fee 
>   exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java b02051b 
>   exec/java-exec/src/test/java/org/apache/drill/exec/RunRootExec.java 4258e60 
>   exec/java-exec/src/test/java/org/apache/drill/exec/client/DumpCatTest.java f4f4966 
>   exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/ExpressionInterpreterTest.java 4942185 
>   exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java 1f0951b 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/SimpleRootExec.java 2536bbb 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestCastFunctions.java e5448ac 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestComparisonFunctions.java 0f6fd43 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/filter/TestSimpleFilter.java a112d92 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/join/TestHashJoin.java ef3a330 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/partitionsender/TestPartitionSender.java 46bcc60 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/trace/TestTraceMultiRecordBatch.java d0d4005 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/trace/TestTraceOutputDump.java f6766b1 
> 
> Diff: https://reviews.apache.org/r/33782/diff/
> 
> 
> Testing
> -------
> 
> regression/unit
> 
> 
> Thanks,
> 
> Jacques Nadeau
> 
>

Re: Review Request 33782: DRILL-2826: Centralize operator cleanup

Posted by Steven Phillips <sp...@maprtech.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33782/#review82329
-----------------------------------------------------------

Ship it!


LGTM


exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java
<https://reviews.apache.org/r/33782/#comment133053>

    Is this really necessary?
    I think it's fine for now, but maybe in the future we should come up with a way to handle this more cleanly, maybe using a Listener of some sort.


- Steven Phillips


On May 2, 2015, 5:27 p.m., Jacques Nadeau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33782/
> -----------------------------------------------------------
> 
> (Updated May 2, 2015, 5:27 p.m.)
> 
> 
> Review request for drill, abdelhakim deneche and Steven Phillips.
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> DRILL-2826: Simplify and centralize Operator Cleanup
> 
> - Remove cleanup method from RecordBatch interface
> - Make OperatorContext creation and closing the management of FragmentContext
> - Make OperatorContext an abstract class and the impl only available to FragmentContext
> - Make RecordBatch closing the responsibility of the RootExec
> - Make all closes be suppresing closes to maximize memory release in failure
> - Add new CloseableRecordBatch interface used by RootExec
> - Make RootExec AutoCloseable
> - Update RecordBatchCreator to return CloseableRecordBatches so that RootExec can maintain list
> - Generate list of operators through change in ImplCreator
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/drill/common/StackTrace.java 454c3a8 
>   common/src/main/java/org/apache/drill/common/exceptions/UserExceptionContext.java 1054c7f 
>   contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseScanBatchCreator.java 9256157 
>   contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveScanBatchCreator.java a1273c0 
>   contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoScanBatchCreator.java c4597b5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java d22651e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java e6d5acd 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorContext.java ccafa67 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorContextImpl.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/BaseRootExec.java 628dcd3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/BatchCreator.java 1cf7da7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ImplCreator.java 912dfd7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/MergingReceiverCreator.java daef44c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/RootExec.java 8fd68b2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java ca2a048 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java 2069d35 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SingleSenderCreator.java 18ea71d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNBatch.java 681c3e3 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNSortBatchCreator.java aa8b611 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/WriterRecordBatch.java b419f71 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java c29fbf2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatchCreator.java 8c60541 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java 87cd4d6 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java ed5b415 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatchCreator.java 0203b81 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/broadcastsender/BroadcastSenderRootExec.java b3a6a8f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterBatchCreator.java 7f2fe8e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterRecordBatch.java 064d5c8 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/flatten/FlattenBatchCreator.java 6f02824 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java 79fe177 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatchCreator.java bfe89c0 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java 1a7e60e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinCreator.java 7d100af 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/NestedLoopJoinBatch.java 4fb1409 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/NestedLoopJoinBatchCreator.java 12588ac 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitBatchCreator.java e71daba 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitRecordBatch.java 7e66893 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java 40cbc89 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java b26c78a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java 7e3f4b2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/producer/ProducerConsumerBatch.java c2d6166 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/producer/ProducerConsumerBatchCreator.java c568ed4 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ComplexToJsonBatchCreator.java 0df9491 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectBatchCreator.java cb1d4f1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 0a097c1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatchCreator.java 217acf2 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/RemovingRecordBatch.java 1fa759c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/SVRemoverCreator.java 455a5f9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/trace/TraceBatchCreator.java 12afa33 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/trace/TraceRecordBatch.java 8a7d659 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/union/UnionAllBatchCreator.java 7f7e110 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/union/UnionAllRecordBatch.java 52b1794 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unorderedreceiver/UnorderedReceiverBatch.java 094865e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unorderedreceiver/UnorderedReceiverCreator.java d9864f9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/validate/IteratorValidatorBatchIterator.java 66ec22f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/validate/IteratorValidatorCreator.java 5d08afb 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/values/ValuesBatchCreator.java d526a84 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFrameBatchCreator.java 285e2cd 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFrameRecordBatch.java bc86390 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java ca93a72 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatchCreator.java eb5d83b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/AbstractRecordBatch.java c96cb7c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/AbstractSingleRecordBatch.java 3cfe177 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/CloseableRecordBatch.java PRE-CREATION 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatch.java 0a8ece5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/avro/AvroRecordReader.java 489a989 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/easy/EasyFormatPlugin.java f1271b1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/easy/EasyReaderBatchCreator.java ac0d2e7 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/easy/EasyWriterBatchCreator.java c91ceba 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/direct/DirectBatchCreator.java 84587a9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONRecordReader.java 2cccc64 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaBatchCreator.java b38a33f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/mock/MockScanBatchCreator.java 0bfd038 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetFormatPlugin.java 7298f53 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetRecordWriter.java 3e35721 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetScanBatchCreator.java 52dccd9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetWriterBatchCreator.java 10dd26d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet2/DrillParquetReader.java 4d837c1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemTableBatchCreator.java 92f676a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java 3368412 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/UnlimitedRawBatchBuffer.java 85262de 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java edbcfde 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java 0783fee 
>   exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java b02051b 
>   exec/java-exec/src/test/java/org/apache/drill/exec/RunRootExec.java 4258e60 
>   exec/java-exec/src/test/java/org/apache/drill/exec/client/DumpCatTest.java f4f4966 
>   exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/ExpressionInterpreterTest.java 4942185 
>   exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java 1f0951b 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/SimpleRootExec.java 2536bbb 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestCastFunctions.java e5448ac 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestComparisonFunctions.java 0f6fd43 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/filter/TestSimpleFilter.java a112d92 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/join/TestHashJoin.java ef3a330 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/partitionsender/TestPartitionSender.java 46bcc60 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/trace/TestTraceMultiRecordBatch.java d0d4005 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/trace/TestTraceOutputDump.java f6766b1 
> 
> Diff: https://reviews.apache.org/r/33782/diff/
> 
> 
> Testing
> -------
> 
> regression/unit
> 
> 
> Thanks,
> 
> Jacques Nadeau
> 
>

Re: Review Request 33782: DRILL-2826: Centralize operator cleanup

Posted by Jacques Nadeau <ja...@gmail.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33782/
-----------------------------------------------------------

(Updated May 2, 2015, 5:27 p.m.)


Review request for drill, abdelhakim deneche and Steven Phillips.


Summary (updated)
-----------------

DRILL-2826: Centralize operator cleanup


Repository: drill-git


Description
-------

DRILL-2826: Simplify and centralize Operator Cleanup

- Remove cleanup method from RecordBatch interface
- Make OperatorContext creation and closing the management of FragmentContext
- Make OperatorContext an abstract class and the impl only available to FragmentContext
- Make RecordBatch closing the responsibility of the RootExec
- Make all closes be suppresing closes to maximize memory release in failure
- Add new CloseableRecordBatch interface used by RootExec
- Make RootExec AutoCloseable
- Update RecordBatchCreator to return CloseableRecordBatches so that RootExec can maintain list
- Generate list of operators through change in ImplCreator


Diffs
-----

  common/src/main/java/org/apache/drill/common/StackTrace.java 454c3a8 
  common/src/main/java/org/apache/drill/common/exceptions/UserExceptionContext.java 1054c7f 
  contrib/storage-hbase/src/main/java/org/apache/drill/exec/store/hbase/HBaseScanBatchCreator.java 9256157 
  contrib/storage-hive/core/src/main/java/org/apache/drill/exec/store/hive/HiveScanBatchCreator.java a1273c0 
  contrib/storage-mongo/src/main/java/org/apache/drill/exec/store/mongo/MongoScanBatchCreator.java c4597b5 
  exec/java-exec/src/main/java/org/apache/drill/exec/memory/TopLevelAllocator.java d22651e 
  exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java e6d5acd 
  exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorContext.java ccafa67 
  exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorContextImpl.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/BaseRootExec.java 628dcd3 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/BatchCreator.java 1cf7da7 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ImplCreator.java 912dfd7 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/MergingReceiverCreator.java daef44c 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/RootExec.java 8fd68b2 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java ca2a048 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java 2069d35 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SingleSenderCreator.java 18ea71d 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNBatch.java 681c3e3 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/TopN/TopNSortBatchCreator.java aa8b611 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/WriterRecordBatch.java b419f71 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java c29fbf2 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatchCreator.java 8c60541 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggTemplate.java 87cd4d6 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java ed5b415 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatchCreator.java 0203b81 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/broadcastsender/BroadcastSenderRootExec.java b3a6a8f 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterBatchCreator.java 7f2fe8e 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/filter/FilterRecordBatch.java 064d5c8 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/flatten/FlattenBatchCreator.java 6f02824 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java 79fe177 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatchCreator.java bfe89c0 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinBatch.java 1a7e60e 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/MergeJoinCreator.java 7d100af 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/NestedLoopJoinBatch.java 4fb1409 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/NestedLoopJoinBatchCreator.java 12588ac 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitBatchCreator.java e71daba 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/limit/LimitRecordBatch.java 7e66893 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/mergereceiver/MergingRecordBatch.java 40cbc89 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/orderedpartitioner/OrderedPartitionRecordBatch.java b26c78a 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java 7e3f4b2 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/producer/ProducerConsumerBatch.java c2d6166 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/producer/ProducerConsumerBatchCreator.java c568ed4 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ComplexToJsonBatchCreator.java 0df9491 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectBatchCreator.java cb1d4f1 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatch.java 0a097c1 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/sort/SortBatchCreator.java 217acf2 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/RemovingRecordBatch.java 1fa759c 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/svremover/SVRemoverCreator.java 455a5f9 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/trace/TraceBatchCreator.java 12afa33 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/trace/TraceRecordBatch.java 8a7d659 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/union/UnionAllBatchCreator.java 7f7e110 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/union/UnionAllRecordBatch.java 52b1794 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unorderedreceiver/UnorderedReceiverBatch.java 094865e 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/unorderedreceiver/UnorderedReceiverCreator.java d9864f9 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/validate/IteratorValidatorBatchIterator.java 66ec22f 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/validate/IteratorValidatorCreator.java 5d08afb 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/values/ValuesBatchCreator.java d526a84 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFrameBatchCreator.java 285e2cd 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/window/WindowFrameRecordBatch.java bc86390 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatch.java ca93a72 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/xsort/ExternalSortBatchCreator.java eb5d83b 
  exec/java-exec/src/main/java/org/apache/drill/exec/record/AbstractRecordBatch.java c96cb7c 
  exec/java-exec/src/main/java/org/apache/drill/exec/record/AbstractSingleRecordBatch.java 3cfe177 
  exec/java-exec/src/main/java/org/apache/drill/exec/record/CloseableRecordBatch.java PRE-CREATION 
  exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatch.java 0a8ece5 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/avro/AvroRecordReader.java 489a989 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/easy/EasyFormatPlugin.java f1271b1 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/easy/EasyReaderBatchCreator.java ac0d2e7 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/dfs/easy/EasyWriterBatchCreator.java c91ceba 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/direct/DirectBatchCreator.java 84587a9 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/JSONRecordReader.java 2cccc64 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/ischema/InfoSchemaBatchCreator.java b38a33f 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/mock/MockScanBatchCreator.java 0bfd038 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetFormatPlugin.java 7298f53 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetRecordWriter.java 3e35721 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetScanBatchCreator.java 52dccd9 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet/ParquetWriterBatchCreator.java 10dd26d 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/parquet2/DrillParquetReader.java 4d837c1 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/sys/SystemTableBatchCreator.java 92f676a 
  exec/java-exec/src/main/java/org/apache/drill/exec/store/text/DrillTextRecordReader.java 3368412 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/UnlimitedRawBatchBuffer.java 85262de 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java edbcfde 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java 0783fee 
  exec/java-exec/src/test/java/org/apache/drill/BaseTestQuery.java b02051b 
  exec/java-exec/src/test/java/org/apache/drill/exec/RunRootExec.java 4258e60 
  exec/java-exec/src/test/java/org/apache/drill/exec/client/DumpCatTest.java f4f4966 
  exec/java-exec/src/test/java/org/apache/drill/exec/fn/interp/ExpressionInterpreterTest.java 4942185 
  exec/java-exec/src/test/java/org/apache/drill/exec/memory/TestAllocators.java 1f0951b 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/SimpleRootExec.java 2536bbb 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestCastFunctions.java e5448ac 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestComparisonFunctions.java 0f6fd43 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/filter/TestSimpleFilter.java a112d92 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/join/TestHashJoin.java ef3a330 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/partitionsender/TestPartitionSender.java 46bcc60 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/trace/TestTraceMultiRecordBatch.java d0d4005 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/trace/TestTraceOutputDump.java f6766b1 

Diff: https://reviews.apache.org/r/33782/diff/


Testing
-------

regression/unit


Thanks,

Jacques Nadeau