-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
feat(prune): add pruner log with INFO
level
#4573
Conversation
Codecov Report
... and 29 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more.
|
This is counting how many variants |
crates/prune/src/pruner.rs
Outdated
trace!(target: "pruner", %tip_block_number, ?elapsed, "Pruner finished"); | ||
info!( | ||
target: "pruner", | ||
%tip_block_number, | ||
%done, | ||
"Pruner finished after {:?} nanosec, pruned {} entries", | ||
elapsed.subsec_nanos(), | ||
total_pruned_entries | ||
); |
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.
Several things:
- As @joshieDo said, we want to log total number of pruned entries, and not the number of pruned parts. It would be nice to show the summary of pruning, i.e. number of entries per part. So each prune part will have its own field.
- Total elapsed time is nice, but let's put it into a field instead of the log message. Log message should be a human-readable text, and all the additional data should go to fields.
- New
info
log now replicates thetrace
one, and sinceINFO < TRACE
in terms of log levels, we can remove the oldtrace
log.
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.
@shekhirin Points 2 and 3 should be resolved, I'm not very familiar with the concept of pruned entries inside pruned parts because I don't know this part very well, can you give me an example of how to calculate that I imagine to place it in a HashMap
which will summarize pruned entries sorted by pruned parts?
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.
yes, that's right. But I just realized it might a bit tricky because account and storage history pruning methods have two internal counters for pruned entries, so it's not clear which one to put in the log.
Let's just have a separate field for each of pruning parts with done
value returned by the pruning method? So, something like
info!(
target: "pruner",
%tip_block_number,
?elapsed,
%done,
?parts_done,
"Pruner finished"
);
where parts_done
is a HashMap<PrunePart, bool>
.
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.
@shekhirin I agree, this is done now
Co-authored-by: Alexey Shekhirin <a.shekhirin@gmail.com>
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.
LGTM, thank you!
Should solve #4563.