[
https://issues.apache.org/jira/browse/ZOOKEEPER-59?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12611754#action_12611754
]
Andrew Kornev commented on ZOOKEEPER-59:
----------------------------------------
I agree with Flavio's comment in
https://issues.apache.org/jira/browse/ZOOKEEPER-59?focusedCommentId=12611527#action_12611527
: incInProcess() should be called prior to calling first request processor.
Also, getInProcess() should be a synchronized method. Even better solution
would be to make requestsInProcess an atomic integer: there is really no need
to synchronize on the ZooKeeper instance to update/access the variable.
Having said that, I don't think this will fix the the timeout problem. From
what I saw in the server logs during my investigation of a similar incident was
that the server was still receiving and processing client requests (in other
words, the counters we're trying to fix here didn't cause throttling). The
problem was that the responses weren't being sent to the client which was
causing the client to timeout.
> Synchronized block in NIOServerCnxn
> -----------------------------------
>
> Key: ZOOKEEPER-59
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-59
> Project: Zookeeper
> Issue Type: Bug
> Components: server
> Reporter: Flavio Paiva Junqueira
> Assignee: Benjamin Reed
> Attachments: ZOOKEEPER-59_1.patch, ZOOKEEPER-59_2.patch
>
>
> There are two synchronized blocks locking on different objects, and to me
> they should be guarded by the same object. Here are the parts of the code I'm
> talking about:
> {noformat}
> [EMAIL PROTECTED]
> ...
> synchronized (this) {
> outstandingRequests++;
> // check throttling
> if (zk.getInProcess() > factory.outstandingLimit) {
> disableRecv();
> // following lines should not be needed since we are
> already
> // reading
> // } else {
> // enableRecv();
> }
> }
> {noformat}
> {noformat}
> [EMAIL PROTECTED]
> ...
> synchronized (this.factory) {
> outstandingRequests--;
> // check throttling
> if (zk.getInProcess() < factory.outstandingLimit
> || outstandingRequests < 1) {
> sk.selector().wakeup();
> enableRecv();
> }
> }
> {noformat}
> I think the second one is correct, and the first synchronized block should be
> guarded by "this.factory".
> This could be related to issue ZOOKEEPER-57, but I have no concrete
> indication that this is the case so far.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
