zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From hanm <...@git.apache.org>
Subject [GitHub] zookeeper issue #330: ZOOKEEPER-2471: ZK Java client should not count sleep ...
Date Fri, 18 Aug 2017 21:23:59 GMT
Github user hanm commented on the issue:

    https://github.com/apache/zookeeper/pull/330
  
    The `now` variable was there when this project starts, and `updateNow` was introduced
in ZOOKEEPER-909 which was just a refactoring so it's irrelevant to the core logic. I had
a chat with @phunt who wrote the original code and the `now` was not introduced by accident
- according to Pat it's used to solve two problems:
    * A performance optimization by caching as on some platforms getting current time is not
cheap.
    * On some platforms get current time will go backwards so to prevent that a cached time
was used at the very start.
    Now after 10 years and many changes (like using monotonic clock) the original design might
become irrelevant, and it might make sense to instead calculate the now time when needed.
Though, I'd like to have another look into this issue before making a conclusion.
    
    One idea is to have a test case that shows the bug (client infinity connect loop etc),
and then verify that the bug gets fixed with the patch. This will be more convincing. I understand
writing such test takes more effort than the patch itself, but eventually it's something must
to have for such a change.
    
    On a side note - this patch is doing fine with my stress test and does not cause any regressions.
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message