maoling edited a comment on issue #1065: ZOOKEEPER-3135: ClientCnxnSocket#updateLastSendAndHeard() method update lastSend、lastHeard has some problem URL: https://github.com/apache/zookeeper/pull/1065#issuecomment-531453107 @anmolnar ``` public void run() { clientCnxnSocket.updateNow(); // t1. update lastSend、lastHeard clientCnxnSocket.updateLastSendAndHeard(); while (state.isAlive()) { try { if (!clientCnxnSocket.isConnected()) { // t2...some operations startConnect(serverAddress); // t3. update lastSend、lastHeard clientCnxnSocket.updateLastSendAndHeard(); } // ...................... // ...................... } catch (Throwable e) { // t4..................... cleanAndNotifyState(); } } } ``` - This issue happens when client reconnects from one server to another. If we don't `updateNow` at place `t3`, the code will use the timestamp updated by `t4`(Notice: not `t1`) and in `startConnect` method which has slept almost 1 second(`isFirstConnect=false`). so `getIdleRecv` and `getIdleSend` will be smaller than the expected value, and `timeToNextPing` is bigger than the expected value, which will make the ping from client to server delay. ``` int timeToNextPing = readTimeout / 2 - clientCnxnSocket.getIdleSend() - ((clientCnxnSocket.getIdleSend() > 1000) ? 1000 : 0); ``` - Overall, after reconnecting successfully, re-record the timestamp of `lastHeard`, `lastSend` with current timestamp is really needed - On the other hand, `updateLastSendAndHeard` is always next to `updateNow` by observing others places where calls the method `updateLastSendAndHeard`.
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
