You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by timothyjward <gi...@git.apache.org> on 2018/05/23 09:58:01 UTC
[GitHub] zookeeper pull request #530: [ZOOKEEPER-2474] Support session reconnection w...
GitHub user timothyjward opened a pull request:
https://github.com/apache/zookeeper/pull/530
[ZOOKEEPER-2474] Support session reconnection when using ZKClientConfig
Signed-off-by: Tim Ward <ti...@apache.org>
This PR simply adds constructors which support session reconnection when using ZKClientConfig
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/timothyjward/zookeeper ZOOKEEPER-2474
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zookeeper/pull/530.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 #530
----
commit a188c5bce1b1c54376c8e4fd7a92e3a4badd5715
Author: Tim Ward <ti...@...>
Date: 2018-05-23T09:53:13Z
[ZOOKEEPER-2474] Support session reconnection when using ZKClientConfig
Signed-off-by: Tim Ward <ti...@apache.org>
----
---
[GitHub] zookeeper issue #530: [ZOOKEEPER-2474] Support session reconnection when usi...
Posted by anmolnar <gi...@git.apache.org>.
Github user anmolnar commented on the issue:
https://github.com/apache/zookeeper/pull/530
@timothyjward Okay, I hear you.
I'll check the Jira, because I think this should go to master too and commit soon.
---
[GitHub] zookeeper issue #530: [ZOOKEEPER-2474] Support session reconnection when usi...
Posted by timothyjward <gi...@git.apache.org>.
Github user timothyjward commented on the issue:
https://github.com/apache/zookeeper/pull/530
I'm still not sure what additional testing is possible. The way the patch is constructed the changed code (all three lines of it) is already exercised by the existing ZooKeeper tests. The new constructors all delegate to existing constructors, so I'm not sure how to create a "different" test rather than just duplicating another test, which seems like a poor practice to me.
---
[GitHub] zookeeper pull request #530: [ZOOKEEPER-2474] Support session reconnection w...
Posted by maoling <gi...@git.apache.org>.
Github user maoling commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/530#discussion_r194383541
--- Diff: src/java/main/org/apache/zookeeper/ZooKeeper.java ---
@@ -1218,6 +1294,79 @@ public ZooKeeper(String connectString, int sessionTimeout, Watcher watcher,
canBeReadOnly, createDefaultHostProvider(connectString));
}
+ /**
+ * To create a ZooKeeper client object, the application needs to pass a
+ * connection string containing a comma separated list of host:port pairs,
+ * each corresponding to a ZooKeeper server.
+ * <p>
+ * Session establishment is asynchronous. This constructor will initiate
+ * connection to the server and return immediately - potentially (usually)
+ * before the session is fully established. The watcher argument specifies
+ * the watcher that will be notified of any changes in state. This
+ * notification can come at any point before or after the constructor call
+ * has returned.
+ * <p>
+ * The instantiated ZooKeeper client object will pick an arbitrary server
+ * from the connectString and attempt to connect to it. If establishment of
+ * the connection fails, another server in the connect string will be tried
+ * (the order is non-deterministic, as we random shuffle the list), until a
+ * connection is established. The client will continue attempts until the
+ * session is explicitly closed (or the session is expired by the server).
+ * <p>
+ * Added in 3.2.0: An optional "chroot" suffix may also be appended to the
+ * connection string. This will run the client commands while interpreting
+ * all paths relative to this root (similar to the unix chroot command).
+ * <p>
+ * Use {@link #getSessionId} and {@link #getSessionPasswd} on an established
+ * client connection, these values must be passed as sessionId and
+ * sessionPasswd respectively if reconnecting. Otherwise, if not
+ * reconnecting, use the other constructor which does not require these
+ * parameters.
+ * <p>
+ * This constructor uses a StaticHostProvider; there is another one
+ * to enable custom behaviour.
+ *
+ * @param connectString
+ * comma separated host:port pairs, each corresponding to a zk
+ * server. e.g. "127.0.0.1:3000,127.0.0.1:3001,127.0.0.1:3002"
+ * If the optional chroot suffix is used the example would look
+ * like: "127.0.0.1:3000,127.0.0.1:3001,127.0.0.1:3002/app/a"
+ * where the client would be rooted at "/app/a" and all paths
+ * would be relative to this root - ie getting/setting/etc...
+ * "/foo/bar" would result in operations being run on
+ * "/app/a/foo/bar" (from the server perspective).
+ * @param sessionTimeout
+ * session timeout in milliseconds
+ * @param watcher
+ * a watcher object which will be notified of state changes, may
+ * also be notified for node events
+ * @param sessionId
+ * specific session id to use if reconnecting
+ * @param sessionPasswd
+ * password for this session
+ * @param canBeReadOnly
+ * (added in 3.4) whether the created client is allowed to go to
+ * read-only mode in case of partitioning. Read-only mode
+ * basically means that if the client can't find any majority
+ * servers but there's partitioned server it could reach, it
+ * connects to one in read-only mode, i.e. read requests are
+ * allowed while write requests are not. It continues seeking for
+ * majority in the background.
+ * @param conf
+ * (added in 3.5.2) passing this conf object gives each client the flexibility of
+ * configuring properties differently compared to other instances
+ * @throws IOException in cases of network failure
+ * @throws IllegalArgumentException if an invalid chroot path is specified
+ */
+ public ZooKeeper(String connectString, int sessionTimeout, Watcher watcher,
+ long sessionId, byte[] sessionPasswd, boolean canBeReadOnly, ZKClientConfig conf)
+ throws IOException {
+ this(connectString, sessionTimeout, watcher, sessionId, sessionPasswd,
+ canBeReadOnly, createDefaultHostProvider(connectString));
+ }
+
+
+
// default hostprovider
private static HostProvider createDefaultHostProvider(String connectString) {
return new StaticHostProvider(
--- End diff --
+l,LGTM
need a Unit Test,some inspirations can be found at the [old-patch](https://issues.apache.org/jira/secure/attachment/12822755/ZOOKEEPER-2474-01.patch) which may be better to add `newZKClient.close() ` in the last line
---
[GitHub] zookeeper issue #530: [ZOOKEEPER-2474] Support session reconnection when usi...
Posted by maoling <gi...@git.apache.org>.
Github user maoling commented on the issue:
https://github.com/apache/zookeeper/pull/530
@timothyjward Are you still working on this?
Or could someone else pick up this?
---
[GitHub] zookeeper issue #530: [ZOOKEEPER-2474] Support session reconnection when usi...
Posted by maoling <gi...@git.apache.org>.
Github user maoling commented on the issue:
https://github.com/apache/zookeeper/pull/530
@timothyjward
Some inspirations can be found at the [old-patch](https://issues.apache.org/jira/secure/attachment/12822755/ZOOKEEPER-2474-01.patch).
This patch LGTM and I had tested it. ping @anmolnar
---
[GitHub] zookeeper issue #530: [ZOOKEEPER-2474] Support session reconnection when usi...
Posted by maoling <gi...@git.apache.org>.
Github user maoling commented on the issue:
https://github.com/apache/zookeeper/pull/530
@timothyjward Can we move on ?
---
[GitHub] zookeeper issue #530: [ZOOKEEPER-2474] Support session reconnection when usi...
Posted by anmolnar <gi...@git.apache.org>.
Github user anmolnar commented on the issue:
https://github.com/apache/zookeeper/pull/530
@timothyjward Do you still working on this?
Would you please rebase and create a separate patch for the master branch?
Once it's applied in there, we can merge this one too and close the ticket.
---
[GitHub] zookeeper issue #530: [ZOOKEEPER-2474] Support session reconnection when usi...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:
https://github.com/apache/zookeeper/pull/530
Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2515/
---
[GitHub] zookeeper issue #530: [ZOOKEEPER-2474] Support session reconnection when usi...
Posted by timothyjward <gi...@git.apache.org>.
Github user timothyjward commented on the issue:
https://github.com/apache/zookeeper/pull/530
I'm happy for this to be merged. The code paths are tested already because of the way the constructors delegate. Is there anything further?
---