You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Steven Phillips <sp...@maprtech.com> on 2015/04/10 22:17:36 UTC
Re: Review Request 32949: Patch for DRILL-2718
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32949/
-----------------------------------------------------------
(Updated April 10, 2015, 8:17 p.m.)
Review request for drill, Chris Westin and Jacques Nadeau.
Summary (updated)
-----------------
Patch for DRILL-2718
Bugs: DRILL-2718
https://issues.apache.org/jira/browse/DRILL-2718
Repository: drill-git
Description (updated)
-------
DRILL-2718: Move counting and tracking of sent batches to FragmentContext
Creates wrapper classes FragmentDataTunnel and FragmentUserDataTunnel which wrap
the DataTunnel and UserClientConnection, respectively, allowing us to use DataTunnels
and UserClientConnections from a global pool, but track pending batches and send status
at the FragmentContext level.
Consolidates the various StatusListener implementations used by the various senders and
instead uses just one implementation.
Diffs (updated)
-----
exec/java-exec/src/main/java/org/apache/drill/exec/ops/AccountingDataTunnel.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/ops/AccountingUserConnection.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java 18b93e90853bb18971ad7a21f9a6d4bd9853c1bb
exec/java-exec/src/main/java/org/apache/drill/exec/ops/StatusHandler.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/BaseRootExec.java a00df9d27a981fd6b85d3fd2a272a00c4aba80d6
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java 8038527a50a1e1f6cdb6dce68b7d5438869f928f
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SendingAccountor.java 21fc800cc9855ee2da309934a4422dd37644f4bf
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SingleSenderCreator.java 1ef7bbd4691f0ced794e0f8d1dc3921b60044931
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/broadcastsender/BroadcastSenderRootExec.java d17fdd4ba8bc3037a26cfcc791aeb18f3a112cfe
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java 6a73cdd6b4124f7410e3eb3cfaf9c7a3e17fed8d
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/Partitioner.java 9d6e98ff9ad59450bbd274ea99417a8f1acf5915
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java 33d6f9560133c4d8f97c1d214c9fa007216197ea
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/StatusHandler.java 5e21878d450825731317742638ef1e6d3f3cee30
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserClient.java 8022c95821e3bac6c5d0f249cf52c85af29fd1e8
Diff: https://reviews.apache.org/r/32949/diff/
Testing
-------
No new functionality, so no new tests. Current tests all pass.
Thanks,
Steven Phillips
Re: Review Request 32949: Patch for DRILL-2718
Posted by Chris Westin <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32949/#review79911
-----------------------------------------------------------
Ship it!
Ship it (non-binding).
- Chris Westin
On April 10, 2015, 11:47 p.m., Steven Phillips wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32949/
> -----------------------------------------------------------
>
> (Updated April 10, 2015, 11:47 p.m.)
>
>
> Review request for drill, Chris Westin and Jacques Nadeau.
>
>
> Bugs: DRILL-2718
> https://issues.apache.org/jira/browse/DRILL-2718
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> DRILL-2718: Move counting and tracking of sent batches to FragmentContext
>
> Creates wrapper classes FragmentDataTunnel and FragmentUserDataTunnel which wrap
> the DataTunnel and UserClientConnection, respectively, allowing us to use DataTunnels
> and UserClientConnections from a global pool, but track pending batches and send status
> at the FragmentContext level.
>
> Consolidates the various StatusListener implementations used by the various senders and
> instead uses just one implementation.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/ops/AccountingDataTunnel.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/ops/AccountingUserConnection.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/ops/Consumer.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java 18b93e90853bb18971ad7a21f9a6d4bd9853c1bb
> exec/java-exec/src/main/java/org/apache/drill/exec/ops/StatusHandler.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/BaseRootExec.java a00df9d27a981fd6b85d3fd2a272a00c4aba80d6
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java 8038527a50a1e1f6cdb6dce68b7d5438869f928f
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SendingAccountor.java 21fc800cc9855ee2da309934a4422dd37644f4bf
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SingleSenderCreator.java 1ef7bbd4691f0ced794e0f8d1dc3921b60044931
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/broadcastsender/BroadcastSenderRootExec.java d17fdd4ba8bc3037a26cfcc791aeb18f3a112cfe
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java 6a73cdd6b4124f7410e3eb3cfaf9c7a3e17fed8d
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/Partitioner.java 9d6e98ff9ad59450bbd274ea99417a8f1acf5915
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java 33d6f9560133c4d8f97c1d214c9fa007216197ea
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/StatusHandler.java 5e21878d450825731317742638ef1e6d3f3cee30
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserClient.java 8022c95821e3bac6c5d0f249cf52c85af29fd1e8
>
> Diff: https://reviews.apache.org/r/32949/diff/
>
>
> Testing
> -------
>
> No new functionality, so no new tests. Current tests all pass.
>
>
> Thanks,
>
> Steven Phillips
>
>
Re: Review Request 32949: Patch for DRILL-2718
Posted by Steven Phillips <sp...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32949/
-----------------------------------------------------------
(Updated April 11, 2015, 6:47 a.m.)
Review request for drill, Chris Westin and Jacques Nadeau.
Bugs: DRILL-2718
https://issues.apache.org/jira/browse/DRILL-2718
Repository: drill-git
Description
-------
DRILL-2718: Move counting and tracking of sent batches to FragmentContext
Creates wrapper classes FragmentDataTunnel and FragmentUserDataTunnel which wrap
the DataTunnel and UserClientConnection, respectively, allowing us to use DataTunnels
and UserClientConnections from a global pool, but track pending batches and send status
at the FragmentContext level.
Consolidates the various StatusListener implementations used by the various senders and
instead uses just one implementation.
Diffs (updated)
-----
exec/java-exec/src/main/java/org/apache/drill/exec/ops/AccountingDataTunnel.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/ops/AccountingUserConnection.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/ops/Consumer.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java 18b93e90853bb18971ad7a21f9a6d4bd9853c1bb
exec/java-exec/src/main/java/org/apache/drill/exec/ops/StatusHandler.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/BaseRootExec.java a00df9d27a981fd6b85d3fd2a272a00c4aba80d6
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java 8038527a50a1e1f6cdb6dce68b7d5438869f928f
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SendingAccountor.java 21fc800cc9855ee2da309934a4422dd37644f4bf
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SingleSenderCreator.java 1ef7bbd4691f0ced794e0f8d1dc3921b60044931
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/broadcastsender/BroadcastSenderRootExec.java d17fdd4ba8bc3037a26cfcc791aeb18f3a112cfe
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java 6a73cdd6b4124f7410e3eb3cfaf9c7a3e17fed8d
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/Partitioner.java 9d6e98ff9ad59450bbd274ea99417a8f1acf5915
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java 33d6f9560133c4d8f97c1d214c9fa007216197ea
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/StatusHandler.java 5e21878d450825731317742638ef1e6d3f3cee30
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserClient.java 8022c95821e3bac6c5d0f249cf52c85af29fd1e8
Diff: https://reviews.apache.org/r/32949/diff/
Testing
-------
No new functionality, so no new tests. Current tests all pass.
Thanks,
Steven Phillips
Re: Review Request 32949: Patch for DRILL-2718
Posted by Chris Westin <ch...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32949/#review79747
-----------------------------------------------------------
Just a few minor comments on comments and strings, so if you agree with those and apply them, Ship it (non-binding).
exec/java-exec/src/main/java/org/apache/drill/exec/ops/AccountingDataTunnel.java
<https://reviews.apache.org/r/32949/#comment129278>
Just "... that tracks the status ..." ?
exec/java-exec/src/main/java/org/apache/drill/exec/ops/StatusHandler.java
<https://reviews.apache.org/r/32949/#comment129281>
It might not be a fragment, which is why I removed the second sentence from the sample I sent you earlier.
exec/java-exec/src/main/java/org/apache/drill/exec/ops/Consumer.java
<https://reviews.apache.org/r/32949/#comment129280>
// TODO replace this when we get JDK8, which includes this
- Chris Westin
On April 10, 2015, 1:20 p.m., Steven Phillips wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32949/
> -----------------------------------------------------------
>
> (Updated April 10, 2015, 1:20 p.m.)
>
>
> Review request for drill, Chris Westin and Jacques Nadeau.
>
>
> Bugs: DRILL-2718
> https://issues.apache.org/jira/browse/DRILL-2718
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> DRILL-2718: Move counting and tracking of sent batches to FragmentContext
>
> Creates wrapper classes FragmentDataTunnel and FragmentUserDataTunnel which wrap
> the DataTunnel and UserClientConnection, respectively, allowing us to use DataTunnels
> and UserClientConnections from a global pool, but track pending batches and send status
> at the FragmentContext level.
>
> Consolidates the various StatusListener implementations used by the various senders and
> instead uses just one implementation.
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/ops/AccountingDataTunnel.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/ops/AccountingUserConnection.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/ops/Consumer.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java 18b93e90853bb18971ad7a21f9a6d4bd9853c1bb
> exec/java-exec/src/main/java/org/apache/drill/exec/ops/StatusHandler.java PRE-CREATION
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/BaseRootExec.java a00df9d27a981fd6b85d3fd2a272a00c4aba80d6
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java 8038527a50a1e1f6cdb6dce68b7d5438869f928f
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SendingAccountor.java 21fc800cc9855ee2da309934a4422dd37644f4bf
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SingleSenderCreator.java 1ef7bbd4691f0ced794e0f8d1dc3921b60044931
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/broadcastsender/BroadcastSenderRootExec.java d17fdd4ba8bc3037a26cfcc791aeb18f3a112cfe
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java 6a73cdd6b4124f7410e3eb3cfaf9c7a3e17fed8d
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/Partitioner.java 9d6e98ff9ad59450bbd274ea99417a8f1acf5915
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java 33d6f9560133c4d8f97c1d214c9fa007216197ea
> exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/StatusHandler.java 5e21878d450825731317742638ef1e6d3f3cee30
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserClient.java 8022c95821e3bac6c5d0f249cf52c85af29fd1e8
>
> Diff: https://reviews.apache.org/r/32949/diff/
>
>
> Testing
> -------
>
> No new functionality, so no new tests. Current tests all pass.
>
>
> Thanks,
>
> Steven Phillips
>
>
Re: Review Request 32949: Patch for DRILL-2718
Posted by Steven Phillips <sp...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32949/
-----------------------------------------------------------
(Updated April 10, 2015, 8:20 p.m.)
Review request for drill, Chris Westin and Jacques Nadeau.
Bugs: DRILL-2718
https://issues.apache.org/jira/browse/DRILL-2718
Repository: drill-git
Description
-------
DRILL-2718: Move counting and tracking of sent batches to FragmentContext
Creates wrapper classes FragmentDataTunnel and FragmentUserDataTunnel which wrap
the DataTunnel and UserClientConnection, respectively, allowing us to use DataTunnels
and UserClientConnections from a global pool, but track pending batches and send status
at the FragmentContext level.
Consolidates the various StatusListener implementations used by the various senders and
instead uses just one implementation.
Diffs (updated)
-----
exec/java-exec/src/main/java/org/apache/drill/exec/ops/AccountingDataTunnel.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/ops/AccountingUserConnection.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/ops/Consumer.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java 18b93e90853bb18971ad7a21f9a6d4bd9853c1bb
exec/java-exec/src/main/java/org/apache/drill/exec/ops/StatusHandler.java PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/BaseRootExec.java a00df9d27a981fd6b85d3fd2a272a00c4aba80d6
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/ScreenCreator.java 8038527a50a1e1f6cdb6dce68b7d5438869f928f
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SendingAccountor.java 21fc800cc9855ee2da309934a4422dd37644f4bf
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SingleSenderCreator.java 1ef7bbd4691f0ced794e0f8d1dc3921b60044931
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/broadcastsender/BroadcastSenderRootExec.java d17fdd4ba8bc3037a26cfcc791aeb18f3a112cfe
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java 6a73cdd6b4124f7410e3eb3cfaf9c7a3e17fed8d
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/Partitioner.java 9d6e98ff9ad59450bbd274ea99417a8f1acf5915
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java 33d6f9560133c4d8f97c1d214c9fa007216197ea
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/StatusHandler.java 5e21878d450825731317742638ef1e6d3f3cee30
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserClient.java 8022c95821e3bac6c5d0f249cf52c85af29fd1e8
Diff: https://reviews.apache.org/r/32949/diff/
Testing
-------
No new functionality, so no new tests. Current tests all pass.
Thanks,
Steven Phillips