[jira] [Updated] (GOBBLIN-796) Add support partial updates for flowConfig

2019-06-07 Thread Jack Moseley (JIRA)


 [ 
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

2019-06-07 Thread ASF GitHub Bot (JIRA)


 [ 
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

2019-06-07 Thread GitBox
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

2019-06-07 Thread ASF GitHub Bot (JIRA)


 [ 
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…

2019-06-07 Thread GitBox
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

2019-06-07 Thread ASF GitHub Bot (JIRA)


 [ 
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

2019-06-07 Thread ASF GitHub Bot (JIRA)


 [ 
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…

2019-06-07 Thread GitBox
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

2019-06-07 Thread ASF GitHub Bot (JIRA)


 [ 
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…

2019-06-07 Thread GitBox
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

2019-06-07 Thread ASF GitHub Bot (JIRA)


 [ 
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

2019-06-07 Thread ASF GitHub Bot (JIRA)


 [ 
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

2019-06-07 Thread GitBox
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

2019-06-07 Thread GitBox
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

2019-06-07 Thread ASF GitHub Bot (JIRA)


 [ 
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

2019-06-07 Thread GitBox
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

2019-06-07 Thread GitBox
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

2019-06-07 Thread ASF GitHub Bot (JIRA)


 [ 
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…

2019-06-07 Thread GitBox
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

2019-06-07 Thread ASF GitHub Bot (JIRA)


 [ 
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…

2019-06-07 Thread GitBox
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

2019-06-07 Thread ASF GitHub Bot (JIRA)


 [ 
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…

2019-06-07 Thread GitBox
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…

2019-06-07 Thread GitBox
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…

2019-06-07 Thread GitBox
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

2019-06-07 Thread ASF GitHub Bot (JIRA)


 [ 
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

2019-06-07 Thread ASF GitHub Bot (JIRA)


 [ 
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

2019-06-07 Thread Zihan Li (JIRA)
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)