Re: [PR] TEZ-4547: Add Tez AM JobID to the JobConf [tez]

2024-07-10 Thread via GitHub


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


##
tez-mapreduce/src/main/java/org/apache/tez/mapreduce/hadoop/MRJobConfig.java:
##
@@ -131,6 +131,11 @@ public interface MRJobConfig {
 
   public static final String CACHE_ARCHIVES_VISIBILITIES = 
"mapreduce.job.cache.archives.visibilities";
 
+  /**
+   * Used by committers to set a job-wide UUID.
+   */
+  public static final String JOB_COMMITTER_UUID = "job.committer.uuid";

Review Comment:
   There is a corresponding change I have in my Hadoop code where it will 
consult this property similar to how it consults the property Spark sets for 
this purpose.



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



Re: [PR] TEZ-4547: Add Tez AM JobID to the JobConf [tez]

2024-07-10 Thread via GitHub


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


##
tez-mapreduce/src/main/java/org/apache/tez/mapreduce/hadoop/MRJobConfig.java:
##
@@ -131,6 +131,11 @@ public interface MRJobConfig {
 
   public static final String CACHE_ARCHIVES_VISIBILITIES = 
"mapreduce.job.cache.archives.visibilities";
 
+  /**
+   * Used by committers to set a job-wide UUID.
+   */
+  public static final String JOB_COMMITTER_UUID = "job.committer.uuid";

Review Comment:
   This is not the setting used by s3 committer right? How will it work ?



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



Re: [PR] TEZ-4547: Add Tez AM JobID to the JobConf [tez]

2024-07-10 Thread via GitHub


tez-yetus commented on PR #339:
URL: https://github.com/apache/tez/pull/339#issuecomment-2221144651

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |::|--:|:|:|
   | +0 :ok: |  reexec  |   0m  0s |  Docker mode activated.  |
   | -1 :x: |  docker  |   0m 20s |  Docker failed to build 
yetus/tez:86b11997b.  |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | GITHUB PR | https://github.com/apache/tez/pull/339 |
   | JIRA Issue | TEZ-4547 |
   | Console output | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/9/console |
   | versions | git=2.34.1 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


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



Re: [PR] TEZ-4547: Add Tez AM JobID to the JobConf [tez]

2024-05-28 Thread via GitHub


tez-yetus commented on PR #339:
URL: https://github.com/apache/tez/pull/339#issuecomment-2136045430

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |::|--:|:|:|
   | +0 :ok: |  reexec  |  27m 44s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files 
found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any 
@author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 
1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   5m 31s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  10m 37s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 55s |  master passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  compile  |   1m 44s |  master passed with JDK Private 
Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06  |
   | +1 :green_heart: |  checkstyle  |   1m 59s |  master passed  |
   | +1 :green_heart: |  javadoc  |   1m 43s |  master passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  javadoc  |   1m 31s |  master passed with JDK Private 
Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06  |
   | +0 :ok: |  spotbugs  |   1m 20s |  Used deprecated FindBugs config; 
considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   3m 46s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 10s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   1m  9s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 17s |  the patch passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  javac  |   1m 17s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m  6s |  the patch passed with JDK 
Private Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06  |
   | +1 :green_heart: |  javac  |   1m  6s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 12s |  tez-api: The patch generated 1 
new + 16 unchanged - 0 fixed = 17 total (was 16)  |
   | -0 :warning: |  checkstyle  |   0m 18s |  tez-mapreduce: The patch 
generated 2 new + 368 unchanged - 0 fixed = 370 total (was 368)  |
   | -0 :warning: |  checkstyle  |   0m 27s |  tez-dag: The patch generated 1 
new + 281 unchanged - 0 fixed = 282 total (was 281)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace 
issues.  |
   | +1 :green_heart: |  javadoc  |   0m 53s |  the patch passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 53s |  the patch passed with JDK 
Private Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06  |
   | +1 :green_heart: |  findbugs  |   3m  3s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   2m 15s |  tez-api in the patch passed.  |
   | +1 :green_heart: |  unit  |   1m 24s |  tez-mapreduce in the patch passed. 
 |
   | +1 :green_heart: |  unit  |   5m  2s |  tez-dag in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 32s |  The patch does not generate 
ASF License warnings.  |
   |  |   |  76m 54s |   |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | Docker | ClientAPI=1.45 ServerAPI=1.45 base: 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/7/artifact/out/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/tez/pull/339 |
   | JIRA Issue | TEZ-4547 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs 
checkstyle compile |
   | uname | Linux 72ee90ea8cae 5.15.0-106-generic #116-Ubuntu SMP Wed Apr 17 
09:17:56 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/tez.sh |
   | git revision | master / 38c5aaccd |
   | Default Java | Private Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06 |
   | Multi-JDK versions | 
/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1 
/usr/lib/jvm/java-8-openjdk-amd64:Private 
Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06 |
   | checkstyle | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/7/artifact/out/diff-checkstyle-tez-api.txt
 |
   | checkstyle | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/7/artifact/out/diff-checkstyle-tez-mapreduce.txt
 |
   | checkstyle | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/7/artifact/out/diff-checkstyle-tez-dag.txt
 |
   |  Test Results | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/7/testReport/ |
   | Max. process+thread count | 410 (vs. ulimit of 5500) |
   | modules | C: tez-api tez-mapreduce tez-dag U: . |
   | Console output | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/7/console |
   | versions | git=2.34.1 maven=3.6.3 findbugs=3.0.1 |
   | Powered by 

Re: [PR] TEZ-4547: Add Tez AM JobID to the JobConf [tez]

2024-05-28 Thread via GitHub


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


##
tez-mapreduce/src/main/java/org/apache/tez/mapreduce/hadoop/MRJobConfig.java:
##
@@ -131,6 +131,11 @@ public interface MRJobConfig {
 
   public static final String CACHE_ARCHIVES_VISIBILITIES = 
"mapreduce.job.cache.archives.visibilities";
 
+  /**
+   * Used by Hadoop's MagicS3Guard and Staging committers to set a job-wide 
UUID
+   */
+  public static final String FS_S3A_COMMITTER_UUID = "fs.s3a.committer.uuid";

Review Comment:
   Renamed and changed the descriptive comment.



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



Re: [PR] TEZ-4547: Add Tez AM JobID to the JobConf [tez]

2024-05-28 Thread via GitHub


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


##
tez-dag/src/main/java/org/apache/tez/dag/app/dag/impl/OutputCommitterContextImpl.java:
##
@@ -34,14 +34,16 @@ public class OutputCommitterContextImpl implements 
OutputCommitterContext {
   private final String dagName;
   private final String vertexName;
   private final int vertexIdx;
+  private final int dagIdentifier;
   private final RootInputLeafOutput output;
 
   public OutputCommitterContextImpl(ApplicationId applicationId,
   int dagAttemptNumber,
   String dagName,
   String vertexName,
   RootInputLeafOutput output,
-  int vertexIdx) {
+  int vertexIdx,
+  int dagIdentifier) {

Review Comment:
   Refactored.



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



Re: [PR] TEZ-4547: Add Tez AM JobID to the JobConf [tez]

2024-05-14 Thread via GitHub


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


##
tez-dag/src/main/java/org/apache/tez/dag/app/dag/impl/OutputCommitterContextImpl.java:
##
@@ -34,14 +34,16 @@ public class OutputCommitterContextImpl implements 
OutputCommitterContext {
   private final String dagName;
   private final String vertexName;
   private final int vertexIdx;
+  private final int dagIdentifier;
   private final RootInputLeafOutput output;
 
   public OutputCommitterContextImpl(ApplicationId applicationId,
   int dagAttemptNumber,
   String dagName,
   String vertexName,
   RootInputLeafOutput output,
-  int vertexIdx) {
+  int vertexIdx,
+  int dagIdentifier) {

Review Comment:
   if we're already changing the signature, what about another order that looks 
better?
   is this context used somewhere else other than tez code? I guess it's not, 
we can use whatever we want, what about:
   ```
   dagAttemptNumber
   dagName
   vertexName
   dagId
   vertexId
   output
   ```



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



Re: [PR] TEZ-4547: Add Tez AM JobID to the JobConf [tez]

2024-05-14 Thread via GitHub


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


##
tez-mapreduce/src/main/java/org/apache/tez/mapreduce/hadoop/MRJobConfig.java:
##
@@ -131,6 +131,11 @@ public interface MRJobConfig {
 
   public static final String CACHE_ARCHIVES_VISIBILITIES = 
"mapreduce.job.cache.archives.visibilities";
 
+  /**
+   * Used by Hadoop's MagicS3Guard and Staging committers to set a job-wide 
UUID
+   */
+  public static final String FS_S3A_COMMITTER_UUID = "fs.s3a.committer.uuid";

Review Comment:
   I don't think this option should be "S3" specific, even if this is 
implemented due to S3
   the new option's name should tell this UUID is job-scoped



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



Re: [PR] TEZ-4547: Add Tez AM JobID to the JobConf [tez]

2024-05-07 Thread via GitHub


VenkatSNarayanan commented on PR #339:
URL: https://github.com/apache/tez/pull/339#issuecomment-2099211212

   @abstractdog @shameersss1 Is there anything else needed? 


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



Re: [PR] TEZ-4547: Add Tez AM JobID to the JobConf [tez]

2024-04-08 Thread via GitHub


tez-yetus commented on PR #339:
URL: https://github.com/apache/tez/pull/339#issuecomment-2043555966

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |::|--:|:|:|
   | +0 :ok: |  reexec  |  26m  4s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files 
found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any 
@author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 
1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   6m 25s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  12m 58s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 54s |  master passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  compile  |   1m 46s |  master passed with JDK Private 
Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06  |
   | +1 :green_heart: |  checkstyle  |   2m  8s |  master passed  |
   | +1 :green_heart: |  javadoc  |   1m 44s |  master passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  javadoc  |   1m 29s |  master passed with JDK Private 
Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06  |
   | +0 :ok: |  spotbugs  |   1m 20s |  Used deprecated FindBugs config; 
considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   3m 50s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 11s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   1m  9s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 18s |  the patch passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  javac  |   1m 18s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m  5s |  the patch passed with JDK 
Private Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06  |
   | +1 :green_heart: |  javac  |   1m  5s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 12s |  tez-api: The patch generated 1 
new + 16 unchanged - 0 fixed = 17 total (was 16)  |
   | -0 :warning: |  checkstyle  |   0m 18s |  tez-mapreduce: The patch 
generated 2 new + 368 unchanged - 0 fixed = 370 total (was 368)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace 
issues.  |
   | +1 :green_heart: |  javadoc  |   0m 53s |  the patch passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 51s |  the patch passed with JDK 
Private Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06  |
   | +1 :green_heart: |  findbugs  |   3m  5s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   2m 18s |  tez-api in the patch passed.  |
   | +1 :green_heart: |  unit  |   1m 23s |  tez-mapreduce in the patch passed. 
 |
   | +1 :green_heart: |  unit  |   4m 58s |  tez-dag in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 33s |  The patch does not generate 
ASF License warnings.  |
   |  |   |  78m 42s |   |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | Docker | ClientAPI=1.45 ServerAPI=1.45 base: 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/6/artifact/out/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/tez/pull/339 |
   | JIRA Issue | TEZ-4547 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs 
checkstyle compile |
   | uname | Linux efb9ed2193d6 5.15.0-94-generic #104-Ubuntu SMP Tue Jan 9 
15:25:40 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/tez.sh |
   | git revision | master / f080031f5 |
   | Default Java | Private Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06 |
   | Multi-JDK versions | 
/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1 
/usr/lib/jvm/java-8-openjdk-amd64:Private 
Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06 |
   | checkstyle | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/6/artifact/out/diff-checkstyle-tez-api.txt
 |
   | checkstyle | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/6/artifact/out/diff-checkstyle-tez-mapreduce.txt
 |
   |  Test Results | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/6/testReport/ |
   | Max. process+thread count | 492 (vs. ulimit of 5500) |
   | modules | C: tez-api tez-mapreduce tez-dag U: . |
   | Console output | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/6/console |
   | versions | git=2.34.1 maven=3.6.3 findbugs=3.0.1 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


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

Re: [PR] TEZ-4547: Add Tez AM JobID to the JobConf [tez]

2024-04-05 Thread via GitHub


shameersss1 commented on PR #339:
URL: https://github.com/apache/tez/pull/339#issuecomment-2039108306

   @abstractdog - Could you please review the same ?


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



Re: [PR] TEZ-4547: Add Tez AM JobID to the JobConf [tez]

2024-04-05 Thread via GitHub


tez-yetus commented on PR #339:
URL: https://github.com/apache/tez/pull/339#issuecomment-2039101580

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |::|--:|:|:|
   | +0 :ok: |  reexec  |   0m 36s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  1s |  No case conflicting files 
found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any 
@author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 
1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   6m 17s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  13m 11s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 57s |  master passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  compile  |   1m 45s |  master passed with JDK Private 
Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06  |
   | +1 :green_heart: |  checkstyle  |   2m  2s |  master passed  |
   | +1 :green_heart: |  javadoc  |   1m 46s |  master passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  javadoc  |   1m 30s |  master passed with JDK Private 
Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06  |
   | +0 :ok: |  spotbugs  |   1m 21s |  Used deprecated FindBugs config; 
considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   3m 44s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 10s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   1m  9s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 16s |  the patch passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  javac  |   1m 16s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m  5s |  the patch passed with JDK 
Private Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06  |
   | +1 :green_heart: |  javac  |   1m  5s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 12s |  tez-api: The patch generated 1 
new + 16 unchanged - 0 fixed = 17 total (was 16)  |
   | -0 :warning: |  checkstyle  |   0m 18s |  tez-mapreduce: The patch 
generated 4 new + 368 unchanged - 0 fixed = 372 total (was 368)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace 
issues.  |
   | +1 :green_heart: |  javadoc  |   0m 51s |  the patch passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 51s |  the patch passed with JDK 
Private Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06  |
   | +1 :green_heart: |  findbugs  |   3m  3s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   2m 16s |  tez-api in the patch passed.  |
   | +1 :green_heart: |  unit  |   1m 23s |  tez-mapreduce in the patch passed. 
 |
   | +1 :green_heart: |  unit  |   5m  0s |  tez-dag in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 32s |  The patch does not generate 
ASF License warnings.  |
   |  |   |  53m  6s |   |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | Docker | ClientAPI=1.45 ServerAPI=1.45 base: 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/5/artifact/out/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/tez/pull/339 |
   | JIRA Issue | TEZ-4547 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs 
checkstyle compile |
   | uname | Linux 7c897a5b673b 5.15.0-94-generic #104-Ubuntu SMP Tue Jan 9 
15:25:40 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/tez.sh |
   | git revision | master / f080031f5 |
   | Default Java | Private Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06 |
   | Multi-JDK versions | 
/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1 
/usr/lib/jvm/java-8-openjdk-amd64:Private 
Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06 |
   | checkstyle | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/5/artifact/out/diff-checkstyle-tez-api.txt
 |
   | checkstyle | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/5/artifact/out/diff-checkstyle-tez-mapreduce.txt
 |
   |  Test Results | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/5/testReport/ |
   | Max. process+thread count | 372 (vs. ulimit of 5500) |
   | modules | C: tez-api tez-mapreduce tez-dag U: . |
   | Console output | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/5/console |
   | versions | git=2.34.1 maven=3.6.3 findbugs=3.0.1 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


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

Re: [PR] TEZ-4547: Add Tez AM JobID to the JobConf [tez]

2024-04-04 Thread via GitHub


VenkatSNarayanan commented on PR #339:
URL: https://github.com/apache/tez/pull/339#issuecomment-2039000568

   @shameersss1 We could actually just set fs.s3a.committer.uuid directly 
instead of the indirection through the other setting.


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



Re: [PR] TEZ-4547: Add Tez AM JobID to the JobConf [tez]

2024-04-03 Thread via GitHub


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


##
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:
   Moved code to initialize.



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



Re: [PR] TEZ-4547: Add Tez AM JobID to the JobConf [tez]

2024-04-03 Thread via GitHub


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


##
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:
   Addressed.



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



Re: [PR] TEZ-4547: Add Tez AM JobID to the JobConf [tez]

2024-04-03 Thread via GitHub


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


##
tez-mapreduce/src/main/java/org/apache/tez/mapreduce/output/MROutput.java:
##
@@ -413,6 +414,7 @@ protected List 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:
   Addressed in update.



##
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:
   Added javadoc description.



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



Re: [PR] TEZ-4547: Add Tez AM JobID to the JobConf [tez]

2024-04-01 Thread via GitHub


tez-yetus commented on PR #339:
URL: https://github.com/apache/tez/pull/339#issuecomment-2030457385

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |::|--:|:|:|
   | +0 :ok: |  reexec  |  26m 48s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  1s |  No case conflicting files 
found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any 
@author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 
1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   5m 33s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  10m 15s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 54s |  master passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  compile  |   1m 44s |  master passed with JDK Private 
Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06  |
   | +1 :green_heart: |  checkstyle  |   2m  2s |  master passed  |
   | +1 :green_heart: |  javadoc  |   1m 41s |  master passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  javadoc  |   1m 29s |  master passed with JDK Private 
Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06  |
   | +0 :ok: |  spotbugs  |   1m 22s |  Used deprecated FindBugs config; 
considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   3m 47s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 10s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   1m  9s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 16s |  the patch passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  javac  |   1m 16s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m  5s |  the patch passed with JDK 
Private Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06  |
   | +1 :green_heart: |  javac  |   1m  5s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 13s |  tez-api: The patch generated 1 
new + 16 unchanged - 0 fixed = 17 total (was 16)  |
   | -0 :warning: |  checkstyle  |   0m 18s |  tez-mapreduce: The patch 
generated 4 new + 368 unchanged - 0 fixed = 372 total (was 368)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace 
issues.  |
   | +1 :green_heart: |  javadoc  |   0m 51s |  the patch passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 51s |  the patch passed with JDK 
Private Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06  |
   | +1 :green_heart: |  findbugs  |   3m  4s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   2m 15s |  tez-api in the patch passed.  |
   | +1 :green_heart: |  unit  |   1m 21s |  tez-mapreduce in the patch passed. 
 |
   | +1 :green_heart: |  unit  |   5m  0s |  tez-dag in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 33s |  The patch does not generate 
ASF License warnings.  |
   |  |   |  75m 27s |   |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | Docker | ClientAPI=1.45 ServerAPI=1.45 base: 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/4/artifact/out/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/tez/pull/339 |
   | JIRA Issue | TEZ-4547 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs 
checkstyle compile |
   | uname | Linux 143e09ab5003 5.15.0-94-generic #104-Ubuntu SMP Tue Jan 9 
15:25:40 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/tez.sh |
   | git revision | master / 34bb628e3 |
   | Default Java | Private Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06 |
   | Multi-JDK versions | 
/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1 
/usr/lib/jvm/java-8-openjdk-amd64:Private 
Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06 |
   | checkstyle | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/4/artifact/out/diff-checkstyle-tez-api.txt
 |
   | checkstyle | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/4/artifact/out/diff-checkstyle-tez-mapreduce.txt
 |
   |  Test Results | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/4/testReport/ |
   | Max. process+thread count | 403 (vs. ulimit of 5500) |
   | modules | C: tez-api tez-mapreduce tez-dag U: . |
   | Console output | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/4/console |
   | versions | git=2.34.1 maven=3.6.3 findbugs=3.0.1 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


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

Re: [PR] TEZ-4547: Add Tez AM JobID to the JobConf [tez]

2024-03-20 Thread via GitHub


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



Re: [PR] TEZ-4547: Add Tez AM JobID to the JobConf [tez]

2024-03-19 Thread via GitHub


tez-yetus commented on PR #339:
URL: https://github.com/apache/tez/pull/339#issuecomment-2007871466

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |::|--:|:|:|
   | +0 :ok: |  reexec  |  28m 32s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files 
found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any 
@author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 
1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +0 :ok: |  mvndep  |   6m 34s |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  14m 31s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 55s |  master passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  compile  |   1m 43s |  master passed with JDK Private 
Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06  |
   | +1 :green_heart: |  checkstyle  |   2m  9s |  master passed  |
   | +1 :green_heart: |  javadoc  |   1m 48s |  master passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  javadoc  |   1m 27s |  master passed with JDK Private 
Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06  |
   | +0 :ok: |  spotbugs  |   1m 23s |  Used deprecated FindBugs config; 
considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   3m 55s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 10s |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   1m 13s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 20s |  the patch passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  javac  |   1m 20s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 10s |  the patch passed with JDK 
Private Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06  |
   | +1 :green_heart: |  javac  |   1m 10s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 14s |  tez-api: The patch generated 1 
new + 16 unchanged - 0 fixed = 17 total (was 16)  |
   | -0 :warning: |  checkstyle  |   0m 18s |  tez-mapreduce: The patch 
generated 5 new + 368 unchanged - 0 fixed = 373 total (was 368)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace 
issues.  |
   | +1 :green_heart: |  javadoc  |   0m 54s |  the patch passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 54s |  the patch passed with JDK 
Private Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06  |
   | +1 :green_heart: |  findbugs  |   3m 18s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   2m 23s |  tez-api in the patch passed.  |
   | +1 :green_heart: |  unit  |   1m 24s |  tez-mapreduce in the patch passed. 
 |
   | +1 :green_heart: |  unit  |   5m  6s |  tez-dag in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 37s |  The patch does not generate 
ASF License warnings.  |
   |  |   |  83m 43s |   |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | Docker | ClientAPI=1.44 ServerAPI=1.44 base: 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/3/artifact/out/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/tez/pull/339 |
   | JIRA Issue | TEZ-4547 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs 
checkstyle compile |
   | uname | Linux 6d51356225eb 5.15.0-94-generic #104-Ubuntu SMP Tue Jan 9 
15:25:40 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/tez.sh |
   | git revision | master / 34bb628e3 |
   | Default Java | Private Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06 |
   | Multi-JDK versions | 
/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1 
/usr/lib/jvm/java-8-openjdk-amd64:Private 
Build-1.8.0_402-8u402-ga-2ubuntu1~22.04-b06 |
   | checkstyle | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/3/artifact/out/diff-checkstyle-tez-api.txt
 |
   | checkstyle | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/3/artifact/out/diff-checkstyle-tez-mapreduce.txt
 |
   |  Test Results | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/3/testReport/ |
   | Max. process+thread count | 507 (vs. ulimit of 5500) |
   | modules | C: tez-api tez-mapreduce tez-dag U: . |
   | Console output | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/3/console |
   | versions | git=2.34.1 maven=3.6.3 findbugs=3.0.1 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


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

Re: [PR] TEZ-4547: Add Tez AM JobID to the JobConf [tez]

2024-03-18 Thread via GitHub


VenkatSNarayanan commented on PR #339:
URL: https://github.com/apache/tez/pull/339#issuecomment-2004624012

   > If my understanding is correct, Hive/Pig would use the value from 
`mapreduce.parent.job.id` to set the correct committer UUID right?
   
   Yes, that was the plan. The property name was just chosen arbitrarily so I 
could put the PR up, any suggestions for a better one are welcome.


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



Re: [PR] TEZ-4547: Add Tez AM JobID to the JobConf [tez]

2024-03-17 Thread via GitHub


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


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

Review Comment:
   Can we have` 
String.valueOf(getContext().getApplicationId().getClusterTimestamp()), 
getContext().getApplicationId().getId()).toString(` inside a method and re-use 
it in MROutput.java as well ?



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



Re: [PR] TEZ-4547: Add Tez AM JobID to the JobConf [tez]

2024-03-13 Thread via GitHub


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


##
tez-mapreduce/src/test/java/org/apache/tez/mapreduce/output/TestMROutput.java:
##
@@ -131,6 +133,26 @@ public void testMergeConfig() throws Exception {
 assertEquals("base-value", mergedConf.get("base-key"));
   }
 
+  @Test
+  public void testParentJobIDSet() throws Exception {
+Configuration conf = new Configuration();
+conf.setBoolean(MRConfig.IS_MAP_PROCESSOR, true);
+DataSinkDescriptor dataSink = MROutput
+.createConfigBuilder(conf, TextOutputFormat.class,
+tmpDir.getPath())
+.build();
+
+OutputContext outputContext = 
createMockOutputContext(dataSink.getOutputDescriptor().getUserPayload(),
+new Configuration(false));
+MROutput output = new MROutput(outputContext, 2);
+output.initialize();
+String invalidJobID = "invalid default";
+String parentJobID = output.jobConf.get(MRJobConfig.MR_PARENT_JOB_ID, 
invalidJobID);
+assertNotEquals(parentJobID,invalidJobID);
+
assertNotEquals(output.jobConf.get(org.apache.hadoop.mapred.JobContext.TASK_ATTEMPT_ID),parentJobID);

Review Comment:
   Fix code formatting. space after ,



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



Re: [PR] TEZ-4547: Add Tez AM JobID to the JobConf [tez]

2024-03-13 Thread via GitHub


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


##
tez-mapreduce/src/main/java/org/apache/tez/mapreduce/output/MROutput.java:
##
@@ -417,6 +418,7 @@ protected List initializeBase() throws IOException, 
InterruptedException
 
.createMockTaskAttemptID(getContext().getApplicationId().getClusterTimestamp(),
 getContext().getTaskVertexIndex(), 
getContext().getApplicationId().getId(),
 getContext().getTaskIndex(), getContext().getTaskAttemptNumber(), 
isMapperOutput);
+jobConf.set(MRJobConfig.MR_PARENT_JOB_ID, new 
JobID(String.valueOf(getContext().getApplicationId().getClusterTimestamp()), 
getContext().getApplicationId().getId()).toString());

Review Comment:
   Move this to line above  TaskAttemptID taskAttemptId =



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



Re: [PR] TEZ-4547: Add Tez AM JobID to the JobConf [tez]

2024-03-13 Thread via GitHub


tez-yetus commented on PR #339:
URL: https://github.com/apache/tez/pull/339#issuecomment-1995452059

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |::|--:|:|:|
   | +0 :ok: |  reexec  |   0m 34s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files 
found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any 
@author tags.  |
   | +1 :green_heart: |  test4tests  |   0m  0s |  The patch appears to include 
1 new or modified test files.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  28m 11s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 37s |  master passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  compile  |   0m 34s |  master passed with JDK Private 
Build-1.8.0_392-8u392-ga-1~22.04-b08  |
   | +1 :green_heart: |  checkstyle  |   1m 38s |  master passed  |
   | +1 :green_heart: |  javadoc  |   0m 48s |  master passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 29s |  master passed with JDK Private 
Build-1.8.0_392-8u392-ga-1~22.04-b08  |
   | +0 :ok: |  spotbugs  |   1m 46s |  Used deprecated FindBugs config; 
considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   1m 43s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 23s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 27s |  the patch passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  javac  |   0m 27s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 22s |  the patch passed with JDK 
Private Build-1.8.0_392-8u392-ga-1~22.04-b08  |
   | +1 :green_heart: |  javac  |   0m 22s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 20s |  tez-mapreduce: The patch 
generated 7 new + 368 unchanged - 0 fixed = 375 total (was 368)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace 
issues.  |
   | +1 :green_heart: |  javadoc  |   0m 20s |  the patch passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 18s |  the patch passed with JDK 
Private Build-1.8.0_392-8u392-ga-1~22.04-b08  |
   | +1 :green_heart: |  findbugs  |   1m  0s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 40s |  tez-mapreduce in the patch passed. 
 |
   | +1 :green_heart: |  asflicense  |   0m 18s |  The patch does not generate 
ASF License warnings.  |
   |  |   |  40m 49s |   |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | Docker | ClientAPI=1.44 ServerAPI=1.44 base: 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/2/artifact/out/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/tez/pull/339 |
   | JIRA Issue | TEZ-4547 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs 
checkstyle compile |
   | uname | Linux 67ed1b8bb238 5.15.0-94-generic #104-Ubuntu SMP Tue Jan 9 
15:25:40 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/tez.sh |
   | git revision | master / 34bb628e3 |
   | Default Java | Private Build-1.8.0_392-8u392-ga-1~22.04-b08 |
   | Multi-JDK versions | 
/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1 
/usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_392-8u392-ga-1~22.04-b08 |
   | checkstyle | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/2/artifact/out/diff-checkstyle-tez-mapreduce.txt
 |
   |  Test Results | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/2/testReport/ |
   | Max. process+thread count | 227 (vs. ulimit of 5500) |
   | modules | C: tez-mapreduce U: tez-mapreduce |
   | Console output | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/2/console |
   | versions | git=2.34.1 maven=3.6.3 findbugs=3.0.1 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


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



Re: [PR] TEZ-4547: Add Tez AM JobID to the JobConf [tez]

2024-03-13 Thread via GitHub


tez-yetus commented on PR #339:
URL: https://github.com/apache/tez/pull/339#issuecomment-1995452045

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |::|--:|:|:|
   | +0 :ok: |  reexec  |  26m 49s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files 
found.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any 
@author tags.  |
   | -1 :x: |  test4tests  |   0m  0s |  The patch doesn't appear to include 
any new or modified tests. Please justify why no new tests are needed for this 
patch. Also please list what manual steps were performed to verify this patch.  
|
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |  31m 10s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 38s |  master passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  compile  |   0m 33s |  master passed with JDK Private 
Build-1.8.0_392-8u392-ga-1~22.04-b08  |
   | +1 :green_heart: |  checkstyle  |   1m 38s |  master passed  |
   | +1 :green_heart: |  javadoc  |   0m 48s |  master passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 30s |  master passed with JDK Private 
Build-1.8.0_392-8u392-ga-1~22.04-b08  |
   | +0 :ok: |  spotbugs  |   1m 45s |  Used deprecated FindBugs config; 
considering switching to SpotBugs.  |
   | +1 :green_heart: |  findbugs  |   1m 43s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   0m 24s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 27s |  the patch passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  javac  |   0m 27s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 22s |  the patch passed with JDK 
Private Build-1.8.0_392-8u392-ga-1~22.04-b08  |
   | +1 :green_heart: |  javac  |   0m 22s |  the patch passed  |
   | -0 :warning: |  checkstyle  |   0m 20s |  tez-mapreduce: The patch 
generated 3 new + 349 unchanged - 0 fixed = 352 total (was 349)  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace 
issues.  |
   | +1 :green_heart: |  javadoc  |   0m 20s |  the patch passed with JDK 
Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1  |
   | +1 :green_heart: |  javadoc  |   0m 18s |  the patch passed with JDK 
Private Build-1.8.0_392-8u392-ga-1~22.04-b08  |
   | +1 :green_heart: |  findbugs  |   1m  0s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   1m 38s |  tez-mapreduce in the patch passed. 
 |
   | +1 :green_heart: |  asflicense  |   0m 18s |  The patch does not generate 
ASF License warnings.  |
   |  |   |  70m  3s |   |
   
   
   | Subsystem | Report/Notes |
   |--:|:-|
   | Docker | ClientAPI=1.44 ServerAPI=1.44 base: 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/1/artifact/out/Dockerfile
 |
   | GITHUB PR | https://github.com/apache/tez/pull/339 |
   | JIRA Issue | TEZ-4547 |
   | Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs 
checkstyle compile |
   | uname | Linux 07d7e29a04db 5.15.0-94-generic #104-Ubuntu SMP Tue Jan 9 
15:25:40 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | personality/tez.sh |
   | git revision | master / 34bb628e3 |
   | Default Java | Private Build-1.8.0_392-8u392-ga-1~22.04-b08 |
   | Multi-JDK versions | 
/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1 
/usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_392-8u392-ga-1~22.04-b08 |
   | checkstyle | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/1/artifact/out/diff-checkstyle-tez-mapreduce.txt
 |
   |  Test Results | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/1/testReport/ |
   | Max. process+thread count | 231 (vs. ulimit of 5500) |
   | modules | C: tez-mapreduce U: tez-mapreduce |
   | Console output | 
https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-339/1/console |
   | versions | git=2.34.1 maven=3.6.3 findbugs=3.0.1 |
   | Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


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