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.

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

graph: Fix an error in KillState::new #1806

Merged
merged 2 commits into from
Jul 28, 2020
Merged

graph: Fix an error in KillState::new #1806

merged 2 commits into from
Jul 28, 2020

Conversation

lutter
Copy link
Collaborator

@lutter lutter commented Jul 24, 2020

On Mac OSX, Instant::now() is the time since the last boot, which might
have been less than a day ago, in which case KillState::new would panic. We
now use 'now - 60s' as a time in the past for initialization, and if that
fails, we'll use just 'now'. There's not much danger in using 'now', it
might just lead to spurious logging and a too-soon update of the kill_rate
if the node becomes immediately overloaded after starting.

Thanks to kevholder for root-causing the issue and demonstrating a fix.

Fixes #1805

@leoyvens
Copy link
Collaborator

This seems worth a comment

@lutter
Copy link
Collaborator Author

lutter commented Jul 24, 2020

True - I made the mistake of putting the comment into the commit message. Fixed now.

@leoyvens
Copy link
Collaborator

@lutter I think you forgot to push the change.

@lutter lutter force-pushed the lutter/lm-init branch 2 times, most recently from 8b5bfc4 to 7b049ba Compare July 24, 2020 23:33
@lutter
Copy link
Collaborator Author

lutter commented Jul 24, 2020

@lutter I think you forgot to push the change.

I did indeed; rebased to latest master and pushed for realz

On Mac OSX, Instant::now() is the time since the last boot, which might
have been less than a day ago, in which case KillState::new would panic. We
now use 'now - 60s' as a time in the past for initialization, and if that
fails, we'll use just 'now'. There's not much danger in using 'now', it
might just lead to spurious logging and a too-soon update of the kill_rate
if the node becomes immediately overloaded after starting.

Thanks to kevholder for root-causing the issue and demonstrating a fix.

Fixes #1805
@lutter lutter merged commit a5dec3c into master Jul 28, 2020
@lutter lutter deleted the lutter/lm-init branch July 29, 2020 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

overflow when subtracting duration from instant
2 participants