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

accounts-index: allow evicting refcount=2 entries from in-memory index (wip) #3184

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

HaoranYi
Copy link

@HaoranYi HaoranYi commented Oct 15, 2024

Problem

When skip rewrites and ancient storage is on, we are seeing that there are much
more ref_count=2 entries in in-memory index.

So instead of only flushing ref_count=1 index entries to disk, this PR allows
to flush ref-count=2 index entries, if they contains a slot that's more than
one epoch old.

Summary of Changes

  • Evict ref_count=2 index entry if they contains a slot that's more than one
    epoch old.
  • Change disk index to allow single_slot_ref_2 entry to be stored in index bucket.

Fixes #

@HaoranYi HaoranYi changed the title allow evict refcount=2 entries from in-memory index accounts-index: allow evict refcount=2 entries from in-memory index Oct 15, 2024
@HaoranYi HaoranYi changed the title accounts-index: allow evict refcount=2 entries from in-memory index accounts-index: allow evicting refcount=2 entries from in-memory index Oct 15, 2024
@HaoranYi HaoranYi force-pushed the accounts-db/multi_disk_index branch from 36794f4 to 1b3fc23 Compare October 15, 2024 19:36
@HaoranYi HaoranYi force-pushed the accounts-db/multi_disk_index branch from 1b3fc23 to 592889e Compare October 15, 2024 19:38
let mut newest_slot = Slot::MIN;
for (slot, info) in slot_list.iter() {
cached = cached || info.is_cached();
oldest_slot = oldest_slot.min(*slot);

Choose a reason for hiding this comment

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

i don't think we care if oldest or newest is older than an epoch. Also, we can't write a slotlist.len() = 2 easily at all. I think the refcount==2, slot_list.len() == 1 is writable with 1 bit.

Copy link
Author

Choose a reason for hiding this comment

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

Age is only u8. Even with max age to flush, there is going to be 102s max (255*400ms). I was thinking this is too frequent.
After we flush these ref_count > 1 to disk, someone may touch them again, and we bring it to memory.
That's why I added one epoch old check, to make sure they are really cold (i.e. in ancient storage) then we flush.

Choose a reason for hiding this comment

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

I'm confused with the connection to age here. Age is the u8 used to try to write items to disk in a bucket.

Copy link
Author

@HaoranYi HaoranYi Oct 15, 2024

Choose a reason for hiding this comment

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

My understanding as follows.

Every 400ms, age increase by 1.
For each entry inserted, it will set it age_to_flush based on whether it is in cache
if not in cache, current_age + 5
if in cache, current_age + 255

So ref_count >2 entry if it is not in cache, after 5 age, then it is about 2s. then it will be flushed to disk if allowed.

Are these correct?

@@ -1283,10 +1283,6 @@ impl<T: IndexValue, U: DiskIndexValue + From<T> + Into<T>> InMemAccountsIndex<T,
// If the entry *was* updated, re-mark it as dirty then
// skip to the next pubkey/entry.
let ref_count = v.ref_count();
if ref_count != 1 || slot_list.len() != 1 {

Choose a reason for hiding this comment

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

if we write slot_list.len() != 1, disk index creates the data file for the bucket.

Copy link
Author

@HaoranYi HaoranYi Oct 16, 2024

Choose a reason for hiding this comment

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

Yes. We'd better not write slit_list.len()!=1. And they shouldn't be long lived.

Copy link
Author

Choose a reason for hiding this comment

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

I pushed a commit that allows single_slot_ref_2 to be stored in index bucket.

@jeffwashington jeffwashington self-requested a review October 15, 2024 20:27
@HaoranYi HaoranYi changed the title accounts-index: allow evicting refcount=2 entries from in-memory index accounts-index: allow evicting refcount=2 entries from in-memory index (wip) Oct 16, 2024
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.

2 participants