You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@zookeeper.apache.org by GitBox <gi...@apache.org> on 2020/03/19 14:12:46 UTC

[GitHub] [zookeeper] symat opened a new pull request #1289: ZOOKEEPER-3756: Members slow to rejoin quorum using Kubernetes

symat opened a new pull request #1289: ZOOKEEPER-3756: Members slow to rejoin quorum using Kubernetes
URL: https://github.com/apache/zookeeper/pull/1289
 
 
   Whenever we close the current master ZooKeeper server, a new leader election
   is triggered. During the new election, a connection will be established between
   all the servers, by calling the synchronized 'connectOne' method in
   QuorumCnxManager. The method will open the socket and send a single small
   initial message to the other server, usually very quickly. If the destination
   host is unreachable, it should fail immediately.
   
   However, when we use Kubernetes, then the destination host is always reachable
   as it points to Kubernetes services. If the actual container / pod is not
   available then the 'socket.connect' method will timeout (by default after 5 sec)
   instead of failing immediately with NoRouteToHostException. As the 'connectOne'
   method is synchronized, this timeout will block the creation of other
   connections, so a single unreachable host can cause timeout in the leader
   election protocol.
   
   One workaround is to decrease the socket connection timeout with the
   '-Dzookeeper.cnxTimeout' stystem property, but the proper fix would be to
   make the connection initiation fully asynchronous, as using very low timeout can
   have its own side effect. Fortunately most of the initial message sending
   is already made async: the SASL authentication can take more time, so the
   second (authentication + initial message sending) part of the initiation protocol 
   is already called in a separate thread, when Quorum SASL authentication is enabled.
   
   In the following patch I made the whole connection initiation async, by
   always using the async executor (not only when Quorum SASL is enabled) and
   also moving the socket.connect call into the async thread.
   
   I also created a unit test to verify my fix. I added a static socket factory that can be 
   changed by the tests using a packet private setter method. My test failed (and
   produced the same error logs as we see in the original Jira ticket) before I applied
   my changes and a time-outed as no leader election succeeded after 15 seconds.
   After the changes the test runs very quickly, in 1-2 seconds.
   
   Note: due to the multiAddress changes, we will need different PRs to the branch 3.5 
   and to the 3.6+ branches. I will submit the other PR once this got reviewed.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [zookeeper] symat commented on issue #1289: ZOOKEEPER-3756: Members slow to rejoin quorum using Kubernetes

Posted by GitBox <gi...@apache.org>.
symat commented on issue #1289: ZOOKEEPER-3756: Members slow to rejoin quorum using Kubernetes
URL: https://github.com/apache/zookeeper/pull/1289#issuecomment-602543283
 
 
   FYI: I also submitted the PR for branch 3.5:
   https://github.com/apache/zookeeper/pull/1293

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [zookeeper] eolivelli commented on issue #1289: ZOOKEEPER-3756: Members slow to rejoin quorum using Kubernetes

Posted by GitBox <gi...@apache.org>.
eolivelli commented on issue #1289: ZOOKEEPER-3756: Members slow to rejoin quorum using Kubernetes
URL: https://github.com/apache/zookeeper/pull/1289#issuecomment-601320396
 
 
   What was the error on CI?
   I hope we are not introducing some kind on instability.
   In theory this change should make more stable the system 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [zookeeper] nkalmar commented on issue #1289: ZOOKEEPER-3756: Members slow to rejoin quorum using Kubernetes

Posted by GitBox <gi...@apache.org>.
nkalmar commented on issue #1289: ZOOKEEPER-3756: Members slow to rejoin quorum using Kubernetes
URL: https://github.com/apache/zookeeper/pull/1289#issuecomment-602670239
 
 
   Thanks @symat , merged to master and branch-3.6. I'll check the PR for 3.5.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [zookeeper] symat commented on issue #1289: ZOOKEEPER-3756: Members slow to rejoin quorum using Kubernetes

Posted by GitBox <gi...@apache.org>.
symat commented on issue #1289: ZOOKEEPER-3756: Members slow to rejoin quorum using Kubernetes
URL: https://github.com/apache/zookeeper/pull/1289#issuecomment-601298352
 
 
   restart maven build

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [zookeeper] symat commented on a change in pull request #1289: ZOOKEEPER-3756: Members slow to rejoin quorum using Kubernetes

