You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by "steveloughran (via GitHub)" <gi...@apache.org> on 2026/04/12 21:00:16 UTC

[PR] MAPREDUCE-6706. TaskUmbilicalProtocol to use ProtobufRPCEngine [hadoop]

steveloughran opened a new pull request, #8422:
URL: https://github.com/apache/hadoop/pull/8422

   
   Add .proto definition of the MR TaskUmbilicalProtocol and adopt it.
   Because this is only used within an MR task, no need to worry about compatibility...
   the old protocol can be cut.
   
   Implemented pretty much entirely with copilot and claude, with human review of the results, extra comments.
   
   Note that many of the complex datatypes (jvm_task, jvm_context, TaskStatus) are actually
   marshalled into bytes as a Writable and then converted at the far end.
   What is different is there's no dynamic inclusion of class info in the messages.
   
   Also taking opportunity to look at old code and convert log statements with string concat into {} templated logs, and some other IDE-hints/actions.
   
   Contains content generated by Github Copilot.
   
   ### How was this patch tested?
   
   Updated MR
   
   ### For code changes:
   
   - [X] Does the title or this PR starts with the corresponding JIRA issue id (e.g. 'HADOOP-17799. Your PR title ...')?
   - [ ] Object storage: have the integration tests been executed and the endpoint declared according to the connector-specific documentation?
   - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
   - [ ] If applicable, have you updated the `LICENSE`, `LICENSE-binary`, `NOTICE-binary` files?
   
   ### AI Tooling
   
   If an AI tool was used:
   
   - [X] The PR includes the phrase "Contains content generated by <tool>"
         where <tool> is the name of the AI tool used.
   - [X] My use of AI contributions follows the ASF legal policy
         https://www.apache.org/legal/generative-tooling.html


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


Re: [PR] MAPREDUCE-6706. TaskUmbilicalProtocol to use ProtobufRPCEngine [hadoop]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #8422:
URL: https://github.com/apache/hadoop/pull/8422#discussion_r3070720885


