----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34008/#review83316 -----------------------------------------------------------
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/control/WorkEventBus.java <https://reviews.apache.org/r/34008/#comment134254> comment missing a word ? "The manager for the ? will ..." exec/java-exec/src/main/java/org/apache/drill/exec/rpc/control/WorkEventBus.java <https://reviews.apache.org/r/34008/#comment134255> shouldn't we log a warning if the manager we are trying to remove is not in the map ? exec/java-exec/src/main/java/org/apache/drill/exec/rpc/control/WorkEventBus.java <https://reviews.apache.org/r/34008/#comment134257> we should probably explain what the returned boolean mean exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java <https://reviews.apache.org/r/34008/#comment134258> logging the running fragments should be done at a debug level exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java <https://reviews.apache.org/r/34008/#comment134260> the fragment can also be a root fragment. Maybe just say "the manager for the fragment" exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/ControlMessageHandler.java <https://reviews.apache.org/r/34008/#comment134262> where is Case 1 ??? exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java <https://reviews.apache.org/r/34008/#comment134267> we don't need this change, see next comment exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryManager.java <https://reviews.apache.org/r/34008/#comment134266> drillbitContext.getWorkBus() can be used to access the work event bus, so you don't need to pass the WorkerBee - abdelhakim deneche On May 11, 2015, 11:16 p.m., Sudheesh Katkam wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/34008/ > ----------------------------------------------------------- > > (Updated May 11, 2015, 11:16 p.m.) > > > Review request for drill, abdelhakim deneche, Jacques Nadeau, and Venki > Korukanti. > > > Repository: drill-git > > > Description > ------- > > [DRILL-2977](https://issues.apache.org/jira/browse/DRILL-2977), > [DRILL-2978](https://issues.apache.org/jira/browse/DRILL-2978): Swap fragment > execution methods and cancellation changes > > Execution: In WorkManager, > + swap implementations of startFragmentPendingRemote() and addFragmentRunner() > + warn if there are running fragments in close() > > Cancellation: > + for fragments waiting on data, delegate cancellations to WorkEventBus (in > Foreman and ControlMessageHandler) > + documentation > > > Diffs > ----- > > > exec/java-exec/src/main/java/org/apache/drill/exec/rpc/control/WorkEventBus.java > d90096a > exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java > 1d3a0b0 > > exec/java-exec/src/main/java/org/apache/drill/exec/work/batch/ControlMessageHandler.java > d12e6d5 > > exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/Foreman.java > bf62ccb > > exec/java-exec/src/main/java/org/apache/drill/exec/work/foreman/QueryManager.java > 090a377 > > exec/java-exec/src/main/java/org/apache/drill/exec/work/fragment/RootFragmentManager.java > 67ef9b8 > > Diff: https://reviews.apache.org/r/34008/diff/ > > > Testing > ------- > > Passes all unit tests and regression tests. > > > Thanks, > > Sudheesh Katkam > >
