Add accounts_data_len to Bank#21781
Conversation
| let total_accounts_stats = bank.get_total_accounts_stats().unwrap(); | ||
| bank.accounts_data_len | ||
| .store(total_accounts_stats.data_len as u64, Release); |
There was a problem hiding this comment.
I was/am worried where Bank::default_with_accounts() is called. I've only seen it in tests, and in those cases the accounts data len here is always zero, and the time the get_total_accounts_stats() takes to run is 50-100 us on my MBP. Let me know if this should be changed though.
There was a problem hiding this comment.
maybe we rename it to default_for_tests or something and make tests be explicit?
There was a problem hiding this comment.
Ah, I see, Bank::default_with_accounts() is called by Bank::new_with_paths(), which is called by blockstore_processor when starting from Bank 0. So I'm going to leave the bank functions named the way they are. I feel pretty good, since starting from Bank 0 should have zero accounts (or very few), and the get_total_accounts_stats to get the accounts_data_len will be fast.
Codecov Report
@@ Coverage Diff @@
## master #21781 +/- ##
=======================================
Coverage 81.3% 81.3%
=======================================
Files 515 515
Lines 144019 144025 +6
=======================================
+ Hits 117099 117123 +24
+ Misses 26920 26902 -18 |
jeffwashington
left a comment
jeffwashington
left a comment
There was a problem hiding this comment.
I think I'm ok with this. Maybe rename the default function.
brooksprumo
force-pushed
the
bank-accounts-data-len
branch
from
890d6d6 to
4030507
Compare
mergify
Bot
dismissed
jeffwashington’s stale review
Pull request has been modified.
(cherry picked from commit eeb97fe)
2 participants
Problem
To set a cap on the accounts data size, transaction processing needs to know about the current size of the accounts data. This falls to the bank, which doesn't currently know about the size of the accounts data.
Summary of Changes
Add a field to Bank for storing the current accounts data size.
Build on PR #21757
Related to Issue #21604