encapsulate-non-helix-job-launch-logic #2193
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,149 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||
| * contributor license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright ownership. | ||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| * (the "License"); you may not use this file except in compliance with | ||
| * the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.apache.gobblin.cluster; | ||
|
|
||
| import java.io.IOException; | ||
| import java.util.List; | ||
|
|
||
| import org.apache.hadoop.fs.FileSystem; | ||
| import org.apache.hadoop.fs.Path; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| import com.google.common.collect.Lists; | ||
| import com.typesafe.config.ConfigFactory; | ||
|
|
||
| import org.apache.gobblin.broker.SharedResourcesBrokerFactory; | ||
| import org.apache.gobblin.broker.gobblin_scopes.GobblinScopeTypes; | ||
| import org.apache.gobblin.broker.gobblin_scopes.JobScopeInstance; | ||
| import org.apache.gobblin.broker.iface.SharedResourcesBroker; | ||
| import org.apache.gobblin.runtime.AbstractJobLauncher; | ||
| import org.apache.gobblin.runtime.GobblinMultiTaskAttempt; | ||
| import org.apache.gobblin.runtime.JobState; | ||
| import org.apache.gobblin.runtime.util.StateStores; | ||
| import org.apache.gobblin.source.workunit.MultiWorkUnit; | ||
| import org.apache.gobblin.source.workunit.WorkUnit; | ||
| import org.apache.gobblin.util.JobLauncherUtils; | ||
| import org.apache.gobblin.util.SerializationUtils; | ||
|
|
||
|
|
||
| public class SingleHelixTask { | ||
|
|
||
| private static final Logger _logger = LoggerFactory.getLogger(SingleHelixTask.class); | ||
|
|
||
| private GobblinMultiTaskAttempt taskAttempt; | ||
| private String jobId; | ||
| private Path workUnitFilePath; | ||
| private Path jobStateFilePath; | ||
| private FileSystem fs; | ||
| private TaskAttemptBuilder taskAttemptBuilder; | ||
| private StateStores stateStores; | ||
|
|
||
| public SingleHelixTask(String jobId, Path workUnitFilePath, Path jobStateFilePath, FileSystem fs, | ||
| TaskAttemptBuilder taskAttemptBuilder, StateStores stateStores) { | ||
| this.jobId = jobId; | ||
| this.workUnitFilePath = workUnitFilePath; | ||
| this.jobStateFilePath = jobStateFilePath; | ||
| this.fs = fs; | ||
| this.taskAttemptBuilder = taskAttemptBuilder; | ||
| this.stateStores = stateStores; | ||
| } | ||
|
|
||
| public int run() | ||
|
There was a problem hiding this comment. Please add java docs for the return type. |
||
| throws IOException, InterruptedException { | ||
| SharedResourcesBroker<GobblinScopeTypes> globalBroker = null; | ||
| try { | ||
| return runInternal(globalBroker); | ||
| }finally { | ||
|
|
||
| closeGlobalBroker(globalBroker); | ||
| } | ||
| } | ||
|
|
||
| private int runInternal(SharedResourcesBroker<GobblinScopeTypes> globalBroker) | ||
|
There was a problem hiding this comment. Can you put docs for the return param? |
||
| throws IOException, InterruptedException { | ||
| List<WorkUnit> workUnits = getWorkUnits(); | ||
| int workUnitSize = workUnits.size(); | ||
|
|
||
| JobState jobState = getJobState(); | ||
|
|
||
| globalBroker = SharedResourcesBrokerFactory | ||
| .createDefaultTopLevelBroker(ConfigFactory.parseProperties(jobState.getProperties()), GobblinScopeTypes.GLOBAL.defaultScopeInstance()); | ||
| SharedResourcesBroker<GobblinScopeTypes> jobBroker = | ||
| globalBroker.newSubscopedBuilder(new JobScopeInstance(jobState.getJobName(), jobState.getJobId())).build(); | ||
|
|
||
| this.taskAttempt = this.taskAttemptBuilder.build(workUnits.iterator(), this.jobId, jobState, jobBroker); | ||
| this.taskAttempt.runAndOptionallyCommitTaskAttempt(GobblinMultiTaskAttempt.CommitPolicy.IMMEDIATE); | ||
| return workUnitSize; | ||
|
|
||
| } | ||
| private void closeGlobalBroker(SharedResourcesBroker<GobblinScopeTypes> globalBroker) { | ||
|
|
||
| if (globalBroker != null) { | ||
| try { | ||
| globalBroker.close(); | ||
| } catch (IOException ioe) { | ||
| _logger.error("Could not close shared resources broker.", ioe); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private JobState getJobState() | ||
| throws java.io.IOException { | ||
| JobState jobState = new JobState(); | ||
| SerializationUtils.deserializeState(this.fs, jobStateFilePath, jobState); | ||
| return jobState; | ||
| } | ||
|
|
||
| private List<WorkUnit> getWorkUnits() | ||
| throws IOException { | ||
|
There was a problem hiding this comment. Is this a coding style that exceptions will be a new line? There was a problem hiding this comment. IDE auto formatted it based on the style config Gobblin provides. |
||
| String fileName = workUnitFilePath.getName(); | ||
| String storeName = workUnitFilePath.getParent().getName(); | ||
| WorkUnit workUnit; | ||
|
|
||
| if (workUnitFilePath.getName().endsWith(AbstractJobLauncher.MULTI_WORK_UNIT_FILE_EXTENSION)) { | ||
| workUnit = stateStores.mwuStateStore.getAll(storeName, fileName).get(0); | ||
| } else { | ||
| workUnit = stateStores.wuStateStore.getAll(storeName, fileName).get(0); | ||
| } | ||
|
|
||
| // The list of individual WorkUnits (flattened) to run | ||
| List<WorkUnit> workUnits = Lists.newArrayList(); | ||
|
|
||
| if (workUnit instanceof MultiWorkUnit) { | ||
| // Flatten the MultiWorkUnit so the job configuration properties can be added to each individual WorkUnits | ||
| List<WorkUnit> flattenedWorkUnits = JobLauncherUtils.flattenWorkUnits(((MultiWorkUnit) workUnit).getWorkUnits()); | ||
| workUnits.addAll(flattenedWorkUnits); | ||
|
There was a problem hiding this comment. Is it possible to simplify this? Maybe return flattenedWorkunits directly in if condition? In else condition just return ImmutableList.of(workunit)? There was a problem hiding this comment. Maybe, |
||
| } else { | ||
| workUnits.add(workUnit); | ||
| } | ||
| return workUnits; | ||
| } | ||
|
|
||
| public void cancel() { | ||
| if (this.taskAttempt != null) { | ||
| try { | ||
| _logger.info("Task cancelled: Shutdown starting for tasks with jobId: {}", this.jobId); | ||
| this.taskAttempt.shutdownTasks(); | ||
| _logger.info("Task cancelled: Shutdown complete for tasks with jobId: {}", this.jobId); | ||
| } catch (InterruptedException e) { | ||
| throw new RuntimeException("Interrupted while shutting down task with jobId: " + this.jobId, e); | ||
| } | ||
| } else { | ||
| _logger.error("Task cancelled but taskAttempt is null, so ignoring."); | ||
| } | ||
| } | ||
| } | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the change here? We usually avoid mixing '_' and this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The existing code doesn't use this consistently anyway.
This change is made to make logger usage more consistent and fit the google coding style and Linkedin coding style.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replaced all the fields with _ prefix and removed all usages of this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see a consistent usage of this.logger elsewhere if the convention is not to mix _ with this. Did I miss something?