[jira] [Updated] (GOBBLIN-796) Add support partial updates for flowConfig
[ https://issues.apache.org/jira/browse/GOBBLIN-796?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jack Moseley updated GOBBLIN-796: - Summary: Add support partial updates for flowConfig (was: Add option to trigger a flow) > Add support partial updates for flowConfig > -- > > Key: GOBBLIN-796 > URL: https://issues.apache.org/jira/browse/GOBBLIN-796 > Project: Apache Gobblin > Issue Type: Improvement >Reporter: Jack Moseley >Priority: Major > Time Spent: 1h 10m > Remaining Estimate: 0h > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (GOBBLIN-796) Add option to trigger a flow
[ https://issues.apache.org/jira/browse/GOBBLIN-796?focusedWorklogId=256304=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-256304 ] ASF GitHub Bot logged work on GOBBLIN-796: -- Author: ASF GitHub Bot Created on: 08/Jun/19 00:34 Start Date: 08/Jun/19 00:34 Worklog Time Spent: 10m Work Description: jack-moseley commented on issue #2663: [GOBBLIN-796] Add support partial updates for flowConfig URL: https://github.com/apache/incubator-gobblin/pull/2663#issuecomment-500077560 @sv2000 as suggested, repurposed this PR to add partial updates for flow config and then triggering a flow can be done by partial update with `runImmediately=true`. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 256304) Time Spent: 1h 10m (was: 1h) > Add option to trigger a flow > > > Key: GOBBLIN-796 > URL: https://issues.apache.org/jira/browse/GOBBLIN-796 > Project: Apache Gobblin > Issue Type: Improvement >Reporter: Jack Moseley >Priority: Major > Time Spent: 1h 10m > Remaining Estimate: 0h > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] [incubator-gobblin] jack-moseley commented on issue #2663: [GOBBLIN-796] Add support partial updates for flowConfig
jack-moseley commented on issue #2663: [GOBBLIN-796] Add support partial updates for flowConfig URL: https://github.com/apache/incubator-gobblin/pull/2663#issuecomment-500077560 @sv2000 as suggested, repurposed this PR to add partial updates for flow config and then triggering a flow can be done by partial update with `runImmediately=true`. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[jira] [Work logged] (GOBBLIN-798) Clean up workflows from Helix when the Gobblin application master starts
[ https://issues.apache.org/jira/browse/GOBBLIN-798?focusedWorklogId=256244=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-256244 ] ASF GitHub Bot logged work on GOBBLIN-798: -- Author: ASF GitHub Bot Created on: 07/Jun/19 21:50 Start Date: 07/Jun/19 21:50 Worklog Time Spent: 10m Work Description: htran1 commented on pull request #2665: [GOBBLIN-798] Clean up workflows from Helix when the Gobblin applicat… URL: https://github.com/apache/incubator-gobblin/pull/2665#discussion_r291765182 ## File path: gobblin-cluster/src/main/java/org/apache/gobblin/cluster/GobblinClusterConfigurationKeys.java ## @@ -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"; + public static final boolean DEFAULT_CLEAN_UP_JOBS_ON_MANAGER_START = false; Review comment: Yes, this is to keep behavior the same. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 256244) Time Spent: 1h 40m (was: 1.5h) > Clean up workflows from Helix when the Gobblin application master starts > > > Key: GOBBLIN-798 > URL: https://issues.apache.org/jira/browse/GOBBLIN-798 > Project: Apache Gobblin > Issue Type: Task >Reporter: Hung Tran >Assignee: Hung Tran >Priority: Major > Time Spent: 1h 40m > Remaining Estimate: 0h > > 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. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] [incubator-gobblin] htran1 commented on a change in pull request #2665: [GOBBLIN-798] Clean up workflows from Helix when the Gobblin applicat…
htran1 commented on a change in pull request #2665: [GOBBLIN-798] Clean up workflows from Helix when the Gobblin applicat… URL: https://github.com/apache/incubator-gobblin/pull/2665#discussion_r291765182 ## File path: gobblin-cluster/src/main/java/org/apache/gobblin/cluster/GobblinClusterConfigurationKeys.java ## @@ -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"; + public static final boolean DEFAULT_CLEAN_UP_JOBS_ON_MANAGER_START = false; Review comment: Yes, this is to keep behavior the same. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[jira] [Work logged] (GOBBLIN-798) Clean up workflows from Helix when the Gobblin application master starts
[ https://issues.apache.org/jira/browse/GOBBLIN-798?focusedWorklogId=256240=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-256240 ] ASF GitHub Bot logged work on GOBBLIN-798: -- Author: ASF GitHub Bot Created on: 07/Jun/19 21:42 Start Date: 07/Jun/19 21:42 Worklog Time Spent: 10m Work Description: htran1 commented on pull request #2665: [GOBBLIN-798] Clean up workflows from Helix when the Gobblin applicat… URL: https://github.com/apache/incubator-gobblin/pull/2665#discussion_r291763549 ## File path: gobblin-cluster/src/main/java/org/apache/gobblin/cluster/GobblinHelixMultiManager.java ## @@ -363,12 +359,26 @@ void handleLeadershipChange(NotificationContext changeContext) { } } + /** + * Delete jobs from the helix cluster + */ + @VisibleForTesting + public void cleanUpJobs() { +cleanUpJobs(this.jobClusterHelixManager); + +if (this.taskDriverHelixManager.isPresent()) { + cleanUpJobs(this.taskDriverHelixManager.get()); +} + } + private void cleanUpJobs(HelixManager helixManager) { // Clean up existing jobs TaskDriver taskDriver = new TaskDriver(helixManager); Map workflows = taskDriver.getWorkflows(); +log.debug("cleanUpJobs workflow count {} workflows {}", workflows.size(), workflows); Review comment: Sure. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 256240) Time Spent: 1h 20m (was: 1h 10m) > Clean up workflows from Helix when the Gobblin application master starts > > > Key: GOBBLIN-798 > URL: https://issues.apache.org/jira/browse/GOBBLIN-798 > Project: Apache Gobblin > Issue Type: Task >Reporter: Hung Tran >Assignee: Hung Tran >Priority: Major > Time Spent: 1h 20m > Remaining Estimate: 0h > > 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. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (GOBBLIN-798) Clean up workflows from Helix when the Gobblin application master starts
[ https://issues.apache.org/jira/browse/GOBBLIN-798?focusedWorklogId=256239=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-256239 ] ASF GitHub Bot logged work on GOBBLIN-798: -- Author: ASF GitHub Bot Created on: 07/Jun/19 21:39 Start Date: 07/Jun/19 21:39 Worklog Time Spent: 10m Work Description: htran1 commented on pull request #2665: [GOBBLIN-798] Clean up workflows from Helix when the Gobblin applicat… URL: https://github.com/apache/incubator-gobblin/pull/2665#discussion_r291762902 ## File path: gobblin-yarn/src/main/java/org/apache/gobblin/yarn/GobblinApplicationMaster.java ## @@ -81,7 +83,10 @@ public GobblinApplicationMaster(String applicationName, ContainerId containerId, Config config, YarnConfiguration yarnConfiguration) throws Exception { super(applicationName, containerId.getApplicationAttemptId().getApplicationId().toString(), -GobblinClusterUtils.addDynamicConfig(config), Optional.absent()); +GobblinClusterUtils.addDynamicConfig(config) +.withFallback(ConfigFactory.parseMap( + ImmutableMap.of(GobblinClusterConfigurationKeys.CLEAN_UP_JOBS_ON_MANAGER_START, "true"))), Review comment: No, this is explicitly setting it to "true" for the Gobblin app master. All other gobblin cluster modes use the default of "false". 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 256239) Time Spent: 1h 10m (was: 1h) > Clean up workflows from Helix when the Gobblin application master starts > > > Key: GOBBLIN-798 > URL: https://issues.apache.org/jira/browse/GOBBLIN-798 > Project: Apache Gobblin > Issue Type: Task >Reporter: Hung Tran >Assignee: Hung Tran >Priority: Major > Time Spent: 1h 10m > Remaining Estimate: 0h > > 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. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] [incubator-gobblin] htran1 commented on a change in pull request #2665: [GOBBLIN-798] Clean up workflows from Helix when the Gobblin applicat…
htran1 commented on a change in pull request #2665: [GOBBLIN-798] Clean up workflows from Helix when the Gobblin applicat… URL: https://github.com/apache/incubator-gobblin/pull/2665#discussion_r291762902 ## File path: gobblin-yarn/src/main/java/org/apache/gobblin/yarn/GobblinApplicationMaster.java ## @@ -81,7 +83,10 @@ public GobblinApplicationMaster(String applicationName, ContainerId containerId, Config config, YarnConfiguration yarnConfiguration) throws Exception { super(applicationName, containerId.getApplicationAttemptId().getApplicationId().toString(), -GobblinClusterUtils.addDynamicConfig(config), Optional.absent()); +GobblinClusterUtils.addDynamicConfig(config) +.withFallback(ConfigFactory.parseMap( + ImmutableMap.of(GobblinClusterConfigurationKeys.CLEAN_UP_JOBS_ON_MANAGER_START, "true"))), Review comment: No, this is explicitly setting it to "true" for the Gobblin app master. All other gobblin cluster modes use the default of "false". 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[jira] [Work logged] (GOBBLIN-798) Clean up workflows from Helix when the Gobblin application master starts
[ https://issues.apache.org/jira/browse/GOBBLIN-798?focusedWorklogId=256236=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-256236 ] ASF GitHub Bot logged work on GOBBLIN-798: -- Author: ASF GitHub Bot Created on: 07/Jun/19 21:36 Start Date: 07/Jun/19 21:36 Worklog Time Spent: 10m Work Description: htran1 commented on pull request #2665: [GOBBLIN-798] Clean up workflows from Helix when the Gobblin applicat… URL: https://github.com/apache/incubator-gobblin/pull/2665#discussion_r291762313 ## File path: gobblin-cluster/src/main/java/org/apache/gobblin/cluster/GobblinClusterConfigurationKeys.java ## @@ -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"; Review 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. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 256236) Time Spent: 1h (was: 50m) > Clean up workflows from Helix when the Gobblin application master starts > > > Key: GOBBLIN-798 > URL: https://issues.apache.org/jira/browse/GOBBLIN-798 > Project: Apache Gobblin > Issue Type: Task >Reporter: Hung Tran >Assignee: Hung Tran >Priority: Major > Time Spent: 1h > Remaining Estimate: 0h > > 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. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] [incubator-gobblin] htran1 commented on a change in pull request #2665: [GOBBLIN-798] Clean up workflows from Helix when the Gobblin applicat…
htran1 commented on a change in pull request #2665: [GOBBLIN-798] Clean up workflows from Helix when the Gobblin applicat… URL: https://github.com/apache/incubator-gobblin/pull/2665#discussion_r291762313 ## File path: gobblin-cluster/src/main/java/org/apache/gobblin/cluster/GobblinClusterConfigurationKeys.java ## @@ -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"; Review 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. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[jira] [Work logged] (GOBBLIN-800) Remove the metric context cache from GobblinMetricsRegistry
[ https://issues.apache.org/jira/browse/GOBBLIN-800?focusedWorklogId=256229=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-256229 ] ASF GitHub Bot logged work on GOBBLIN-800: -- Author: ASF GitHub Bot Created on: 07/Jun/19 21:23 Start Date: 07/Jun/19 21:23 Worklog Time Spent: 10m Work Description: sv2000 commented on pull request #2667: [GOBBLIN-800] Remove the metric context cache from GobblinMetricsRegistry URL: https://github.com/apache/incubator-gobblin/pull/2667#discussion_r291757745 ## File path: gobblin-runtime/src/main/java/org/apache/gobblin/runtime/util/ForkMetrics.java ## @@ -0,0 +1,80 @@ +/* + * 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.runtime.util; + +import java.util.List; +import java.util.concurrent.Callable; + +import com.google.common.collect.ImmutableList; + +import org.apache.gobblin.configuration.ConfigurationKeys; +import org.apache.gobblin.configuration.State; +import org.apache.gobblin.metrics.GobblinMetrics; +import org.apache.gobblin.metrics.MetricContext; +import org.apache.gobblin.metrics.Tag; +import org.apache.gobblin.runtime.TaskState; +import org.apache.gobblin.runtime.fork.Fork; + +/** + * An extension to {@link GobblinMetrics} specifically for {@link Fork}. + */ +public class ForkMetrics extends GobblinMetrics { + private static final String FORK_METRICS_BRANCH_NAME_KEY = "forkBranchName"; + + protected ForkMetrics(TaskState taskState, int index) { +super(name(taskState, index), parentContextForFork(taskState), getForkMetricsTags(taskState, index)); + } + + private static MetricContext parentContextForFork(TaskState taskState) { +return TaskMetrics.get(METRICS_ID_PREFIX + taskState.getJobId() + "." + taskState.getTaskId()).getMetricContext(); + } + + public static ForkMetrics get(final TaskState taskState, int index) { +return (ForkMetrics) GOBBLIN_METRICS_REGISTRY.getOrDefault(name(taskState, index), new Callable() { + @Override + public GobblinMetrics call() throws Exception { +return new ForkMetrics(taskState, index); + } +}); + } + + /** + * Creates a unique {@link String} representing this branch. + */ + private static String getForkMetricsId(State state, int index) { +return state.getProp(ConfigurationKeys.FORK_BRANCH_NAME_KEY + "." + index, +ConfigurationKeys.DEFAULT_FORK_BRANCH_NAME + index); + } + + /** + * Creates a {@link List} of {@link Tag}s for a {@link Fork} instance. The {@link Tag}s are purely based on the + * index and the branch name. + */ + private static List> getForkMetricsTags(State state, int index) { +return ImmutableList.>of(new Tag<>(FORK_METRICS_BRANCH_NAME_KEY, getForkMetricsId(state, index))); Review comment: Maybe add a null check here i.e. if (getForkMetricsId(state,index) != null)? Else, ImmutableList.of() may throw an exception. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 256229) Time Spent: 0.5h (was: 20m) > Remove the metric context cache from GobblinMetricsRegistry > --- > > Key: GOBBLIN-800 > URL: https://issues.apache.org/jira/browse/GOBBLIN-800 > Project: Apache Gobblin > Issue Type: Bug >Reporter: Kuai Yu >Priority: Major > Time Spent: 0.5h > Remaining Estimate: 0h > > Remove the metric context cache from GobblinMetricsRegistry -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (GOBBLIN-800) Remove the metric context cache from GobblinMetricsRegistry
[ https://issues.apache.org/jira/browse/GOBBLIN-800?focusedWorklogId=256228=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-256228 ] ASF GitHub Bot logged work on GOBBLIN-800: -- Author: ASF GitHub Bot Created on: 07/Jun/19 21:23 Start Date: 07/Jun/19 21:23 Worklog Time Spent: 10m Work Description: sv2000 commented on pull request #2667: [GOBBLIN-800] Remove the metric context cache from GobblinMetricsRegistry URL: https://github.com/apache/incubator-gobblin/pull/2667#discussion_r291756656 ## File path: gobblin-runtime/src/main/java/org/apache/gobblin/runtime/util/TaskMetrics.java ## @@ -61,15 +62,35 @@ public GobblinMetrics call() throws Exception { /** * Remove the {@link TaskMetrics} instance for the task with the given {@link TaskMetrics} instance. + * Please note this method is invoked by job driver so it won't delete any underlying {@link ForkMetrics} + * because the {@link org.apache.gobblin.runtime.fork.Fork} can be created on different nodes. * * @param taskState the given {@link TaskState} instance */ public static void remove(TaskState taskState) { remove(name(taskState)); } + /** + * Remove the {@link TaskMetrics} instance for the task with the given {@link TaskMetrics} instance. + * Please note that this will also delete the underlying {@link ForkMetrics} related to this specific task. + * + * @param task the given task instance + */ + public static void remove(Task task) { +task.getForks().forEach(forkOpt -> { + remove(ForkMetrics.name(task.getTaskState(), forkOpt.get().getIndex())); +}); + +remove(name(task)); + } + private static String name(TaskState taskState) { -return "gobblin.metrics." + taskState.getJobId() + "." + taskState.getTaskId(); +return METRICS_ID_PREFIX + taskState.getJobId() + "." + taskState.getTaskId(); + } + + private static String name(Task task) { +return METRICS_ID_PREFIX + task.getJobId() + "." + task.getTaskId(); Review comment: return name(task.getTaskState())? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 256228) Time Spent: 0.5h (was: 20m) > Remove the metric context cache from GobblinMetricsRegistry > --- > > Key: GOBBLIN-800 > URL: https://issues.apache.org/jira/browse/GOBBLIN-800 > Project: Apache Gobblin > Issue Type: Bug >Reporter: Kuai Yu >Priority: Major > Time Spent: 0.5h > Remaining Estimate: 0h > > Remove the metric context cache from GobblinMetricsRegistry -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] [incubator-gobblin] sv2000 commented on a change in pull request #2667: [GOBBLIN-800] Remove the metric context cache from GobblinMetricsRegistry
sv2000 commented on a change in pull request #2667: [GOBBLIN-800] Remove the metric context cache from GobblinMetricsRegistry URL: https://github.com/apache/incubator-gobblin/pull/2667#discussion_r291757745 ## File path: gobblin-runtime/src/main/java/org/apache/gobblin/runtime/util/ForkMetrics.java ## @@ -0,0 +1,80 @@ +/* + * 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.runtime.util; + +import java.util.List; +import java.util.concurrent.Callable; + +import com.google.common.collect.ImmutableList; + +import org.apache.gobblin.configuration.ConfigurationKeys; +import org.apache.gobblin.configuration.State; +import org.apache.gobblin.metrics.GobblinMetrics; +import org.apache.gobblin.metrics.MetricContext; +import org.apache.gobblin.metrics.Tag; +import org.apache.gobblin.runtime.TaskState; +import org.apache.gobblin.runtime.fork.Fork; + +/** + * An extension to {@link GobblinMetrics} specifically for {@link Fork}. + */ +public class ForkMetrics extends GobblinMetrics { + private static final String FORK_METRICS_BRANCH_NAME_KEY = "forkBranchName"; + + protected ForkMetrics(TaskState taskState, int index) { +super(name(taskState, index), parentContextForFork(taskState), getForkMetricsTags(taskState, index)); + } + + private static MetricContext parentContextForFork(TaskState taskState) { +return TaskMetrics.get(METRICS_ID_PREFIX + taskState.getJobId() + "." + taskState.getTaskId()).getMetricContext(); + } + + public static ForkMetrics get(final TaskState taskState, int index) { +return (ForkMetrics) GOBBLIN_METRICS_REGISTRY.getOrDefault(name(taskState, index), new Callable() { + @Override + public GobblinMetrics call() throws Exception { +return new ForkMetrics(taskState, index); + } +}); + } + + /** + * Creates a unique {@link String} representing this branch. + */ + private static String getForkMetricsId(State state, int index) { +return state.getProp(ConfigurationKeys.FORK_BRANCH_NAME_KEY + "." + index, +ConfigurationKeys.DEFAULT_FORK_BRANCH_NAME + index); + } + + /** + * Creates a {@link List} of {@link Tag}s for a {@link Fork} instance. The {@link Tag}s are purely based on the + * index and the branch name. + */ + private static List> getForkMetricsTags(State state, int index) { +return ImmutableList.>of(new Tag<>(FORK_METRICS_BRANCH_NAME_KEY, getForkMetricsId(state, index))); Review comment: Maybe add a null check here i.e. if (getForkMetricsId(state,index) != null)? Else, ImmutableList.of() may throw an exception. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] sv2000 commented on a change in pull request #2667: [GOBBLIN-800] Remove the metric context cache from GobblinMetricsRegistry
sv2000 commented on a change in pull request #2667: [GOBBLIN-800] Remove the metric context cache from GobblinMetricsRegistry URL: https://github.com/apache/incubator-gobblin/pull/2667#discussion_r291756656 ## File path: gobblin-runtime/src/main/java/org/apache/gobblin/runtime/util/TaskMetrics.java ## @@ -61,15 +62,35 @@ public GobblinMetrics call() throws Exception { /** * Remove the {@link TaskMetrics} instance for the task with the given {@link TaskMetrics} instance. + * Please note this method is invoked by job driver so it won't delete any underlying {@link ForkMetrics} + * because the {@link org.apache.gobblin.runtime.fork.Fork} can be created on different nodes. * * @param taskState the given {@link TaskState} instance */ public static void remove(TaskState taskState) { remove(name(taskState)); } + /** + * Remove the {@link TaskMetrics} instance for the task with the given {@link TaskMetrics} instance. + * Please note that this will also delete the underlying {@link ForkMetrics} related to this specific task. + * + * @param task the given task instance + */ + public static void remove(Task task) { +task.getForks().forEach(forkOpt -> { + remove(ForkMetrics.name(task.getTaskState(), forkOpt.get().getIndex())); +}); + +remove(name(task)); + } + private static String name(TaskState taskState) { -return "gobblin.metrics." + taskState.getJobId() + "." + taskState.getTaskId(); +return METRICS_ID_PREFIX + taskState.getJobId() + "." + taskState.getTaskId(); + } + + private static String name(Task task) { +return METRICS_ID_PREFIX + task.getJobId() + "." + task.getTaskId(); Review comment: return name(task.getTaskState())? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[jira] [Work logged] (GOBBLIN-800) Remove the metric context cache from GobblinMetricsRegistry
[ https://issues.apache.org/jira/browse/GOBBLIN-800?focusedWorklogId=256152=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-256152 ] ASF GitHub Bot logged work on GOBBLIN-800: -- Author: ASF GitHub Bot Created on: 07/Jun/19 18:53 Start Date: 07/Jun/19 18:53 Worklog Time Spent: 10m Work Description: yukuai518 commented on issue #2667: [GOBBLIN-800] Remove the metric context cache from GobblinMetricsRegistry URL: https://github.com/apache/incubator-gobblin/pull/2667#issuecomment-49595 @sv2000 can you help review? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 256152) Time Spent: 20m (was: 10m) > Remove the metric context cache from GobblinMetricsRegistry > --- > > Key: GOBBLIN-800 > URL: https://issues.apache.org/jira/browse/GOBBLIN-800 > Project: Apache Gobblin > Issue Type: Bug >Reporter: Kuai Yu >Priority: Major > Time Spent: 20m > Remaining Estimate: 0h > > Remove the metric context cache from GobblinMetricsRegistry -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] [incubator-gobblin] yukuai518 commented on issue #2667: [GOBBLIN-800] Remove the metric context cache from GobblinMetricsRegistry
yukuai518 commented on issue #2667: [GOBBLIN-800] Remove the metric context cache from GobblinMetricsRegistry URL: https://github.com/apache/incubator-gobblin/pull/2667#issuecomment-49595 @sv2000 can you help review? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] ZihanLi58 opened a new pull request #2666: Schema check bug
ZihanLi58 opened a new pull request #2666: Schema check bug URL: https://github.com/apache/incubator-gobblin/pull/2666 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 - [ ] My PR addresses the following [Gobblin JIRA](https://issues.apache.org/jira/browse/GOBBLIN/) issues and references them in the PR title. For example, "[GOBBLIN-XXX] My Gobblin PR" - https://issues.apache.org/jira/browse/GOBBLIN-799 ### Description - [ ] Here are some details about my PR, including screenshots (if applicable): Add return statement to fix the bug. ### Tests - [ ] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason: Add unit test to test default schema check strategy. ### Commits - [ ] My commits all reference JIRA issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)": 1. Subject is separated from body by a blank line 2. Subject is limited to 50 characters 3. Subject does not end with a period 4. Subject uses the imperative mood ("add", not "adding") 5. Body wraps at 72 characters 6. Body explains "what" and "why", not "how" 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[jira] [Work logged] (GOBBLIN-798) Clean up workflows from Helix when the Gobblin application master starts
[ https://issues.apache.org/jira/browse/GOBBLIN-798?focusedWorklogId=256109=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-256109 ] ASF GitHub Bot logged work on GOBBLIN-798: -- Author: ASF GitHub Bot Created on: 07/Jun/19 17:46 Start Date: 07/Jun/19 17:46 Worklog Time Spent: 10m Work Description: sv2000 commented on pull request #2665: [GOBBLIN-798] Clean up workflows from Helix when the Gobblin applicat… URL: https://github.com/apache/incubator-gobblin/pull/2665#discussion_r291687392 ## File path: gobblin-yarn/src/main/java/org/apache/gobblin/yarn/GobblinApplicationMaster.java ## @@ -81,7 +83,10 @@ public GobblinApplicationMaster(String applicationName, ContainerId containerId, Config config, YarnConfiguration yarnConfiguration) throws Exception { super(applicationName, containerId.getApplicationAttemptId().getApplicationId().toString(), -GobblinClusterUtils.addDynamicConfig(config), Optional.absent()); +GobblinClusterUtils.addDynamicConfig(config) +.withFallback(ConfigFactory.parseMap( + ImmutableMap.of(GobblinClusterConfigurationKeys.CLEAN_UP_JOBS_ON_MANAGER_START, "true"))), Review comment: Should it be ImmutableMap.of(GobblinClusterConfigurationKeys.CLEAN_UP_JOBS_ON_MANAGER_START, DEFAULT_CLEAN_UP_JOBS_ON_MANAGER_START) ? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 256109) Time Spent: 40m (was: 0.5h) > Clean up workflows from Helix when the Gobblin application master starts > > > Key: GOBBLIN-798 > URL: https://issues.apache.org/jira/browse/GOBBLIN-798 > Project: Apache Gobblin > Issue Type: Task >Reporter: Hung Tran >Assignee: Hung Tran >Priority: Major > Time Spent: 40m > Remaining Estimate: 0h > > 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. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] [incubator-gobblin] sv2000 commented on a change in pull request #2665: [GOBBLIN-798] Clean up workflows from Helix when the Gobblin applicat…
sv2000 commented on a change in pull request #2665: [GOBBLIN-798] Clean up workflows from Helix when the Gobblin applicat… URL: https://github.com/apache/incubator-gobblin/pull/2665#discussion_r291687913 ## File path: gobblin-cluster/src/main/java/org/apache/gobblin/cluster/GobblinHelixMultiManager.java ## @@ -363,12 +359,26 @@ void handleLeadershipChange(NotificationContext changeContext) { } } + /** + * Delete jobs from the helix cluster + */ + @VisibleForTesting + public void cleanUpJobs() { +cleanUpJobs(this.jobClusterHelixManager); + +if (this.taskDriverHelixManager.isPresent()) { + cleanUpJobs(this.taskDriverHelixManager.get()); +} + } + private void cleanUpJobs(HelixManager helixManager) { // Clean up existing jobs TaskDriver taskDriver = new TaskDriver(helixManager); Map workflows = taskDriver.getWorkflows(); +log.debug("cleanUpJobs workflow count {} workflows {}", workflows.size(), workflows); Review comment: Maybe just dump workflows.keySet() instead of the entire map? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[jira] [Work logged] (GOBBLIN-798) Clean up workflows from Helix when the Gobblin application master starts
[ https://issues.apache.org/jira/browse/GOBBLIN-798?focusedWorklogId=256107=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-256107 ] ASF GitHub Bot logged work on GOBBLIN-798: -- Author: ASF GitHub Bot Created on: 07/Jun/19 17:46 Start Date: 07/Jun/19 17:46 Worklog Time Spent: 10m Work Description: sv2000 commented on pull request #2665: [GOBBLIN-798] Clean up workflows from Helix when the Gobblin applicat… URL: https://github.com/apache/incubator-gobblin/pull/2665#discussion_r291685805 ## File path: gobblin-cluster/src/main/java/org/apache/gobblin/cluster/GobblinClusterConfigurationKeys.java ## @@ -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"; Review comment: Shouldn't we always clean up on restart? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 256107) Time Spent: 0.5h (was: 20m) > Clean up workflows from Helix when the Gobblin application master starts > > > Key: GOBBLIN-798 > URL: https://issues.apache.org/jira/browse/GOBBLIN-798 > Project: Apache Gobblin > Issue Type: Task >Reporter: Hung Tran >Assignee: Hung Tran >Priority: Major > Time Spent: 0.5h > Remaining Estimate: 0h > > 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. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] [incubator-gobblin] sv2000 commented on a change in pull request #2665: [GOBBLIN-798] Clean up workflows from Helix when the Gobblin applicat…
sv2000 commented on a change in pull request #2665: [GOBBLIN-798] Clean up workflows from Helix when the Gobblin applicat… URL: https://github.com/apache/incubator-gobblin/pull/2665#discussion_r291690862 ## File path: gobblin-cluster/src/main/java/org/apache/gobblin/cluster/GobblinClusterManager.java ## @@ -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) { Review 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? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[jira] [Work logged] (GOBBLIN-798) Clean up workflows from Helix when the Gobblin application master starts
[ https://issues.apache.org/jira/browse/GOBBLIN-798?focusedWorklogId=256110=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-256110 ] ASF GitHub Bot logged work on GOBBLIN-798: -- Author: ASF GitHub Bot Created on: 07/Jun/19 17:46 Start Date: 07/Jun/19 17:46 Worklog Time Spent: 10m Work Description: sv2000 commented on pull request #2665: [GOBBLIN-798] Clean up workflows from Helix when the Gobblin applicat… URL: https://github.com/apache/incubator-gobblin/pull/2665#discussion_r291685487 ## File path: gobblin-cluster/src/main/java/org/apache/gobblin/cluster/GobblinClusterConfigurationKeys.java ## @@ -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"; + public static final boolean DEFAULT_CLEAN_UP_JOBS_ON_MANAGER_START = false; Review comment: Is the default "false" to preserve the current behavior with Gobblin cluster in non-Yarn mode? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 256110) Time Spent: 50m (was: 40m) > Clean up workflows from Helix when the Gobblin application master starts > > > Key: GOBBLIN-798 > URL: https://issues.apache.org/jira/browse/GOBBLIN-798 > Project: Apache Gobblin > Issue Type: Task >Reporter: Hung Tran >Assignee: Hung Tran >Priority: Major > Time Spent: 50m > Remaining Estimate: 0h > > 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. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] [incubator-gobblin] sv2000 commented on a change in pull request #2665: [GOBBLIN-798] Clean up workflows from Helix when the Gobblin applicat…
sv2000 commented on a change in pull request #2665: [GOBBLIN-798] Clean up workflows from Helix when the Gobblin applicat… URL: https://github.com/apache/incubator-gobblin/pull/2665#discussion_r291685487 ## File path: gobblin-cluster/src/main/java/org/apache/gobblin/cluster/GobblinClusterConfigurationKeys.java ## @@ -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"; + public static final boolean DEFAULT_CLEAN_UP_JOBS_ON_MANAGER_START = false; Review comment: Is the default "false" to preserve the current behavior with Gobblin cluster in non-Yarn mode? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] sv2000 commented on a change in pull request #2665: [GOBBLIN-798] Clean up workflows from Helix when the Gobblin applicat…
sv2000 commented on a change in pull request #2665: [GOBBLIN-798] Clean up workflows from Helix when the Gobblin applicat… URL: https://github.com/apache/incubator-gobblin/pull/2665#discussion_r291685805 ## File path: gobblin-cluster/src/main/java/org/apache/gobblin/cluster/GobblinClusterConfigurationKeys.java ## @@ -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"; Review comment: Shouldn't we always clean up on restart? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-gobblin] sv2000 commented on a change in pull request #2665: [GOBBLIN-798] Clean up workflows from Helix when the Gobblin applicat…
sv2000 commented on a change in pull request #2665: [GOBBLIN-798] Clean up workflows from Helix when the Gobblin applicat… URL: https://github.com/apache/incubator-gobblin/pull/2665#discussion_r291687392 ## File path: gobblin-yarn/src/main/java/org/apache/gobblin/yarn/GobblinApplicationMaster.java ## @@ -81,7 +83,10 @@ public GobblinApplicationMaster(String applicationName, ContainerId containerId, Config config, YarnConfiguration yarnConfiguration) throws Exception { super(applicationName, containerId.getApplicationAttemptId().getApplicationId().toString(), -GobblinClusterUtils.addDynamicConfig(config), Optional.absent()); +GobblinClusterUtils.addDynamicConfig(config) +.withFallback(ConfigFactory.parseMap( + ImmutableMap.of(GobblinClusterConfigurationKeys.CLEAN_UP_JOBS_ON_MANAGER_START, "true"))), Review comment: Should it be ImmutableMap.of(GobblinClusterConfigurationKeys.CLEAN_UP_JOBS_ON_MANAGER_START, DEFAULT_CLEAN_UP_JOBS_ON_MANAGER_START) ? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[jira] [Work logged] (GOBBLIN-798) Clean up workflows from Helix when the Gobblin application master starts
[ https://issues.apache.org/jira/browse/GOBBLIN-798?focusedWorklogId=256106=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-256106 ] ASF GitHub Bot logged work on GOBBLIN-798: -- Author: ASF GitHub Bot Created on: 07/Jun/19 17:46 Start Date: 07/Jun/19 17:46 Worklog Time Spent: 10m Work Description: sv2000 commented on pull request #2665: [GOBBLIN-798] Clean up workflows from Helix when the Gobblin applicat… URL: https://github.com/apache/incubator-gobblin/pull/2665#discussion_r291687913 ## File path: gobblin-cluster/src/main/java/org/apache/gobblin/cluster/GobblinHelixMultiManager.java ## @@ -363,12 +359,26 @@ void handleLeadershipChange(NotificationContext changeContext) { } } + /** + * Delete jobs from the helix cluster + */ + @VisibleForTesting + public void cleanUpJobs() { +cleanUpJobs(this.jobClusterHelixManager); + +if (this.taskDriverHelixManager.isPresent()) { + cleanUpJobs(this.taskDriverHelixManager.get()); +} + } + private void cleanUpJobs(HelixManager helixManager) { // Clean up existing jobs TaskDriver taskDriver = new TaskDriver(helixManager); Map workflows = taskDriver.getWorkflows(); +log.debug("cleanUpJobs workflow count {} workflows {}", workflows.size(), workflows); Review comment: Maybe just dump workflows.keySet() instead of the entire map? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 256106) Time Spent: 20m (was: 10m) > Clean up workflows from Helix when the Gobblin application master starts > > > Key: GOBBLIN-798 > URL: https://issues.apache.org/jira/browse/GOBBLIN-798 > Project: Apache Gobblin > Issue Type: Task >Reporter: Hung Tran >Assignee: Hung Tran >Priority: Major > Time Spent: 20m > Remaining Estimate: 0h > > 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. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Work logged] (GOBBLIN-798) Clean up workflows from Helix when the Gobblin application master starts
[ https://issues.apache.org/jira/browse/GOBBLIN-798?focusedWorklogId=256108=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-256108 ] ASF GitHub Bot logged work on GOBBLIN-798: -- Author: ASF GitHub Bot Created on: 07/Jun/19 17:46 Start Date: 07/Jun/19 17:46 Worklog Time Spent: 10m Work Description: sv2000 commented on pull request #2665: [GOBBLIN-798] Clean up workflows from Helix when the Gobblin applicat… URL: https://github.com/apache/incubator-gobblin/pull/2665#discussion_r291690862 ## File path: gobblin-cluster/src/main/java/org/apache/gobblin/cluster/GobblinClusterManager.java ## @@ -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) { Review 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? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org Issue Time Tracking --- Worklog Id: (was: 256108) Time Spent: 40m (was: 0.5h) > Clean up workflows from Helix when the Gobblin application master starts > > > Key: GOBBLIN-798 > URL: https://issues.apache.org/jira/browse/GOBBLIN-798 > Project: Apache Gobblin > Issue Type: Task >Reporter: Hung Tran >Assignee: Hung Tran >Priority: Major > Time Spent: 40m > Remaining Estimate: 0h > > 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. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Created] (GOBBLIN-799) Bugs in AvroSchemaCheckDefaultStrategy that not return after check ENUM and FIXED
Zihan Li created GOBBLIN-799: Summary: Bugs in AvroSchemaCheckDefaultStrategy that not return after check ENUM and FIXED Key: GOBBLIN-799 URL: https://issues.apache.org/jira/browse/GOBBLIN-799 Project: Apache Gobblin Issue Type: Bug Reporter: Zihan Li There are bugs in AvroSchemaCheckDefaultStrategy that not return after check ENUM and FIXED, just need to add return statement -- This message was sent by Atlassian JIRA (v7.6.3#76005)