shameersss1 commented on code in PR #339:
URL: https://github.com/apache/tez/pull/339#discussion_r1531567044


##########
tez-mapreduce/src/main/java/org/apache/tez/mapreduce/committer/MROutputCommitter.java:
##########
@@ -119,6 +120,7 @@ public void abortOutput(VertexStatus.State finalState) 
throws IOException {
         || jobConf.getBoolean("mapred.mapper.new-api", false))  {
       newApiCommitter = true;
     }
+    jobConf.set(MRJobConfig.MR_JOB_UUID, 
Utils.createJobUUID(getContext().getApplicationId().getClusterTimestamp(), 
getContext().getApplicationId().getId(), getContext().getDagIdentifier()));

Review Comment:
   [Question] Shouldn't we be doing this in the `initialize` method ?



##########
tez-mapreduce/src/main/java/org/apache/tez/mapreduce/common/Utils.java:
##########
@@ -63,5 +64,8 @@ public static Counter getMRCounter(TezCounter tezCounter) {
     Objects.requireNonNull(tezCounter);
     return new MRCounters.MRCounter(tezCounter);
   }
-  
+
+  public static String createJobUUID(long clusterId, int appId, int 
dagIdentifier) {
+    return new JobID(String.valueOf(clusterId), 
appId).toString()+"_"+String.valueOf(dagIdentifier);

Review Comment:
   space after two operators 
   `return new JobID(String.valueOf(clusterId), appId).toString() + "_" + 
String.valueOf(dagIdentifier);`



##########
tez-mapreduce/src/main/java/org/apache/tez/mapreduce/hadoop/MRJobConfig.java:
##########
@@ -131,6 +131,8 @@ public interface MRJobConfig {
 
   public static final String CACHE_ARCHIVES_VISIBILITIES = 
"mapreduce.job.cache.archives.visibilities";
 
+  public static final String MR_JOB_UUID = "mapreduce.job.uuid";

Review Comment:
   Can we javadoc on the usage of this config ? Like downstream apps like Hive, 
pig etc can use this to set committer id etc.



##########
tez-mapreduce/src/main/java/org/apache/tez/mapreduce/output/MROutput.java:
##########
@@ -413,6 +414,7 @@ protected List<Event> initializeBase() throws IOException, 
InterruptedException
     }
     jobConf.setInt(MRJobConfig.APPLICATION_ATTEMPT_ID,
         getContext().getDAGAttemptNumber());
+    jobConf.set(MRJobConfig.MR_JOB_UUID, 
Utils.createJobUUID(getContext().getApplicationId().getClusterTimestamp(), 
getContext().getApplicationId().getId(), getContext().getDagIdentifier()));

Review Comment:
   check for checkstyle violation, This might exceed the limit of character in 
a line.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@tez.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to