[GitHub] [zookeeper] maoling edited a comment on issue #1065: ZOOKEEPER-3135: ClientCnxnSocket#updateLastSendAndHeard() method update lastSend、lastHeard has some problem

[GitBox <gi...@apache.org>]

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:
users@infra.apache.org


With regards,
Apache Git Services