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

add ancient.total_alive_bytes metric #2828

Merged
merged 2 commits into from
Sep 4, 2024
Merged

Conversation

jeffwashington
Copy link

Problem

ancient packing needs to know the # of alive bytes across all ancient storages for future optimizations, such as a dynamic ideal size.

Summary of Changes

Calculate and add a metric for all alive bytes across all ancient storages.

Fixes #

@HaoranYi HaoranYi marked this pull request as ready for review September 4, 2024 13:36
HaoranYi
HaoranYi previously approved these changes Sep 4, 2024
Copy link

@HaoranYi HaoranYi left a comment

Choose a reason for hiding this comment

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

lgtm.

accounts-db/src/accounts_db.rs Show resolved Hide resolved
.map(|info| total_dead_bytes += info.capacity.saturating_sub(info.alive_bytes))
.map(|info| {
total_dead_bytes += info.capacity.saturating_sub(info.alive_bytes);
total_alive_bytes += info.alive_bytes;

Choose a reason for hiding this comment

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

I was initially curious how we guaranteed slots was all the ancient storages, otherwise this wouldn't be a 'total', per se.

I see we get slots by calling AccountsDb::get_sorted_potential_ancient_slots(). I believe it is safe for this function to assume/require that slots is correct.

And assuming we use 'pack', then the stats are reported after each invocation of combine_ancient_slots_packed(), so I think that also ensures we don't sum up multiple 'totals' too.

Copy link

@HaoranYi HaoranYi left a comment

Choose a reason for hiding this comment

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

lgtm

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.

:shipit:

@jeffwashington jeffwashington added the automerge automerge Merge this Pull Request automatically once CI passes label Sep 4, 2024
@mergify mergify bot merged commit f017972 into anza-xyz:master Sep 4, 2024
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge automerge Merge this Pull Request automatically once CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants