Github user hanm commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/184#discussion_r114167558
  
    --- Diff: 
src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java ---
    @@ -438,9 +447,18 @@ synchronized private boolean connectOne(long sid, 
InetSocketAddress electionAddr
             Socket sock = null;
             try {
                  LOG.debug("Opening channel to server " + sid);
    -             sock = new Socket();
    -             setSockOpts(sock);
    -             sock.connect(electionAddr, cnxTO);
    +             if (self.isSslQuorum()) {
    +                 SSLSocket sslSock = x509Util.createSSLSocket();
    +                 setSockOpts(sslSock);
    +                 sslSock.connect(electionAddr, cnxTO);
    +                 sslSock.startHandshake();
    --- End diff --
    
    This is a synchronous call and will block connectOne until the hand shake 
finishes. Current cnx manager does blocking IO when handling connections so 
while hand shake is going on other peers will not be able to connect to this 
server. This might not be a big problem with carefully tuned initLimit / 
syncLimit, but I thought we might want mention this somewhere in doc or wiki.
    
    An alternative would be making hand shake async so cnx manager does not 
block, but I am currently leaning towards not do this and instead use 
ZOOKEEPER-900/901 to fix the root cause.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to