You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pig.apache.org by Daniel Dai <da...@gmail.com> on 2015/04/12 00:12:18 UTC
Re: Review Request 32492: PIG-4434: Improve auto-parallelism for tez
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32492/
-----------------------------------------------------------
(Updated April 11, 2015, 10:12 p.m.)
Review request for pig and Rohini Palaniswamy.
Repository: pig
Description
-------
See PIG-4434
Diffs (updated)
-----
trunk/ivy/libraries.properties 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/TezDagBuilder.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezEdgeDescriptor.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezOperPlan.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezOperator.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/LoaderProcessor.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/ParallelismSetter.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/TezEstimatedParallelismClearer.java PRE-CREATION
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/TezOperDependencyParallelismEstimator.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/runtime/PartitionerDefinedVertexManager.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/runtime/PigGraceShuffleVertexManager.java PRE-CREATION
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/util/TezCompilerUtil.java 1672805
trunk/src/org/apache/pig/impl/plan/OperatorKey.java 1672805
trunk/test/org/apache/pig/test/Util.java 1672805
trunk/test/org/apache/pig/tez/TestTezGraceParallelism.java PRE-CREATION
trunk/test/org/apache/pig/tez/TestTezJobControlCompiler.java 1672805
Diff: https://reviews.apache.org/r/32492/diff/
Testing
-------
All unit tests/e2e tests pass.
Thanks,
Daniel Dai
Re: Review Request 32492: PIG-4434: Improve auto-parallelism for tez
Posted by Daniel Dai <da...@gmail.com>.
> On April 13, 2015, 3:50 a.m., Rohini Palaniswamy wrote:
> > - What about Cross parallelism?
> > - What about updating findquantiles constant for sampling?
> > - Can we have a config to turn off grace parallelism if there are issues and fall back to the previous auto parallelism?
The cross parallelism is an optimization. As long as GFCross use the same parallelism in different vertexes, we still get the right result. And grace parallelism does not change that.
Order by / skewed join is using PartitionerDefinedVertexManager. It is not affected by grace parallelism
Adding a flag pig.tez.grace.parallelism for that.
> On April 13, 2015, 3:50 a.m., Rohini Palaniswamy wrote:
> > trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/TezDagBuilder.java, line 440
> > <https://reviews.apache.org/r/32492/diff/1/?file=905799#file905799line440>
> >
> > Why should the datamovement type be null? Can you add a comment explaining why
Put datamovement to null to prevent vertex "to" from starting. It will be started by PigGraceShuffleVertexManager
> On April 13, 2015, 3:50 a.m., Rohini Palaniswamy wrote:
> > trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezOperator.java, line 510
> > <https://reviews.apache.org/r/32492/diff/1/?file=905802#file905802line510>
> >
> > Why would plan be null?
TezOperator.plan is marked transient. It will be null in the backend (PigGraceShuffleVertexManager)
> On April 13, 2015, 3:50 a.m., Rohini Palaniswamy wrote:
> > trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/ParallelismSetter.java, line 164
> > <https://reviews.apache.org/r/32492/diff/1/?file=905804#file905804line164>
> >
> > Is this the case of vertex group for sample aggregation? In that case shouldn't we update the successor of the vertex group?
Same as before: TezOperator.plan is marked transient. It will be null in the backend (PigGraceShuffleVertexManager)
> On April 13, 2015, 3:50 a.m., Rohini Palaniswamy wrote:
> > trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/ParallelismSetter.java, line 110
> > <https://reviews.apache.org/r/32492/diff/1/?file=905804#file905804line110>
> >
> > Why do we need this check with 1-1 parallelism?
If tezOp.estimatedParallelism already set, don't override. PigGraceShuffleVertexManager set the estimated parallelism according to the output data size of the node. Add comments.
- Daniel
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32492/#review79498
-----------------------------------------------------------
On April 12, 2015, 3:51 p.m., Daniel Dai wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32492/
> -----------------------------------------------------------
>
> (Updated April 12, 2015, 3:51 p.m.)
>
>
> Review request for pig and Rohini Palaniswamy.
>
>
> Repository: pig
>
>
> Description
> -------
>
> See PIG-4434
>
>
> Diffs
> -----
>
> trunk/ivy/libraries.properties 1672805
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/TezDagBuilder.java 1672805
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezEdgeDescriptor.java 1672805
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezOperPlan.java 1672805
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezOperator.java 1672805
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/LoaderProcessor.java 1672805
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/ParallelismSetter.java 1672805
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/TezEstimatedParallelismClearer.java PRE-CREATION
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/TezOperDependencyParallelismEstimator.java 1672805
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/runtime/PartitionerDefinedVertexManager.java 1672805
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/runtime/PigGraceShuffleVertexManager.java PRE-CREATION
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/util/TezCompilerUtil.java 1672805
> trunk/src/org/apache/pig/impl/plan/OperatorKey.java 1672805
> trunk/test/org/apache/pig/newplan/logical/optimizer/TestImplicitSplitOnTuple.java 1672805
> trunk/test/org/apache/pig/test/Util.java 1672805
> trunk/test/org/apache/pig/tez/TestTezGraceParallelism.java PRE-CREATION
> trunk/test/org/apache/pig/tez/TestTezJobControlCompiler.java 1672805
> trunk/test/org/apache/pig/tez/TestTezLauncher.java 1672805
>
> Diff: https://reviews.apache.org/r/32492/diff/
>
>
> Testing
> -------
>
> All unit test pass pending (TEZ-2310), which is a Tez 0.7.0 regression not related to the patch.
>
>
> Thanks,
>
> Daniel Dai
>
>
Re: Review Request 32492: PIG-4434: Improve auto-parallelism for tez
Posted by Rohini Palaniswamy <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32492/#review79498
-----------------------------------------------------------
- What about Cross parallelism?
- What about updating findquantiles constant for sampling?
- Can we have a config to turn off grace parallelism if there are issues and fall back to the previous auto parallelism?
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/TezDagBuilder.java
<https://reviews.apache.org/r/32492/#comment129422>
Why should the datamovement type be null? Can you add a comment explaining why
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezOperPlan.java
<https://reviews.apache.org/r/32492/#comment129419>
private transient
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezOperPlan.java
<https://reviews.apache.org/r/32492/#comment129423>
Can you add some comments explaining what is done here. i.e why we terminate if there is vertexgroup, not include pred because it is too late, etc
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezOperator.java
<https://reviews.apache.org/r/32492/#comment128906>
Can you add a comment saying plan, input splits are transient as they are big and we don't want them serialized. Just to avoid someone doing that accidentally in future.
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezOperator.java
<https://reviews.apache.org/r/32492/#comment129420>
Why would plan be null?
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/ParallelismSetter.java
<https://reviews.apache.org/r/32492/#comment129426>
Why do we need this check with 1-1 parallelism?
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/ParallelismSetter.java
<https://reviews.apache.org/r/32492/#comment129425>
Is this the case of vertex group for sample aggregation? In that case shouldn't we update the successor of the vertex group?
trunk/test/org/apache/pig/test/Util.java
<https://reviews.apache.org/r/32492/#comment128904>
Can we get rid of this method and replace this with the new method added? It is only used in two test classes.
- Rohini Palaniswamy
On April 12, 2015, 3:51 p.m., Daniel Dai wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32492/
> -----------------------------------------------------------
>
> (Updated April 12, 2015, 3:51 p.m.)
>
>
> Review request for pig and Rohini Palaniswamy.
>
>
> Repository: pig
>
>
> Description
> -------
>
> See PIG-4434
>
>
> Diffs
> -----
>
> trunk/ivy/libraries.properties 1672805
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/TezDagBuilder.java 1672805
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezEdgeDescriptor.java 1672805
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezOperPlan.java 1672805
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezOperator.java 1672805
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/LoaderProcessor.java 1672805
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/ParallelismSetter.java 1672805
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/TezEstimatedParallelismClearer.java PRE-CREATION
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/TezOperDependencyParallelismEstimator.java 1672805
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/runtime/PartitionerDefinedVertexManager.java 1672805
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/runtime/PigGraceShuffleVertexManager.java PRE-CREATION
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/util/TezCompilerUtil.java 1672805
> trunk/src/org/apache/pig/impl/plan/OperatorKey.java 1672805
> trunk/test/org/apache/pig/newplan/logical/optimizer/TestImplicitSplitOnTuple.java 1672805
> trunk/test/org/apache/pig/test/Util.java 1672805
> trunk/test/org/apache/pig/tez/TestTezGraceParallelism.java PRE-CREATION
> trunk/test/org/apache/pig/tez/TestTezJobControlCompiler.java 1672805
> trunk/test/org/apache/pig/tez/TestTezLauncher.java 1672805
>
> Diff: https://reviews.apache.org/r/32492/diff/
>
>
> Testing
> -------
>
> All unit test pass pending (TEZ-2310), which is a Tez 0.7.0 regression not related to the patch.
>
>
> Thanks,
>
> Daniel Dai
>
>
Re: Review Request 32492: PIG-4434: Improve auto-parallelism for tez
Posted by Rohini Palaniswamy <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32492/#review79879
-----------------------------------------------------------
Ship it!
Ship It!
- Rohini Palaniswamy
On April 13, 2015, 7:09 a.m., Daniel Dai wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32492/
> -----------------------------------------------------------
>
> (Updated April 13, 2015, 7:09 a.m.)
>
>
> Review request for pig and Rohini Palaniswamy.
>
>
> Repository: pig
>
>
> Description
> -------
>
> See PIG-4434
>
>
> Diffs
> -----
>
> trunk/conf/pig.properties 1672805
> trunk/ivy/libraries.properties 1672805
> trunk/src/org/apache/pig/PigConfiguration.java 1672805
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/TezDagBuilder.java 1672805
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezEdgeDescriptor.java 1672805
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezOperPlan.java 1672805
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezOperator.java 1672805
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/LoaderProcessor.java 1672805
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/ParallelismSetter.java 1672805
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/TezEstimatedParallelismClearer.java PRE-CREATION
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/TezOperDependencyParallelismEstimator.java 1672805
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/runtime/PartitionerDefinedVertexManager.java 1672805
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/runtime/PigGraceShuffleVertexManager.java PRE-CREATION
> trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/util/TezCompilerUtil.java 1672805
> trunk/src/org/apache/pig/impl/plan/OperatorKey.java 1672805
> trunk/test/org/apache/pig/newplan/logical/optimizer/TestImplicitSplitOnTuple.java 1672805
> trunk/test/org/apache/pig/test/Util.java 1672805
> trunk/test/org/apache/pig/tez/TestTezAutoParallelism.java 1672805
> trunk/test/org/apache/pig/tez/TestTezGraceParallelism.java PRE-CREATION
> trunk/test/org/apache/pig/tez/TestTezJobControlCompiler.java 1672805
> trunk/test/org/apache/pig/tez/TestTezLauncher.java 1672805
>
> Diff: https://reviews.apache.org/r/32492/diff/
>
>
> Testing
> -------
>
> All unit test pass pending (TEZ-2310), which is a Tez 0.7.0 regression not related to the patch.
>
>
> Thanks,
>
> Daniel Dai
>
>
Re: Review Request 32492: PIG-4434: Improve auto-parallelism for tez
Posted by Daniel Dai <da...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32492/
-----------------------------------------------------------
(Updated April 13, 2015, 7:09 a.m.)
Review request for pig and Rohini Palaniswamy.
Repository: pig
Description
-------
See PIG-4434
Diffs (updated)
-----
trunk/conf/pig.properties 1672805
trunk/ivy/libraries.properties 1672805
trunk/src/org/apache/pig/PigConfiguration.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/TezDagBuilder.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezEdgeDescriptor.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezOperPlan.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezOperator.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/LoaderProcessor.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/ParallelismSetter.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/TezEstimatedParallelismClearer.java PRE-CREATION
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/TezOperDependencyParallelismEstimator.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/runtime/PartitionerDefinedVertexManager.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/runtime/PigGraceShuffleVertexManager.java PRE-CREATION
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/util/TezCompilerUtil.java 1672805
trunk/src/org/apache/pig/impl/plan/OperatorKey.java 1672805
trunk/test/org/apache/pig/newplan/logical/optimizer/TestImplicitSplitOnTuple.java 1672805
trunk/test/org/apache/pig/test/Util.java 1672805
trunk/test/org/apache/pig/tez/TestTezAutoParallelism.java 1672805
trunk/test/org/apache/pig/tez/TestTezGraceParallelism.java PRE-CREATION
trunk/test/org/apache/pig/tez/TestTezJobControlCompiler.java 1672805
trunk/test/org/apache/pig/tez/TestTezLauncher.java 1672805
Diff: https://reviews.apache.org/r/32492/diff/
Testing
-------
All unit test pass pending (TEZ-2310), which is a Tez 0.7.0 regression not related to the patch.
Thanks,
Daniel Dai
Re: Review Request 32492: PIG-4434: Improve auto-parallelism for tez
Posted by Daniel Dai <da...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32492/
-----------------------------------------------------------
(Updated April 12, 2015, 3:51 p.m.)
Review request for pig and Rohini Palaniswamy.
Repository: pig
Description
-------
See PIG-4434
Diffs (updated)
-----
trunk/ivy/libraries.properties 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/TezDagBuilder.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezEdgeDescriptor.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezOperPlan.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezOperator.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/LoaderProcessor.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/ParallelismSetter.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/TezEstimatedParallelismClearer.java PRE-CREATION
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/TezOperDependencyParallelismEstimator.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/runtime/PartitionerDefinedVertexManager.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/runtime/PigGraceShuffleVertexManager.java PRE-CREATION
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/util/TezCompilerUtil.java 1672805
trunk/src/org/apache/pig/impl/plan/OperatorKey.java 1672805
trunk/test/org/apache/pig/newplan/logical/optimizer/TestImplicitSplitOnTuple.java 1672805
trunk/test/org/apache/pig/test/Util.java 1672805
trunk/test/org/apache/pig/tez/TestTezGraceParallelism.java PRE-CREATION
trunk/test/org/apache/pig/tez/TestTezJobControlCompiler.java 1672805
trunk/test/org/apache/pig/tez/TestTezLauncher.java 1672805
Diff: https://reviews.apache.org/r/32492/diff/
Testing (updated)
-------
All unit test pass pending (TEZ-2310), which is a Tez 0.7.0 regression not related to the patch.
Thanks,
Daniel Dai
Re: Review Request 32492: PIG-4434: Improve auto-parallelism for tez
Posted by Daniel Dai <da...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32492/
-----------------------------------------------------------
(Updated April 11, 2015, 10:13 p.m.)
Review request for pig and Rohini Palaniswamy.
Repository: pig
Description
-------
See PIG-4434
Diffs
-----
trunk/ivy/libraries.properties 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/TezDagBuilder.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezEdgeDescriptor.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezOperPlan.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/TezOperator.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/LoaderProcessor.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/ParallelismSetter.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/TezEstimatedParallelismClearer.java PRE-CREATION
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/plan/optimizer/TezOperDependencyParallelismEstimator.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/runtime/PartitionerDefinedVertexManager.java 1672805
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/runtime/PigGraceShuffleVertexManager.java PRE-CREATION
trunk/src/org/apache/pig/backend/hadoop/executionengine/tez/util/TezCompilerUtil.java 1672805
trunk/src/org/apache/pig/impl/plan/OperatorKey.java 1672805
trunk/test/org/apache/pig/test/Util.java 1672805
trunk/test/org/apache/pig/tez/TestTezGraceParallelism.java PRE-CREATION
trunk/test/org/apache/pig/tez/TestTezJobControlCompiler.java 1672805
Diff: https://reviews.apache.org/r/32492/diff/
Testing (updated)
-------
There are some unit test failures not related to this patch, but tez 0.7.0 regression (TEZ-2310)
Thanks,
Daniel Dai