[ 
https://issues.apache.org/jira/browse/MAPREDUCE-7538?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18085286#comment-18085286
 ] 

ASF GitHub Bot commented on MAPREDUCE-7538:
-------------------------------------------

steveloughran commented on code in PR #8331:
URL: https://github.com/apache/hadoop/pull/8331#discussion_r3336342503


##########
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/main/java/org/apache/hadoop/mapred/YARNRunner.java:
##########
@@ -503,7 +503,9 @@ private List<String> setupAMCommand(Configuration jobConf) {
       }
     }
 
-    vargs.add(MRJobConfig.APPLICATION_MASTER_CLASS);
+    String amClass = jobConf.get("yarn.app.mapreduce.am",
+        MRJobConfig.APPLICATION_MASTER_CLASS);
+    vargs.add(amClass);

Review Comment:
   Would you want to do any preflight checking here?
   
   we can't look for the class on the CP as it may arrive in the referenced 
jar, but should fail fast if the classname is null/""



##########
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/main/java/org/apache/hadoop/mapred/YARNRunner.java:
##########
@@ -503,7 +503,9 @@ private List<String> setupAMCommand(Configuration jobConf) {
       }
     }
 
-    vargs.add(MRJobConfig.APPLICATION_MASTER_CLASS);
+    String amClass = jobConf.get("yarn.app.mapreduce.am",

Review Comment:
   use getTrimmed() to trim whitespace



##########
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/main/java/org/apache/hadoop/mapred/YARNRunner.java:
##########
@@ -503,7 +503,9 @@ private List<String> setupAMCommand(Configuration jobConf) {
       }
     }
 
-    vargs.add(MRJobConfig.APPLICATION_MASTER_CLASS);
+    String amClass = jobConf.get("yarn.app.mapreduce.am",

Review Comment:
   I concur with everything @K0K0V0K says.
   - public string constant
   - updated default file
   - doc change.
   
   Test wise,
   
   1. Create a subclass of the normal MR AM which does something observable in 
startup (touch a file on a configured path?). Launch a minimal MR job and 
assert the file exists.
   
   2. Launch an MR job with a configuration that refers to a class which isn't 
present. Expect failure.
   





> Make MapReduce Application Master class configurable in YARNRunner
> ------------------------------------------------------------------
>
>                 Key: MAPREDUCE-7538
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-7538
>             Project: Hadoop Map/Reduce
>          Issue Type: New Feature
>    Affects Versions: 3.4.3
>            Reporter: Lewis John McGibbney
>            Priority: Minor
>              Labels: patch, pull-request-available
>
> YARNRunner currently always uses {{MRJobConfig.APPLICATION_MASTER_CLASS}} 
> when building the command to start the Application Master, and does not read 
> {{{}yarn.app.mapreduce.am{}}}. As a result, custom AM classes (e.g. 
> Celeborn’s {{{}MRAppMasterWithCeleborn{}}}) are never used when submitting 
> jobs via the standard job client.
> This patch changes YARNRunner to take the AM class from configuration: it 
> uses {{jobConf.get("yarn.app.mapreduce.am", 
> MRJobConfig.APPLICATION_MASTER_CLASS)}} and passes that value into the launch 
> arguments. If {{yarn.app.mapreduce.am}} is set, that class is used; otherwise 
> behavior is unchanged (default AM class). This allows remote shuffle and 
> other custom AMs to be used without forking the job client.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to