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

fix accounts index startup stat and add two new stats for dups #3112

Merged
merged 4 commits into from
Oct 12, 2024

Conversation

HaoranYi
Copy link

@HaoranYi HaoranYi commented Oct 8, 2024

Problem

  1. In accounts index startup stats, "total_item, max_bin_size, min_bin_size" are not populated. We are reading these stats too early - before the index are actually flushed. That's why they are all zeros. We should wait to read them after flush.
[2024-10-08T14:43:43.225032940Z
INFO
solana_metrics::metrics]
datapoint:
generate_index
overall_us=590639563i
total_us=505808369i
scan_stores_us=7810839i
insertion_time_us=67372816i
min_bin_size=0i
max_bin_size=0i
storage_size_storages_us=92035i
index_flush_us=424152i
total_rent_paying=0i
amount_to_top_off_rent=0i
total_items_including_duplicates=650345708i
total_items=0i
accounts_data_len_dedup_time_us=81218852i
total_duplicate_slot_keys=51300596i
populate_duplicate_keys_us=3019104i
total_slots=433370i
slots_to_clean=411148i

  1. "total_item, max_bin_size, min_bin_size" actually represent the index items stored inside in-memory cache. Rename them to make it explicit that they are stats for in-memory index.

  2. Add a stats to count how many dups (accounts_duplicates_num) and total_num_unique_duplicate_keys encountered during create account index at startup.

Summary of Changes

  • fix account index startup stats
  • add stats for dup account num at startup
[2024-10-08T20:12:36.127914984Z
INFO
solana_metrics::metrics]
datapoint:
generate_index
overall_us=145751031i
total_us=87320399i
scan_stores_us=1163008i
insertion_time_us=98630200i
min_bin_size_in_mem=989i
max_bin_size_in_mem=68834i
storage_size_storages_us=78822i
index_flush_us=525962i
total_rent_paying=0i
amount_to_top_off_rent=0i
total_items_including_duplicates=665224689i
total_items_in_mem=10070323i
accounts_data_len_dedup_time_us=55835509i
total_duplicate_slot_keys=52628546i
accounts_duplicates_num=42558223i
populate_duplicate_keys_us=1942864i
total_slots=418500i
slots_to_clean=418406i

Fixes #

@HaoranYi HaoranYi force-pushed the fix_accounts_index_startup_stat branch 3 times, most recently from efa7ebb to ca33900 Compare October 8, 2024 23:38
Copy link

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

I think the code looks good. Some nits and requests to rename datapoints, since they are notoriously hard to change later.

accounts-db/src/accounts_db.rs Outdated Show resolved Hide resolved
accounts-db/src/accounts_db.rs Show resolved Hide resolved
Comment on lines 685 to 686
pub total_num_unique_duplicate_keys: u64,
pub accounts_duplicates_num: u64,

Choose a reason for hiding this comment

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

I kinda wish this was in a separate PR. So one PR for fixing the existing stats, and a second PR for adding the new stats.

@HaoranYi HaoranYi force-pushed the fix_accounts_index_startup_stat branch from 83118db to 724380f Compare October 11, 2024 20:54
Copy link

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

Looks good. I think this is correct.

Copy link

@jeffwashington jeffwashington left a comment

Choose a reason for hiding this comment

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

lgtm

@HaoranYi HaoranYi changed the title fix accounts index startup stat fix accounts index startup stat and add two new stats for dups Oct 11, 2024
@HaoranYi HaoranYi merged commit 6210454 into anza-xyz:master Oct 12, 2024
38 checks passed
@HaoranYi HaoranYi deleted the fix_accounts_index_startup_stat branch October 12, 2024 00:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants