From f2f6ec8c7b6bee49c8a8cb164d16d6a2f8bc5df5 Mon Sep 17 00:00:00 2001 From: David Lutterkort Date: Thu, 23 Jul 2020 17:24:48 -0700 Subject: [PATCH 1/2] graph: Fix an error in KillState::new 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 https://github.com/graphprotocol/graph-node/issues/1805 --- graph/src/data/graphql/effort.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/graph/src/data/graphql/effort.rs b/graph/src/data/graphql/effort.rs index b26d22baff6..d39c9f52973 100644 --- a/graph/src/data/graphql/effort.rs +++ b/graph/src/data/graphql/effort.rs @@ -149,12 +149,14 @@ struct KillState { impl KillState { fn new() -> Self { - let long_ago = Duration::from_secs(86400); + let long_ago = Duration::from_secs(60); + let now = Instant::now(); + let before = now.checked_sub(long_ago).unwrap_or(now); Self { kill_rate: 0.0, - last_update: Instant::now() - long_ago, + last_update: before, overload_start: None, - last_overload_log: Instant::now() - long_ago, + last_overload_log: before, } } From a5dec3c8c3298130e4a70eb1fb3e8150d5284452 Mon Sep 17 00:00:00 2001 From: David Lutterkort Date: Fri, 24 Jul 2020 11:18:51 -0700 Subject: [PATCH 2/2] graph: Explain how and why we initialize timestamps in KillState::new --- graph/src/data/graphql/effort.rs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/graph/src/data/graphql/effort.rs b/graph/src/data/graphql/effort.rs index d39c9f52973..d713c0e6399 100644 --- a/graph/src/data/graphql/effort.rs +++ b/graph/src/data/graphql/effort.rs @@ -149,9 +149,19 @@ struct KillState { impl KillState { fn new() -> Self { - let long_ago = Duration::from_secs(60); - let now = Instant::now(); - let before = now.checked_sub(long_ago).unwrap_or(now); + // Set before to an instant long enough ago so that we don't + // immediately log or adjust the kill rate if the node is already + // under load. Unfortunately, on OSX, `Instant` measures time from + // the last boot, and if that was less than 60s ago, we can't + // subtract 60s from `now`. Since the worst that can happen if + // we set `before` to `now` is that we might log more than strictly + // necessary, and adjust the kill rate one time too often right after + // node start, it is acceptable to fall back to `now` + let before = { + let long_ago = Duration::from_secs(60); + let now = Instant::now(); + now.checked_sub(long_ago).unwrap_or(now) + }; Self { kill_rate: 0.0, last_update: before,