Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Elapsed time calculations should use System.nanoTime() #13

Closed
sleberknight opened this issue Jan 5, 2021 · 0 comments · Fixed by #33
Closed

Elapsed time calculations should use System.nanoTime() #13

sleberknight opened this issue Jan 5, 2021 · 0 comments · Fixed by #33
Assignees
@sleberknight
Labels
enhancement A request for change or improvement to an existing feature
Milestone
0.5.0

Comments

@sleberknight
Copy link
Member

The original code in guava-retrying in Retryer#call used System.nanoTime() to measure time elapsed since the first attempt was made, but the forked re-retrying library changed to use System.currentTimeMillis().

Change this code back to use nanoTime()..

See https://stackoverflow.com/a/1776053 for one good answer or dropwizard/metrics#1361 for another, more interesting, scenario...
@sleberknight sleberknight added the enhancement A request for change or improvement to an existing feature label Jan 5, 2021
@sleberknight sleberknight added this to the 0.5.0 milestone Jan 5, 2021
@sleberknight sleberknight self-assigned this Jan 5, 2021
sleberknight added a commit that referenced this issue Jan 7, 2021
@sleberknight
Use nanoTime to compute delay time since first attempt
324c007 
This was (incorrectly) changed in the re-retrying fork to use
currentTimeMillis() instead of nanoTime() to make the delay computation
"simpler", but it resulted in a potentially problematic calculation
since nanoTime is intended to be used for elapsed time computations in
Java. Using currentTimeMillis can can odd issues (e.g. a call to
currentTimeMillis could return a number less than a preceding call
and thus result in a negative elapsed time)

Fixes #13
@sleberknight sleberknight mentioned this issue Jan 7, 2021
Merged
chrisrohr pushed a commit that referenced this issue Jan 7, 2021
@sleberknight
Use nanoTime to compute delay time since first attempt (#33)
e6d14f8 
This was (incorrectly) changed in the re-retrying fork to use
currentTimeMillis() instead of nanoTime() to make the delay computation
"simpler", but it resulted in a potentially problematic calculation
since nanoTime is intended to be used for elapsed time computations in
Java. Using currentTimeMillis can can odd issues (e.g. a call to
currentTimeMillis could return a number less than a preceding call
and thus result in a negative elapsed time)

Fixes #13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sleberknight sleberknight

Labels
enhancement A request for change or improvement to an existing feature
Projects
None yet
Milestone
0.5.0
Development

Successfully merging a pull request may close this issue.

Use nanoTime to compute delay time since first attempt kiwiproject/retrying-again
1 participant
@sleberknight