##########
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/protocolPB/package-info.java:
##########
@@ -0,0 +1,29 @@
+/*

Review Comment:
   ~~why track these generated classes in the source tree? can't they be generated on-the-fly?~~
   
   noop, I misread it.



-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


Re: [PR] MAPREDUCE-6706. TaskUmbilicalProtocol to use ProtobufRPCEngine [hadoop]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #8422:
URL: https://github.com/apache/hadoop/pull/8422#discussion_r3092812851


##########
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapreduce/security/TestUmbilicalProtocolWithJobToken.java:
##########
@@ -51,10 +55,11 @@
 import org.junit.jupiter.api.Test;
 import static org.slf4j.LoggerFactory.getLogger;
 
-/** Unit tests for using Job Token over RPC. 
- * 
+/** Unit tests for using Job Token over RPC.
+ *
  * System properties required:
- * -Djava.security.krb5.conf=.../hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/target/test-classes/krb5.conf 
+ * -Djava.security.krb5.conf=.../hadoop-mapreduce-client/
+ *   hadoop-mapreduce-client-jobclient/target/test-classes/krb5.conf

Review Comment:
   why remove the leading folder, and should these two lines be merged?



-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


Re: [PR] MAPREDUCE-6706. TaskUmbilicalProtocol to use ProtobufRPCEngine [hadoop]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #8422:
URL: https://github.com/apache/hadoop/pull/8422#discussion_r3070720885


##########
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/protocolPB/package-info.java:
##########
@@ -0,0 +1,29 @@
+/*

Review Comment:
   why track these generated classes in the source tree? can't they be generated on-the-fly?



-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


Re: [PR] MAPREDUCE-6706. TaskUmbilicalProtocol to use ProtobufRPCEngine [hadoop]

Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on code in PR #8422:
URL: https://github.com/apache/hadoop/pull/8422#discussion_r3092984134


##########
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapreduce/security/TestUmbilicalProtocolWithJobToken.java:
##########
@@ -51,10 +55,11 @@
 import org.junit.jupiter.api.Test;
 import static org.slf4j.LoggerFactory.getLogger;
 
-/** Unit tests for using Job Token over RPC. 
- * 
+/** Unit tests for using Job Token over RPC.
+ *
  * System properties required:
- * -Djava.security.krb5.conf=.../hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/target/test-classes/krb5.conf 
+ * -Djava.security.krb5.conf=.../hadoop-mapreduce-client/
+ *   hadoop-mapreduce-client-jobclient/target/test-classes/krb5.conf

Review Comment:
   looks like an error. will fix



-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


Re: [PR] MAPREDUCE-6706. TaskUmbilicalProtocol to use ProtobufRPCEngine [hadoop]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #8422:
URL: https://github.com/apache/hadoop/pull/8422#discussion_r3073295927


##########
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/TaskStatus.java:
##########
@@ -523,5 +523,32 @@ static TaskStatus createTaskStatus(boolean isMap) {
     return (isMap) ? new MapTaskStatus() : new ReduceTaskStatus();
   }
 
+  /**
+   * Write a TaskStatus to a DataOutput, prefixed by an isMap discriminator byte.
+   * Used by the Protobuf-based RPC layer to serialize TaskStatus values.
+   * @param out output stream
+   * @param status status to write (must not be null)
+   * @throws IOException on I/O error
+   */
+  public static void writeTaskStatusForPB(DataOutput out, TaskStatus status)
+      throws IOException {
+    out.writeBoolean(status.getIsMap());

Review Comment:
   int or string (class name) sgtm



-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


Re: [PR] MAPREDUCE-6706. TaskUmbilicalProtocol to use ProtobufRPCEngine [hadoop]

Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #8422:
URL: https://github.com/apache/hadoop/pull/8422#issuecomment-4236793957

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m 22s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  0s |  |  detect-secrets was not available.  |
   | +0 :ok: |  xmllint  |   0m  0s |  |  xmllint was not available.  |
   | +0 :ok: |  buf  |   0m  0s |  |  buf was not available.  |
   | +0 :ok: |  buf  |   0m  0s |  |  buf was not available.  |
   | +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.  |
   |||| _ trunk Compile Tests _ |
   | +0 :ok: |  mvndep  |   1m 44s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  25m 24s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |   1m 10s |  |  trunk passed with JDK Ubuntu-21.0.10+7-Ubuntu-124.04  |
   | +1 :green_heart: |  compile  |   1m  4s |  |  trunk passed with JDK Ubuntu-17.0.18+8-Ubuntu-124.04.1  |
   | +1 :green_heart: |  checkstyle  |   0m 54s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   2m  3s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   1m 48s |  |  trunk passed with JDK Ubuntu-21.0.10+7-Ubuntu-124.04  |
   | +1 :green_heart: |  javadoc  |   1m 43s |  |  trunk passed with JDK Ubuntu-17.0.18+8-Ubuntu-124.04.1  |
   | +1 :green_heart: |  spotbugs  |   2m 40s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  15m 37s |  |  branch has no errors when building and testing our client artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 18s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   1m  3s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 54s |  |  the patch passed with JDK Ubuntu-21.0.10+7-Ubuntu-124.04  |
   | +1 :green_heart: |  cc  |   0m 54s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 54s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 51s |  |  the patch passed with JDK Ubuntu-17.0.18+8-Ubuntu-124.04.1  |
   | +1 :green_heart: |  cc  |   0m 51s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 51s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | -0 :warning: |  checkstyle  |   0m 32s | [/results-checkstyle-hadoop-mapreduce-project_hadoop-mapreduce-client.txt](https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8422/2/artifact/out/results-checkstyle-hadoop-mapreduce-project_hadoop-mapreduce-client.txt) |  hadoop-mapreduce-project/hadoop-mapreduce-client: The patch generated 8 new + 72 unchanged - 5 fixed = 80 total (was 77)  |
   | +1 :green_heart: |  mvnsite  |   1m  4s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   0m 50s |  |  the patch passed with JDK Ubuntu-21.0.10+7-Ubuntu-124.04  |
   | +1 :green_heart: |  javadoc  |   0m 49s |  |  the patch passed with JDK Ubuntu-17.0.18+8-Ubuntu-124.04.1  |
   | +1 :green_heart: |  spotbugs  |   2m 19s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  14m 46s |  |  patch has no errors when building and testing our client artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   6m 14s |  |  hadoop-mapreduce-client-core in the patch passed.  |
   | +1 :green_heart: |  unit  |   6m 22s |  |  hadoop-mapreduce-client-app in the patch passed.  |
   | +1 :green_heart: |  unit  |  95m  2s |  |  hadoop-mapreduce-client-jobclient in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 31s |  |  The patch does not generate ASF License warnings.  |
   |  |   | 188m 23s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.54 ServerAPI=1.54 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8422/2/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/8422 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient codespell detsecrets xmllint spotbugs checkstyle cc buflint bufcompat |
   | uname | Linux 4c21825637c4 5.15.0-173-generic #183-Ubuntu SMP Fri Mar 6 13:29:34 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / 916951600d44e65f7072b212397d95943c47004f |
   | Default Java | Ubuntu-17.0.18+8-Ubuntu-124.04.1 |
   | Multi-JDK versions | /usr/lib/jvm/java-21-openjdk-amd64:Ubuntu-21.0.10+7-Ubuntu-124.04 /usr/lib/jvm/java-17-openjdk-amd64:Ubuntu-17.0.18+8-Ubuntu-124.04.1 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8422/2/testReport/ |
   | Max. process+thread count | 1587 (vs. ulimit of 10000) |
   | modules | C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient U: hadoop-mapreduce-project/hadoop-mapreduce-client |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8422/2/console |
   | versions | git=2.43.0 maven=3.9.11 spotbugs=4.9.7 |
   | Powered by | Apache Yetus 0.14.1 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: common-issues-unsubscribe@hadoop.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


Re: [PR] MAPREDUCE-6706. TaskUmbilicalProtocol to use ProtobufRPCEngine2 [hadoop]

Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #8422:
URL: https://github.com/apache/hadoop/pull/8422#issuecomment-4262028139

   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |   0m  0s |  |  Docker mode activated.  |
   | -1 :x: |  patch  |   0m 16s |  |  https://github.com/apache/hadoop/pull/8422 does not apply to trunk. Rebase required? Wrong Branch? See https://cwiki.apache.org/confluence/display/HADOOP/How+To+Contribute for help.  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | GITHUB PR | https://github.com/apache/hadoop/pull/8422 |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8422/4/console |
   | versions | git=2.34.1 |
   | Powered by | Apache Yetus 0.14.1 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: common-issues-unsubscribe@hadoop.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


Re: [PR] MAPREDUCE-6706. TaskUmbilicalProtocol to use ProtobufRPCEngine [hadoop]

Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran merged PR #8422:
URL: https://github.com/apache/hadoop/pull/8422


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


Re: [PR] MAPREDUCE-6706. TaskUmbilicalProtocol to use ProtobufRPCEngine [hadoop]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #8422:
URL: https://github.com/apache/hadoop/pull/8422#issuecomment-4259682298

   nit: change ProtobufRPCEngine => ProtobufRPCEngine2 in PR title


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


Re: [PR] MAPREDUCE-6706. TaskUmbilicalProtocol to use ProtobufRPCEngine [hadoop]

Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #8422:
URL: https://github.com/apache/hadoop/pull/8422#issuecomment-4261274432

   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime |  Logfile | Comment |
   |:----:|----------:|--------:|:--------:|:-------:|
   | +0 :ok: |  reexec  |  95m 15s |  |  Docker mode activated.  |
   |||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  1s |  |  No case conflicting files found.  |
   | +0 :ok: |  codespell  |   0m  0s |  |  codespell was not available.  |
   | +0 :ok: |  detsecrets  |   0m  0s |  |  detect-secrets was not available.  |
   | +0 :ok: |  xmllint  |   0m  0s |  |  xmllint was not available.  |
   | +0 :ok: |  buf  |   0m  0s |  |  buf was not available.  |
   | +0 :ok: |  buf  |   0m  0s |  |  buf was not available.  |
   | +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.  |
   |||| _ trunk Compile Tests _ |
   | +0 :ok: |  mvndep  |   1m 47s |  |  Maven dependency ordering for branch  |
   | +1 :green_heart: |  mvninstall  |  25m 46s |  |  trunk passed  |
   | +1 :green_heart: |  compile  |   1m 12s |  |  trunk passed with JDK Ubuntu-21.0.10+7-Ubuntu-124.04  |
   | +1 :green_heart: |  compile  |   1m  9s |  |  trunk passed with JDK Ubuntu-17.0.18+8-Ubuntu-124.04.1  |
   | +1 :green_heart: |  checkstyle  |   0m 53s |  |  trunk passed  |
   | +1 :green_heart: |  mvnsite  |   2m  1s |  |  trunk passed  |
   | +1 :green_heart: |  javadoc  |   1m 47s |  |  trunk passed with JDK Ubuntu-21.0.10+7-Ubuntu-124.04  |
   | +1 :green_heart: |  javadoc  |   1m 43s |  |  trunk passed with JDK Ubuntu-17.0.18+8-Ubuntu-124.04.1  |
   | +1 :green_heart: |  spotbugs  |   2m 46s |  |  trunk passed  |
   | +1 :green_heart: |  shadedclient  |  15m 42s |  |  branch has no errors when building and testing our client artifacts.  |
   |||| _ Patch Compile Tests _ |
   | +0 :ok: |  mvndep  |   0m 17s |  |  Maven dependency ordering for patch  |
   | +1 :green_heart: |  mvninstall  |   1m  2s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 56s |  |  the patch passed with JDK Ubuntu-21.0.10+7-Ubuntu-124.04  |
   | +1 :green_heart: |  cc  |   0m 56s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 56s |  |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 51s |  |  the patch passed with JDK Ubuntu-17.0.18+8-Ubuntu-124.04.1  |
   | +1 :green_heart: |  cc  |   0m 51s |  |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 51s |  |  the patch passed  |
   | +1 :green_heart: |  blanks  |   0m  0s |  |  The patch has no blanks issues.  |
   | +1 :green_heart: |  checkstyle  |   0m 32s |  |  hadoop-mapreduce-project/hadoop-mapreduce-client: The patch generated 0 new + 71 unchanged - 6 fixed = 71 total (was 77)  |
   | +1 :green_heart: |  mvnsite  |   1m  8s |  |  the patch passed  |
   | +1 :green_heart: |  javadoc  |   0m 50s |  |  the patch passed with JDK Ubuntu-21.0.10+7-Ubuntu-124.04  |
   | +1 :green_heart: |  javadoc  |   0m 50s |  |  the patch passed with JDK Ubuntu-17.0.18+8-Ubuntu-124.04.1  |
   | +1 :green_heart: |  spotbugs  |   2m 21s |  |  the patch passed  |
   | +1 :green_heart: |  shadedclient  |  14m 42s |  |  patch has no errors when building and testing our client artifacts.  |
   |||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   6m 10s |  |  hadoop-mapreduce-client-core in the patch passed.  |
   | +1 :green_heart: |  unit  |   6m 25s |  |  hadoop-mapreduce-client-app in the patch passed.  |
   | +1 :green_heart: |  unit  |  95m 22s |  |  hadoop-mapreduce-client-jobclient in the patch passed.  |
   | +1 :green_heart: |  asflicense  |   0m 32s |  |  The patch does not generate ASF License warnings.  |
   |  |   | 283m 56s |  |  |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | ClientAPI=1.54 ServerAPI=1.54 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8422/3/artifact/out/Dockerfile |
   | GITHUB PR | https://github.com/apache/hadoop/pull/8422 |
   | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient codespell detsecrets xmllint spotbugs checkstyle cc buflint bufcompat |
   | uname | Linux fa8a57c6a5ed 5.15.0-173-generic #183-Ubuntu SMP Fri Mar 6 13:29:34 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/bin/hadoop.sh |
   | git revision | trunk / e908269ad82146dbe57717ab0aa36a354e76ac2c |
   | Default Java | Ubuntu-17.0.18+8-Ubuntu-124.04.1 |
   | Multi-JDK versions | /usr/lib/jvm/java-21-openjdk-amd64:Ubuntu-21.0.10+7-Ubuntu-124.04 /usr/lib/jvm/java-17-openjdk-amd64:Ubuntu-17.0.18+8-Ubuntu-124.04.1 |
   |  Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8422/3/testReport/ |
   | Max. process+thread count | 1595 (vs. ulimit of 10000) |
   | modules | C: hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient U: hadoop-mapreduce-project/hadoop-mapreduce-client |
   | Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8422/3/console |
   | versions | git=2.43.0 maven=3.9.11 spotbugs=4.9.7 |
   | Powered by | Apache Yetus 0.14.1 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: common-issues-unsubscribe@hadoop.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


Re: [PR] MAPREDUCE-6706. TaskUmbilicalProtocol to use ProtobufRPCEngine [hadoop]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #8422:
URL: https://github.com/apache/hadoop/pull/8422#discussion_r3070720885


##########
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/protocolPB/package-info.java:
##########
@@ -0,0 +1,29 @@
+/*

Review Comment:
   ~~why track these generated classes in the source tree? can't they be generated on-the-fly?~~



##########
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/protocolPB/package-info.java:
##########
@@ -0,0 +1,29 @@
+/*

Review Comment:
   noop, I misread it.



-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


Re: [PR] MAPREDUCE-6706. TaskUmbilicalProtocol to use ProtobufRPCEngine [hadoop]

Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on code in PR #8422:
URL: https://github.com/apache/hadoop/pull/8422#discussion_r3073095477


##########
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/TaskStatus.java:
##########
@@ -523,5 +523,32 @@ static TaskStatus createTaskStatus(boolean isMap) {
     return (isMap) ? new MapTaskStatus() : new ReduceTaskStatus();
   }
 
+  /**
+   * Write a TaskStatus to a DataOutput, prefixed by an isMap discriminator byte.
+   * Used by the Protobuf-based RPC layer to serialize TaskStatus values.
+   * @param out output stream
+   * @param status status to write (must not be null)
+   * @throws IOException on I/O error
+   */
+  public static void writeTaskStatusForPB(DataOutput out, TaskStatus status)
+      throws IOException {
+    out.writeBoolean(status.getIsMap());

Review Comment:
   if MR ever gets updated (who? why?) they can update the private protocol then. 
   
   alternatively: include the classname as the field and do a switch on it?



##########
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/protocolPB/package-info.java:
##########
@@ -0,0 +1,29 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/**
+ * This package contains the protobuf implementation of the MR TaskUmbilicalProtocol which was originally
+ * a {@code WritableRPC} protocol.
+ * To marshall some of the complex MR datatypes, such as {@code Task}, some objects are serialized to bytes
+ * and send in the RPC messages as binary objects, rather than fully defined in protobuf messages.
+ */
+
+@InterfaceAudience.Private
+package org.apache.hadoop.mapred.protocolPB;
+
+import org.apache.hadoop.classification.InterfaceAudience;

Review Comment:
   nit: newline



-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


Re: [PR] MAPREDUCE-6706. TaskUmbilicalProtocol to use ProtobufRPCEngine2 [hadoop]

Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on PR #8422:
URL: https://github.com/apache/hadoop/pull/8422#issuecomment-4835674945

   going to cherrypick to 3.5.x


-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


Re: [PR] MAPREDUCE-6706. TaskUmbilicalProtocol to use ProtobufRPCEngine [hadoop]

Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on code in PR #8422:
URL: https://github.com/apache/hadoop/pull/8422#discussion_r3078844146


##########
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/TaskStatus.java:
##########
@@ -523,5 +523,32 @@ static TaskStatus createTaskStatus(boolean isMap) {
     return (isMap) ? new MapTaskStatus() : new ReduceTaskStatus();
   }
 
+  /**
+   * Write a TaskStatus to a DataOutput, prefixed by an isMap discriminator byte.
+   * Used by the Protobuf-based RPC layer to serialize TaskStatus values.
+   * @param out output stream
+   * @param status status to write (must not be null)
+   * @throws IOException on I/O error
+   */
+  public static void writeTaskStatusForPB(DataOutput out, TaskStatus status)
+      throws IOException {
+    out.writeBoolean(status.getIsMap());

Review Comment:
   I'll enum in java code mapped to an int in protobuf.



-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


Re: [PR] MAPREDUCE-6706. TaskUmbilicalProtocol to use ProtobufRPCEngine [hadoop]

Posted by "steveloughran (via GitHub)" <gi...@apache.org>.
steveloughran commented on code in PR #8422:
URL: https://github.com/apache/hadoop/pull/8422#discussion_r3072155013


##########
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/main/java/org/apache/hadoop/mapred/protocolPB/package-info.java:
##########
@@ -0,0 +1,29 @@
+/*

Review Comment:
   not generated, that's why



-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org


Re: [PR] MAPREDUCE-6706. TaskUmbilicalProtocol to use ProtobufRPCEngine [hadoop]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #8422:
URL: https://github.com/apache/hadoop/pull/8422#discussion_r3072379145


##########
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapred/TaskStatus.java:
##########
@@ -523,5 +523,32 @@ static TaskStatus createTaskStatus(boolean isMap) {
     return (isMap) ? new MapTaskStatus() : new ReduceTaskStatus();
   }
 
+  /**
+   * Write a TaskStatus to a DataOutput, prefixed by an isMap discriminator byte.
+   * Used by the Protobuf-based RPC layer to serialize TaskStatus values.
+   * @param out output stream
+   * @param status status to write (must not be null)
+   * @throws IOException on I/O error
+   */
+  public static void writeTaskStatusForPB(DataOutput out, TaskStatus status)
+      throws IOException {
+    out.writeBoolean(status.getIsMap());

Review Comment:
   this looks a little bit hacky - in the current codebase, it is either `MapTaskStatus` or `ReduceTaskStatus`, a bool flag is sufficient to distinguish them, but this also stops the evolution possibility.
   
   if I'm not wrong, in `WritableRpcEngine`, it uses `ObjectWritable` to serialize the `Writable` instance, which carries the class name in the metadata, so no issues if we introduce a new `TaskStatus` in the future.
   
   maybe we need another PB data struct instead of a bool flag, though the evolution of the MR framework may not happen ...



-- 
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: common-issues-unsubscribe@hadoop.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org