[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-2366?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15295141#comment-15295141
 ] 

Alexander Shraer commented on ZOOKEEPER-2366:
---------------------------------------------

Thanks for the patch. A few comments:
- In case the new port is taken, perhaps its better to leave the old port in 
effect ? (rather than to remain without a client port at all like now, which is 
expected in the new test)
- Error message in Netty method is less informative. Consider making messages 
consistent in both methods.
- I'm not sure simply returning false in  Leader.java is sufficient. That 
operation may forever remain in the queue ? I'm not sure about this since the
new test somehow works.
- Indentation seems a bit weird in the changes in QuorumPeer and in the test.


> Reconfiguration of client port causes a socket leak
> ---------------------------------------------------
>
>                 Key: ZOOKEEPER-2366
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2366
>             Project: ZooKeeper
>          Issue Type: Bug
>          Components: quorum
>    Affects Versions: 3.5.0
>            Reporter: Timothy Ward
>            Assignee: Flavio Junqueira
>            Priority: Blocker
>             Fix For: 3.5.2
>
>         Attachments: ZOOKEEPER-2366.patch, ZOOKEEPER-2366.patch, 
> zookeeper.patch
>
>
> The NIOServerCnxnFactory reconfigure method can leak server sockets, and 
> hence make ports unusable until the JVM restarts:
> The first line of the method takes a reference to the current 
> ServerSocketChannel and then the next line replaces it. The subsequent 
> interactions with the server socket can fail (for example if the 
> reconfiguration tries to bind to an in-use port). If they fail *before* the  
> call to oldSS.close() then oldSS is *never* closed. This holds that port open 
> forever, and prevents the user from rolling back to the previous port!
> The code from reconfigure is shown below:
>  ServerSocketChannel oldSS = ss;        
>         try {
>            this.ss = ServerSocketChannel.open();
>            ss.socket().setReuseAddress(true);
>            LOG.info("binding to port " + addr);
>            ss.socket().bind(addr);
>            ss.configureBlocking(false);
>            acceptThread.setReconfiguring();
>            oldSS.close();           
>            acceptThread.wakeupSelector();
>            try {
>                         acceptThread.join();
>                  } catch (InterruptedException e) {
>                          LOG.error("Error joining old acceptThread when 
> reconfiguring client port " + e.getMessage());
>                  }
>            acceptThread = new AcceptThread(ss, addr, selectorThreads);
>            acceptThread.start();
>         } catch(IOException e) {
>            LOG.error("Error reconfiguring client port to " + addr + " " + 
> e.getMessage());
>         }



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to