You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by Hanifi Gunes <hg...@maprtech.com> on 2015/01/15 01:47:33 UTC
Review Request 29907: DRILL-2004: Foreman should account for fragment
cancellations
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29907/
-----------------------------------------------------------
Review request for drill, Jacques Nadeau, Parth Chandra, and Steven Phillips.
Bugs: DRILL-2004
https://issues.apache.org/jira/browse/DRILL-2004
Repository: drill-git
Description
-------
DRILL-2004: Foreman should account for early termination of fragments
Diffs
-----
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/control/WorkEventBus.java b9f0a26
exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java 5efc9fa
exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryManager.java d4c87d4
exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java 27038d3
Diff: https://reviews.apache.org/r/29907/diff/
Testing
-------
all tests pass
Thanks,
Hanifi Gunes
Re: Review Request 29907: DRILL-2004: Foreman should account for
fragment cancellations
Posted by Hanifi Gunes <hg...@maprtech.com>.
> On Jan. 15, 2015, 9:18 p.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryManager.java, line 61
> > <https://reviews.apache.org/r/29907/diff/1/?file=822064#file822064line61>
> >
> > QueryManager should communicate to Foreman through the stateListener abstraction. It shouldn't hold a direct reference.
i used this for debugging not referenced. will remove.
> On Jan. 15, 2015, 9:18 p.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java, line 57
> > <https://reviews.apache.org/r/29907/diff/1/?file=822065#file822065line57>
> >
> > ?
unused will remove.
> On Jan. 15, 2015, 9:18 p.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java, line 219
> > <https://reviews.apache.org/r/29907/diff/1/?file=822065#file822065line219>
> >
> > This error message doesn't make sense in the method.
seems like a copy & paste problem. will fix.
> On Jan. 15, 2015, 9:18 p.m., Jacques Nadeau wrote:
> > exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java, line 221
> > <https://reviews.apache.org/r/29907/diff/1/?file=822065#file822065line221>
> >
> > Why runtimeexception? Should be sublcass of drill exception
used the same code. will fix.
- Hanifi
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29907/#review68311
-----------------------------------------------------------
On Jan. 15, 2015, 12:47 a.m., Hanifi Gunes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29907/
> -----------------------------------------------------------
>
> (Updated Jan. 15, 2015, 12:47 a.m.)
>
>
> Review request for drill, Jacques Nadeau, Parth Chandra, and Steven Phillips.
>
>
> Bugs: DRILL-2004
> https://issues.apache.org/jira/browse/DRILL-2004
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> DRILL-2004: Foreman should account for early termination of fragments
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/control/WorkEventBus.java b9f0a26
> exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java 5efc9fa
> exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryManager.java d4c87d4
> exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java 27038d3
>
> Diff: https://reviews.apache.org/r/29907/diff/
>
>
> Testing
> -------
>
> all tests pass
>
>
> Thanks,
>
> Hanifi Gunes
>
>
Re: Review Request 29907: DRILL-2004: Foreman should account for
fragment cancellations
Posted by Jacques Nadeau <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29907/#review68311
-----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryManager.java
<https://reviews.apache.org/r/29907/#comment112476>
QueryManager should communicate to Foreman through the stateListener abstraction. It shouldn't hold a direct reference.
exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java
<https://reviews.apache.org/r/29907/#comment112478>
?
exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java
<https://reviews.apache.org/r/29907/#comment112494>
This error message doesn't make sense in the method.
exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java
<https://reviews.apache.org/r/29907/#comment112479>
Why runtimeexception? Should be sublcass of drill exception
- Jacques Nadeau
On Jan. 15, 2015, 12:47 a.m., Hanifi Gunes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29907/
> -----------------------------------------------------------
>
> (Updated Jan. 15, 2015, 12:47 a.m.)
>
>
> Review request for drill, Jacques Nadeau, Parth Chandra, and Steven Phillips.
>
>
> Bugs: DRILL-2004
> https://issues.apache.org/jira/browse/DRILL-2004
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> DRILL-2004: Foreman should account for early termination of fragments
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/control/WorkEventBus.java b9f0a26
> exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java 5efc9fa
> exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryManager.java d4c87d4
> exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java 27038d3
>
> Diff: https://reviews.apache.org/r/29907/diff/
>
>
> Testing
> -------
>
> all tests pass
>
>
> Thanks,
>
> Hanifi Gunes
>
>
Re: Review Request 29907: DRILL-2004: Foreman should account for
fragment cancellations
Posted by Parth Chandra <pc...@maprtech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29907/#review68341
-----------------------------------------------------------
Ship it!
Checked changes after review comments.
- Parth Chandra
On Jan. 15, 2015, 12:47 a.m., Hanifi Gunes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29907/
> -----------------------------------------------------------
>
> (Updated Jan. 15, 2015, 12:47 a.m.)
>
>
> Review request for drill, Jacques Nadeau, Parth Chandra, and Steven Phillips.
>
>
> Bugs: DRILL-2004
> https://issues.apache.org/jira/browse/DRILL-2004
>
>
> Repository: drill-git
>
>
> Description
> -------
>
> DRILL-2004: Foreman should account for early termination of fragments
>
>
> Diffs
> -----
>
> exec/java-exec/src/main/java/org/apache/drill/exec/rpc/control/WorkEventBus.java b9f0a26
> exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java 5efc9fa
> exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryManager.java d4c87d4
> exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/FragmentExecutor.java 27038d3
>
> Diff: https://reviews.apache.org/r/29907/diff/
>
>
> Testing
> -------
>
> all tests pass
>
>
> Thanks,
>
> Hanifi Gunes
>
>