You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by jtstorck <gi...@git.apache.org> on 2016/12/05 21:56:47 UTC
[GitHub] nifi pull request #1301: NIFI-3150 Added logic to wait for the zk client to ...
GitHub user jtstorck opened a pull request:
https://github.com/apache/nifi/pull/1301
NIFI-3150 Added logic to wait for the zk client to connect to the con\u2026
\u2026figured server
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jtstorck/nifi NIFI-3150
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi/pull/1301.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 #1301
----
commit f439f5a42f5d5962ec6e4ccf3beaf6b2189b8374
Author: Jeff Storck <jt...@gmail.com>
Date: 2016-12-05T18:19:31Z
NIFI-3150 Added logic to wait for the zk client to connect to the configured server
----
---
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] nifi pull request #1301: NIFI-3150 Added logic to wait for the zk client to ...
Posted by jtstorck <gi...@git.apache.org>.
Github user jtstorck commented on a diff in the pull request:
https://github.com/apache/nifi/pull/1301#discussion_r90972151
--- Diff: nifi-toolkit/nifi-toolkit-zookeeper-migrator/src/main/java/org/apache/nifi/toolkit/zkmigrator/ZooKeeperMigrator.java ---
@@ -284,8 +287,29 @@ private Stat transmitNode(ZooKeeper zooKeeper, DataStatAclNode node) {
}
private ZooKeeper getZooKeeper(ZooKeeperEndpointConfig zooKeeperEndpointConfig, AuthMode authMode, byte[] authData) throws IOException {
+ CountDownLatch connectionLatch = new CountDownLatch(1);
ZooKeeper zooKeeper = new ZooKeeper(zooKeeperEndpointConfig.getConnectString(), 3000, watchedEvent -> {
+ if (LOGGER.isDebugEnabled()) {
+ LOGGER.debug("ZooKeeper server state changed to {} in {}", watchedEvent.getState(), zooKeeperEndpointConfig);
+ }
+ if (watchedEvent.getType().equals(Watcher.Event.EventType.None) && watchedEvent.getState().equals(Watcher.Event.KeeperState.SyncConnected)) {
+ connectionLatch.countDown();
+ }
});
+
+ final boolean connected;
+ try {
+ connected = connectionLatch.await(5, TimeUnit.SECONDS);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new IOException(String.format("interrupted while waiting for ZooKeeper connection to %s", zooKeeperEndpointConfig), e);
+ }
+
+ if (!connected) {
+ throw new IOException(String.format("unable to connect to %s, state is %s", zooKeeperEndpointConfig, zooKeeper.getState()));
--- End diff --
I don't think it's necessary, since the migrator will exit if a connection with ZK cannot be established, but I will add it for the sake of explicit cleanup.
---
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] nifi issue #1301: NIFI-3150 Added logic to wait for the zk client to connect...
Posted by brosander <gi...@git.apache.org>.
Github user brosander commented on the issue:
https://github.com/apache/nifi/pull/1301
Merge commit: https://github.com/apache/nifi/commit/78de10dec0127598fbbbb8ac7f7048cec3aecb6b
---
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] nifi issue #1301: NIFI-3150 Added logic to wait for the zk client to connect...
Posted by brosander <gi...@git.apache.org>.
Github user brosander commented on the issue:
https://github.com/apache/nifi/pull/1301
@jtstorck is it safe to remove that? What happens if the main method terminates before the reads/writes are finished?
---
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] nifi issue #1301: NIFI-3150 Added logic to wait for the zk client to connect...
Posted by brosander <gi...@git.apache.org>.
Github user brosander commented on the issue:
https://github.com/apache/nifi/pull/1301
@jtstorck makes sense, +1, merging
---
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] nifi pull request #1301: NIFI-3150 Added logic to wait for the zk client to ...
Posted by jtstorck <gi...@git.apache.org>.
Github user jtstorck commented on a diff in the pull request:
https://github.com/apache/nifi/pull/1301#discussion_r90969390
--- Diff: nifi-toolkit/nifi-toolkit-zookeeper-migrator/src/main/java/org/apache/nifi/toolkit/zkmigrator/ZooKeeperMigrator.java ---
@@ -284,8 +286,40 @@ private Stat transmitNode(ZooKeeper zooKeeper, DataStatAclNode node) {
}
private ZooKeeper getZooKeeper(ZooKeeperEndpointConfig zooKeeperEndpointConfig, AuthMode authMode, byte[] authData) throws IOException {
+ CountDownLatch connectionLatch = new CountDownLatch(1);
ZooKeeper zooKeeper = new ZooKeeper(zooKeeperEndpointConfig.getConnectString(), 3000, watchedEvent -> {
+ if (LOGGER.isDebugEnabled()) {
+ LOGGER.debug("ZooKeeper server state changed to {} in {}", watchedEvent.getState(), zooKeeperEndpointConfig);
+ }
+ switch (watchedEvent.getType()) {
+ case None:
+ switch (watchedEvent.getState()) {
+ case SyncConnected:
+ connectionLatch.countDown();
+ break;
+ case Expired:
+ case AuthFailed:
+ case ConnectedReadOnly:
+ case SaslAuthenticated:
+ case Disconnected:
+ break;
+ }
+ }
});
+
+ final boolean connected;
+ try {
+ connected = connectionLatch.await(5, TimeUnit.SECONDS);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new RuntimeException(String.format("interrupted while waiting for ZooKeeper connection to %s", zooKeeperEndpointConfig), e);
--- End diff --
Sure, I can change the RuntimeExceptions to IOExceptions.
---
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] nifi pull request #1301: NIFI-3150 Added logic to wait for the zk client to ...
Posted by brosander <gi...@git.apache.org>.
Github user brosander commented on a diff in the pull request:
https://github.com/apache/nifi/pull/1301#discussion_r90969885
--- Diff: nifi-toolkit/nifi-toolkit-zookeeper-migrator/src/main/java/org/apache/nifi/toolkit/zkmigrator/ZooKeeperMigrator.java ---
@@ -284,8 +287,29 @@ private Stat transmitNode(ZooKeeper zooKeeper, DataStatAclNode node) {
}
private ZooKeeper getZooKeeper(ZooKeeperEndpointConfig zooKeeperEndpointConfig, AuthMode authMode, byte[] authData) throws IOException {
+ CountDownLatch connectionLatch = new CountDownLatch(1);
ZooKeeper zooKeeper = new ZooKeeper(zooKeeperEndpointConfig.getConnectString(), 3000, watchedEvent -> {
+ if (LOGGER.isDebugEnabled()) {
+ LOGGER.debug("ZooKeeper server state changed to {} in {}", watchedEvent.getState(), zooKeeperEndpointConfig);
+ }
+ if (watchedEvent.getType().equals(Watcher.Event.EventType.None) && watchedEvent.getState().equals(Watcher.Event.KeeperState.SyncConnected)) {
+ connectionLatch.countDown();
+ }
});
+
+ final boolean connected;
+ try {
+ connected = connectionLatch.await(5, TimeUnit.SECONDS);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new IOException(String.format("interrupted while waiting for ZooKeeper connection to %s", zooKeeperEndpointConfig), e);
+ }
+
+ if (!connected) {
+ throw new IOException(String.format("unable to connect to %s, state is %s", zooKeeperEndpointConfig, zooKeeper.getState()));
--- End diff --
Should we still try to close the zk connection to free any resources 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] nifi issue #1301: NIFI-3150 Added logic to wait for the zk client to connect...
Posted by jtstorck <gi...@git.apache.org>.
Github user jtstorck commented on the issue:
https://github.com/apache/nifi/pull/1301
Was merged, closing
---
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] nifi issue #1301: NIFI-3150 Added logic to wait for the zk client to connect...
Posted by jtstorck <gi...@git.apache.org>.
Github user jtstorck commented on the issue:
https://github.com/apache/nifi/pull/1301
@brosander Those close calls were not in the original contrib for the ZooKeeper Migrator. I will investigate this further and make another contribution if it is discovered that there is an issue preventing all nodes being read/written.
---
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] nifi issue #1301: NIFI-3150 Added logic to wait for the zk client to connect...
Posted by brosander <gi...@git.apache.org>.
Github user brosander commented on the issue:
https://github.com/apache/nifi/pull/1301
@jtstorck thanks for closing, I forgot the "this closes line"
---
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] nifi pull request #1301: NIFI-3150 Added logic to wait for the zk client to ...
Posted by brosander <gi...@git.apache.org>.
Github user brosander commented on a diff in the pull request:
https://github.com/apache/nifi/pull/1301#discussion_r90969779
--- Diff: nifi-toolkit/nifi-toolkit-zookeeper-migrator/src/main/java/org/apache/nifi/toolkit/zkmigrator/ZooKeeperMigrator.java ---
@@ -284,8 +287,29 @@ private Stat transmitNode(ZooKeeper zooKeeper, DataStatAclNode node) {
}
private ZooKeeper getZooKeeper(ZooKeeperEndpointConfig zooKeeperEndpointConfig, AuthMode authMode, byte[] authData) throws IOException {
+ CountDownLatch connectionLatch = new CountDownLatch(1);
ZooKeeper zooKeeper = new ZooKeeper(zooKeeperEndpointConfig.getConnectString(), 3000, watchedEvent -> {
+ if (LOGGER.isDebugEnabled()) {
+ LOGGER.debug("ZooKeeper server state changed to {} in {}", watchedEvent.getState(), zooKeeperEndpointConfig);
+ }
+ if (watchedEvent.getType().equals(Watcher.Event.EventType.None) && watchedEvent.getState().equals(Watcher.Event.KeeperState.SyncConnected)) {
+ connectionLatch.countDown();
+ }
});
+
+ final boolean connected;
+ try {
+ connected = connectionLatch.await(5, TimeUnit.SECONDS);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new RuntimeException(String.format("interrupted while waiting for ZooKeeper connection to %s", zooKeeperEndpointConfig), e);
+ }
+
+ if (!connected) {
--- End diff --
Should we still try to close the zk connection to free any resources 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] nifi pull request #1301: NIFI-3150 Added logic to wait for the zk client to ...
Posted by jtstorck <gi...@git.apache.org>.
Github user jtstorck closed the pull request at:
https://github.com/apache/nifi/pull/1301
---
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] nifi pull request #1301: NIFI-3150 Added logic to wait for the zk client to ...
Posted by jtstorck <gi...@git.apache.org>.
Github user jtstorck commented on a diff in the pull request:
https://github.com/apache/nifi/pull/1301#discussion_r90969482
--- Diff: nifi-toolkit/nifi-toolkit-zookeeper-migrator/src/main/java/org/apache/nifi/toolkit/zkmigrator/ZooKeeperMigrator.java ---
@@ -284,8 +286,40 @@ private Stat transmitNode(ZooKeeper zooKeeper, DataStatAclNode node) {
}
private ZooKeeper getZooKeeper(ZooKeeperEndpointConfig zooKeeperEndpointConfig, AuthMode authMode, byte[] authData) throws IOException {
+ CountDownLatch connectionLatch = new CountDownLatch(1);
ZooKeeper zooKeeper = new ZooKeeper(zooKeeperEndpointConfig.getConnectString(), 3000, watchedEvent -> {
+ if (LOGGER.isDebugEnabled()) {
+ LOGGER.debug("ZooKeeper server state changed to {} in {}", watchedEvent.getState(), zooKeeperEndpointConfig);
+ }
+ switch (watchedEvent.getType()) {
+ case None:
+ switch (watchedEvent.getState()) {
+ case SyncConnected:
+ connectionLatch.countDown();
+ break;
+ case Expired:
+ case AuthFailed:
+ case ConnectedReadOnly:
+ case SaslAuthenticated:
+ case Disconnected:
+ break;
+ }
+ }
});
+
+ final boolean connected;
+ try {
+ connected = connectionLatch.await(5, TimeUnit.SECONDS);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new RuntimeException(String.format("interrupted while waiting for ZooKeeper connection to %s", zooKeeperEndpointConfig), e);
--- End diff --
However, there are other areas in the code that throw RuntimeExceptions. Could address those in another PR if necessary.
---
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.
---