Details

    • Type: Sub-task Sub-task
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 3.5.0
    • Fix Version/s: 3.5.2, 3.6.0
    • Component/s: tests
    • Labels:
      None

      Description

      testPortChange changes all ports and role of the server and thus causes existing clients to disconnect, while this wouldn't happen if only the client port changes. Need to fix it to only change client port and not all the other parameters and make sure that the clients don't disconnect, while new clients shouldn't be able to connect to the old port.

      1. ZOOKEEPER-2000.patch
        6 kB
        Alexander Shraer
      2. ZOOKEEPER-2000.patch
        6 kB
        Michi Mutsuzaki

        Issue Links

          Activity

          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Patch Available Patch Available Resolved Resolved
          16d 8h 56m 1 Michi Mutsuzaki 20/Aug/14 05:08
          Resolved Resolved Reopened Reopened
          7d 10h 1m 1 Patrick Hunt 27/Aug/14 15:10
          Reopened Reopened Patch Available Patch Available
          193d 7h 16m 1 Michi Mutsuzaki 08/Mar/15 21:26
          Open Open Patch Available Patch Available
          1d 7h 24m 3 Hongchao Deng 10/Mar/15 03:47
          Patch Available Patch Available Open Open
          3d 12h 57m 3 Hongchao Deng 10/Mar/15 05:24
          Michi Mutsuzaki made changes -
          Fix Version/s 3.5.2 [ 12331981 ]
          Fix Version/s 3.5.1 [ 12326786 ]
          Hide
          Michi Mutsuzaki added a comment -

          remember to undo 2137 before closing this issue

          Show
          Michi Mutsuzaki added a comment - remember to undo 2137 before closing this issue
          Michi Mutsuzaki made changes -
          Link This issue requires ZOOKEEPER-2137 [ ZOOKEEPER-2137 ]
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12703583/ZOOKEEPER-2000.patch
          against trunk revision 1665315.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 6 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2554//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2554//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2554//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12703583/ZOOKEEPER-2000.patch against trunk revision 1665315. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2554//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2554//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2554//console This message is automatically generated.
          Hongchao Deng made changes -
          Attachment ZOOKEEPER-2000.patch [ 12703583 ]
          Hongchao Deng made changes -
          Status Patch Available [ 10002 ] Open [ 1 ]
          Hide
          Hongchao Deng added a comment -

          Will make it in another JIRA.

          Show
          Hongchao Deng added a comment - Will make it in another JIRA.
          Hide
          Hongchao Deng added a comment -

          Good advice.

          Show
          Hongchao Deng added a comment - Good advice.
          Hongchao Deng made changes -
          Description Previous changes:
                 
             testPortChange changes all ports and role of the server and thus causes existing clients to disconnect, while this wouldn't happen if only the client port changes. Need to fix it to only change client port and not all the other parameters and make sure that the clients don't disconnect, while new clients shouldn't be able to connect to the old port.

          ========
          Later changes:

          This is what I have figured out:

          * Server (1) and (2) were followers, (3) was the leader.
          * client connected to (1), did a reconfig().
          * (1) and (2) formed a quorum, reconfig was successful, and returned.
          * (3) still thinks he's the leader, so using LeaderZooKeeperServer.
          * client connected to (3) did a sync(), and the sync didn't go through a
          quorum. THE CLIENT WHO DID SYNC() GETS WRONG BEHAVIOR. There's a split
          brain here for sync().
          * Then (3) gradually moves to the new quorum config.

          I'm proposing to change sync() to need quorum acks.
          testPortChange changes all ports and role of the server and thus causes existing clients to disconnect, while this wouldn't happen if only the client port changes. Need to fix it to only change client port and not all the other parameters and make sure that the clients don't disconnect, while new clients shouldn't be able to connect to the old port.
          Hongchao Deng made changes -
          Assignee Hongchao Deng [ hdeng ] Alexander Shraer [ shralex ]
          Hide
          Alexander Shraer added a comment -

          Another thing - it makes sense that this could be the problem, but do you have a way of making it reproducible ? The port-change test constantly passes on my laptop, even if I increase synclimit and initlimit. This would be useful to test your fix anyway.

          Show
          Alexander Shraer added a comment - Another thing - it makes sense that this could be the problem, but do you have a way of making it reproducible ? The port-change test constantly passes on my laptop, even if I increase synclimit and initlimit. This would be useful to test your fix anyway.
          Hide
          Alexander Shraer added a comment -

          BTW, I think you'd need to change PrepRequestProcessor - you need to prepare a transaction for the sync, similar to create/delete/setdata.

          Show
          Alexander Shraer added a comment - BTW, I think you'd need to change PrepRequestProcessor - you need to prepare a transaction for the sync, similar to create/delete/setdata.
          Hide
          Alexander Shraer added a comment -

          Hongchao, how about creating a new JIRA for making sync a quorum operation ? It seems like it will make the proposed change more visible to people that may care about the issue than if you make the changes here.

          Show
          Alexander Shraer added a comment - Hongchao, how about creating a new JIRA for making sync a quorum operation ? It seems like it will make the proposed change more visible to people that may care about the issue than if you make the changes here.
          Hongchao Deng made changes -
          Description           testPortChange changes all ports and role of the server and thus causes existing clients to disconnect, while this wouldn't happen if only the client port changes. Need to fix it to only change client port and not all the other parameters and make sure that the clients don't disconnect, while new clients shouldn't be able to connect to the old port. Previous changes:
                 
             testPortChange changes all ports and role of the server and thus causes existing clients to disconnect, while this wouldn't happen if only the client port changes. Need to fix it to only change client port and not all the other parameters and make sure that the clients don't disconnect, while new clients shouldn't be able to connect to the old port.

          ========
          Later changes:

          This is what I have figured out:

          * Server (1) and (2) were followers, (3) was the leader.
          * client connected to (1), did a reconfig().
          * (1) and (2) formed a quorum, reconfig was successful, and returned.
          * (3) still thinks he's the leader, so using LeaderZooKeeperServer.
          * client connected to (3) did a sync(), and the sync didn't go through a
          quorum. THE CLIENT WHO DID SYNC() GETS WRONG BEHAVIOR. There's a split
          brain here for sync().
          * Then (3) gradually moves to the new quorum config.

          I'm proposing to change sync() to need quorum acks.
          Hongchao Deng made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Assignee Alexander Shraer [ shralex ] Hongchao Deng [ hdeng ]
          Hongchao Deng made changes -
          Attachment ZOOKEEPER-2000.patch [ 12703583 ]
          Hide
          Hongchao Deng added a comment -

          Hi Alexander Shraer.

          I've figured out the root cause of the problem.
          When the leader changed the port and renounced the leadership, it should also change to user FollowerZooKeeperServer, whose CommitProcessor#matchSyncs = true. But in the case it still used LeaderZooKeeperServer, whose CommitProcessor#matchSyncs = false.

          So the sync isn't ack-ed by a quorum. And the reader gets old data. Can you help fix it or give some advice how to fix it on this?

          Show
          Hongchao Deng added a comment - Hi Alexander Shraer . I've figured out the root cause of the problem. When the leader changed the port and renounced the leadership, it should also change to user FollowerZooKeeperServer, whose CommitProcessor#matchSyncs = true. But in the case it still used LeaderZooKeeperServer, whose CommitProcessor#matchSyncs = false. So the sync isn't ack-ed by a quorum. And the reader gets old data. Can you help fix it or give some advice how to fix it on this?
          Michi Mutsuzaki made changes -
          Status Patch Available [ 10002 ] Open [ 1 ]
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12659564/ZOOKEEPER-2000.patch
          against trunk revision 1663127.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 7 new or modified tests.

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2550//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12659564/ZOOKEEPER-2000.patch against trunk revision 1663127. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 7 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2550//console This message is automatically generated.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12659564/ZOOKEEPER-2000.patch
          against trunk revision 1663127.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 7 new or modified tests.

          -1 patch. The patch command could not apply the patch.

          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2549//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12659564/ZOOKEEPER-2000.patch against trunk revision 1663127. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 7 new or modified tests. -1 patch. The patch command could not apply the patch. Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2549//console This message is automatically generated.
          Michi Mutsuzaki made changes -
          Status Reopened [ 4 ] Patch Available [ 10002 ]
          Michi Mutsuzaki made changes -
          Parent ZOOKEEPER-2135 [ 12780346 ]
          Issue Type Bug [ 1 ] Sub-task [ 7 ]
          Lam Thien Son made changes -
          Link This issue is blocked by LOGGING-160 [ LOGGING-160 ]
          Hide
          Michi Mutsuzaki added a comment -

          Hi Alex, is this patch good to go? I'm seeing this test fail once in a while in the precommit build:

          https://builds.apache.org/view/S-Z/view/ZooKeeper/job/PreCommit-ZOOKEEPER-Build/2389/

          Show
          Michi Mutsuzaki added a comment - Hi Alex, is this patch good to go? I'm seeing this test fail once in a while in the precommit build: https://builds.apache.org/view/S-Z/view/ZooKeeper/job/PreCommit-ZOOKEEPER-Build/2389/
          Patrick Hunt made changes -
          Fix Version/s 3.5.1 [ 12326786 ]
          Fix Version/s 3.6.0 [ 12326518 ]
          Fix Version/s 3.5.0 [ 12316644 ]
          Hide
          Hongchao Deng added a comment -

          Alexander Shraer
          Could there be a race between multiple clients because ports are changed?

          Show
          Hongchao Deng added a comment - Alexander Shraer Could there be a race between multiple clients because ports are changed?
          Hide
          Alexander Shraer added a comment -

          btw changing leader port will always cause a leader re-election by design - it doesn't mean that the leader port change failed.

          Show
          Alexander Shraer added a comment - btw changing leader port will always cause a leader re-election by design - it doesn't mean that the leader port change failed.
          Hide
          Alexander Shraer added a comment -

          no, the operation completes when the commit&activate message is sent. This message may cause a leader election, but the reconfig operation may have completed by then.

          The error "expected:<test[1]> but was:<test[0]>" seems like a safety issue - data is being read but its the wrong data, this shouldn't happen.

          Show
          Alexander Shraer added a comment - no, the operation completes when the commit&activate message is sent. This message may cause a leader election, but the reconfig operation may have completed by then. The error "expected:<test [1] > but was:<test [0] >" seems like a safety issue - data is being read but its the wrong data, this shouldn't happen.
          Hide
          Hongchao Deng added a comment -

          I looked at a few other testPortChange failures and they have something in common – they all failed in changing the leader's port, which enforces another leader election.

          Alexander Shraer Does the completion of reconfig guarantee that a new leader having been elected?

          Show
          Hongchao Deng added a comment - I looked at a few other testPortChange failures and they have something in common – they all failed in changing the leader's port, which enforces another leader election. Alexander Shraer Does the completion of reconfig guarantee that a new leader having been elected?
          Patrick Hunt made changes -
          Resolution Fixed [ 1 ]
          Status Resolved [ 5 ] Reopened [ 4 ]
          Hide
          Patrick Hunt added a comment -

          Since this change was committed the upstream jenkins trunk test has been failing intermittently for this test. Please investigate.

          e.g.: https://builds.apache.org/view/S-Z/view/ZooKeeper/job/ZooKeeper-trunk/2420/testReport/junit/org.apache.zookeeper.test/ReconfigTest/testPortChange/

          Show
          Patrick Hunt added a comment - Since this change was committed the upstream jenkins trunk test has been failing intermittently for this test. Please investigate. e.g.: https://builds.apache.org/view/S-Z/view/ZooKeeper/job/ZooKeeper-trunk/2420/testReport/junit/org.apache.zookeeper.test/ReconfigTest/testPortChange/
          Michi Mutsuzaki made changes -
          Description testPortChange changes all ports and role of the server and thus causes existing clients to disconnect, while this wouldn't happen if only the client port changes. Need to fix it to only change client port and not all the other parameters and make sure that the clients don't disconnect, while new clients shouldn't be able to connect to the old port.           testPortChange changes all ports and role of the server and thus causes existing clients to disconnect, while this wouldn't happen if only the client port changes. Need to fix it to only change client port and not all the other parameters and make sure that the clients don't disconnect, while new clients shouldn't be able to connect to the old port.
          Show
          Michi Mutsuzaki added a comment - branch-3.5: http://svn.apache.org/viewvc?view=revision&revision=1619278
          Hide
          Hudson added a comment -

          FAILURE: Integrated in ZooKeeper-trunk #2412 (See https://builds.apache.org/job/ZooKeeper-trunk/2412/)
          ZOOKEEPER-2000. Fix ReconfigTest.testPortChange (Alexander Shraer via michim) (michim: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1619030)

          • /zookeeper/trunk/CHANGES.txt
          • /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/ReconfigTest.java
          Show
          Hudson added a comment - FAILURE: Integrated in ZooKeeper-trunk #2412 (See https://builds.apache.org/job/ZooKeeper-trunk/2412/ ) ZOOKEEPER-2000 . Fix ReconfigTest.testPortChange (Alexander Shraer via michim) (michim: http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1619030 ) /zookeeper/trunk/CHANGES.txt /zookeeper/trunk/src/java/test/org/apache/zookeeper/test/ReconfigTest.java
          Michi Mutsuzaki made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Show
          Michi Mutsuzaki added a comment - trunk: http://svn.apache.org/viewvc?view=revision&revision=1619030
          Michi Mutsuzaki made changes -
          Fix Version/s 3.5.0 [ 12316644 ]
          Fix Version/s 3.5.1 [ 12326786 ]
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12659564/ZOOKEEPER-2000.patch
          against trunk revision 1615240.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 7 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2264//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2264//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2264//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12659564/ZOOKEEPER-2000.patch against trunk revision 1615240. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 7 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2264//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2264//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2264//console This message is automatically generated.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12659564/ZOOKEEPER-2000.patch
          against trunk revision 1615240.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 7 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2263//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2263//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2263//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12659564/ZOOKEEPER-2000.patch against trunk revision 1615240. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 7 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2263//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2263//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2263//console This message is automatically generated.
          Hide
          Alexander Shraer added a comment -

          Thanks a lot, Michi! I think this should be for 3.5.0.

          Show
          Alexander Shraer added a comment - Thanks a lot, Michi! I think this should be for 3.5.0.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12659564/ZOOKEEPER-2000.patch
          against trunk revision 1615240.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 7 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2262//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2262//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2262//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12659564/ZOOKEEPER-2000.patch against trunk revision 1615240. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 7 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2262//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2262//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2262//console This message is automatically generated.
          Hide
          Michi Mutsuzaki added a comment -

          Addressed Hongchao's comment. I'll check this in once the build passes.

          By the way, is this for 3.5.0 or 3.5.1? I think this can go into 3.5.0.

          Show
          Michi Mutsuzaki added a comment - Addressed Hongchao's comment. I'll check this in once the build passes. By the way, is this for 3.5.0 or 3.5.1? I think this can go into 3.5.0.
          Michi Mutsuzaki made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Michi Mutsuzaki made changes -
          Attachment ZOOKEEPER-2000.patch [ 12659564 ]
          Michi Mutsuzaki made changes -
          Status Patch Available [ 10002 ] Open [ 1 ]
          Hide
          Hongchao Deng added a comment -

          Right.
          I missed this line

                  quorumPort = PortAssignment.unique();
          

          Better to use "newQuorumPort"??

          Show
          Hongchao Deng added a comment - Right. I missed this line quorumPort = PortAssignment.unique(); Better to use "newQuorumPort"??
          Hide
          Alexander Shraer added a comment -

          thanks Hongchao. the same test changes all three ports, one at a time.

          Show
          Alexander Shraer added a comment - thanks Hongchao. the same test changes all three ports, one at a time.
          Hide
          Hongchao Deng added a comment -

          +1, Patch looks good.
          It would better to cover quorum port change.

          Show
          Hongchao Deng added a comment - +1, Patch looks good. It would better to cover quorum port change.
          Hide
          Hongchao Deng added a comment -
                              public void process(WatchedEvent event) {}});
                  for (int i = 0; i < 10; i++) {
                      try {
                          Thread.sleep(1000);
                          zkArr[followerIndex].setData("/test", "teststr".getBytes(), -1);
                          Assert.fail("New client connected to old client port!");
                      } catch (KeeperException.ConnectionLossException e) {
                      }
          
          

          code format.

          Show
          Hongchao Deng added a comment - public void process(WatchedEvent event) {}}); for ( int i = 0; i < 10; i++) { try { Thread .sleep(1000); zkArr[followerIndex].setData( "/test" , "teststr" .getBytes(), -1); Assert.fail( "New client connected to old client port!" ); } catch (KeeperException.ConnectionLossException e) { } code format.
          Patrick Hunt made changes -
          Fix Version/s 3.5.1 [ 12326786 ]
          Fix Version/s 3.5.0 [ 12316644 ]
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12658866/ZOOKEEPER-2000.patch
          against trunk revision 1613553.

          +1 @author. The patch does not contain any @author tags.

          +1 tests included. The patch appears to include 7 new or modified tests.

          +1 javadoc. The javadoc tool did not generate any warning messages.

          +1 javac. The applied patch does not increase the total number of javac compiler warnings.

          +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.

          +1 release audit. The applied patch does not increase the total number of release audit warnings.

          -1 core tests. The patch failed core unit tests.

          +1 contrib tests. The patch passed contrib unit tests.

          Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2250//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2250//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2250//console

          This message is automatically generated.

          Show
          Hadoop QA added a comment - -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12658866/ZOOKEEPER-2000.patch against trunk revision 1613553. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 7 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2250//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2250//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/2250//console This message is automatically generated.
          Alexander Shraer made changes -
          Attachment ZOOKEEPER-2000.patch [ 12658866 ]
          Alexander Shraer made changes -
          Fix Version/s 3.5.0 [ 12316644 ]
          Alexander Shraer made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Alexander Shraer made changes -
          Assignee Alexander Shraer [ shralex ]
          Alexander Shraer made changes -
          Component/s tests [ 12312427 ]
          Alexander Shraer made changes -
          Priority Major [ 3 ] Minor [ 4 ]
          Alexander Shraer made changes -
          Description Currently when a reconfiguration changes the client port / address of a server, clients connected to that server will be disconnected. It may be possible to avoid this.

          The relevant code is in NIOServerCnxmFactory.java, reconfigure() and NettyServerCnxnFactory.java reconfigure()
          testPortChange changes all ports and role of the server and thus causes existing clients to disconnect, while this wouldn't happen if only the client port changes. Need to fix it to only change client port and not all the other parameters and make sure that the clients don't disconnect, while new clients shouldn't be able to connect to the old port.
          Alexander Shraer made changes -
          Summary Fix ReconfigTest.testPortChange to keep Fix ReconfigTest.testPortChange
          Alexander Shraer made changes -
          Summary Fix Fix ReconfigTest.testPortChange to keep
          Alexander Shraer made changes -
          Field Original Value New Value
          Summary Avoid disconnecting existing clients on client port change Fix
          Alexander Shraer created issue -

            People

            • Assignee:
              Alexander Shraer
              Reporter:
              Alexander Shraer
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:

                Development