zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From hanm <...@git.apache.org>
Subject [GitHub] zookeeper pull request #700: ZOOKEEPER-1441 - JAVA 11 - Some test cases are ...
Date Wed, 21 Nov 2018 00:30:02 GMT
Github user hanm commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/700#discussion_r235218313
  
    --- Diff: zookeeper-server/src/main/java/org/apache/zookeeper/server/NIOServerCnxnFactory.java
---
    @@ -209,6 +209,8 @@ public void run() {
                             LOG.warn("Ignoring unexpected exception", e);
                         }
                     }
    +            } catch (IOException e) {
    +                LOG.error("Exception when running accept thread. Unable to register selector?",
e);
    --- End diff --
    
    Yeah, please ignore the port binding part of my statement previously. I was actually referring
to port selection. Before this fix, we have selector registration in constructor of `AcceptorThread`:
    `this.acceptKey =
                    acceptSocket.register(selector, SelectionKey.OP_ACCEPT);`
    If we get an exception here, we will bail out and zk server will not start.
    
    If we selector registration inside AcceptorThread and got an exception, then the acceptor
thread will shutdown, but the caller will not be aware of this so the zk server could be in
a weird state. That's my only concern of this patch. Does this sound a reasonable concern?


---

Mime
View raw message