[GOBBLIN-798] Clean up workflows from Helix when the Gobblin applicat… #2665
Conversation
…ion master starts
|
|
||
| // for cleaning up jobs on cluster manager startup | ||
| public static final String CLEAN_UP_JOBS_ON_MANAGER_START = GOBBLIN_CLUSTER_PREFIX + "cleanUpJobsOnManagerStart"; | ||
| public static final boolean DEFAULT_CLEAN_UP_JOBS_ON_MANAGER_START = false; |
There was a problem hiding this comment.
Is the default "false" to preserve the current behavior with Gobblin cluster in non-Yarn mode?
There was a problem hiding this comment.
Yes, this is to keep behavior the same.
There was a problem hiding this comment.
Removed this option since the existing behavior of not cleaning up on startup lead to the issue being fixed in this PR, so it is reasonable to not have an option to preserve the buggy behavior.
| @@ -161,4 +161,8 @@ | |||
|
|
|||
| public static final String CANCEL_RUNNING_JOB_ON_DELETE = GOBBLIN_CLUSTER_PREFIX + "job.cancelRunningJobOnDelete"; | |||
| public static final String DEFAULT_CANCEL_RUNNING_JOB_ON_DELETE = "false"; | |||
|
|
|||
| // for cleaning up jobs on cluster manager startup | |||
| public static final String CLEAN_UP_JOBS_ON_MANAGER_START = GOBBLIN_CLUSTER_PREFIX + "cleanUpJobsOnManagerStart"; | |||
There was a problem hiding this comment.
Shouldn't we always clean up on restart?
There was a problem hiding this comment.
I have this default to true for YARN mode. For standalone mode we clean up on leader ship change. For all other modes the existing behavior is maintained. I wanted to avoid changing behavior as much as possible. Initial startup already blows away the Helix cluster. This is only for yarn restart of the Gobblin application master without a restart of the application launcher.
| GobblinClusterUtils.addDynamicConfig(config), Optional.<Path>absent()); | ||
| GobblinClusterUtils.addDynamicConfig(config) | ||
| .withFallback(ConfigFactory.parseMap( | ||
| ImmutableMap.of(GobblinClusterConfigurationKeys.CLEAN_UP_JOBS_ON_MANAGER_START, "true"))), |
There was a problem hiding this comment.
Should it be ImmutableMap.of(GobblinClusterConfigurationKeys.CLEAN_UP_JOBS_ON_MANAGER_START, DEFAULT_CLEAN_UP_JOBS_ON_MANAGER_START) ?
There was a problem hiding this comment.
No, this is explicitly setting it to "true" for the Gobblin app master. All other gobblin cluster modes use the default of "false".
| private void cleanUpJobs(HelixManager helixManager) { | ||
| // Clean up existing jobs | ||
| TaskDriver taskDriver = new TaskDriver(helixManager); | ||
|
|
||
| Map<String, WorkflowConfig> workflows = taskDriver.getWorkflows(); | ||
|
|
||
| log.debug("cleanUpJobs workflow count {} workflows {}", workflows.size(), workflows); |
There was a problem hiding this comment.
Maybe just dump workflows.keySet() instead of the entire map?
| @@ -264,6 +268,12 @@ public synchronized void start() { | |||
| this.eventBus.register(this); | |||
| this.multiManager.connect(); | |||
|
|
|||
| // Standalone mode registers a handler to clean up on leadership change, so don't do the cleanup | |||
| // now even if the option to clean up on startup is set. | |||
| if (this.cleanUpJobsOnStartup && !this.isStandaloneMode) { | |||
There was a problem hiding this comment.
Is this check needed for correctness or to avoid duplicate clean up calls? If it is the latter, shouldn't the 2nd call be handled as a No-op?
There was a problem hiding this comment.
It is to avoid duplicate clean up calls. The code would be correct without this check. A standalone mode manager can transition through multiple leadership change events and there is only one clean up call per transition, so there is no need to dedupe/no-op in that code path.
Workflows will always be deleted on cluster manager startup in non-standalone mode. For standalone mode the cleanup will be in the existing leadership change path.
…ion master starts
Dear Gobblin maintainers,
Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!
JIRA
Description
If the application master aborts a new one may be spawned by YARN. The second application master will resubmit the jobs. This results in duplicate jobs in Helix and multiple instances of the job may run, resulting in duplicate data.
The Gobblin application master should clean up all workflows on startup to avoid executing multiple instances of a job.
Tests
GobblinYarnAppLauncherTest.testJobCleanup()
Commits