-
Notifications
You must be signed in to change notification settings - Fork 4.5k
Fix accounts_db store counts in purging accounts logic #8218
Conversation
Codecov Report
@@ Coverage Diff @@
## master #8218 +/- ##
========================================
+ Coverage 82% 82% +<.1%
========================================
Files 249 249
Lines 53530 53576 +46
========================================
+ Hits 43913 43959 +46
Misses 9617 9617 |
} | ||
} | ||
if no_delete { | ||
for (_slot_id, account_info) in account_infos { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I carefully re-read the purge_zero_lamport_accounts
. And I think this new logic works as intended. Very nice and thank you so much! It's sad for me not to notice nor write this... ;)
My previous perception was wrong... Especially the DAG thing. I wonder why I was urged to think so. Maybe, too tired...
And this makes purge_zero_lamport_accounts
yet more conservative. So, regardless a hostile or normal situation, I think accumulated zero lamport accounts impose us no-op wasted CPU time by bunch of loops for each snapshot_slot_interval
to some extent.
Especially, this loop exhibits some contaminating/cascading behavior, easily approaching to the worst cost of O(N)
where N is the total number of all updates to every accounts which became 0 lamport in the past at least once. That's because making no_delete
be true
for vast of unrelated pubkey
s can easily be done by triggering the avalanche counting just with a single zero account with many updates.
In near future, I think we'd need some compaction mechanism as implied by #8168...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Is this a possible fix for #8168? |
Sweet! Read to land? ? |
yea |
* Show insufficient purge_zero_lamport_account logic * Add another pass to detect non-deleted values and increment the count Co-authored-by: Ryo Onodera <ryoqun@gmail.com> (cherry picked from commit a8028fb)
Problem
When deleting, purge_zero_lamport accounts doesn't re-adjust the store counts once an account is determined not to be purged.
Summary of Changes
Take a pass to readjust the store counts before determining which stores can be purged.
Fixes #8130