Github user awholegunch commented on a diff in the pull request:
https://github.com/apache/incubator-twill/pull/70#discussion_r42194494
--- Diff:
twill-yarn/src/main/java/org/apache/twill/internal/appmaster/ApplicationMasterMain.java
---
@@ -229,4 +234,82 @@ protected void shutDown() throws Exception {
}
}
}
+
+ private static final class AppMasterTwillZKPathService extends
TwillZKPathService {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(AppMasterTwillZKPathService.class);
+ private final ZKClient zkClient;
+
+ public AppMasterTwillZKPathService(ZKClient zkClient, RunId runId) {
+ super(zkClient, runId);
+ this.zkClient = zkClient;
+ }
+
+ @Override
+ protected void shutDown() throws Exception {
+ super.shutDown();
+
+ // Deletes ZK nodes created for the application execution
+ // We don't have to worry about race condition when another instance
of the same app starts at the same time
+ // when removal is performed because we always create node with
"createParent == true", which will take care of
+ // the the parent node recreation if it is getting removed from here.
+
+ // Try to delete the /instances path. It may throws
NotEmptyException if there are other instances of the
+ // same app running, which can safely ignore and return.
+ if (!delete(Constants.INSTANCES_PATH_PREFIX)) {
+ return;
+ }
+
+ // Try to delete children under /discovery. It may fail with
NotEmptyException if there are other instances
+ // of the same app running that has discovery services running.
+ List<String> children =
zkClient.getChildren(Constants.DISCOVERY_PATH_PREFIX)
+ .get(TIMEOUT_SECONDS,
TimeUnit.SECONDS).getChildren();
+ List<OperationFuture<?>> deleteFutures = new ArrayList<>();
+ for (String child : children) {
+ String path = Constants.DISCOVERY_PATH_PREFIX + "/" + child;
+ LOG.info("Removing ZK path: {}{}", zkClient.getConnectString(),
path);
+ deleteFutures.add(zkClient.delete(path));
+ }
+ Futures.successfulAsList(deleteFutures).get(TIMEOUT_SECONDS,
TimeUnit.SECONDS);
+ for (OperationFuture<?> future : deleteFutures) {
+ try {
+ future.get();
+ } catch (ExecutionException e) {
+ if (e.getCause() instanceof KeeperException.NotEmptyException) {
+ return;
+ }
+ throw e;
+ }
+ }
+
+ // Delete the /discovery. It may fail with NotEmptyException (due to
race between apps),
+ // which can safely ignore and return.
+ if (!delete(Constants.DISCOVERY_PATH_PREFIX)) {
+ return;
+ }
+
+ // Delete the ZK path for the app namespace.
+ delete("/");
+ }
+
+ /**
+ * Deletes the given ZK path.
+ *
+ * @param path path to delete
+ * @return true if the path is delete, false if failed to delete due
to {@link KeeperException.NotEmptyException}.
+ * @throws Exception if failed to delete
--- End diff --
@throws Exception if it failed to delete the path
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
