Skip to content
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

don't log shrink metrics on first call #17328

Merged
merged 2 commits into from
May 20, 2021

Conversation

jeffwashington
Copy link
Contributor

@jeffwashington jeffwashington commented May 19, 2021

Problem

Shrink metrics always log on first call after creation. This causes noise and bifurcated metrics.

Summary of Changes

Wait to log until it has been 1 second.
Fixes #

@jeffwashington
Copy link
Contributor Author

this pr:
datapoint: shrink_stats num_slots_shrunk=2990i storage_read_elapsed=0i index_read_elapsed=307621i find_alive_elapsed=10353i create_and_insert_store_elapsed=55275i store_accounts_elapsed=354651i update_index_elapsed=177512i handle_reclaims_elapsed=86620i write_storage_elapsed=2i rewrite_elapsed=693354i drop_storage_entries_elapsed=26i recycle_stores_write_time=225i accounts_removed=4165i bytes_removed=2875392i store_append_accounts=161636i store_find_store=0i store_hash_accounts=0i build_meta_time=168044i read_only_cache=129070i
master:
datapoint: shrink_stats num_slots_shrunk=1i storage_read_elapsed=0i index_read_elapsed=59i find_alive_elapsed=3i create_and_insert_store_elapsed=192i store_accounts_elapsed=100i update_index_elapsed=43i handle_reclaims_elapsed=32i write_storage_elapsed=0i rewrite_elapsed=373i drop_storage_entries_elapsed=0i recycle_stores_write_time=0i accounts_removed=1i bytes_removed=0i store_append_accounts=45i store_find_store=0i store_hash_accounts=0i build_meta_time=46i read_only_cache=36i
datapoint: shrink_stats num_slots_shrunk=2989i storage_read_elapsed=0i index_read_elapsed=309356i find_alive_elapsed=10085i create_and_insert_store_elapsed=56001i store_accounts_elapsed=353106i update_index_elapsed=180224i handle_reclaims_elapsed=88388i write_storage_elapsed=0i rewrite_elapsed=696774i drop_storage_entries_elapsed=84i recycle_stores_write_time=360i accounts_removed=4164i bytes_removed=2875392i store_append_accounts=161879i store_find_store=0i store_hash_accounts=0i build_meta_time=166724i read_only_cache=128213i

@jeffwashington jeffwashington marked this pull request as ready for review May 19, 2021 14:56
@jeffwashington jeffwashington requested a review from carllin May 19, 2021 14:56
@codecov
Copy link

codecov bot commented May 19, 2021

Codecov Report

Merging #17328 (50f034a) into master (6cba534) will decrease coverage by 0.0%.
The diff coverage is 90.6%.

❗ Current head 50f034a differs from pull request most recent head c58af22. Consider uploading reports for the commit c58af22 to get more accurate results

@@            Coverage Diff            @@
##           master   #17328     +/-   ##
=========================================
- Coverage    82.7%    82.7%   -0.1%     
=========================================
  Files         421      423      +2     
  Lines      118048   118054      +6     
=========================================
- Hits        97664    97645     -19     
- Misses      20384    20409     +25     

@jeffwashington
Copy link
Contributor Author

@sakridge suggested simpler logic.

carllin
carllin previously approved these changes May 19, 2021
runtime/src/accounts_db.rs Outdated Show resolved Hide resolved
@mergify mergify bot dismissed carllin’s stale review May 19, 2021 21:56

Pull request has been modified.

@jeffwashington jeffwashington added the automerge Merge this Pull Request automatically once CI passes label May 19, 2021
@mergify mergify bot merged commit a544010 into solana-labs:master May 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Merge this Pull Request automatically once CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants