[ https://issues.apache.org/jira/browse/ZOOKEEPER-3652?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17041899#comment-17041899 ]
Sylvain Wallez commented on ZOOKEEPER-3652: ------------------------------------------- [~maoling] I submitted a [pull request|https://github.com/apache/zookeeper/pull/1257] with a fix! > Improper synchronization in ClientCnxn > -------------------------------------- > > Key: ZOOKEEPER-3652 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-3652 > Project: ZooKeeper > Issue Type: Bug > Components: java client > Affects Versions: 3.5.6 > Reporter: Sylvain Wallez > Priority: Major > Labels: pull-request-available > Time Spent: 10m > Remaining Estimate: 0h > > ZOOKEEPER-2111 introduced {{synchronized(state)}} statements in > {{ClientCnxn}} and {{ClientCnxn.SendThread}} to coordinate insertion in > {{outgoingQueue}} and draining it when the client connection isn't alive. > There are several issues with this approach: > - the value of the {{state}} field is not stable, meaning we don't always > synchronize on the same object. > - the {{state}} field is an enum value, which are global objects. So in an > application with several ZooKeeper clients connected to different servers, > this causes some contention between clients. > An easy fix is change those {{synchronized(state)}} statements to > {{synchronized(outgoingQueue)}} since it is local to each client and is what > we want to coordinate. > I'll be happy to prepare a PR with the above change if this is deemed to be > the correct way to fix it. > > Another issue that makes contention worse is > {{ClientCnxnSocketNIO.cleanup()}} that is called from within the above > synchronized block and contains {{Thread.sleep(100)}}. Why is this sleep > statement needed, and can we remove it? > -- This message was sent by Atlassian Jira (v8.3.4#803005)