Posted by GitBox <gi...@apache.org>.
symat commented on a change in pull request #1289: ZOOKEEPER-3756: Members slow to rejoin quorum using Kubernetes
URL: https://github.com/apache/zookeeper/pull/1289#discussion_r395082580
 
 

 ##########
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java
 ##########
 @@ -359,20 +359,49 @@ public Thread newThread(Runnable r) {
      *
      * @param sid
      */
-    public void testInitiateConnection(long sid) throws Exception {
+    public void testInitiateConnection(long sid) {
         LOG.debug("Opening channel to server {}", sid);
-        Socket sock = new Socket();
-        setSockOpts(sock);
-        InetSocketAddress address = self.getVotingView().get(sid).electionAddr.getReachableOrOne();
-        sock.connect(address, cnxTO);
-        initiateConnection(sock, sid);
+        initiateConnection(self.getVotingView().get(sid).electionAddr, sid);
     }
 
     /**
+     * First we create the socket, perform SSL handshake and authentication if needed.
+     * Then we perform the initiaion protocol.
      * If this server has initiated the connection, then it gives up on the
      * connection if it loses challenge. Otherwise, it keeps the connection.
      */
-    public void initiateConnection(final Socket sock, final Long sid) {
+    public void initiateConnection(final MultipleAddresses electionAddr, final Long sid) {
+        Socket sock = null;
+        try {
+            LOG.debug("Opening channel to server {}", sid);
+            if (self.isSslQuorum()) {
 
 Review comment:
   The makeSocket() is also a nice idea :)
   
   I was thinking 1-2 times already to use PowerMock. On the other hand the new JDK versions seems to be more strict with reflection... so I am not sure how 'future-proof' it would be.
   
   I was also thinking if we should use the `zookeeper.serverCnxnFactory` system property in the test, but we don't use the `ServerCnxnFactory` approach for the leaser election AFAICS. And implementing that would have been a longer story. Maybe for 4.0 it would be good, if we would touch the Leader Election part.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [zookeeper] eolivelli commented on a change in pull request #1289: ZOOKEEPER-3756: Members slow to rejoin quorum using Kubernetes

Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #1289: ZOOKEEPER-3756: Members slow to rejoin quorum using Kubernetes
URL: https://github.com/apache/zookeeper/pull/1289#discussion_r395073912
 
 

 ##########
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java
 ##########
 @@ -359,20 +359,49 @@ public Thread newThread(Runnable r) {
      *
      * @param sid
      */
-    public void testInitiateConnection(long sid) throws Exception {
+    public void testInitiateConnection(long sid) {
         LOG.debug("Opening channel to server {}", sid);
-        Socket sock = new Socket();
-        setSockOpts(sock);
-        InetSocketAddress address = self.getVotingView().get(sid).electionAddr.getReachableOrOne();
-        sock.connect(address, cnxTO);
-        initiateConnection(sock, sid);
+        initiateConnection(self.getVotingView().get(sid).electionAddr, sid);
     }
 
     /**
+     * First we create the socket, perform SSL handshake and authentication if needed.
+     * Then we perform the initiaion protocol.
      * If this server has initiated the connection, then it gives up on the
      * connection if it loses challenge. Otherwise, it keeps the connection.
      */
-    public void initiateConnection(final Socket sock, final Long sid) {
+    public void initiateConnection(final MultipleAddresses electionAddr, final Long sid) {
+        Socket sock = null;
+        try {
+            LOG.debug("Opening channel to server {}", sid);
+            if (self.isSslQuorum()) {
 
 Review comment:
   A better approach would be to add a
   Socket makeSocket() 
   method 
   and override it with PowerMock
   But we don't have powermock :(

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [zookeeper] eolivelli commented on issue #1289: ZOOKEEPER-3756: Members slow to rejoin quorum using Kubernetes

Posted by GitBox <gi...@apache.org>.
eolivelli commented on issue #1289: ZOOKEEPER-3756: Members slow to rejoin quorum using Kubernetes
URL: https://github.com/apache/zookeeper/pull/1289#issuecomment-601215372
 
 
   LGTM 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [zookeeper] symat commented on issue #1289: ZOOKEEPER-3756: Members slow to rejoin quorum using Kubernetes

Posted by GitBox <gi...@apache.org>.
symat commented on issue #1289: ZOOKEEPER-3756: Members slow to rejoin quorum using Kubernetes
URL: https://github.com/apache/zookeeper/pull/1289#issuecomment-601240666
 
 
   retest maven build

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [zookeeper] symat commented on a change in pull request #1289: ZOOKEEPER-3756: Members slow to rejoin quorum using Kubernetes

Posted by GitBox <gi...@apache.org>.
symat commented on a change in pull request #1289: ZOOKEEPER-3756: Members slow to rejoin quorum using Kubernetes
URL: https://github.com/apache/zookeeper/pull/1289#discussion_r395082580
 
 

 ##########
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java
 ##########
 @@ -359,20 +359,49 @@ public Thread newThread(Runnable r) {
      *
      * @param sid
      */
-    public void testInitiateConnection(long sid) throws Exception {
+    public void testInitiateConnection(long sid) {
         LOG.debug("Opening channel to server {}", sid);
-        Socket sock = new Socket();
-        setSockOpts(sock);
-        InetSocketAddress address = self.getVotingView().get(sid).electionAddr.getReachableOrOne();
-        sock.connect(address, cnxTO);
-        initiateConnection(sock, sid);
+        initiateConnection(self.getVotingView().get(sid).electionAddr, sid);
     }
 
     /**
+     * First we create the socket, perform SSL handshake and authentication if needed.
+     * Then we perform the initiaion protocol.
      * If this server has initiated the connection, then it gives up on the
      * connection if it loses challenge. Otherwise, it keeps the connection.
      */
-    public void initiateConnection(final Socket sock, final Long sid) {
+    public void initiateConnection(final MultipleAddresses electionAddr, final Long sid) {
+        Socket sock = null;
+        try {
+            LOG.debug("Opening channel to server {}", sid);
+            if (self.isSslQuorum()) {
 
 Review comment:
   The makeSocket() is also a nice idea :)
   
   I was thinking 1-2 times already to use PowerMock. On the other hand the new JDK versions seems to be more strict with reflection... so I am not sure how 'future-proof' it would be.
   
   I was also thinking if we should use the `zookeeper.serverCnxnFactory` system property in the test, but we don't use the `ServerCnxnFactory` approach for the leader election AFAICS. And implementing that would have been a longer story. Maybe for 4.0 it would be good, if we would touch the Leader Election part.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [zookeeper] asfgit closed pull request #1289: ZOOKEEPER-3756: Members slow to rejoin quorum using Kubernetes

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #1289: ZOOKEEPER-3756: Members slow to rejoin quorum using Kubernetes
URL: https://github.com/apache/zookeeper/pull/1289
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [zookeeper] eolivelli commented on issue #1289: ZOOKEEPER-3756: Members slow to rejoin quorum using Kubernetes

Posted by GitBox <gi...@apache.org>.
eolivelli commented on issue #1289: ZOOKEEPER-3756: Members slow to rejoin quorum using Kubernetes
URL: https://github.com/apache/zookeeper/pull/1289#issuecomment-601884191
 
 
   @anmolnar @nkalmar PTAL

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [zookeeper] symat commented on issue #1289: ZOOKEEPER-3756: Members slow to rejoin quorum using Kubernetes

Posted by GitBox <gi...@apache.org>.
symat commented on issue #1289: ZOOKEEPER-3756: Members slow to rejoin quorum using Kubernetes
URL: https://github.com/apache/zookeeper/pull/1289#issuecomment-602686312
 
 
   thanks @eolivelli  and @nkalmar  for the quick. reviews!

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [zookeeper] symat commented on issue #1289: ZOOKEEPER-3756: Members slow to rejoin quorum using Kubernetes

Posted by GitBox <gi...@apache.org>.
symat commented on issue #1289: ZOOKEEPER-3756: Members slow to rejoin quorum using Kubernetes
URL: https://github.com/apache/zookeeper/pull/1289#issuecomment-601568101
 
 
   Yes, I agree. This should make things more stable. But also it changes an important part of the leader election... so we should be sure it is stable.
   
   This was the failed jenkins job: https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build-maven/org.apache.zookeeper$zookeeper/1909/
   
   The `org.apache.zookeeper.test.WatcherTest` is failing only. As far as I can see, this test is using only a single ZK server, so the leader election shouldn't even triggered.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [zookeeper] eolivelli commented on issue #1289: ZOOKEEPER-3756: Members slow to rejoin quorum using Kubernetes

Posted by GitBox <gi...@apache.org>.
eolivelli commented on issue #1289: ZOOKEEPER-3756: Members slow to rejoin quorum using Kubernetes
URL: https://github.com/apache/zookeeper/pull/1289#issuecomment-601594104
 
 
   ok
   ready to ship it as soon as we have an other approval

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [zookeeper] eolivelli commented on issue #1289: ZOOKEEPER-3756: Members slow to rejoin quorum using Kubernetes

Posted by GitBox <gi...@apache.org>.
eolivelli commented on issue #1289: ZOOKEEPER-3756: Members slow to rejoin quorum using Kubernetes
URL: https://github.com/apache/zookeeper/pull/1289#issuecomment-601227377
 
 
   With  the new release schedule of JDK the future is now ;) 
   Don't bother too much 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services