> On May 15, 2014, 10:17 p.m., Rohini Palaniswamy wrote: > > src/org/apache/pig/backend/hadoop/executionengine/tez/TezLauncher.java, > > line 147 > > <https://reviews.apache.org/r/21465/diff/1/?file=582126#file582126line147> > > > > Shouldn't it be job.getMapredJobId()?
job.getMapredJobId() returns null in Tez. But I can add getAppId() to TezJob and use it instead. Thanks for pointing this out. > On May 15, 2014, 10:17 p.m., Rohini Palaniswamy wrote: > > src/org/apache/pig/backend/hadoop/executionengine/tez/TezLauncher.java, > > line 172 > > <https://reviews.apache.org/r/21465/diff/1/?file=582126#file582126line172> > > > > Can we set id to that of the application > > (controlledjob.getMapredJobID()) as tts.getName() is already vertex name? No, this is intentional. PPNL uses tts.getJobId() to identify MR jobs / Tez vertices in a Pig job. So if I change getJobId() to return AppId in Tez, I have to maintain two code paths in PPNL for MR and Tez. I'd prefer keeping getJobId() returns vertex name in Tez. That is in fact clearly indicated in TezTaskStats code- @Override public String getJobId() { return (vertexName == null) ? "" : vertexName; } - Cheolsoo ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21465/#review43167 ----------------------------------------------------------- On May 14, 2014, 11:49 p.m., Cheolsoo Park wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/21465/ > ----------------------------------------------------------- > > (Updated May 14, 2014, 11:49 p.m.) > > > Review request for pig, Daniel Dai and Rohini Palaniswamy. > > > Bugs: PIG-3918 > https://issues.apache.org/jira/browse/PIG-3918 > > > Repository: pig-git > > > Description > ------- > > This patch makes PPNL work with Pig-on-Tez: > > * Emit event notifications to PPNL in TezLauncher. > * Add a TezJob handle to TezStats. PPNL needs to access to DAG status to > report progress. > * Along the way, two bug fixes- > * TezDagBuilder wasn't setting POLoad#lFile while constructing > POSimpleTezLoad. This causes NPE in PPNL. > * BaseOperatorPlan#connect() shouldn't add nulls to edge maps. This causes > NPE in PPNL. > * Fix unit tests. Some tests were failing after my changes because of > ClassCastException from TezScriptState to MRScriptState. > > > Diffs > ----- > > src/org/apache/pig/backend/hadoop/executionengine/tez/POSimpleTezLoad.java > f5ea1a5 > src/org/apache/pig/backend/hadoop/executionengine/tez/TezDagBuilder.java > 692cf08 > src/org/apache/pig/backend/hadoop/executionengine/tez/TezJob.java da587f8 > src/org/apache/pig/backend/hadoop/executionengine/tez/TezJobControl.java > de01a9f > src/org/apache/pig/backend/hadoop/executionengine/tez/TezLauncher.java > f5a8461 > src/org/apache/pig/newplan/BaseOperatorPlan.java d86b60b > src/org/apache/pig/tools/pigstats/tez/TezStats.java f94b954 > test/org/apache/pig/test/TestAccumulator.java 16f067a > test/org/apache/pig/test/TestBuiltin.java d33ed89 > test/org/apache/pig/test/TestLocalRearrange.java 7ffc872 > test/org/apache/pig/test/TestSkewedJoin.java 1ce3350 > > Diff: https://reviews.apache.org/r/21465/diff/ > > > Testing > ------- > > > Thanks, > > Cheolsoo Park > >
