Re: Review Request 31107: Ability to make PartitionSender multithreaded - useful in case of LocalExchange being enabled, as it allows to deal with high volume of incoming data
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31107/#review76711 --- Ship it! Ship It! - Jacques Nadeau On March 17, 2015, 4:17 a.m., Yuliya Feldman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31107/ --- (Updated March 17, 2015, 4:17 a.m.) Review request for drill, Chris Westin, Jacques Nadeau, Steven Phillips, and Venki Korukanti. Bugs: DRILL-2210 https://issues.apache.org/jira/browse/DRILL-2210 Repository: drill-git Description --- In addition to description Fixed few classes that did not handle multithreading well Added/Changed some Stats behavior to allow stats merge from multiple threads, since again this class is not suitable to be used in multithreaded environment Introduced new decorator class to handle multi thrteading (or not) to minimize changes to ParitionSenderRootExec class Diffs - exec/java-exec/src/main/java/org/apache/drill/exec/compile/CodeCompiler.java 7cc350e exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java 108f5bb exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorStats.java 0e9da0e exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/IteratorValidator.java 64cf7c5 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SendingAccountor.java 7af7b65 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java ccbd289 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/Partitioner.java 5ed9c39 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerDecorator.java PRE-CREATION exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java 1d9088a exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/Materializer.java 9b0944e exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/PlannerSettings.java abbc910 exec/java-exec/src/main/java/org/apache/drill/exec/server/DrillbitContext.java 0fb10ff exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java d821af8 exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java 99c6ab8 exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestOptiqPlans.java 478 exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/partitionsender/TestPartitionSender.java PRE-CREATION Diff: https://reviews.apache.org/r/31107/diff/ Testing --- Still need to provide Unit Tests. Functional tests are passing Performance tests were run and look promising for some queries Thanks, Yuliya Feldman
Re: Review Request 31107: Ability to make PartitionSender multithreaded - useful in case of LocalExchange being enabled, as it allows to deal with high volume of incoming data
On March 15, 2015, 1:28 p.m., Jacques Nadeau wrote: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java, line 85 https://reviews.apache.org/r/31107/diff/7/?file=889340#file889340line85 returning null here seems weird. When would that happen? Since there can be 1 partitioners current partitioner may not correspond to the index. There is a method in PartitionerDecorator that loops over partitioners and return one that matches the index. I will add comments to the method. On March 15, 2015, 1:28 p.m., Jacques Nadeau wrote: exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java, line 234 https://reviews.apache.org/r/31107/diff/7/?file=889333#file889333line234 shouldn't this parameter be instanceCount? instanceNumber seems like an index. will do On March 15, 2015, 1:28 p.m., Jacques Nadeau wrote: exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorStats.java, line 70 https://reviews.apache.org/r/31107/diff/7/?file=889334#file889334line70 missing description of what isClean means. will do - Yuliya --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31107/#review76510 --- On March 9, 2015, 9:31 a.m., Yuliya Feldman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31107/ --- (Updated March 9, 2015, 9:31 a.m.) Review request for drill, Chris Westin, Jacques Nadeau, Steven Phillips, and Venki Korukanti. Bugs: DRILL-2210 https://issues.apache.org/jira/browse/DRILL-2210 Repository: drill-git Description --- In addition to description Fixed few classes that did not handle multithreading well Added/Changed some Stats behavior to allow stats merge from multiple threads, since again this class is not suitable to be used in multithreaded environment Introduced new decorator class to handle multi thrteading (or not) to minimize changes to ParitionSenderRootExec class Diffs - exec/java-exec/src/main/java/org/apache/drill/exec/compile/CodeCompiler.java 7cc350e exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java e413921 exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorStats.java 0e9da0e exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/IteratorValidator.java 64cf7c5 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SendingAccountor.java 7af7b65 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java a23bd7a exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/Partitioner.java 5ed9c39 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerDecorator.java PRE-CREATION exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java 71ffd41 exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/Materializer.java 961b603 exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/PlannerSettings.java bbfbbcb exec/java-exec/src/main/java/org/apache/drill/exec/server/DrillbitContext.java 0fb10ff exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java 3d3e96f exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java 99c6ab8 exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestOptiqPlans.java 478 exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/partitionsender/TestPartitionSender.java PRE-CREATION Diff: https://reviews.apache.org/r/31107/diff/ Testing --- Still need to provide Unit Tests. Functional tests are passing Performance tests were run and look promising for some queries Thanks, Yuliya Feldman
Re: Review Request 31107: Ability to make PartitionSender multithreaded - useful in case of LocalExchange being enabled, as it allows to deal with high volume of incoming data
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31107/ --- (Updated March 9, 2015, 9:31 a.m.) Review request for drill, Chris Westin, Jacques Nadeau, Steven Phillips, and Venki Korukanti. Changes --- More unit tests for failure scenarios Optimized more partitioners distribution algorithm Fixed OperatorStats merging metrics Bugs: DRILL-2210 https://issues.apache.org/jira/browse/DRILL-2210 Repository: drill-git Description --- In addition to description Fixed few classes that did not handle multithreading well Added/Changed some Stats behavior to allow stats merge from multiple threads, since again this class is not suitable to be used in multithreaded environment Introduced new decorator class to handle multi thrteading (or not) to minimize changes to ParitionSenderRootExec class Diffs (updated) - exec/java-exec/src/main/java/org/apache/drill/exec/compile/CodeCompiler.java 7cc350e exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java e413921 exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorStats.java 0e9da0e exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/IteratorValidator.java 64cf7c5 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SendingAccountor.java 7af7b65 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java a23bd7a exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/Partitioner.java 5ed9c39 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerDecorator.java PRE-CREATION exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java 71ffd41 exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/Materializer.java 961b603 exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/PlannerSettings.java bbfbbcb exec/java-exec/src/main/java/org/apache/drill/exec/server/DrillbitContext.java 0fb10ff exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java 3d3e96f exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java 99c6ab8 exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestOptiqPlans.java 478 exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/partitionsender/TestPartitionSender.java PRE-CREATION Diff: https://reviews.apache.org/r/31107/diff/ Testing --- Still need to provide Unit Tests. Functional tests are passing Performance tests were run and look promising for some queries Thanks, Yuliya Feldman
Re: Review Request 31107: Ability to make PartitionSender multithreaded - useful in case of LocalExchange being enabled, as it allows to deal with high volume of incoming data
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31107/#review75779 --- Just a few minor suggestions. exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/partitionsender/TestPartitionSender.java https://reviews.apache.org/r/31107/#comment123054 Can divisor and longTail be final? exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/partitionsender/TestPartitionSender.java https://reviews.apache.org/r/31107/#comment123055 No need for a local, just throw this. - Chris Westin On March 9, 2015, 9:31 a.m., Yuliya Feldman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31107/ --- (Updated March 9, 2015, 9:31 a.m.) Review request for drill, Chris Westin, Jacques Nadeau, Steven Phillips, and Venki Korukanti. Bugs: DRILL-2210 https://issues.apache.org/jira/browse/DRILL-2210 Repository: drill-git Description --- In addition to description Fixed few classes that did not handle multithreading well Added/Changed some Stats behavior to allow stats merge from multiple threads, since again this class is not suitable to be used in multithreaded environment Introduced new decorator class to handle multi thrteading (or not) to minimize changes to ParitionSenderRootExec class Diffs - exec/java-exec/src/main/java/org/apache/drill/exec/compile/CodeCompiler.java 7cc350e exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java e413921 exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorStats.java 0e9da0e exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/IteratorValidator.java 64cf7c5 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SendingAccountor.java 7af7b65 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java a23bd7a exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/Partitioner.java 5ed9c39 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerDecorator.java PRE-CREATION exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java 71ffd41 exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/Materializer.java 961b603 exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/PlannerSettings.java bbfbbcb exec/java-exec/src/main/java/org/apache/drill/exec/server/DrillbitContext.java 0fb10ff exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java 3d3e96f exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java 99c6ab8 exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestOptiqPlans.java 478 exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/partitionsender/TestPartitionSender.java PRE-CREATION Diff: https://reviews.apache.org/r/31107/diff/ Testing --- Still need to provide Unit Tests. Functional tests are passing Performance tests were run and look promising for some queries Thanks, Yuliya Feldman
Re: Review Request 31107: Ability to make PartitionSender multithreaded - useful in case of LocalExchange being enabled, as it allows to deal with high volume of incoming data
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31107/ --- (Updated March 6, 2015, 2:59 p.m.) Review request for drill, Chris Westin, Jacques Nadeau, Steven Phillips, and Venki Korukanti. Changes --- Added Unit tests Added number of threads as a metric to PartitionSender operator Fixed partitioners distribution algorithm to do even distribution between threads Bugs: DRILL-2210 https://issues.apache.org/jira/browse/DRILL-2210 Repository: drill-git Description --- In addition to description Fixed few classes that did not handle multithreading well Added/Changed some Stats behavior to allow stats merge from multiple threads, since again this class is not suitable to be used in multithreaded environment Introduced new decorator class to handle multi thrteading (or not) to minimize changes to ParitionSenderRootExec class Diffs (updated) - exec/java-exec/src/main/java/org/apache/drill/exec/compile/CodeCompiler.java 7cc350e exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java e413921 exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorStats.java 0e9da0e exec/java-exec/src/main/java/org/apache/drill/exec/physical/config/IteratorValidator.java 64cf7c5 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SendingAccountor.java 7af7b65 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java a23bd7a exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/Partitioner.java 5ed9c39 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerDecorator.java PRE-CREATION exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java 71ffd41 exec/java-exec/src/main/java/org/apache/drill/exec/planner/fragment/Materializer.java 961b603 exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/PlannerSettings.java bbfbbcb exec/java-exec/src/main/java/org/apache/drill/exec/server/DrillbitContext.java 0fb10ff exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java 3d3e96f exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java 99c6ab8 exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestOptiqPlans.java 478 exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/partitionsender/TestPartitionSender.java PRE-CREATION Diff: https://reviews.apache.org/r/31107/diff/ Testing --- Still need to provide Unit Tests. Functional tests are passing Performance tests were run and look promising for some queries Thanks, Yuliya Feldman
Re: Review Request 31107: Ability to make PartitionSender multithreaded - useful in case of LocalExchange being enabled, as it allows to deal with high volume of incoming data
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31107/ --- (Updated Feb. 25, 2015, 11:24 p.m.) Review request for drill, Chris Westin, Jacques Nadeau, Steven Phillips, and Venki Korukanti. Changes --- Changes based on latest code review with Jacques and Venki 1. Reuse of ExecutorService from WorkManager 2. Not using anonymous objects 3. Not using callables in favor of runnables 4. other small corrections Bugs: DRILL-2210 https://issues.apache.org/jira/browse/DRILL-2210 Repository: drill-git Description --- In addition to description Fixed few classes that did not handle multithreading well Added/Changed some Stats behavior to allow stats merge from multiple threads, since again this class is not suitable to be used in multithreaded environment Introduced new decorator class to handle multi thrteading (or not) to minimize changes to ParitionSenderRootExec class Diffs (updated) - exec/java-exec/src/main/java/org/apache/drill/exec/compile/CodeCompiler.java 7cc350e exec/java-exec/src/main/java/org/apache/drill/exec/ops/FragmentContext.java e413921 exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorStats.java 0e9da0e exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SendingAccountor.java 7af7b65 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java f09acaa exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/Partitioner.java 5ed9c39 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerDecorator.java PRE-CREATION exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java 4292c09 exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/PlannerSettings.java faa8546 exec/java-exec/src/main/java/org/apache/drill/exec/server/DrillbitContext.java 83a89df exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java aa0a5ad exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java 99c6ab8 exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/TestOptiqPlans.java 478 Diff: https://reviews.apache.org/r/31107/diff/ Testing --- Still need to provide Unit Tests. Functional tests are passing Performance tests were run and look promising for some queries Thanks, Yuliya Feldman
Re: Review Request 31107: Ability to make PartitionSender multithreaded - useful in case of LocalExchange being enabled, as it allows to deal with high volume of incoming data
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31107/#review73593 --- exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerDecorator.java https://reviews.apache.org/r/31107/#comment119992 You could use Thread.currentThread() once here, and then reuse. - Chris Westin On Feb. 20, 2015, 5:39 p.m., Yuliya Feldman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31107/ --- (Updated Feb. 20, 2015, 5:39 p.m.) Review request for drill, Chris Westin, Jacques Nadeau, Steven Phillips, and Venki Korukanti. Bugs: DRILL-2210 https://issues.apache.org/jira/browse/DRILL-2210 Repository: drill-git Description --- In addition to description Fixed few classes that did not handle multithreading well Added/Changed some Stats behavior to allow stats merge from multiple threads, since again this class is not suitable to be used in multithreaded environment Introduced new decorator class to handle multi thrteading (or not) to minimize changes to ParitionSenderRootExec class Diffs - exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorStats.java 0e9da0e exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SendingAccountor.java 7af7b65 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java f09acaa exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/Partitioner.java 5ed9c39 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerDecorator.java PRE-CREATION exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java 4292c09 exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/PlannerSettings.java faa8546 exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java aa0a5ad Diff: https://reviews.apache.org/r/31107/diff/ Testing --- Still need to provide Unit Tests. Functional tests are passing Performance tests were run and look promising for some queries Thanks, Yuliya Feldman
Re: Review Request 31107: Ability to make PartitionSender multithreaded - useful in case of LocalExchange being enabled, as it allows to deal with high volume of incoming data
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31107/ --- (Updated Feb. 23, 2015, 3:29 p.m.) Review request for drill, Chris Westin, Jacques Nadeau, Steven Phillips, and Venki Korukanti. Changes --- Addressing review comments Bugs: DRILL-2210 https://issues.apache.org/jira/browse/DRILL-2210 Repository: drill-git Description --- In addition to description Fixed few classes that did not handle multithreading well Added/Changed some Stats behavior to allow stats merge from multiple threads, since again this class is not suitable to be used in multithreaded environment Introduced new decorator class to handle multi thrteading (or not) to minimize changes to ParitionSenderRootExec class Diffs (updated) - exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorStats.java 0e9da0e exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SendingAccountor.java 7af7b65 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java f09acaa exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/Partitioner.java 5ed9c39 exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerDecorator.java PRE-CREATION exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java 4292c09 exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/PlannerSettings.java faa8546 exec/java-exec/src/main/java/org/apache/drill/exec/server/options/SystemOptionManager.java aa0a5ad Diff: https://reviews.apache.org/r/31107/diff/ Testing --- Still need to provide Unit Tests. Functional tests are passing Performance tests were run and look promising for some queries Thanks, Yuliya Feldman
Re: Review Request 31107: Ability to make PartitionSender multithreaded - useful in case of LocalExchange being enabled, as it allows to deal with high volume of incoming data
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31107/#review73233
---
exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorStats.java
https://reviews.apache.org/r/31107/#comment119501
Why not call this(original, false) to avoid duplicating the code?
exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorStats.java
https://reviews.apache.org/r/31107/#comment119502
how about
final IntLongOpenHashmap fromMetrics = from.longMetrics;
final long[] fromValues = fromMetrics.values;
for(int i : fromMetrics.keys) {
final long value = fromValues[i];
longMetrics.putOrAdd(i, value, value);
}
This avoids multiple evaluation of the member accesses on every pass of the
loop, and multiple bounds checks within the loop body.
Similar improvements can be made to the doubleMetrics loop below.
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SendingAccountor.java
https://reviews.apache.org/r/31107/#comment119503
I'd make these final too, while you're here.
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SendingAccountor.java
https://reviews.apache.org/r/31107/#comment119504
This should do something other than swallow InterruptedException. See
http://www.ibm.com/developerworks/library/j-jtp05236/ .
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java
https://reviews.apache.org/r/31107/#comment119506
Reuse the id variable from above (which looks like it should be made final).
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerDecorator.java
https://reviews.apache.org/r/31107/#comment119507
Could this be final?
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerDecorator.java
https://reviews.apache.org/r/31107/#comment119508
Call part.getStats() once at the beginning, and reuse the local throughout.
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerDecorator.java
https://reviews.apache.org/r/31107/#comment119510
part.getStats() once.
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerDecorator.java
https://reviews.apache.org/r/31107/#comment119511
Call Thread.currentThread() once at the beginning, and then reuse.
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerDecorator.java
https://reviews.apache.org/r/31107/#comment119513
Would it make sense to restore the original thread name here? If so, wrap
the iface.execute() in a try and restore the name in a finally.
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerDecorator.java
https://reviews.apache.org/r/31107/#comment119514
Is IOException the best choice here? Would RuntimeException make more
sense? Either way, would a message be helpful in case there are problems? How
would a user know what to do if this exception fired?
- Chris Westin
On Feb. 17, 2015, 12:31 a.m., Yuliya Feldman wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31107/
---
(Updated Feb. 17, 2015, 12:31 a.m.)
Review request for drill, Chris Westin, Jacques Nadeau, Steven Phillips, and
Venki Korukanti.
Bugs: DRILL-2210
https://issues.apache.org/jira/browse/DRILL-2210
Repository: drill-git
Description
---
In addition to description
Fixed few classes that did not handle multithreading well
Added/Changed some Stats behavior to allow stats merge from multiple threads,
since again this class is not suitable to be used in multithreaded environment
Introduced new decorator class to handle multi thrteading (or not) to
minimize changes to ParitionSenderRootExec class
Diffs
-
exec/java-exec/src/main/java/org/apache/drill/exec/ops/OperatorStats.java
0e9da0e
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/SendingAccountor.java
7af7b65
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionSenderRootExec.java
f09acaa
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/Partitioner.java
5ed9c39
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerDecorator.java
PRE-CREATION
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/partitionsender/PartitionerTemplate.java
4292c09
exec/java-exec/src/main/java/org/apache/drill/exec/planner/physical/PlannerSettings.java
faa8546
