You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@gobblin.apache.org by GitBox <gi...@apache.org> on 2022/09/13 21:29:40 UTC
[GitHub] [gobblin] homatthew opened a new pull request, #3561: [GOBBLIN-1704] Purge offline helix instances during startup
homatthew opened a new pull request, #3561:
URL: https://github.com/apache/gobblin/pull/3561
To avoid accumulation of helix instances, we should clean up over time. But we can only perform the clean up at startup because it's unsafe to call the API while instances are being created / removed.
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-XXX
### Description
- [ ] Here are some details about my PR, including screenshots (if applicable):
### Tests
- [ ] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason:
### 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.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] homatthew commented on a diff in pull request #3561: [GOBBLIN-1704] Purge offline helix instances during startup
Posted by GitBox <gi...@apache.org>.
homatthew commented on code in PR #3561:
URL: https://github.com/apache/gobblin/pull/3561#discussion_r972323434
##########
gobblin-yarn/src/main/java/org/apache/gobblin/yarn/YarnService.java:
##########
@@ -339,6 +342,15 @@ protected void startUp() throws Exception {
LOGGER.info("ApplicationMaster registration response: " + response);
this.maxResourceCapacity = Optional.of(response.getMaximumResourceCapability());
+ // All previous helix instances should be purged on startup. Gobblin task runners are stateless from helix
+ // perspective because all important state is persisted separately in Workunit State Store or Watermark store.
+ // Offline duration of 0 means any offline instance should be purged (Note: there aren't any online instances
+ // when this code runs, this is during startup before any containers are allocated).
+ LOGGER.info("Purging offline helix instances before allocating containers for helixClusterName={}, connectionString={}", helixManager.getClusterName(), helixManager.getMetadataStoreConnectionString());
+ long offlineDuration = 0;
+ this.helixAdmin.purgeOfflineInstances(this.helixManager.getClusterName(), offlineDuration);
Review Comment:
Well kind of. As mentioned
> But adding a timeout adds a potential bug / risk where we timeout and then helix starts purging instances while we are allocating new instances (bad bug with nondeterministic behavior)
We put ourselves at risk of having a live instances without an INSTANCE config. That would essentially bork the live instance and I don't know that the system would auto recover. Although I do think we should send a GTE if this call is taking too long
--
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.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] homatthew commented on a diff in pull request #3561: [GOBBLIN-1704] Purge offline helix instances during startup
Posted by GitBox <gi...@apache.org>.
homatthew commented on code in PR #3561:
URL: https://github.com/apache/gobblin/pull/3561#discussion_r970335122
##########
gobblin-yarn/src/main/java/org/apache/gobblin/yarn/YarnService.java:
##########
@@ -339,6 +342,15 @@ protected void startUp() throws Exception {
LOGGER.info("ApplicationMaster registration response: " + response);
this.maxResourceCapacity = Optional.of(response.getMaximumResourceCapability());
+ // All previous helix instances should be purged on startup. Gobblin task runners are stateless from helix
+ // perspective because all important state is persisted separately in Workunit State Store or Watermark store.
+ // Offline duration of 0 means any offline instance should be purged (Note: there aren't any online instances
+ // when this code runs, this is during startup before any containers are allocated).
+ LOGGER.info("Purging offline helix instances before allocating containers for helixClusterName={}, connectionString={}", helixManager.getClusterName(), helixManager.getMetadataStoreConnectionString());
+ long offlineDuration = 0;
+ this.helixAdmin.purgeOfflineInstances(this.helixManager.getClusterName(), offlineDuration);
Review Comment:
Not sure if we should have some sort of timeout in case the API takes a long time to respond. Currently this a blocking call and I wonder if this call fails we could block containers from being allocated in the YarnService (leading to starvation).
But adding a timeout adds a potential bug / risk where we timeout and then helix starts purging instances while we are allocating new instances (bad bug with nondeterministic behavior)
--
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.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] homatthew commented on a diff in pull request #3561: [GOBBLIN-1704] Purge offline helix instances during startup
Posted by GitBox <gi...@apache.org>.
homatthew commented on code in PR #3561:
URL: https://github.com/apache/gobblin/pull/3561#discussion_r972324729
##########
gobblin-yarn/src/test/java/org/apache/gobblin/yarn/YarnServiceTest.java:
##########
@@ -303,19 +303,18 @@ public void testExceedResourceLimit() {
static class TestYarnService extends YarnService {
public TestYarnService(Config config, String applicationName, String applicationId, YarnConfiguration yarnConfiguration,
FileSystem fs, EventBus eventBus) throws Exception {
- super(config, applicationName, applicationId, yarnConfiguration, fs, eventBus, getMockHelixManager(config));
+ super(config, applicationName, applicationId, yarnConfiguration, fs, eventBus, getMockHelixManager(config), getMockHelixAdmin());
}
private static HelixManager getMockHelixManager(Config config) {
HelixManager helixManager = Mockito.mock(HelixManager.class);
- HelixAdmin helixAdmin = Mockito.mock(HelixAdmin.class);
HelixDataAccessor helixDataAccessor = Mockito.mock(HelixDataAccessor.class);
PropertyKey propertyKey = Mockito.mock(PropertyKey.class);
PropertyKey.Builder propertyKeyBuilder = Mockito.mock(PropertyKey.Builder.class);
Mockito.when(helixManager.getInstanceName()).thenReturn("helixInstance1");
Mockito.when(helixManager.getClusterName()).thenReturn(config.getString(GobblinClusterConfigurationKeys.HELIX_CLUSTER_NAME_KEY));
- Mockito.doNothing().when(helixAdmin).enableInstance(Mockito.anyString(), Mockito.anyString(), Mockito.anyBoolean());
+//
Review Comment:
TODO: remove this random //
##########
gobblin-yarn/src/test/java/org/apache/gobblin/yarn/YarnServiceTest.java:
##########
@@ -303,19 +303,18 @@ public void testExceedResourceLimit() {
static class TestYarnService extends YarnService {
public TestYarnService(Config config, String applicationName, String applicationId, YarnConfiguration yarnConfiguration,
FileSystem fs, EventBus eventBus) throws Exception {
- super(config, applicationName, applicationId, yarnConfiguration, fs, eventBus, getMockHelixManager(config));
+ super(config, applicationName, applicationId, yarnConfiguration, fs, eventBus, getMockHelixManager(config), getMockHelixAdmin());
}
private static HelixManager getMockHelixManager(Config config) {
HelixManager helixManager = Mockito.mock(HelixManager.class);
- HelixAdmin helixAdmin = Mockito.mock(HelixAdmin.class);
HelixDataAccessor helixDataAccessor = Mockito.mock(HelixDataAccessor.class);
PropertyKey propertyKey = Mockito.mock(PropertyKey.class);
PropertyKey.Builder propertyKeyBuilder = Mockito.mock(PropertyKey.Builder.class);
Mockito.when(helixManager.getInstanceName()).thenReturn("helixInstance1");
Mockito.when(helixManager.getClusterName()).thenReturn(config.getString(GobblinClusterConfigurationKeys.HELIX_CLUSTER_NAME_KEY));
- Mockito.doNothing().when(helixAdmin).enableInstance(Mockito.anyString(), Mockito.anyString(), Mockito.anyBoolean());
+//
Review Comment:
TODO: remove this random // on next iteration
--
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.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] umustafi commented on a diff in pull request #3561: [GOBBLIN-1704] Purge offline helix instances during startup
Posted by GitBox <gi...@apache.org>.
umustafi commented on code in PR #3561:
URL: https://github.com/apache/gobblin/pull/3561#discussion_r972235376
##########
gobblin-yarn/src/main/java/org/apache/gobblin/yarn/YarnService.java:
##########
@@ -339,6 +342,15 @@ protected void startUp() throws Exception {
LOGGER.info("ApplicationMaster registration response: " + response);
this.maxResourceCapacity = Optional.of(response.getMaximumResourceCapability());
+ // All previous helix instances should be purged on startup. Gobblin task runners are stateless from helix
+ // perspective because all important state is persisted separately in Workunit State Store or Watermark store.
+ // Offline duration of 0 means any offline instance should be purged (Note: there aren't any online instances
+ // when this code runs, this is during startup before any containers are allocated).
+ LOGGER.info("Purging offline helix instances before allocating containers for helixClusterName={}, connectionString={}", helixManager.getClusterName(), helixManager.getMetadataStoreConnectionString());
+ long offlineDuration = 0;
+ this.helixAdmin.purgeOfflineInstances(this.helixManager.getClusterName(), offlineDuration);
Review Comment:
We should block on this call as you're saying so we don't start allocating new instances while purging his happening in a different thread. However, we don't want the whole `YarnService` to fail initialization because of a Helix issue. We should block and have a timeout. Then if it fails, emit a **metric** (in addition to log) that Helix instances have failed to be purged. We can set alerts on this metric so oncall can respond to this spikes in the metric emission and investigate failure to purge Helix instances so they don't grow unbounded. With a log warning/error it's easy to get ignored and miss it until there is a GCN level issue.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] ZihanLi58 merged pull request #3561: [GOBBLIN-1704] Purge offline helix instances during startup
Posted by GitBox <gi...@apache.org>.
ZihanLi58 merged PR #3561:
URL: https://github.com/apache/gobblin/pull/3561
--
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.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] ZihanLi58 commented on a diff in pull request #3561: [GOBBLIN-1704] Purge offline helix instances during startup
Posted by GitBox <gi...@apache.org>.
ZihanLi58 commented on code in PR #3561:
URL: https://github.com/apache/gobblin/pull/3561#discussion_r973475222
##########
gobblin-yarn/src/main/java/org/apache/gobblin/yarn/YarnService.java:
##########
@@ -339,6 +342,15 @@ protected void startUp() throws Exception {
LOGGER.info("ApplicationMaster registration response: " + response);
this.maxResourceCapacity = Optional.of(response.getMaximumResourceCapability());
+ // All previous helix instances should be purged on startup. Gobblin task runners are stateless from helix
+ // perspective because all important state is persisted separately in Workunit State Store or Watermark store.
+ // Offline duration of 0 means any offline instance should be purged (Note: there aren't any online instances
+ // when this code runs, this is during startup before any containers are allocated).
+ LOGGER.info("Purging offline helix instances before allocating containers for helixClusterName={}, connectionString={}", helixManager.getClusterName(), helixManager.getMetadataStoreConnectionString());
+ long offlineDuration = 0;
+ this.helixAdmin.purgeOfflineInstances(this.helixManager.getClusterName(), offlineDuration);
Review Comment:
+1 to make this behavior configurable, so that if something wrong happens on helix side and causing the starvation we can quickly mitigate by disable this behavior. Also it will be good to add instrument here to measure how long it usually take for us to clear the offline instance.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] umustafi commented on a diff in pull request #3561: [GOBBLIN-1704] Purge offline helix instances during startup
Posted by GitBox <gi...@apache.org>.
umustafi commented on code in PR #3561:
URL: https://github.com/apache/gobblin/pull/3561#discussion_r972235376
##########
gobblin-yarn/src/main/java/org/apache/gobblin/yarn/YarnService.java:
##########
@@ -339,6 +342,15 @@ protected void startUp() throws Exception {
LOGGER.info("ApplicationMaster registration response: " + response);
this.maxResourceCapacity = Optional.of(response.getMaximumResourceCapability());
+ // All previous helix instances should be purged on startup. Gobblin task runners are stateless from helix
+ // perspective because all important state is persisted separately in Workunit State Store or Watermark store.
+ // Offline duration of 0 means any offline instance should be purged (Note: there aren't any online instances
+ // when this code runs, this is during startup before any containers are allocated).
+ LOGGER.info("Purging offline helix instances before allocating containers for helixClusterName={}, connectionString={}", helixManager.getClusterName(), helixManager.getMetadataStoreConnectionString());
+ long offlineDuration = 0;
+ this.helixAdmin.purgeOfflineInstances(this.helixManager.getClusterName(), offlineDuration);
Review Comment:
We should block on this call as you're saying so we don't start allocating new instances while purging his happening in a different thread. However, we don't want the whole `YarnService` to fail initialization because of a Helix issue. We should block and have a timeout. Then if it fails, emit a **metric** (in addition to log) that Helix instances have failed to be purged. We should set alerts on this metric so oncall can respond to this metric and investigate failure to purge Helix instances so they don't grow unbounded. With a log warning/error it's easy to get ignored and miss it until there is a GCN level issue.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] homatthew commented on a diff in pull request #3561: [GOBBLIN-1704] Purge offline helix instances during startup
Posted by GitBox <gi...@apache.org>.
homatthew commented on code in PR #3561:
URL: https://github.com/apache/gobblin/pull/3561#discussion_r976834480
##########
gobblin-yarn/src/main/java/org/apache/gobblin/yarn/HelixInstancePurgerWithMetrics.java:
##########
@@ -0,0 +1,140 @@
+/*
+ * 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.yarn;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Stopwatch;
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
+import lombok.AllArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.gobblin.metrics.event.EventSubmitter;
+import org.apache.gobblin.metrics.event.GobblinEventBuilder;
+import org.apache.helix.HelixAdmin;
+
+
+@Slf4j
+@AllArgsConstructor
+public class HelixInstancePurgerWithMetrics {
+ private final EventSubmitter eventSubmitter;
+ private final long pollingRateMs;
+ private static final String PREFIX = "HelixOfflineInstancePurge.";
+ public static final String PURGE_FAILURE_EVENT = PREFIX + "Failure";
+ public static final String PURGE_LAGGING_EVENT = PREFIX + "Lagging";
+ public static final String PURGE_COMPLETED_EVENT = PREFIX + "Completed";
+
+
+ /**
+ * Blocking call for purging all offline helix instances. Provides boiler plate code for providing periodic updates
+ * and sending a GTE if it's an unexpected amount of time.
+ *
+ * All previous helix instances should be purged on startup. Gobblin task runners are stateless from helix
+ * perspective because all important state is persisted separately in Workunit State Store or Watermark store.
+ */
+ public void purgeAllOfflineInstances(HelixAdmin admin, String clusterName, long laggingThresholdMs, Map<String, String> gteMetadata) {
+ CompletableFuture<Void> purgeTask = CompletableFuture.supplyAsync(() -> {
+ long offlineDuration = 0; // 0 means all offline instance should be purged.
+ admin.purgeOfflineInstances(clusterName, offlineDuration);
+ return null;
+ });
+
+ long timeToPurgeMs = waitForPurgeCompletion(purgeTask, laggingThresholdMs, Stopwatch.createUnstarted(), gteMetadata);
+ log.info("Finished purging offline helix instances. It took timeToPurgeMs={}", timeToPurgeMs);
+ }
+
+ @VisibleForTesting
+ long waitForPurgeCompletion(CompletableFuture<Void> purgeTask, long laggingThresholdMs, Stopwatch watch,
+ Map<String, String> gteMetadata) {
+ watch.start();
+ try {
+ boolean haveSubmittedLaggingEvent = false; //
+ while (!purgeTask.isDone()) {
+ long elapsedTimeMs = watch.elapsed(TimeUnit.MILLISECONDS);
+ log.info("Waiting for helix to purge offline instances. Cannot proceed with execution because purging is a "
+ + "non-thread safe call. To disable purging offline instances during startup, change the flag {} "
+ + "elapsedTimeMs={}, laggingThresholdMs={}",
+ GobblinYarnConfigurationKeys.HELIX_PURGE_OFFLINE_INSTANCES_ENABLED, elapsedTimeMs, laggingThresholdMs);
+ if (!haveSubmittedLaggingEvent && elapsedTimeMs > laggingThresholdMs) {
+ submitLaggingEvent(elapsedTimeMs, laggingThresholdMs, gteMetadata);
+ haveSubmittedLaggingEvent = true;
+ }
+ Thread.sleep(this.pollingRateMs);
+ }
+
+ long timeToPurgeMs = watch.elapsed(TimeUnit.MILLISECONDS);
+ if (!haveSubmittedLaggingEvent && timeToPurgeMs > laggingThresholdMs) {
+ submitLaggingEvent(timeToPurgeMs, laggingThresholdMs, gteMetadata);
+ }
+
+ purgeTask.get(); // check for exceptions
+ submitCompletedEvent(timeToPurgeMs, gteMetadata);
+ return timeToPurgeMs;
+ } catch (ExecutionException | InterruptedException e) {
+ log.info("The call to purge offline helix instances failed. This is not a fatal error because it is not mandatory to "
+ + "clean up old helix instances. But repeated failure to purge offline helix instances will cause an accumulation"
+ + "of offline helix instances which may cause large delays in future helix calls.", e);
+ long timeToPurgeMs = watch.elapsed(TimeUnit.MILLISECONDS);
+ submitFailureEvent(timeToPurgeMs, gteMetadata);
+ return timeToPurgeMs;
+ }
+ }
+
+ private void submitFailureEvent(long elapsedTimeMs, Map<String, String> additionalMetadata) {
+ if (eventSubmitter != null) {
+ GobblinEventBuilder eventBuilder = new GobblinEventBuilder(PURGE_FAILURE_EVENT);
+ eventBuilder.addAdditionalMetadata(additionalMetadata);
+ eventBuilder.addMetadata("elapsedTimeMs", String.valueOf(elapsedTimeMs));
+
+ log.info("Submitting GTE because purging offline instances has failed to complete. event={}", eventBuilder);
+ eventSubmitter.submit(eventBuilder);
+ } else {
+ log.info("Cannot submit {} GTE because eventSubmitter is null", PURGE_FAILURE_EVENT);
+ }
+ }
+
+ private void submitCompletedEvent(long timeToPurgeMs, Map<String, String> additionalMetadata) {
+ if (eventSubmitter != null) {
+ GobblinEventBuilder eventBuilder = new GobblinEventBuilder(PURGE_COMPLETED_EVENT);
+ eventBuilder.addAdditionalMetadata(additionalMetadata);
+ eventBuilder.addMetadata("timeToPurgeMs", String.valueOf(timeToPurgeMs));
+
+ log.info("Submitting GTE because purging offline instances has completed successfully. event={}", eventBuilder);
+ eventSubmitter.submit(eventBuilder);
+ } else {
+ log.info("Cannot submit {} GTE because eventSubmitter is null", PURGE_COMPLETED_EVENT);
+ }
+ }
+
+ private void submitLaggingEvent(long elapsedTimeMs, long laggingThresholdMs,
Review Comment:
The events being submitted are Gobblin Tracking Events sent to Kafka. One way to setup general reporting is to query the Hive table specifically for this GobblingTrackingEvent topic.
It is up to us, the users, to setup the infrastructure to consume these events.
GobblinTrackingEvents aren't new, so existing monitoring services can easily consume this and emit alerts.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] homatthew commented on a diff in pull request #3561: [GOBBLIN-1704] Purge offline helix instances during startup
Posted by GitBox <gi...@apache.org>.
homatthew commented on code in PR #3561:
URL: https://github.com/apache/gobblin/pull/3561#discussion_r972323434
##########
gobblin-yarn/src/main/java/org/apache/gobblin/yarn/YarnService.java:
##########
@@ -339,6 +342,15 @@ protected void startUp() throws Exception {
LOGGER.info("ApplicationMaster registration response: " + response);
this.maxResourceCapacity = Optional.of(response.getMaximumResourceCapability());
+ // All previous helix instances should be purged on startup. Gobblin task runners are stateless from helix
+ // perspective because all important state is persisted separately in Workunit State Store or Watermark store.
+ // Offline duration of 0 means any offline instance should be purged (Note: there aren't any online instances
+ // when this code runs, this is during startup before any containers are allocated).
+ LOGGER.info("Purging offline helix instances before allocating containers for helixClusterName={}, connectionString={}", helixManager.getClusterName(), helixManager.getMetadataStoreConnectionString());
+ long offlineDuration = 0;
+ this.helixAdmin.purgeOfflineInstances(this.helixManager.getClusterName(), offlineDuration);
Review Comment:
Well kind of. As mentioned
> But adding a timeout adds a potential bug / risk where we timeout and then helix starts purging instances while we are allocating new instances (bad bug with nondeterministic behavior)
We put ourselves at risk of having a live instances without an INSTANCE config. That would essentially bork the live instance and have a domino effect. Although I do think we should send a GTE if this call is taking too long
--
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.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] hanghangliu commented on a diff in pull request #3561: [GOBBLIN-1704] Purge offline helix instances during startup
Posted by GitBox <gi...@apache.org>.
hanghangliu commented on code in PR #3561:
URL: https://github.com/apache/gobblin/pull/3561#discussion_r972476792
##########
gobblin-yarn/src/main/java/org/apache/gobblin/yarn/YarnService.java:
##########
@@ -339,6 +342,15 @@ protected void startUp() throws Exception {
LOGGER.info("ApplicationMaster registration response: " + response);
this.maxResourceCapacity = Optional.of(response.getMaximumResourceCapability());
+ // All previous helix instances should be purged on startup. Gobblin task runners are stateless from helix
+ // perspective because all important state is persisted separately in Workunit State Store or Watermark store.
+ // Offline duration of 0 means any offline instance should be purged (Note: there aren't any online instances
+ // when this code runs, this is during startup before any containers are allocated).
+ LOGGER.info("Purging offline helix instances before allocating containers for helixClusterName={}, connectionString={}", helixManager.getClusterName(), helixManager.getMetadataStoreConnectionString());
+ long offlineDuration = 0;
+ this.helixAdmin.purgeOfflineInstances(this.helixManager.getClusterName(), offlineDuration);
Review Comment:
We should also add a config here to make this action configurable
##########
gobblin-yarn/src/main/java/org/apache/gobblin/yarn/YarnService.java:
##########
@@ -339,6 +342,15 @@ protected void startUp() throws Exception {
LOGGER.info("ApplicationMaster registration response: " + response);
this.maxResourceCapacity = Optional.of(response.getMaximumResourceCapability());
+ // All previous helix instances should be purged on startup. Gobblin task runners are stateless from helix
+ // perspective because all important state is persisted separately in Workunit State Store or Watermark store.
+ // Offline duration of 0 means any offline instance should be purged (Note: there aren't any online instances
+ // when this code runs, this is during startup before any containers are allocated).
+ LOGGER.info("Purging offline helix instances before allocating containers for helixClusterName={}, connectionString={}", helixManager.getClusterName(), helixManager.getMetadataStoreConnectionString());
+ long offlineDuration = 0;
+ this.helixAdmin.purgeOfflineInstances(this.helixManager.getClusterName(), offlineDuration);
Review Comment:
Agreed that we need to avoid the race condition, according to [ZKHelixAdmin.purgeOfflineInstances()](https://github.com/apache/helix/blob/ee61ff434ee37d4ab8456c739b2229f99d9e0e72/helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixAdmin.java#L270).
Please do add a GTE or an alert here if this takes too long
--
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.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] umustafi commented on a diff in pull request #3561: [GOBBLIN-1704] Purge offline helix instances during startup
Posted by GitBox <gi...@apache.org>.
umustafi commented on code in PR #3561:
URL: https://github.com/apache/gobblin/pull/3561#discussion_r976824839
##########
gobblin-yarn/src/main/java/org/apache/gobblin/yarn/HelixInstancePurgerWithMetrics.java:
##########
@@ -0,0 +1,140 @@
+/*
+ * 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.yarn;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Stopwatch;
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
+import lombok.AllArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.gobblin.metrics.event.EventSubmitter;
+import org.apache.gobblin.metrics.event.GobblinEventBuilder;
+import org.apache.helix.HelixAdmin;
+
+
+@Slf4j
+@AllArgsConstructor
+public class HelixInstancePurgerWithMetrics {
+ private final EventSubmitter eventSubmitter;
+ private final long pollingRateMs;
+ private static final String PREFIX = "HelixOfflineInstancePurge.";
+ public static final String PURGE_FAILURE_EVENT = PREFIX + "Failure";
+ public static final String PURGE_LAGGING_EVENT = PREFIX + "Lagging";
+ public static final String PURGE_COMPLETED_EVENT = PREFIX + "Completed";
+
+
+ /**
+ * Blocking call for purging all offline helix instances. Provides boiler plate code for providing periodic updates
+ * and sending a GTE if it's an unexpected amount of time.
+ *
+ * All previous helix instances should be purged on startup. Gobblin task runners are stateless from helix
+ * perspective because all important state is persisted separately in Workunit State Store or Watermark store.
+ */
+ public void purgeAllOfflineInstances(HelixAdmin admin, String clusterName, long laggingThresholdMs, Map<String, String> gteMetadata) {
+ CompletableFuture<Void> purgeTask = CompletableFuture.supplyAsync(() -> {
+ long offlineDuration = 0; // 0 means all offline instance should be purged.
+ admin.purgeOfflineInstances(clusterName, offlineDuration);
+ return null;
+ });
+
+ long timeToPurgeMs = waitForPurgeCompletion(purgeTask, laggingThresholdMs, Stopwatch.createUnstarted(), gteMetadata);
+ log.info("Finished purging offline helix instances. It took timeToPurgeMs={}", timeToPurgeMs);
+ }
+
+ @VisibleForTesting
+ long waitForPurgeCompletion(CompletableFuture<Void> purgeTask, long laggingThresholdMs, Stopwatch watch,
+ Map<String, String> gteMetadata) {
+ watch.start();
+ try {
+ boolean haveSubmittedLaggingEvent = false; //
+ while (!purgeTask.isDone()) {
+ long elapsedTimeMs = watch.elapsed(TimeUnit.MILLISECONDS);
+ log.info("Waiting for helix to purge offline instances. Cannot proceed with execution because purging is a "
Review Comment:
very helpful log message here!
##########
gobblin-yarn/src/main/java/org/apache/gobblin/yarn/HelixInstancePurgerWithMetrics.java:
##########
@@ -0,0 +1,140 @@
+/*
+ * 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.yarn;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Stopwatch;
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
+import lombok.AllArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.gobblin.metrics.event.EventSubmitter;
+import org.apache.gobblin.metrics.event.GobblinEventBuilder;
+import org.apache.helix.HelixAdmin;
+
+
+@Slf4j
+@AllArgsConstructor
+public class HelixInstancePurgerWithMetrics {
+ private final EventSubmitter eventSubmitter;
+ private final long pollingRateMs;
+ private static final String PREFIX = "HelixOfflineInstancePurge.";
+ public static final String PURGE_FAILURE_EVENT = PREFIX + "Failure";
+ public static final String PURGE_LAGGING_EVENT = PREFIX + "Lagging";
+ public static final String PURGE_COMPLETED_EVENT = PREFIX + "Completed";
+
+
+ /**
+ * Blocking call for purging all offline helix instances. Provides boiler plate code for providing periodic updates
+ * and sending a GTE if it's an unexpected amount of time.
+ *
+ * All previous helix instances should be purged on startup. Gobblin task runners are stateless from helix
+ * perspective because all important state is persisted separately in Workunit State Store or Watermark store.
+ */
+ public void purgeAllOfflineInstances(HelixAdmin admin, String clusterName, long laggingThresholdMs, Map<String, String> gteMetadata) {
+ CompletableFuture<Void> purgeTask = CompletableFuture.supplyAsync(() -> {
+ long offlineDuration = 0; // 0 means all offline instance should be purged.
+ admin.purgeOfflineInstances(clusterName, offlineDuration);
+ return null;
+ });
+
+ long timeToPurgeMs = waitForPurgeCompletion(purgeTask, laggingThresholdMs, Stopwatch.createUnstarted(), gteMetadata);
+ log.info("Finished purging offline helix instances. It took timeToPurgeMs={}", timeToPurgeMs);
+ }
+
+ @VisibleForTesting
+ long waitForPurgeCompletion(CompletableFuture<Void> purgeTask, long laggingThresholdMs, Stopwatch watch,
+ Map<String, String> gteMetadata) {
+ watch.start();
+ try {
+ boolean haveSubmittedLaggingEvent = false; //
+ while (!purgeTask.isDone()) {
+ long elapsedTimeMs = watch.elapsed(TimeUnit.MILLISECONDS);
+ log.info("Waiting for helix to purge offline instances. Cannot proceed with execution because purging is a "
+ + "non-thread safe call. To disable purging offline instances during startup, change the flag {} "
+ + "elapsedTimeMs={}, laggingThresholdMs={}",
+ GobblinYarnConfigurationKeys.HELIX_PURGE_OFFLINE_INSTANCES_ENABLED, elapsedTimeMs, laggingThresholdMs);
+ if (!haveSubmittedLaggingEvent && elapsedTimeMs > laggingThresholdMs) {
+ submitLaggingEvent(elapsedTimeMs, laggingThresholdMs, gteMetadata);
+ haveSubmittedLaggingEvent = true;
+ }
+ Thread.sleep(this.pollingRateMs);
+ }
+
+ long timeToPurgeMs = watch.elapsed(TimeUnit.MILLISECONDS);
+ if (!haveSubmittedLaggingEvent && timeToPurgeMs > laggingThresholdMs) {
+ submitLaggingEvent(timeToPurgeMs, laggingThresholdMs, gteMetadata);
+ }
+
+ purgeTask.get(); // check for exceptions
+ submitCompletedEvent(timeToPurgeMs, gteMetadata);
+ return timeToPurgeMs;
+ } catch (ExecutionException | InterruptedException e) {
+ log.info("The call to purge offline helix instances failed. This is not a fatal error because it is not mandatory to "
Review Comment:
nitpick can we make this a WARN type so it catches our attention more
##########
gobblin-yarn/src/main/java/org/apache/gobblin/yarn/HelixInstancePurgerWithMetrics.java:
##########
@@ -0,0 +1,140 @@
+/*
+ * 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.yarn;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Stopwatch;
+import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
+import lombok.AllArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
+import org.apache.gobblin.metrics.event.EventSubmitter;
+import org.apache.gobblin.metrics.event.GobblinEventBuilder;
+import org.apache.helix.HelixAdmin;
+
+
+@Slf4j
+@AllArgsConstructor
+public class HelixInstancePurgerWithMetrics {
+ private final EventSubmitter eventSubmitter;
+ private final long pollingRateMs;
+ private static final String PREFIX = "HelixOfflineInstancePurge.";
+ public static final String PURGE_FAILURE_EVENT = PREFIX + "Failure";
+ public static final String PURGE_LAGGING_EVENT = PREFIX + "Lagging";
+ public static final String PURGE_COMPLETED_EVENT = PREFIX + "Completed";
+
+
+ /**
+ * Blocking call for purging all offline helix instances. Provides boiler plate code for providing periodic updates
+ * and sending a GTE if it's an unexpected amount of time.
+ *
+ * All previous helix instances should be purged on startup. Gobblin task runners are stateless from helix
+ * perspective because all important state is persisted separately in Workunit State Store or Watermark store.
+ */
+ public void purgeAllOfflineInstances(HelixAdmin admin, String clusterName, long laggingThresholdMs, Map<String, String> gteMetadata) {
+ CompletableFuture<Void> purgeTask = CompletableFuture.supplyAsync(() -> {
+ long offlineDuration = 0; // 0 means all offline instance should be purged.
+ admin.purgeOfflineInstances(clusterName, offlineDuration);
+ return null;
+ });
+
+ long timeToPurgeMs = waitForPurgeCompletion(purgeTask, laggingThresholdMs, Stopwatch.createUnstarted(), gteMetadata);
+ log.info("Finished purging offline helix instances. It took timeToPurgeMs={}", timeToPurgeMs);
+ }
+
+ @VisibleForTesting
+ long waitForPurgeCompletion(CompletableFuture<Void> purgeTask, long laggingThresholdMs, Stopwatch watch,
+ Map<String, String> gteMetadata) {
+ watch.start();
+ try {
+ boolean haveSubmittedLaggingEvent = false; //
+ while (!purgeTask.isDone()) {
+ long elapsedTimeMs = watch.elapsed(TimeUnit.MILLISECONDS);
+ log.info("Waiting for helix to purge offline instances. Cannot proceed with execution because purging is a "
+ + "non-thread safe call. To disable purging offline instances during startup, change the flag {} "
+ + "elapsedTimeMs={}, laggingThresholdMs={}",
+ GobblinYarnConfigurationKeys.HELIX_PURGE_OFFLINE_INSTANCES_ENABLED, elapsedTimeMs, laggingThresholdMs);
+ if (!haveSubmittedLaggingEvent && elapsedTimeMs > laggingThresholdMs) {
+ submitLaggingEvent(elapsedTimeMs, laggingThresholdMs, gteMetadata);
+ haveSubmittedLaggingEvent = true;
+ }
+ Thread.sleep(this.pollingRateMs);
+ }
+
+ long timeToPurgeMs = watch.elapsed(TimeUnit.MILLISECONDS);
+ if (!haveSubmittedLaggingEvent && timeToPurgeMs > laggingThresholdMs) {
+ submitLaggingEvent(timeToPurgeMs, laggingThresholdMs, gteMetadata);
+ }
+
+ purgeTask.get(); // check for exceptions
+ submitCompletedEvent(timeToPurgeMs, gteMetadata);
+ return timeToPurgeMs;
+ } catch (ExecutionException | InterruptedException e) {
+ log.info("The call to purge offline helix instances failed. This is not a fatal error because it is not mandatory to "
+ + "clean up old helix instances. But repeated failure to purge offline helix instances will cause an accumulation"
+ + "of offline helix instances which may cause large delays in future helix calls.", e);
+ long timeToPurgeMs = watch.elapsed(TimeUnit.MILLISECONDS);
+ submitFailureEvent(timeToPurgeMs, gteMetadata);
+ return timeToPurgeMs;
+ }
+ }
+
+ private void submitFailureEvent(long elapsedTimeMs, Map<String, String> additionalMetadata) {
+ if (eventSubmitter != null) {
+ GobblinEventBuilder eventBuilder = new GobblinEventBuilder(PURGE_FAILURE_EVENT);
+ eventBuilder.addAdditionalMetadata(additionalMetadata);
+ eventBuilder.addMetadata("elapsedTimeMs", String.valueOf(elapsedTimeMs));
+
+ log.info("Submitting GTE because purging offline instances has failed to complete. event={}", eventBuilder);
+ eventSubmitter.submit(eventBuilder);
+ } else {
+ log.info("Cannot submit {} GTE because eventSubmitter is null", PURGE_FAILURE_EVENT);
+ }
+ }
+
+ private void submitCompletedEvent(long timeToPurgeMs, Map<String, String> additionalMetadata) {
+ if (eventSubmitter != null) {
+ GobblinEventBuilder eventBuilder = new GobblinEventBuilder(PURGE_COMPLETED_EVENT);
+ eventBuilder.addAdditionalMetadata(additionalMetadata);
+ eventBuilder.addMetadata("timeToPurgeMs", String.valueOf(timeToPurgeMs));
+
+ log.info("Submitting GTE because purging offline instances has completed successfully. event={}", eventBuilder);
+ eventSubmitter.submit(eventBuilder);
+ } else {
+ log.info("Cannot submit {} GTE because eventSubmitter is null", PURGE_COMPLETED_EVENT);
+ }
+ }
+
+ private void submitLaggingEvent(long elapsedTimeMs, long laggingThresholdMs,
Review Comment:
how do we track these events? are they processed somewhere or do we have any automatic monitoring? or will we query them when there's an issue we're noticing with helix?
--
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.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] hanghangliu commented on pull request #3561: [GOBBLIN-1704] Purge offline helix instances during startup
Posted by GitBox <gi...@apache.org>.
hanghangliu commented on PR #3561:
URL: https://github.com/apache/gobblin/pull/3561#issuecomment-1255537970
LGTM +1
--
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.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] codecov-commenter commented on pull request #3561: [GOBBLIN-1704] Purge offline helix instances during startup
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #3561:
URL: https://github.com/apache/gobblin/pull/3561#issuecomment-1245983645
# [Codecov](https://codecov.io/gh/apache/gobblin/pull/3561?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#3561](https://codecov.io/gh/apache/gobblin/pull/3561?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f681323) into [master](https://codecov.io/gh/apache/gobblin/commit/f469b11f088611c8b5da0e13a7ebf3b9f5bf0a14?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f469b11) will **decrease** coverage by `2.92%`.
> The diff coverage is `100.00%`.
```diff
@@ Coverage Diff @@
## master #3561 +/- ##
============================================
- Coverage 46.79% 43.87% -2.93%
+ Complexity 10501 2056 -8445
============================================
Files 2089 406 -1683
Lines 81880 17535 -64345
Branches 9127 2142 -6985
============================================
- Hits 38317 7693 -30624
+ Misses 40018 8987 -31031
+ Partials 3545 855 -2690
```
| [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3561?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ache/gobblin/cluster/GobblinHelixMultiManager.java](https://codecov.io/gh/apache/gobblin/pull/3561/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvR29iYmxpbkhlbGl4TXVsdGlNYW5hZ2VyLmphdmE=) | `54.54% <100.00%> (+1.66%)` | :arrow_up: |
| [.../gobblin/cluster/GobblinHelixTaskStateTracker.java](https://codecov.io/gh/apache/gobblin/pull/3561/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvR29iYmxpbkhlbGl4VGFza1N0YXRlVHJhY2tlci5qYXZh) | `62.50% <0.00%> (-6.25%)` | :arrow_down: |
| [...main/java/org/apache/gobblin/util/HadoopUtils.java](https://codecov.io/gh/apache/gobblin/pull/3561/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvSGFkb29wVXRpbHMuamF2YQ==) | `29.54% <0.00%> (-0.65%)` | :arrow_down: |
| [.../apache/gobblin/yarn/GobblinApplicationMaster.java](https://codecov.io/gh/apache/gobblin/pull/3561/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi15YXJuL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3lhcm4vR29iYmxpbkFwcGxpY2F0aW9uTWFzdGVyLmphdmE=) | | |
| [...main/java/org/apache/gobblin/yarn/YarnService.java](https://codecov.io/gh/apache/gobblin/pull/3561/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi15YXJuL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3lhcm4vWWFyblNlcnZpY2UuamF2YQ==) | | |
| [.../compaction/mapreduce/MRCompactionTaskFactory.java](https://codecov.io/gh/apache/gobblin/pull/3561/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1jb21wYWN0aW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NvbXBhY3Rpb24vbWFwcmVkdWNlL01SQ29tcGFjdGlvblRhc2tGYWN0b3J5LmphdmE=) | | |
| [...ment/copy/replication/ReplicaHadoopFsEndPoint.java](https://codecov.io/gh/apache/gobblin/pull/3561/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1kYXRhLW1hbmFnZW1lbnQvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YS9tYW5hZ2VtZW50L2NvcHkvcmVwbGljYXRpb24vUmVwbGljYUhhZG9vcEZzRW5kUG9pbnQuamF2YQ==) | | |
| [...bblin/recordaccess/FieldDoesNotExistException.java](https://codecov.io/gh/apache/gobblin/pull/3561/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vcmVjb3JkYWNjZXNzL0ZpZWxkRG9lc05vdEV4aXN0RXhjZXB0aW9uLmphdmE=) | | |
| [...rvice/modules/spec/JobExecutionPlanDagFactory.java](https://codecov.io/gh/apache/gobblin/pull/3561/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NlcnZpY2UvbW9kdWxlcy9zcGVjL0pvYkV4ZWN1dGlvblBsYW5EYWdGYWN0b3J5LmphdmE=) | | |
| [...org/apache/gobblin/compaction/dataset/Dataset.java](https://codecov.io/gh/apache/gobblin/pull/3561/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1jb21wYWN0aW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NvbXBhY3Rpb24vZGF0YXNldC9EYXRhc2V0LmphdmE=) | | |
| ... and [1679 more](https://codecov.io/gh/apache/gobblin/pull/3561/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [gobblin] homatthew commented on a diff in pull request #3561: [GOBBLIN-1704] Purge offline helix instances during startup
Posted by GitBox <gi...@apache.org>.
homatthew commented on code in PR #3561:
URL: https://github.com/apache/gobblin/pull/3561#discussion_r972482144
##########
gobblin-yarn/src/main/java/org/apache/gobblin/yarn/YarnService.java:
##########
@@ -339,6 +342,15 @@ protected void startUp() throws Exception {
LOGGER.info("ApplicationMaster registration response: " + response);
this.maxResourceCapacity = Optional.of(response.getMaximumResourceCapability());
+ // All previous helix instances should be purged on startup. Gobblin task runners are stateless from helix
+ // perspective because all important state is persisted separately in Workunit State Store or Watermark store.
+ // Offline duration of 0 means any offline instance should be purged (Note: there aren't any online instances
+ // when this code runs, this is during startup before any containers are allocated).
+ LOGGER.info("Purging offline helix instances before allocating containers for helixClusterName={}, connectionString={}", helixManager.getClusterName(), helixManager.getMetadataStoreConnectionString());
+ long offlineDuration = 0;
+ this.helixAdmin.purgeOfflineInstances(this.helixManager.getClusterName(), offlineDuration);
Review Comment:
Thanks for linking the code documentation. Forgot to link that
--
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.
To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org