[GitHub] [zookeeper] KimRasak commented on issue #1164: Fix haveDelivered wrong implementation.

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

KimRasak commented on issue #1164: Fix haveDelivered wrong implementation.
URL: https://github.com/apache/zookeeper/pull/1164#issuecomment-561656541
 
 
   > @KimRasak
   > 
   > * Your analysis looks reasonable. Think further, it may not affect the correctness of the program. That's why the suspicious logic existed for many years without catching.
   > * It's better to create a [JIRA](https://issues.apache.org/jira/projects/ZOOKEEPER/) issues (sign up JIRA if you don't have an account) to bind this PR to a JIRA-ID. the contributor guideline is [here](https://cwiki.apache.org/confluence/display/ZOOKEEPER/HowToContribute%5D)
   
   Ok, I've not used JIRA before... I'll take a look at it and it's ok not to merge. 
   - I believe my implementation can reduce unnecessary notifications, whlie original implementation can get faster to complete the leader voting by sending more notifications
   - Will the original implementation cause the "broadcast storm"? Think about a scene where there are n=4 servers (s1, s2, s3, s4). If s1 and s2 are broken(the connection is down),  notifications from s3/s4 to s1/s2 will stay in the BlockingQueue, and `sendNotifications();` of s3/s4 will be called again and again. s3 and s4 will send more and more notifications and reply to each other, resulting in the "broadcast storm".
      - Think about the "OSI model", if there are other machines among the zookeeper servers, and n is large, the notifications at the network layer may cause too much burden to other machines and routers.
   - It's also good to rewrite the comment/function name of `sendNotifications();`.

----------------------------------------------------------------
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