[GitHub] [gobblin] ZihanLi58 commented on a diff in pull request #3781: [GOBBLIN-1914] Configurable Gobblin Application Master class for Yarn

["ZihanLi58 (via GitHub)" <gi...@apache.org>]

ZihanLi58 commented on code in PR #3781:
URL: https://github.com/apache/gobblin/pull/3781#discussion_r1332194662


##########
gobblin-yarn/src/main/java/org/apache/gobblin/yarn/GobblinYarnAppLauncher.java:
##########
@@ -801,23 +801,25 @@ private void addJobConfPackage(String jobConfPackagePath, Path destDir, Map<Stri
 
   @VisibleForTesting
   protected String buildApplicationMasterCommand(String applicationId, int memoryMbs) {
-    String appMasterClassName = GobblinApplicationMaster.class.getSimpleName();
+    String appMasterClass = ConfigUtils.getString(
+       config, GobblinYarnConfigurationKeys.APP_MASTER_CLASS, GobblinYarnConfigurationKeys.DEFAULT_APP_MASTER_CLASS);
+    String logFileName = GobblinApplicationMaster.class.getSimpleName();
     return new StringBuilder()
         .append(ApplicationConstants.Environment.JAVA_HOME.$()).append("/bin/java")
         .append(" -Xmx").append((int) (memoryMbs * this.jvmMemoryXmxRatio) - this.jvmMemoryOverheadMbs).append("M")
         .append(" -D").append(GobblinYarnConfigurationKeys.JVM_USER_TIMEZONE_CONFIG).append("=").append(this.containerTimezone)
         .append(" -D").append(GobblinYarnConfigurationKeys.GOBBLIN_YARN_CONTAINER_LOG_DIR_NAME).append("=").append(ApplicationConstants.LOG_DIR_EXPANSION_VAR)
-        .append(" -D").append(GobblinYarnConfigurationKeys.GOBBLIN_YARN_CONTAINER_LOG_FILE_NAME).append("=").append(appMasterClassName).append(".").append(ApplicationConstants.STDOUT)
+        .append(" -D").append(GobblinYarnConfigurationKeys.GOBBLIN_YARN_CONTAINER_LOG_FILE_NAME).append("=").append(logFileName).append(".").append(ApplicationConstants.STDOUT)
         .append(" ").append(JvmUtils.formatJvmArguments(this.appMasterJvmArgs))
-        .append(" ").append(GobblinApplicationMaster.class.getName())
+        .append(" ").append(appMasterClass)
         .append(" --").append(GobblinClusterConfigurationKeys.APPLICATION_NAME_OPTION_NAME)
         .append(" ").append(this.applicationName)
         .append(" --").append(GobblinClusterConfigurationKeys.APPLICATION_ID_OPTION_NAME)
         .append(" ").append(applicationId)
         .append(" 1>").append(ApplicationConstants.LOG_DIR_EXPANSION_VAR).append(File.separator).append(
-            appMasterClassName).append(".").append(ApplicationConstants.STDOUT)
+            logFileName).append(".").append(ApplicationConstants.STDOUT)

Review Comment:
   Why do we want to keep the log name as the GobblinApplicationMaster class name even we change the class? should we just keep it to be appMasterClass?



-- 
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: dev-unsubscribe@gobblin.apache.org

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