zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ivmaykov <...@git.apache.org>
Subject [GitHub] zookeeper pull request #669: ZOOKEEPER-3152: Port ZK netty stack to netty4
Date Wed, 14 Nov 2018 22:57:39 GMT
Github user ivmaykov commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/669#discussion_r233652712
  
    --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxnSocketNetty.java
---
    @@ -103,71 +105,102 @@
         boolean isConnected() {
             // Assuming that isConnected() is only used to initiate connection,
             // not used by some other connection status judgement.
    -        return channel != null;
    +        connectLock.lock();
    +        try {
    +            return channel != null || connectFuture != null;
    --- End diff --
    
    As the comment above says, the `isConnected()` method is only used in the main loop inside
`ClientCnxn$SendThread.run()` to see if a new connection should be initiated. So, this method
should return false if a connection attempt is already in progress. This is the case when
`connectFuture` is not null. Arguably the method should be called `isConnectedOrConnecting()`
but I didn't want to go around refactoring APIs in this diff - can do it if you like.


---

Mime
View raw message