[
https://issues.apache.org/jira/browse/GOBBLIN-1914?focusedWorklogId=881044&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-881044
]
ASF GitHub Bot logged work on GOBBLIN-1914:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 20/Sep/23 23:04
Start Date: 20/Sep/23 23:04
Worklog Time Spent: 10m
Work Description: homatthew commented on code in PR #3781:
URL: https://github.com/apache/gobblin/pull/3781#discussion_r1332259928
##########
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:
With this originally being hard coded for so many years, I wasn't sure if it
made sense to change this or not. I had some concerns about if any assumptions
were made about the name of this class but for external systems.
I saw a few choices:
1. Change the log file name to always be the same as the class impl name
2. Make the log file name configurable to anything arbtirary, with the
default being the log file name
3. Keep the log file always GobblinApplicationMaster.*
I didn't see much value in actually changing the name of the log files when
changing the app master implementation class. To me, the first container
launched by the `YarnAppLauncher` is always a yarn `ApplicationMaster`. So
it's never incorrect to refer to the log files produced by that container as a
`GobblinApplicationMaster`. What are your thoughts? Do you think one of the
other 2 choices makes more sense?
Issue Time Tracking
-------------------
Worklog Id: (was: 881044)
Time Spent: 0.5h (was: 20m)
> Add ability to determine App master class from runtime config
> -------------------------------------------------------------
>
> Key: GOBBLIN-1914
> URL: https://issues.apache.org/jira/browse/GOBBLIN-1914
> Project: Apache Gobblin
> Issue Type: Improvement
> Reporter: Matthew Ho
> Priority: Major
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
