wip: ancient packing algorithm tweaks #2849
Conversation
accounts-db/src/accounts_db.rs
Outdated
| @@ -5082,6 +5104,41 @@ impl AccountsDb { | |||
| } | |||
| }; | |||
|
|
|||
| let mut limit = 10; | |||
There was a problem hiding this comment.
@HaoranYi @brooksprumo this is the algorithm that I hacked up to have shrink pick up some shrinkable ancient storages if shrink isn't overloaded. I don't love this, but it appears to work.
There was a problem hiding this comment.
You can see the cliff when I restarted with this algorithm in place. It took care of a backlog of 160k shrinkable storages over a very short amount of time. This may even be too aggressive.
| db.shrink_candidate_slots.lock().unwrap().insert(slot); | ||
| } | ||
| candidate_for_shrink | ||
| || if can_randomly_shrink && thread_rng().gen_range(0..80000) == 0 { |
There was a problem hiding this comment.
this random constant needs to be proportional to total # ancient storages allowed.
| @@ -577,6 +624,14 @@ impl AccountsDb { | |||
| }) | |||
| .count() | |||
| .saturating_sub(randoms as usize); | |||
| // ideal storage size is total alive bytes of ancient storages / half of max ancient slots | |||
| tuning.ideal_storage_size = NonZeroU64::new( | |||
| (infos.total_alive_bytes.0 / tuning.max_ancient_slots.max(1) as u64 * 2).max(5_000_000), | |||
There was a problem hiding this comment.
i think ideal storage size needs to be dynamically calculated. Here, I picked max ancient slots / 2 should be able to hold all of the alive bytes. We could be more aggressive and maybe say / 4. This constant can be true no matter the width. 10k is the default width today. @HaoranYi this is a new pr.
| return; | ||
| }; | ||
| */ | ||
| let max_ancient_slots = 100_000; // this gives us approx 5M ideal size for testing against mnb now |
There was a problem hiding this comment.
@HaoranYi I think once other changes are made, this number probably needs to be a cli arg and it should probably default to 100k or so. The question is 432k + this number - ancient offset becomes the expected # of storages we may have open at one time. So, that affects the global mmap or file handle limit. The more files we put here, the smaller size per file. Smaller files are great. Larger files are intense in i/o and cpu to rewrite.
There was a problem hiding this comment.
yeah. I will add a cli arg for this one.
| percent_of_alive_shrunk_data: 55, | ||
| max_ancient_slots, | ||
| // don't re-pack anything just to shrink. Shrink will handle these old storages. | ||
| percent_of_alive_shrunk_data: 0, |
There was a problem hiding this comment.
I think we can change this tuning parameter to 0 to stop re-packing shrinkable slots. This mixes warm and cold accounts, which is not good. Once shrink is taking care of ancient storages, the pack algorithm should not mix old and new. It should only mix new and small in order to hit the storage count limit.
jeffwashington
force-pushed
the
4aug50_100k
branch
9 times, most recently
from
487bc42 to
51c34c0
Compare
jeffwashington
force-pushed
the
4aug50_100k
branch
from
51c34c0 to
6aa09a7
Compare
accounts-db/src/accounts_db.rs
Outdated
| limit = shrink_slots.len() + 1; | ||
| } | ||
| let mut num_found = 0; | ||
| if shrink_slots.len() < limit { |
There was a problem hiding this comment.
This condition seems to be always true: shrink_slots().len is 0 or 1, limit is 2, shrink_slots.len() is 2, 3, ... limit is 3, 4, ... shrink_slots.len() + 1.
I'm trying to understand the purpose of this entire block. We add something to the shrink_slots only if the something's capacity is the same as storage capacity for the corresponding slot, but only one such addition is allowed (or two if we start with an empty list of shrink_slots). Could you, maybe, in a few words help me to figure out what we're accomplishing here in the context of the whole shrinking algorithm?
| @@ -550,7 +596,7 @@ impl AccountsDb { | |||
| let max_slot = slots.iter().max().cloned().unwrap_or_default(); | |||
| // heuristic to include some # of newly eligible ancient slots so that the pack algorithm always makes progress | |||
| let high_slot_boundary = max_slot.saturating_sub(HIGH_SLOT_OFFSET); | |||
| let is_high_slot = |slot| slot >= high_slot_boundary; | |||
| let is_high_slot = |slot| false; // nothign is high slot slot >= high_slot_boundary; | |||
There was a problem hiding this comment.
Is this a temporary replacement for experimentation or a permanent change that you intended to make for this PR?
jeffwashington
force-pushed
the
4aug50_100k
branch
from
0d43e08 to
ea5da23
Compare
jeffwashington
force-pushed
the
4aug50_100k
branch
from
d9d6ae6 to
d989c46
Compare
Problem
ancient packing when skipping rewrites has some non-ideal behavior
Summary of Changes
prototype branch to try some things out
Fixes #