K0K0V0K commented on code in PR #8331:
URL: https://github.com/apache/hadoop/pull/8331#discussion_r3246813391
##########
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:
Thanks @lewismc for the improvement!
For me, the proposal looks good overall.
Since we already have **mapreduce.job.map.class** and
**mapreduce.job.reduce.class**, I would suggest naming the new configuration
property **mapreduce.job.am.class** for consistency.
May I also ask which Java versions were tested with this change? I was
initially wondering whether the module system could cause any issues on newer
JDKs, but I think this approach should work fine.
FYI, while I am not fully aware of the intended deployment use case, I
believe there are at least two possible mechanisms for making the class
available on the NodeManagers:
* Bundling it directly with the application
* Adding it to the MapReduce framework tarball via
`org.apache.hadoop.mapred.uploader.FrameworkUploader`
Could I also ask you to add the new property here as well?
https://github.com/apache/hadoop/blob/22008a241a81a5f0a77cf55ead3546502c71df99/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-core/src/main/java/org/apache/hadoop/mapreduce/MRConfig.java#L167
That would provide at least some limited control over what kinds of classes
can be loaded through this mechanism.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
