You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Daniel Barclay <db...@maprtech.com> on 2015/01/29 05:38:04 UTC

Review Request 28686: For DRILL-1735, hook up JDBC connection closing, and fix follow-on resource bugs.

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

Review request for drill, Mehant Baid and Parth Chandra.


Bugs: DRILL-1735
    https://issues.apache.org/jira/browse/DRILL-1735


Repository: drill-git


Description
-------

This change hooks in the previously written but unused DrillHandler so that a local-mode JDBC connection shuts down its embedded DrillBit. 

It also fixes a chain of follow-on bugs (mostly resource-deallocation omissions) exposed by the original bug fixes.  (See the commit message for the list of bugs/fixes and other changes.)


Diffs
-----

  common/src/main/java/org/apache/drill/common/config/DrillConfig.java 4c6c766 
  exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java aa5b702 
  exec/java-exec/src/main/java/io/netty/buffer/DrillBuf.java 8e9d395 
  exec/java-exec/src/main/java/io/netty/buffer/FakeAllocator.java 3de0a75 
  exec/java-exec/src/main/java/io/netty/buffer/PooledByteBufAllocatorL.java c0de544 
  exec/java-exec/src/main/java/io/netty/buffer/UnsafeDirectLittleEndian.java dfdc114 
  exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java 04b955b 
  exec/java-exec/src/main/java/org/apache/drill/exec/memory/BufferAllocator.java 83d9d1e 
  exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java dc47f4e 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/FragmentRoot.java 1721fcf 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/RootExec.java a644c34 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java d68a5b5 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java d884200 
  exec/java-exec/src/main/java/org/apache/drill/exec/record/AbstractRecordBatch.java 2bb29e5 
  exec/java-exec/src/main/java/org/apache/drill/exec/record/AbstractSingleRecordBatch.java f895f47 
  exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatch.java 0a8ece5 
  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/BasicServer.java c00df4e 
  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java 96c9911 
  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java 9015a16 
  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserResultsListener.java 9f83a4f 
  exec/java-exec/src/main/java/org/apache/drill/exec/server/BootStrapContext.java 3da2ea9 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/accessor/AbstractSqlAccessor.java 1605c7d 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/accessor/SqlAccessor.java b8480b4 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/CancelableQuery.java 5b11943 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java 5bc3da1 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java b33042b 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryManager.java 2de3592 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryStatus.java 4e18da6 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java 9ffe643 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/user/UserWorker.java 854f474 
  exec/java-exec/src/main/java/parquet/hadoop/CodecFactoryExposer.java 4d107e4 
  exec/java-exec/src/main/java/parquet/hadoop/ColumnChunkIncReadStore.java 2a3124c 
  exec/java-exec/src/main/java/parquet/hadoop/ColumnChunkPageWriteStoreExposer.java 0e9dec0 
  exec/java-exec/src/test/java/org/apache/drill/exec/ExecTest.java 0272b23 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestComparisonFunctions.java 609bc14 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/AvaticaDrillSqlAccessor.java cf5829a 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillAccessorList.java 7594783 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionConfig.java 584bbb4 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java 4c54780 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillCursor.java fbe611f 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillResultSet.java 88a6c6d 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/Driver.java 974e786 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/InvalidCursorStateSqlException.java PRE-CREATION 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/JdbcApiSqlException.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/DrillResultSetTest.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/DriverTest.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Bug1735ConnectionCloseTest.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Bug1735ResultSetCloseReleasesBuffersTest.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestJdbcDistQuery.java b2f86ea 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestViews.java 28fa4a7 

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


Testing
-------

Development/unit tests run with no errors.

