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?
---
