Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

add metrics to handle_snapshot_requests #17937

Merged
merged 1 commit into from
Jun 14, 2021

Conversation

jeffwashington
Copy link
Contributor

@jeffwashington jeffwashington commented Jun 14, 2021

Problem

As # accounts grows, handle_snapshot_requests code path execution time grows very large. Having metrics of the total processing time and idle time will help find and verify improvements. For example, if clean and hash calculation can be performed in parallel, then overall time could improve while individual times (hash & clean) could get worse.

Summary of Changes

add metrics
Fixes #

@jeffwashington
Copy link
Contributor Author

jeffwashington commented Jun 14, 2021

lemond, against current mnb snapshot

datapoint: handle_snapshot_requests-timing hash_time=3507089i flush_accounts_cache_time=541845i shrink_time=57178i clean_time=996269i snapshot_time=304815i purge_old_snapshots_time=6418i total_us=5413618i non_snapshot_time_us=77834828i

@jeffwashington jeffwashington requested a review from sakridge June 14, 2021 18:43
@jeffwashington jeffwashington marked this pull request as ready for review June 14, 2021 18:43
) -> Option<u64> {
self.snapshot_request_receiver
.try_iter()
.last()
.map(|snapshot_request| {
let mut total_time = Measure::start("wallclock time elapsed");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be nice if this mentions something about snapshots. With nvtx labels "wallclock time elapsed" won't be very descriptive.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will handle in separate pr

Copy link
Contributor

@sakridge sakridge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small nit about Measure text, but looks good

@codecov
Copy link

codecov bot commented Jun 14, 2021

Codecov Report

Merging #17937 (b5d9236) into master (c2191d8) will increase coverage by 0.0%.
The diff coverage is 60.0%.

@@           Coverage Diff           @@
##           master   #17937   +/-   ##
=======================================
  Coverage    82.6%    82.6%           
=======================================
  Files         431      431           
  Lines      121288   121302   +14     
=======================================
+ Hits       100224   100250   +26     
+ Misses      21064    21052   -12     

@jeffwashington jeffwashington merged commit e6bbd4b into solana-labs:master Jun 14, 2021
mergify bot pushed a commit that referenced this pull request Jun 14, 2021
jeffwashington added a commit to jeffwashington/solana that referenced this pull request Jun 14, 2021
mergify bot added a commit that referenced this pull request Jun 14, 2021
(cherry picked from commit e6bbd4b)

Co-authored-by: Jeff Washington (jwash) <[email protected]>
@brooksprumo brooksprumo mentioned this pull request Aug 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants