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

correctly log stats at remove_unrooted_slots #32467

Merged
merged 1 commit into from
Jul 13, 2023

Conversation

jeffwashington
Copy link
Contributor

@jeffwashington jeffwashington commented Jul 12, 2023

Problem

During high account count testing, we discovered remove_unrooted_slots taking a very long time (hours).
Unfortunately, the log metrics had a bug in when to log the metrics. The metric logging code is shared. In this case, the metrics are created new each call. This means the first time to check if sufficient duration has passed will ALWAYS skip the first call and say 'no'. This prevents startup noise and gives periodic, useful metrics over time. However, in this case since we never call it a second time on the same temporary metrics struct, the metrics NEVER get logged. This is related to the skip first parameter within the elapsed time checker.

Summary of Changes

Change duration to None so that we always log the metrics NOW, the one and only time it is called from abs loop.

Fixes #

@jeffwashington jeffwashington marked this pull request as ready for review July 12, 2023 19:12
@codecov
Copy link

codecov bot commented Jul 12, 2023

Codecov Report

Merging #32467 (6a10fdd) into master (a3ada9c) will increase coverage by 0.0%.
The diff coverage is 100.0%.

@@           Coverage Diff           @@
##           master   #32467   +/-   ##
=======================================
  Coverage    82.1%    82.1%           
=======================================
  Files         778      778           
  Lines      210200   210200           
=======================================
+ Hits       172640   172661   +21     
+ Misses      37560    37539   -21     

@jeffwashington jeffwashington merged commit 09ddbd7 into solana-labs:master Jul 13, 2023
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