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 [email protected] or file a JIRA ticket
with INFRA.
---