QA/cluster tests don't seem to fail any worse than they already do for Drill's master branch ( :-( ):

The Functional - Passing - new has failures for:
.../Functional/Passing/partition_pruning/dfs/parquet/plan/parquetselectStarFromPartition.q
.../Functional/Passing/partition_pruning/dfs/json/plan/jsonselectStarFromPartition.q
.../Functional/Passing/partition_pruning/dfs/hierarchical/plan/textSelectStartFromPartition.q

The TPCH SF100 Parquet test suite has failures for:
- .../Advanced/Passing/tpch100/parquet/10.q
- .../Advanced/Passing/tpch100/parquet/18.q
- .../Advanced/Passing/tpch100/parquet/01.q


Thanks,

Daniel Barclay


Re: Review Request 28686: For DRILL-1735, hook up JDBC connection closing, and fix follow-on resource bugs.

Posted by Daniel Barclay <db...@maprtech.com>.

> On Jan. 29, 2015, 9:40 p.m., Mehant Baid wrote:
> > Since this change touches about 50 files, I think it would be easier to review the changes if you separate out the code changes and the comment/TODO changes.

Roger.  Split misc. TODO changes out from code changes.

See new patch.


- Daniel


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


On Feb. 25, 2015, 9:37 p.m., Daniel Barclay wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28686/
> -----------------------------------------------------------
> 
> (Updated Feb. 25, 2015, 9:37 p.m.)
> 
> 
> Review request for drill, Mehant Baid and Parth Chandra.
> 
> 
> Bugs: DRILL-1735
>     https://issues.apache.org/jira/browse/DRILL-1735
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> This change hooks in the previously written but unused DrillHandler so that a local-mode JDBC connection shuts down its embedded DrillBit. 
> 
> It also fixes a chain of follow-on bugs (mostly resource-deallocation omissions) exposed by the original bug fixes.  (See the commit message for the list of bugs/fixes and other changes.)
> 
> 
> Diffs
> -----
> 
>   exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java 04b955b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java d884200 
>   exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java b079428 
>   exec/java-exec/src/main/java/org/apache/drill/exec/server/BootStrapContext.java 3da2ea9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/accessor/SqlAccessor.java b8480b4 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java 99c6ab8 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java 378e81a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java 7ccb64e 
>   exec/java-exec/src/test/java/org/apache/drill/exec/ExecTest.java 0272b23 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/AvaticaDrillSqlAccessor.java cf5829a 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillAccessorList.java 82d51f1 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionConfig.java 54e31b1 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java 2c51ec0 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillCursor.java fbe611f 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillResultSet.java 88a6c6d 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/Driver.java 974e786 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/InvalidCursorStateSqlException.java PRE-CREATION 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/JdbcApiSqlException.java PRE-CREATION 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/DrillResultSetTest.java PRE-CREATION 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/DriverTest.java PRE-CREATION 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Bug1735ConnectionCloseTest.java PRE-CREATION 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Bug1735ResultSetCloseReleasesBuffersTest.java PRE-CREATION 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestJdbcDistQuery.java b2f86ea 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestJdbcQuery.java b627c38 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestMetadataDDL.java 93925fe 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestViews.java 0f9e25e 
> 
> Diff: https://reviews.apache.org/r/28686/diff/
> 
> 
> Testing
> -------
> 
> Ran tests.
> 
> 
> Thanks,
> 
> Daniel Barclay
> 
>


Re: Review Request 28686: For DRILL-1735, hook up JDBC connection closing, and fix follow-on resource bugs.

Posted by Mehant Baid <ba...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28686/#review70282
-----------------------------------------------------------


Since this change touches about 50 files, I think it would be easier to review the changes if you separate out the code changes and the comment/TODO changes.

- Mehant Baid


On Jan. 29, 2015, 5:47 a.m., Daniel Barclay wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28686/
> -----------------------------------------------------------
> 
> (Updated Jan. 29, 2015, 5:47 a.m.)
> 
> 
> Review request for drill, Mehant Baid and Parth Chandra.
> 
> 
> Bugs: DRILL-1735
>     https://issues.apache.org/jira/browse/DRILL-1735
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> This change hooks in the previously written but unused DrillHandler so that a local-mode JDBC connection shuts down its embedded DrillBit. 
> 
> It also fixes a chain of follow-on bugs (mostly resource-deallocation omissions) exposed by the original bug fixes.  (See the commit message for the list of bugs/fixes and other changes.)
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/drill/common/config/DrillConfig.java 4c6c766 
>   exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java aa5b702 
>   exec/java-exec/src/main/java/io/netty/buffer/DrillBuf.java 8e9d395 
>   exec/java-exec/src/main/java/io/netty/buffer/FakeAllocator.java 3de0a75 
>   exec/java-exec/src/main/java/io/netty/buffer/PooledByteBufAllocatorL.java c0de544 
>   exec/java-exec/src/main/java/io/netty/buffer/UnsafeDirectLittleEndian.java dfdc114 
>   exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java 04b955b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/memory/BufferAllocator.java 83d9d1e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java dc47f4e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/FragmentRoot.java 1721fcf 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/RootExec.java a644c34 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java d68a5b5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java d884200 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/AbstractRecordBatch.java 2bb29e5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/AbstractSingleRecordBatch.java f895f47 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatch.java 0a8ece5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/rpc/BasicServer.java c00df4e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java 96c9911 
>   exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java 9015a16 
>   exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserResultsListener.java 9f83a4f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/server/BootStrapContext.java 3da2ea9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/accessor/AbstractSqlAccessor.java 1605c7d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/accessor/SqlAccessor.java b8480b4 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/CancelableQuery.java 5b11943 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java 5bc3da1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java b33042b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryManager.java 2de3592 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryStatus.java 4e18da6 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java 9ffe643 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/user/UserWorker.java 854f474 
>   exec/java-exec/src/main/java/parquet/hadoop/CodecFactoryExposer.java 4d107e4 
>   exec/java-exec/src/main/java/parquet/hadoop/ColumnChunkIncReadStore.java 2a3124c 
>   exec/java-exec/src/main/java/parquet/hadoop/ColumnChunkPageWriteStoreExposer.java 0e9dec0 
>   exec/java-exec/src/test/java/org/apache/drill/exec/ExecTest.java 0272b23 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestComparisonFunctions.java 609bc14 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/AvaticaDrillSqlAccessor.java cf5829a 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillAccessorList.java 7594783 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionConfig.java 584bbb4 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java 4c54780 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillCursor.java fbe611f 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillResultSet.java 88a6c6d 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/Driver.java 974e786 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/InvalidCursorStateSqlException.java PRE-CREATION 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/JdbcApiSqlException.java PRE-CREATION 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/DrillResultSetTest.java PRE-CREATION 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/DriverTest.java PRE-CREATION 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Bug1735ConnectionCloseTest.java PRE-CREATION 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Bug1735ResultSetCloseReleasesBuffersTest.java PRE-CREATION 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestJdbcDistQuery.java b2f86ea 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestViews.java 28fa4a7 
> 
> Diff: https://reviews.apache.org/r/28686/diff/
> 
> 
> Testing
> -------
> 
> Development/unit tests run with no errors.
> 
> QA/cluster tests don't seem to fail any worse than they already do for Drill's master branch ( :-( ):
> 
> The Functional - Passing - new has failures for:
> .../Functional/Passing/partition_pruning/dfs/parquet/plan/parquetselectStarFromPartition.q
> .../Functional/Passing/partition_pruning/dfs/json/plan/jsonselectStarFromPartition.q
> .../Functional/Passing/partition_pruning/dfs/hierarchical/plan/textSelectStartFromPartition.q
> 
> The TPCH SF100 Parquet test suite has failures for:
> - .../Advanced/Passing/tpch100/parquet/10.q
> - .../Advanced/Passing/tpch100/parquet/01.q
> 
> 
> Thanks,
> 
> Daniel Barclay
> 
>


Re: Review Request 28686: For DRILL-1735, hook up JDBC connection closing, and fix follow-on resource bugs.

Posted by Parth Chandra <pc...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28686/#review72477
-----------------------------------------------------------

Ship it!


Ship It!

- Parth Chandra


On Feb. 5, 2015, 7:28 p.m., Daniel Barclay wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28686/
> -----------------------------------------------------------
> 
> (Updated Feb. 5, 2015, 7:28 p.m.)
> 
> 
> Review request for drill, Mehant Baid and Parth Chandra.
> 
> 
> Bugs: DRILL-1735
>     https://issues.apache.org/jira/browse/DRILL-1735
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> This change hooks in the previously written but unused DrillHandler so that a local-mode JDBC connection shuts down its embedded DrillBit. 
> 
> It also fixes a chain of follow-on bugs (mostly resource-deallocation omissions) exposed by the original bug fixes.  (See the commit message for the list of bugs/fixes and other changes.)
> 
> 
> Diffs
> -----
> 
>   exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java 04b955b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java d884200 
>   exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java 9015a16 
>   exec/java-exec/src/main/java/org/apache/drill/exec/server/BootStrapContext.java 3da2ea9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/accessor/SqlAccessor.java b8480b4 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java 99c6ab8 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java 378e81a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java a8f07b5 
>   exec/java-exec/src/test/java/org/apache/drill/exec/ExecTest.java 0272b23 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/AvaticaDrillSqlAccessor.java cf5829a 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillAccessorList.java 7594783 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionConfig.java 54e31b1 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java 2c51ec0 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillCursor.java fbe611f 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillResultSet.java 88a6c6d 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/Driver.java 974e786 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/InvalidCursorStateSqlException.java PRE-CREATION 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/JdbcApiSqlException.java PRE-CREATION 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/DrillResultSetTest.java PRE-CREATION 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/DriverTest.java PRE-CREATION 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Bug1735ConnectionCloseTest.java PRE-CREATION 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Bug1735ResultSetCloseReleasesBuffersTest.java PRE-CREATION 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestJdbcDistQuery.java b2f86ea 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestJdbcQuery.java b627c38 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestMetadataDDL.java c52eafd 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestViews.java 28fa4a7 
> 
> Diff: https://reviews.apache.org/r/28686/diff/
> 
> 
> Testing
> -------
> 
> Ran tests.
> 
> 
> Thanks,
> 
> Daniel Barclay
> 
>


Re: Review Request 28686: For DRILL-1735, hook up JDBC connection closing, and fix follow-on resource bugs.

Posted by Parth Chandra <pc...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28686/#review77154
-----------------------------------------------------------

Ship it!


Perhaps we could change the behavior so that the connection object adds and removes statements from the registry (instead of the statements doing so themselves). Makes the code cleaner. Not a showstopper for this patch.

- Parth Chandra


On March 20, 2015, 12:30 a.m., Daniel Barclay wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28686/
> -----------------------------------------------------------
> 
> (Updated March 20, 2015, 12:30 a.m.)
> 
> 
> Review request for drill, Mehant Baid and Parth Chandra.
> 
> 
> Bugs: DRILL-1735
>     https://issues.apache.org/jira/browse/DRILL-1735
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> This change hooks in the previously written but unused DrillHandler so that a local-mode JDBC connection shuts down its embedded DrillBit. 
> 
> It also fixes a chain of follow-on bugs (mostly resource-deallocation omissions) exposed by the original bug fixes.  (See the commit message for the list of bugs/fixes and other changes.)
> 
> 
> Diffs
> -----
> 
>   exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java c3a873c 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java 2d1a136 
>   exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java b079428 
>   exec/java-exec/src/main/java/org/apache/drill/exec/server/BootStrapContext.java 3da2ea9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java b606707 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/accessor/SqlAccessor.java b8480b4 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java 9650ee5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java b6176db 
>   exec/java-exec/src/test/java/org/apache/drill/exec/ExecTest.java 0272b23 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/AvaticaDrillSqlAccessor.java cf5829a 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillAccessorList.java 82d51f1 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionConfig.java 54e31b1 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java f19aab0 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillCursor.java fbe611f 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillPreparedStatement.java cfcee8c 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillResultSet.java 77b2c37 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillStatement.java fec126e 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillStatementRegistry.java cc797fa 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/Driver.java 974e786 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/InvalidCursorStateSqlException.java PRE-CREATION 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/JdbcApiSqlException.java PRE-CREATION 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/DrillResultSetTest.java PRE-CREATION 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/DriverTest.java PRE-CREATION 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/JdbcTest.java 40b1445 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Bug1735ConnectionCloseTest.java PRE-CREATION 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Bug1735ResultSetCloseReleasesBuffersTest.java PRE-CREATION 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestJdbcDistQuery.java b2f86ea 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestJdbcQuery.java 682fca3 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestMetadataDDL.java 93925fe 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestViews.java 0f9e25e 
> 
> Diff: https://reviews.apache.org/r/28686/diff/
> 
> 
> Testing
> -------
> 
> PENDING (earlier version was working; resolving current problems)
> 
> 
> Thanks,
> 
> Daniel Barclay
> 
>


Re: Review Request 28686: For DRILL-1735, hook up JDBC connection closing, and fix follow-on resource bugs.

Posted by Daniel Barclay <db...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28686/
-----------------------------------------------------------

(Updated March 20, 2015, 12:30 a.m.)


Review request for drill, Mehant Baid and Parth Chandra.


Bugs: DRILL-1735
    https://issues.apache.org/jira/browse/DRILL-1735


Repository: drill-git


Description
-------

This change hooks in the previously written but unused DrillHandler so that a local-mode JDBC connection shuts down its embedded DrillBit. 

It also fixes a chain of follow-on bugs (mostly resource-deallocation omissions) exposed by the original bug fixes.  (See the commit message for the list of bugs/fixes and other changes.)


Diffs (updated)
-----

  exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java c3a873c 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java 2d1a136 
  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java b079428 
  exec/java-exec/src/main/java/org/apache/drill/exec/server/BootStrapContext.java 3da2ea9 
  exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java b606707 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/accessor/SqlAccessor.java b8480b4 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java 9650ee5 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java b6176db 
  exec/java-exec/src/test/java/org/apache/drill/exec/ExecTest.java 0272b23 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/AvaticaDrillSqlAccessor.java cf5829a 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillAccessorList.java 82d51f1 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionConfig.java 54e31b1 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java f19aab0 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillCursor.java fbe611f 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillPreparedStatement.java cfcee8c 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillResultSet.java 77b2c37 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillStatement.java fec126e 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillStatementRegistry.java cc797fa 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/Driver.java 974e786 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/InvalidCursorStateSqlException.java PRE-CREATION 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/JdbcApiSqlException.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/DrillResultSetTest.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/DriverTest.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/JdbcTest.java 40b1445 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Bug1735ConnectionCloseTest.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Bug1735ResultSetCloseReleasesBuffersTest.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestJdbcDistQuery.java b2f86ea 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestJdbcQuery.java 682fca3 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestMetadataDDL.java 93925fe 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestViews.java 0f9e25e 

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


Testing
-------

PENDING (earlier version was working; resolving current problems)


Thanks,

Daniel Barclay


Re: Review Request 28686: For DRILL-1735, hook up JDBC connection closing, and fix follow-on resource bugs.

Posted by Daniel Barclay <db...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28686/
-----------------------------------------------------------

(Updated March 20, 2015, 12:25 a.m.)


Review request for drill, Mehant Baid and Parth Chandra.


Changes
-------

Rebased on Chris's DRILL-2245 patch.


Bugs: DRILL-1735
    https://issues.apache.org/jira/browse/DRILL-1735


Repository: drill-git


Description
-------

This change hooks in the previously written but unused DrillHandler so that a local-mode JDBC connection shuts down its embedded DrillBit. 

It also fixes a chain of follow-on bugs (mostly resource-deallocation omissions) exposed by the original bug fixes.  (See the commit message for the list of bugs/fixes and other changes.)


Diffs (updated)
-----

  exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java c3a873c 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java 2d1a136 
  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java b079428 
  exec/java-exec/src/main/java/org/apache/drill/exec/server/BootStrapContext.java 3da2ea9 
  exec/java-exec/src/main/java/org/apache/drill/exec/server/Drillbit.java b606707 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/accessor/SqlAccessor.java b8480b4 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java 9650ee5 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java b6176db 
  exec/java-exec/src/test/java/org/apache/drill/exec/ExecTest.java 0272b23 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/AvaticaDrillSqlAccessor.java cf5829a 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillAccessorList.java 82d51f1 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionConfig.java 54e31b1 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java f19aab0 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillCursor.java fbe611f 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillPreparedStatement.java cfcee8c 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillResultSet.java 77b2c37 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillStatement.java fec126e 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillStatementRegistry.java cc797fa 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/Driver.java 974e786 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/InvalidCursorStateSqlException.java PRE-CREATION 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/JdbcApiSqlException.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/DrillResultSetTest.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/DriverTest.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/JdbcTest.java 40b1445 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Bug1735ConnectionCloseTest.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Bug1735ResultSetCloseReleasesBuffersTest.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestJdbcDistQuery.java b2f86ea 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestJdbcQuery.java 682fca3 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestMetadataDDL.java 93925fe 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestViews.java 0f9e25e 

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


Testing (updated)
-------

PENDING (earlier version was working; resolving current problems)


Thanks,

Daniel Barclay


Re: Review Request 28686: For DRILL-1735, hook up JDBC connection closing, and fix follow-on resource bugs.

Posted by Daniel Barclay <db...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28686/
-----------------------------------------------------------

(Updated March 19, 2015, 6:19 p.m.)


Review request for drill, Mehant Baid and Parth Chandra.


Changes
-------

Added auto-closing of Statements when Connection is closed.


Bugs: DRILL-1735
    https://issues.apache.org/jira/browse/DRILL-1735


Repository: drill-git


Description
-------

This change hooks in the previously written but unused DrillHandler so that a local-mode JDBC connection shuts down its embedded DrillBit. 

It also fixes a chain of follow-on bugs (mostly resource-deallocation omissions) exposed by the original bug fixes.  (See the commit message for the list of bugs/fixes and other changes.)


Diffs (updated)
-----

  exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java 04b955b 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java d884200 
  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java b079428 
  exec/java-exec/src/main/java/org/apache/drill/exec/server/BootStrapContext.java 3da2ea9 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/accessor/SqlAccessor.java b8480b4 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java 8e0780b 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java e2f7bbf 
  exec/java-exec/src/test/java/org/apache/drill/exec/ExecTest.java 0272b23 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/AvaticaDrillSqlAccessor.java cf5829a 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillAccessorList.java 82d51f1 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionConfig.java 54e31b1 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java 2c51ec0 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillCursor.java fbe611f 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillPreparedStatement.java cfcee8c 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillResultSet.java 77b2c37 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillStatement.java fec126e 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillStatementRegistry.java cc797fa 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/Driver.java 974e786 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/InvalidCursorStateSqlException.java PRE-CREATION 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/JdbcApiSqlException.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/DrillResultSetTest.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/DriverTest.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/JdbcTest.java 40b1445 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Bug1735ConnectionCloseTest.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Bug1735ResultSetCloseReleasesBuffersTest.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestJdbcDistQuery.java b2f86ea 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestJdbcQuery.java 682fca3 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestMetadataDDL.java 93925fe 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestViews.java 0f9e25e 

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


Testing
-------

Ran tests.


Thanks,

Daniel Barclay


Re: Review Request 28686: For DRILL-1735, hook up JDBC connection closing, and fix follow-on resource bugs.

Posted by Daniel Barclay <db...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28686/
-----------------------------------------------------------

(Updated Feb. 25, 2015, 9:37 p.m.)


Review request for drill, Mehant Baid and Parth Chandra.


Changes
-------

Rebased on current, not-yet-split patch for 2245 on today's master and re-made patch.


Bugs: DRILL-1735
    https://issues.apache.org/jira/browse/DRILL-1735


Repository: drill-git


Description
-------

This change hooks in the previously written but unused DrillHandler so that a local-mode JDBC connection shuts down its embedded DrillBit. 

It also fixes a chain of follow-on bugs (mostly resource-deallocation omissions) exposed by the original bug fixes.  (See the commit message for the list of bugs/fixes and other changes.)


Diffs (updated)
-----

  exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java 04b955b 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java d884200 
  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java b079428 
  exec/java-exec/src/main/java/org/apache/drill/exec/server/BootStrapContext.java 3da2ea9 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/accessor/SqlAccessor.java b8480b4 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java 99c6ab8 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java 378e81a 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java 7ccb64e 
  exec/java-exec/src/test/java/org/apache/drill/exec/ExecTest.java 0272b23 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/AvaticaDrillSqlAccessor.java cf5829a 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillAccessorList.java 82d51f1 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionConfig.java 54e31b1 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java 2c51ec0 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillCursor.java fbe611f 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillResultSet.java 88a6c6d 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/Driver.java 974e786 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/InvalidCursorStateSqlException.java PRE-CREATION 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/JdbcApiSqlException.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/DrillResultSetTest.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/DriverTest.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Bug1735ConnectionCloseTest.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Bug1735ResultSetCloseReleasesBuffersTest.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestJdbcDistQuery.java b2f86ea 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestJdbcQuery.java b627c38 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestMetadataDDL.java 93925fe 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestViews.java 0f9e25e 

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


Testing
-------

Ran tests.


Thanks,

Daniel Barclay


Re: Review Request 28686: For DRILL-1735, hook up JDBC connection closing, and fix follow-on resource bugs.

Posted by Daniel Barclay <db...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28686/
-----------------------------------------------------------

(Updated Feb. 5, 2015, 7:28 p.m.)


Review request for drill, Mehant Baid and Parth Chandra.


Bugs: DRILL-1735
    https://issues.apache.org/jira/browse/DRILL-1735


Repository: drill-git


Description
-------

This change hooks in the previously written but unused DrillHandler so that a local-mode JDBC connection shuts down its embedded DrillBit. 

It also fixes a chain of follow-on bugs (mostly resource-deallocation omissions) exposed by the original bug fixes.  (See the commit message for the list of bugs/fixes and other changes.)


Diffs (updated)
-----

  exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java 04b955b 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java d884200 
  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java 9015a16 
  exec/java-exec/src/main/java/org/apache/drill/exec/server/BootStrapContext.java 3da2ea9 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/accessor/SqlAccessor.java b8480b4 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java 99c6ab8 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java 378e81a 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java a8f07b5 
  exec/java-exec/src/test/java/org/apache/drill/exec/ExecTest.java 0272b23 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/AvaticaDrillSqlAccessor.java cf5829a 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillAccessorList.java 7594783 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionConfig.java 54e31b1 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java 2c51ec0 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillCursor.java fbe611f 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillResultSet.java 88a6c6d 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/Driver.java 974e786 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/InvalidCursorStateSqlException.java PRE-CREATION 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/JdbcApiSqlException.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/DrillResultSetTest.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/DriverTest.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Bug1735ConnectionCloseTest.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Bug1735ResultSetCloseReleasesBuffersTest.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestJdbcDistQuery.java b2f86ea 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestJdbcQuery.java b627c38 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestMetadataDDL.java c52eafd 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestViews.java 28fa4a7 

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


Testing (updated)
-------

Ran tests.


Thanks,

Daniel Barclay


Re: Review Request 28686: For DRILL-1735, hook up JDBC connection closing, and fix follow-on resource bugs.

Posted by Daniel Barclay <db...@maprtech.com>.

> On Jan. 30, 2015, 1:08 a.m., Parth Chandra wrote:
> > exec/java-exec/src/main/java/io/netty/buffer/DrillBuf.java, line 37
> > <https://reviews.apache.org/r/28686/diff/1/?file=839365#file839365line37>
> >
> >     Because this (and other similar classes) need to access package private classes.

Yes, I know the part about accessing package-private methods or other members.  

However, since ideally we should not be hacking around published interfaces (the public and protected members), when we currently have to (or just do), we should at least document or highlight it enough.

That way, we can more easily recognize when there's a loose end to be addressed later (perhaps by trying to get the published interface fixed, perhaps by working it out more cleanly, etc.).


Anyway, excluded from this patch.


> On Jan. 30, 2015, 1:08 a.m., Parth Chandra wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/record/AbstractRecordBatch.java, line 138
> > <https://reviews.apache.org/r/28686/diff/1/?file=839376#file839376line138>
> >
> >     This TODO is not very useful

Excluded from patch.


> On Jan. 30, 2015, 1:08 a.m., Parth Chandra wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java, line 125
> > <https://reviews.apache.org/r/28686/diff/1/?file=839380#file839380line125>
> >
> >     This TODO is not very useful.

Excluded from patch.


> On Jan. 30, 2015, 1:08 a.m., Parth Chandra wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java, line 108
> > <https://reviews.apache.org/r/28686/diff/1/?file=839381#file839381line108>
> >
> >     This switch statement sets isTerminalBatch to false for all cases.

Roger.  Fixed switch statement and revisited/cleaned up related logic and comments.


- Daniel


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


On Feb. 25, 2015, 9:37 p.m., Daniel Barclay wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28686/
> -----------------------------------------------------------
> 
> (Updated Feb. 25, 2015, 9:37 p.m.)
> 
> 
> Review request for drill, Mehant Baid and Parth Chandra.
> 
> 
> Bugs: DRILL-1735
>     https://issues.apache.org/jira/browse/DRILL-1735
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> This change hooks in the previously written but unused DrillHandler so that a local-mode JDBC connection shuts down its embedded DrillBit. 
> 
> It also fixes a chain of follow-on bugs (mostly resource-deallocation omissions) exposed by the original bug fixes.  (See the commit message for the list of bugs/fixes and other changes.)
> 
> 
> Diffs
> -----
> 
>   exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java 04b955b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java d884200 
>   exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java b079428 
>   exec/java-exec/src/main/java/org/apache/drill/exec/server/BootStrapContext.java 3da2ea9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/accessor/SqlAccessor.java b8480b4 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java 99c6ab8 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java 378e81a 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java 7ccb64e 
>   exec/java-exec/src/test/java/org/apache/drill/exec/ExecTest.java 0272b23 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/AvaticaDrillSqlAccessor.java cf5829a 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillAccessorList.java 82d51f1 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionConfig.java 54e31b1 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java 2c51ec0 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillCursor.java fbe611f 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillResultSet.java 88a6c6d 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/Driver.java 974e786 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/InvalidCursorStateSqlException.java PRE-CREATION 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/JdbcApiSqlException.java PRE-CREATION 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/DrillResultSetTest.java PRE-CREATION 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/DriverTest.java PRE-CREATION 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Bug1735ConnectionCloseTest.java PRE-CREATION 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Bug1735ResultSetCloseReleasesBuffersTest.java PRE-CREATION 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestJdbcDistQuery.java b2f86ea 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestJdbcQuery.java b627c38 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestMetadataDDL.java 93925fe 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestViews.java 0f9e25e 
> 
> Diff: https://reviews.apache.org/r/28686/diff/
> 
> 
> Testing
> -------
> 
> Ran tests.
> 
> 
> Thanks,
> 
> Daniel Barclay
> 
>


Re: Review Request 28686: For DRILL-1735, hook up JDBC connection closing, and fix follow-on resource bugs.

Posted by Parth Chandra <pc...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28686/#review70300
-----------------------------------------------------------


I would reiterate Mehant's review comment. The formatting, documentation and renaming changes need to go into another patch. With the number of changes in here, this patch is not reviewable. I could see substantial change only in QueryResultHandler which is reviewed.
For the comments that have been added, it is preferable to actually add a comment than to add a TODO that suggests the comment to be added. Some of the TODO comments actually confused me.


exec/java-exec/src/main/java/io/netty/buffer/DrillBuf.java
<https://reviews.apache.org/r/28686/#comment115392>

    Because this (and other similar classes) need to access package private classes.



exec/java-exec/src/main/java/org/apache/drill/exec/record/AbstractRecordBatch.java
<https://reviews.apache.org/r/28686/#comment115393>

    This TODO is not very useful



exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java
<https://reviews.apache.org/r/28686/#comment115394>

    This TODO is not very useful.



exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java
<https://reviews.apache.org/r/28686/#comment115389>

    This switch statement sets isTerminalBatch to false for all cases.


- Parth Chandra


On Jan. 29, 2015, 5:47 a.m., Daniel Barclay wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28686/
> -----------------------------------------------------------
> 
> (Updated Jan. 29, 2015, 5:47 a.m.)
> 
> 
> Review request for drill, Mehant Baid and Parth Chandra.
> 
> 
> Bugs: DRILL-1735
>     https://issues.apache.org/jira/browse/DRILL-1735
> 
> 
> Repository: drill-git
> 
> 
> Description
> -------
> 
> This change hooks in the previously written but unused DrillHandler so that a local-mode JDBC connection shuts down its embedded DrillBit. 
> 
> It also fixes a chain of follow-on bugs (mostly resource-deallocation omissions) exposed by the original bug fixes.  (See the commit message for the list of bugs/fixes and other changes.)
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/drill/common/config/DrillConfig.java 4c6c766 
>   exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java aa5b702 
>   exec/java-exec/src/main/java/io/netty/buffer/DrillBuf.java 8e9d395 
>   exec/java-exec/src/main/java/io/netty/buffer/FakeAllocator.java 3de0a75 
>   exec/java-exec/src/main/java/io/netty/buffer/PooledByteBufAllocatorL.java c0de544 
>   exec/java-exec/src/main/java/io/netty/buffer/UnsafeDirectLittleEndian.java dfdc114 
>   exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java 04b955b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/memory/BufferAllocator.java 83d9d1e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java dc47f4e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/FragmentRoot.java 1721fcf 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/RootExec.java a644c34 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java d68a5b5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java d884200 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/AbstractRecordBatch.java 2bb29e5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/AbstractSingleRecordBatch.java f895f47 
>   exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatch.java 0a8ece5 
>   exec/java-exec/src/main/java/org/apache/drill/exec/rpc/BasicServer.java c00df4e 
>   exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java 96c9911 
>   exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java 9015a16 
>   exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserResultsListener.java 9f83a4f 
>   exec/java-exec/src/main/java/org/apache/drill/exec/server/BootStrapContext.java 3da2ea9 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/accessor/AbstractSqlAccessor.java 1605c7d 
>   exec/java-exec/src/main/java/org/apache/drill/exec/vector/accessor/SqlAccessor.java b8480b4 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/CancelableQuery.java 5b11943 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java 5bc3da1 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java b33042b 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryManager.java 2de3592 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryStatus.java 4e18da6 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java 9ffe643 
>   exec/java-exec/src/main/java/org/apache/drill/exec/work/user/UserWorker.java 854f474 
>   exec/java-exec/src/main/java/parquet/hadoop/CodecFactoryExposer.java 4d107e4 
>   exec/java-exec/src/main/java/parquet/hadoop/ColumnChunkIncReadStore.java 2a3124c 
>   exec/java-exec/src/main/java/parquet/hadoop/ColumnChunkPageWriteStoreExposer.java 0e9dec0 
>   exec/java-exec/src/test/java/org/apache/drill/exec/ExecTest.java 0272b23 
>   exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestComparisonFunctions.java 609bc14 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/AvaticaDrillSqlAccessor.java cf5829a 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillAccessorList.java 7594783 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionConfig.java 584bbb4 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java 4c54780 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillCursor.java fbe611f 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillResultSet.java 88a6c6d 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/Driver.java 974e786 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/InvalidCursorStateSqlException.java PRE-CREATION 
>   exec/jdbc/src/main/java/org/apache/drill/jdbc/JdbcApiSqlException.java PRE-CREATION 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/DrillResultSetTest.java PRE-CREATION 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/DriverTest.java PRE-CREATION 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Bug1735ConnectionCloseTest.java PRE-CREATION 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Bug1735ResultSetCloseReleasesBuffersTest.java PRE-CREATION 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestJdbcDistQuery.java b2f86ea 
>   exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestViews.java 28fa4a7 
> 
> Diff: https://reviews.apache.org/r/28686/diff/
> 
> 
> Testing
> -------
> 
> Development/unit tests run with no errors.
> 
> QA/cluster tests don't seem to fail any worse than they already do for Drill's master branch ( :-( ):
> 
> The Functional - Passing - new has failures for:
> .../Functional/Passing/partition_pruning/dfs/parquet/plan/parquetselectStarFromPartition.q
> .../Functional/Passing/partition_pruning/dfs/json/plan/jsonselectStarFromPartition.q
> .../Functional/Passing/partition_pruning/dfs/hierarchical/plan/textSelectStartFromPartition.q
> 
> The TPCH SF100 Parquet test suite has failures for:
> - .../Advanced/Passing/tpch100/parquet/10.q
> - .../Advanced/Passing/tpch100/parquet/01.q
> 
> 
> Thanks,
> 
> Daniel Barclay
> 
>


Re: Review Request 28686: For DRILL-1735, hook up JDBC connection closing, and fix follow-on resource bugs.

Posted by Daniel Barclay <db...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28686/
-----------------------------------------------------------

(Updated Jan. 29, 2015, 5:47 a.m.)


Review request for drill, Mehant Baid and Parth Chandra.


Bugs: DRILL-1735
    https://issues.apache.org/jira/browse/DRILL-1735


Repository: drill-git


Description
-------

This change hooks in the previously written but unused DrillHandler so that a local-mode JDBC connection shuts down its embedded DrillBit. 

It also fixes a chain of follow-on bugs (mostly resource-deallocation omissions) exposed by the original bug fixes.  (See the commit message for the list of bugs/fixes and other changes.)


Diffs
-----

  common/src/main/java/org/apache/drill/common/config/DrillConfig.java 4c6c766 
  exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java aa5b702 
  exec/java-exec/src/main/java/io/netty/buffer/DrillBuf.java 8e9d395 
  exec/java-exec/src/main/java/io/netty/buffer/FakeAllocator.java 3de0a75 
  exec/java-exec/src/main/java/io/netty/buffer/PooledByteBufAllocatorL.java c0de544 
  exec/java-exec/src/main/java/io/netty/buffer/UnsafeDirectLittleEndian.java dfdc114 
  exec/java-exec/src/main/java/org/apache/drill/exec/client/DrillClient.java 04b955b 
  exec/java-exec/src/main/java/org/apache/drill/exec/memory/BufferAllocator.java 83d9d1e 
  exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java dc47f4e 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/base/FragmentRoot.java 1721fcf 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/RootExec.java a644c34 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScanBatch.java d68a5b5 
  exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java d884200 
  exec/java-exec/src/main/java/org/apache/drill/exec/record/AbstractRecordBatch.java 2bb29e5 
  exec/java-exec/src/main/java/org/apache/drill/exec/record/AbstractSingleRecordBatch.java f895f47 
  exec/java-exec/src/main/java/org/apache/drill/exec/record/RecordBatch.java 0a8ece5 
  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/BasicServer.java c00df4e 
  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/RpcBus.java 96c9911 
  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/QueryResultHandler.java 9015a16 
  exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserResultsListener.java 9f83a4f 
  exec/java-exec/src/main/java/org/apache/drill/exec/server/BootStrapContext.java 3da2ea9 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/accessor/AbstractSqlAccessor.java 1605c7d 
  exec/java-exec/src/main/java/org/apache/drill/exec/vector/accessor/SqlAccessor.java b8480b4 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/CancelableQuery.java 5b11943 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java 5bc3da1 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java b33042b 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryManager.java 2de3592 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryStatus.java 4e18da6 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java 9ffe643 
  exec/java-exec/src/main/java/org/apache/drill/exec/work/user/UserWorker.java 854f474 
  exec/java-exec/src/main/java/parquet/hadoop/CodecFactoryExposer.java 4d107e4 
  exec/java-exec/src/main/java/parquet/hadoop/ColumnChunkIncReadStore.java 2a3124c 
  exec/java-exec/src/main/java/parquet/hadoop/ColumnChunkPageWriteStoreExposer.java 0e9dec0 
  exec/java-exec/src/test/java/org/apache/drill/exec/ExecTest.java 0272b23 
  exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestComparisonFunctions.java 609bc14 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/AvaticaDrillSqlAccessor.java cf5829a 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillAccessorList.java 7594783 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionConfig.java 584bbb4 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillConnectionImpl.java 4c54780 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillCursor.java fbe611f 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/DrillResultSet.java 88a6c6d 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/Driver.java 974e786 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/InvalidCursorStateSqlException.java PRE-CREATION 
  exec/jdbc/src/main/java/org/apache/drill/jdbc/JdbcApiSqlException.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/DrillResultSetTest.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/DriverTest.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Bug1735ConnectionCloseTest.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/Bug1735ResultSetCloseReleasesBuffersTest.java PRE-CREATION 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestJdbcDistQuery.java b2f86ea 
  exec/jdbc/src/test/java/org/apache/drill/jdbc/test/TestViews.java 28fa4a7 

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


Testing (updated)
-------

Development/unit tests run with no errors.

QA/cluster tests don't seem to fail any worse than they already do for Drill's master branch ( :-( ):

The Functional - Passing - new has failures for:
.../Functional/Passing/partition_pruning/dfs/parquet/plan/parquetselectStarFromPartition.q
.../Functional/Passing/partition_pruning/dfs/json/plan/jsonselectStarFromPartition.q
.../Functional/Passing/partition_pruning/dfs/hierarchical/plan/textSelectStartFromPartition.q

The TPCH SF100 Parquet test suite has failures for:
- .../Advanced/Passing/tpch100/parquet/10.q
- .../Advanced/Passing/tpch100/parquet/01.q


Thanks,

Daniel Barclay