You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by mkedwards <gi...@git.apache.org> on 2018/11/25 22:27:04 UTC
[GitHub] zookeeper pull request #721: ZOOKEEPER-3046: wait for clients to reconnect a...
GitHub user mkedwards opened a pull request:
https://github.com/apache/zookeeper/pull/721
ZOOKEEPER-3046: wait for clients to reconnect after restarting server
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mkedwards/zookeeper ZOOKEEPER-3046
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zookeeper/pull/721.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 #721
----
commit 62e6bca2460b261e7cad204566f7b88a41ab0972
Author: Michael Edwards <michael edwards>
Date: 2018-11-25T22:23:29Z
ZOOKEEPER-3046: wait for clients to reconnect after restarting server
----
---
[GitHub] zookeeper pull request #721: ZOOKEEPER-3046: wait for clients to reconnect a...
Posted by mkedwards <gi...@git.apache.org>.
Github user mkedwards commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/721#discussion_r236097804
--- Diff: zookeeper-server/src/test/java/org/apache/zookeeper/test/DisconnectedWatcherTest.java ---
@@ -221,6 +228,7 @@ public void testManyChildWatchersAutoReset() throws Exception {
watcher.waitForDisconnected(30000);
startServer();
watcher.waitForConnected(30000);
+ watcher1.waitForConnected(30000);
--- End diff --
That example is due to an unrelated class of failure, in which the Zookeeper server is permanently unreachable (notice that the entire test timed out); I think many failure cases of that kind are the product of failure to bind() the dynamically-assigned port to the server socket (probably due to collisions with other tests running concurrently on the same build host). I've updated the description of this PR to explain what kind of failure it's intended to fix.
---
[GitHub] zookeeper pull request #721: ZOOKEEPER-3046: wait for clients to reconnect a...
Posted by mkedwards <gi...@git.apache.org>.
Github user mkedwards closed the pull request at:
https://github.com/apache/zookeeper/pull/721
---
[GitHub] zookeeper issue #721: ZOOKEEPER-3046: wait for clients to reconnect after re...
Posted by mkedwards <gi...@git.apache.org>.
Github user mkedwards commented on the issue:
https://github.com/apache/zookeeper/pull/721
Except, er, that seems to make the tests fail. 😝 Investigating.
---
[GitHub] zookeeper issue #721: ZOOKEEPER-3046: wait for clients to reconnect after re...
Posted by mkedwards <gi...@git.apache.org>.
Github user mkedwards commented on the issue:
https://github.com/apache/zookeeper/pull/721
retest this please
---
[GitHub] zookeeper pull request #721: ZOOKEEPER-3046: wait for clients to reconnect a...
Posted by lavacat <gi...@git.apache.org>.
Github user lavacat commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/721#discussion_r236095252
--- Diff: zookeeper-server/src/test/java/org/apache/zookeeper/test/DisconnectedWatcherTest.java ---
@@ -221,6 +228,7 @@ public void testManyChildWatchersAutoReset() throws Exception {
watcher.waitForDisconnected(30000);
startServer();
watcher.waitForConnected(30000);
+ watcher1.waitForConnected(30000);
--- End diff --
If this is fixing this test - that's great.
Trying to understand why. If zk1 isn't connected, should we get CONNECTIONLOSS on line 237 zk1.create?
Here is an example and I don't see it.
https://builds.apache.org/job/ZooKeeper_branch35_java10/263/testReport/junit/org.apache.zookeeper.test/DisconnectedWatcherTest/testManyChildWatchersAutoReset/
Good improvement anyway, I think should be merged.
---
[GitHub] zookeeper issue #721: ZOOKEEPER-3046: wait for clients to reconnect after re...
Posted by mkedwards <gi...@git.apache.org>.
Github user mkedwards commented on the issue:
https://github.com/apache/zookeeper/pull/721
retest this please
---
[GitHub] zookeeper pull request #721: ZOOKEEPER-3046: wait for clients to reconnect a...
Posted by mkedwards <gi...@git.apache.org>.
GitHub user mkedwards reopened a pull request:
https://github.com/apache/zookeeper/pull/721
ZOOKEEPER-3046: wait for clients to reconnect after restarting server
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mkedwards/zookeeper ZOOKEEPER-3046
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zookeeper/pull/721.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 #721
----
commit 62e6bca2460b261e7cad204566f7b88a41ab0972
Author: Michael Edwards <michael edwards>
Date: 2018-11-25T22:23:29Z
ZOOKEEPER-3046: wait for clients to reconnect after restarting server
----
---
[GitHub] zookeeper issue #721: ZOOKEEPER-3046: wait for clients to reconnect after re...
Posted by mkedwards <gi...@git.apache.org>.
Github user mkedwards commented on the issue:
https://github.com/apache/zookeeper/pull/721
I think so too! Done 😄
---
[GitHub] zookeeper issue #721: ZOOKEEPER-3046: wait for clients to reconnect after re...
Posted by mkedwards <gi...@git.apache.org>.
Github user mkedwards commented on the issue:
https://github.com/apache/zookeeper/pull/721
Fixed that (and tested it locally before pushing this time).
---