You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@twill.apache.org by chtyim <gi...@git.apache.org> on 2015/10/16 01:14:34 UTC
[GitHub] incubator-twill pull request: (TWILL-131) Remove ZK node when appl...
GitHub user chtyim opened a pull request:
https://github.com/apache/incubator-twill/pull/70
(TWILL-131) Remove ZK node when application finished.
- Remove the application ZK node when the application terminates
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/chtyim/incubator-twill feature/TWILL-131
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-twill/pull/70.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #70
----
commit 94a54da147cf74d7fd292369f6f28ea24b9d13b4
Author: Terence Yim <ch...@apache.org>
Date: 2015-10-14T20:39:32Z
(TWILL-131) Remove ZK node when application finished.
- Remove the application ZK node when the application terminates
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-twill pull request: (TWILL-131) Remove ZK node when appl...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-twill/pull/70
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-twill pull request: (TWILL-131) Remove ZK node when appl...
Posted by awholegunch <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-twill pull request: (TWILL-131) Remove ZK node when appl...
Posted by gsps1 <gi...@git.apache.org>.
Github user gsps1 commented on a diff in the pull request:
https://github.com/apache/incubator-twill/pull/70#discussion_r42302190
--- 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();
--- End diff --
wouldn't ``Futures.successfulAsList(deleteFutures).get(TIMEOUT_SECONDS, TimeUnit.SECONDS);`` already ensure the futures are completed?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-twill pull request: (TWILL-131) Remove ZK node when appl...
Posted by gsps1 <gi...@git.apache.org>.
Github user gsps1 commented on a diff in the pull request:
https://github.com/apache/incubator-twill/pull/70#discussion_r42302212
--- 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
+ */
+ private boolean delete(String path) throws Exception {
+ try {
+ LOG.info("Removing ZK path: {}{}", zkClient.getConnectString(), path);
--- End diff --
should this be logged as ``debug`` ?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-twill pull request: (TWILL-131) Remove ZK node when appl...
Posted by chtyim <gi...@git.apache.org>.
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/incubator-twill/pull/70#discussion_r42435827
--- 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
+ */
+ private boolean delete(String path) throws Exception {
+ try {
+ LOG.info("Removing ZK path: {}{}", zkClient.getConnectString(), path);
--- End diff --
All ZK node operation are logged as info.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-twill pull request: (TWILL-131) Remove ZK node when appl...
Posted by chtyim <gi...@git.apache.org>.
Github user chtyim commented on a diff in the pull request:
https://github.com/apache/incubator-twill/pull/70#discussion_r42435799
--- 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();
--- End diff --
It does. But we need to call `Future.get()` to determine if the future actually completed successfully or got error.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-twill pull request: (TWILL-131) Remove ZK node when appl...
Posted by awholegunch <gi...@git.apache.org>.
Github user awholegunch commented on a diff in the pull request:
https://github.com/apache/incubator-twill/pull/70#discussion_r42194452
--- 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 --
@return true if the path was deleted, false if it failed to delete due to {@link KeeperException.NotEmptyException}.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-twill pull request: (TWILL-131) Remove ZK node when appl...
Posted by chtyim <gi...@git.apache.org>.
Github user chtyim commented on the pull request:
https://github.com/apache/incubator-twill/pull/70#issuecomment-149610343
Squashed commits.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-twill pull request: (TWILL-131) Remove ZK node when appl...
Posted by awholegunch <gi...@git.apache.org>.
Github user awholegunch commented on a diff in the pull request:
https://github.com/apache/incubator-twill/pull/70#discussion_r42194118
--- 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
--- End diff --
Deletes ZK nodes created for the application execution.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-twill pull request: (TWILL-131) Remove ZK node when appl...
Posted by awholegunch <gi...@git.apache.org>.
Github user awholegunch commented on a diff in the pull request:
https://github.com/apache/incubator-twill/pull/70#discussion_r42194244
--- 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.
--- End diff --
We don't have to worry about a race condition if another instance of the same app starts at the same time
as when removal is performed because we always create the node with "createParent == true", which takes care of
the parent node recreation if it is being removed from here.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-twill pull request: (TWILL-131) Remove ZK node when appl...
Posted by awholegunch <gi...@git.apache.org>.
Github user awholegunch commented on a diff in the pull request:
https://github.com/apache/incubator-twill/pull/70#discussion_r42194105
--- 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.
--- End diff --
same app running, which we can safely ignore and return.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-twill pull request: (TWILL-131) Remove ZK node when appl...
Posted by gsps1 <gi...@git.apache.org>.
Github user gsps1 commented on a diff in the pull request:
https://github.com/apache/incubator-twill/pull/70#discussion_r42443833
--- 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();
--- End diff --
ok
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-twill pull request: (TWILL-131) Remove ZK node when appl...
Posted by gsps1 <gi...@git.apache.org>.
Github user gsps1 commented on the pull request:
https://github.com/apache/incubator-twill/pull/70#issuecomment-149385965
LGTM :+1:
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---