----------------------------------------------------------- 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 > >
