-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
chore(node-core): log errors in debug when ETA fails to calculate #6971
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this does not fix the root cause of the issue, right?
but this prevents the overflow
let Some(processed_since_last) = | ||
current.processed.checked_sub(self.last_checkpoint.processed) | ||
else { | ||
self.eta = None; | ||
debug!(target: "reth::cli", ?current, ?self.last_checkpoint, "Failed to calculate the ETA: processed entities is less than the last checkpoint"); | ||
return | ||
}; | ||
let elapsed = last_checkpoint_time.elapsed(); | ||
let per_second = processed_since_last as f64 / elapsed.as_secs_f64(); | ||
|
||
self.eta = Duration::try_from_secs_f64( | ||
((current.total - current.processed) as f64) / per_second, | ||
) | ||
.ok(); | ||
let Some(remaining) = current.total.checked_sub(current.processed) else { | ||
self.eta = None; | ||
debug!(target: "reth::cli", ?current, "Failed to calculate the ETA: total entities is less than processed entities"); | ||
return | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's add stage_id
arg to Eta::update
and log it as well to identify the source
I couldn't instantly identify the source of this debug mode panic
So instead, on subtract with overflow, we log the error on debug level and reset the ETA instead of crashing or reporting the invalid ETA.