Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

remove purge parameter to accounts #2990

Merged
merged 1 commit into from
Feb 28, 2019
Merged

remove purge parameter to accounts #2990

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 31 additions & 45 deletions runtime/src/accounts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,11 +429,9 @@ impl AccountsDB {
}
}

/// Store the account update. If the update is to delete the account because
/// the token balance is 0, purge needs to be set to true for the delete
/// to occur in place.
fn store_account(&self, fork: Fork, purge: bool, pubkey: &Pubkey, account: &Account) {
if account.tokens == 0 && purge {
/// Store the account update.
fn store_account(&self, fork: Fork, pubkey: &Pubkey, account: &Account) {
if account.tokens == 0 && self.is_squashed(fork) {
// purge if balance is 0 and no checkpoints
let index = self.index_info.index.read().unwrap();
let map = index.get(&pubkey).unwrap();
Expand All @@ -459,20 +457,19 @@ impl AccountsDB {
}
}

pub fn store(&self, fork: Fork, purge: bool, pubkey: &Pubkey, account: &Account) {
pub fn store(&self, fork: Fork, pubkey: &Pubkey, account: &Account) {
{
if !self.index_info.index.read().unwrap().contains_key(&pubkey) {
let mut windex = self.index_info.index.write().unwrap();
windex.insert(*pubkey, AccountMap(RwLock::new(HashMap::new())));
}
}
self.store_account(fork, purge, pubkey, account);
self.store_account(fork, pubkey, account);
}

pub fn store_accounts(
&self,
fork: Fork,
purge: bool,
txs: &[Transaction],
res: &[Result<()>],
loaded: &[Result<(InstructionAccounts, InstructionLoaders)>],
Expand Down Expand Up @@ -506,7 +503,7 @@ impl AccountsDB {
let tx = &txs[i];
let acc = raccs.as_ref().unwrap();
for (key, account) in tx.account_keys.iter().zip(acc.0.iter()) {
self.store(fork, purge, key, account);
self.store(fork, key, account);
}
}
}
Expand Down Expand Up @@ -649,6 +646,16 @@ impl AccountsDB {
parents
}

fn is_squashed(&self, fork: Fork) -> bool {
self.fork_info
Copy link
Contributor

Choose a reason for hiding this comment

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

This is kind of more expensive than the old way, but maybe it doesn't matter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what's the old way?

Copy link
Contributor

Choose a reason for hiding this comment

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

checking the parent pointer on the bank.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the duplication of state/logic was bugging me ;)

.read()
.unwrap()
.get(&fork)
.unwrap()
.parents
.is_empty()
}

fn get_merged_index(
&self,
fork: Fork,
Expand All @@ -668,7 +675,7 @@ impl AccountsDB {
None
}

/// become the root accountsDB
/// make fork a root, i.e. forget its heritage
fn squash(&self, fork: Fork) {
let parents = self.remove_parents(fork);
let tx_count = parents
Expand Down Expand Up @@ -769,10 +776,8 @@ impl Accounts {
}

/// Slow because lock is held for 1 operation insted of many
/// * purge - if the account token value is 0 and purge is true then delete the account.
/// purge should be set to false for overlays, and true for the root checkpoint.
pub fn store_slow(&self, fork: Fork, purge: bool, pubkey: &Pubkey, account: &Account) {
self.accounts_db.store(fork, purge, pubkey, account);
pub fn store_slow(&self, fork: Fork, pubkey: &Pubkey, account: &Account) {
self.accounts_db.store(fork, pubkey, account);
}

fn lock_account(
Expand Down Expand Up @@ -871,18 +876,14 @@ impl Accounts {
}

/// Store the accounts into the DB
/// * purge - if the account token value is 0 and purge is true then delete the account.
/// purge should be set to false for overlays, and true for the root checkpoint.
pub fn store_accounts(
&self,
fork: Fork,
purge: bool,
txs: &[Transaction],
res: &[Result<()>],
loaded: &[Result<(InstructionAccounts, InstructionLoaders)>],
) {
self.accounts_db
.store_accounts(fork, purge, txs, res, loaded)
self.accounts_db.store_accounts(fork, txs, res, loaded)
}

pub fn increment_transaction_count(&self, fork: Fork, tx_count: usize) {
Expand Down Expand Up @@ -914,29 +915,14 @@ mod tests {
use solana_sdk::transaction::Instruction;
use solana_sdk::transaction::Transaction;

#[test]
fn test_purge() {
let paths = "purge".to_string();
let db = AccountsDB::new(0, &paths);
let key = Pubkey::default();
let account = Account::new(0, 0, Pubkey::default());
// accounts are deleted when their token value is 0 and purge is true
db.store(0, false, &key, &account);
assert_eq!(db.load(0, &key, true), Some(account.clone()));
// purge should be set to true for the root checkpoint
db.store(0, true, &key, &account);
assert_eq!(db.load(0, &key, true), None);
cleanup_dirs(&paths);
}

fn load_accounts(
tx: Transaction,
ka: &Vec<(Pubkey, Account)>,
error_counters: &mut ErrorCounters,
) -> Vec<Result<(InstructionAccounts, InstructionLoaders)>> {
let accounts = Accounts::new(0, None);
for ka in ka.iter() {
accounts.store_slow(0, true, &ka.0, &ka.1);
accounts.store_slow(0, &ka.0, &ka.1);
}

let res = accounts.load_accounts(0, &[tx], vec![Ok(())], error_counters);
Expand Down Expand Up @@ -1321,7 +1307,7 @@ mod tests {
let account0 = Account::new(1, 0, key);

// store value 1 in the "root", i.e. db zero
db.store(0, true, &key, &account0);
db.store(0, &key, &account0);

let mut pubkeys: Vec<Pubkey> = vec![];
create_account(&db, &mut pubkeys, 100, 0);
Expand All @@ -1333,9 +1319,9 @@ mod tests {
assert_eq!(compare_account(&default_account, &account), true);
}
db.add_fork(1, Some(0));
// store value 0 in the child, but don't purge (see purge test above)
// store value 0 in the child
let account1 = Account::new(0, 0, key);
db.store(1, false, &key, &account1);
db.store(1, &key, &account1);

// masking accounts is done at the Accounts level, at accountsDB we see
// original account
Expand Down Expand Up @@ -1365,12 +1351,12 @@ mod tests {
let paths = "unsquash".to_string();
let db0 = AccountsDB::new(0, &paths);
let account0 = Account::new(1, 0, key);
db0.store(0, true, &key, &account0);
db0.store(0, &key, &account0);

db0.add_fork(1, Some(0));
// 0 tokens in the child
let account1 = Account::new(0, 0, key);
db0.store(1, false, &key, &account1);
db0.store(1, &key, &account1);

// masking accounts is done at the Accounts level, at accountsDB we see
// original account
Expand Down Expand Up @@ -1400,7 +1386,7 @@ mod tests {
nvote -= 1;
}
assert!(accounts.load(0, &pubkey, true).is_none());
accounts.store(0, true, &pubkey, &default_account);
accounts.store(0, &pubkey, &default_account);
}
}

Expand All @@ -1409,7 +1395,7 @@ mod tests {
let idx = thread_rng().gen_range(0, range);
if let Some(mut account) = accounts.load(0, &pubkeys[idx], true) {
account.tokens = account.tokens + 1;
accounts.store(0, true, &pubkeys[idx], &account);
accounts.store(0, &pubkeys[idx], &account);
if account.tokens == 0 {
assert!(accounts.load(0, &pubkeys[idx], true).is_none());
} else {
Expand Down Expand Up @@ -1491,7 +1477,7 @@ mod tests {
];
let pubkey1 = Keypair::new().pubkey();
let account1 = Account::new(1, ACCOUNT_DATA_FILE_SIZE as usize / 2, pubkey1);
accounts.store(0, true, &pubkey1, &account1);
accounts.store(0, &pubkey1, &account1);
{
let stores = accounts.storage.read().unwrap();
assert_eq!(stores.len(), 1);
Expand All @@ -1501,7 +1487,7 @@ mod tests {

let pubkey2 = Keypair::new().pubkey();
let account2 = Account::new(1, ACCOUNT_DATA_FILE_SIZE as usize / 2, pubkey2);
accounts.store(0, true, &pubkey2, &account2);
accounts.store(0, &pubkey2, &account2);
{
let stores = accounts.storage.read().unwrap();
assert_eq!(stores.len(), 2);
Expand All @@ -1515,7 +1501,7 @@ mod tests {

for i in 0..25 {
let index = i % 2;
accounts.store(0, true, &pubkey1, &account1);
accounts.store(0, &pubkey1, &account1);
{
let stores = accounts.storage.read().unwrap();
assert_eq!(stores.len(), 3);
Expand Down
11 changes: 4 additions & 7 deletions runtime/src/bank.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,6 @@ impl Bank {

self.accounts().store_slow(
self.id,
self.is_root(),
&genesis_block.bootstrap_leader_vote_account_id,
&bootstrap_leader_vote_account,
);
Expand All @@ -260,8 +259,7 @@ impl Bank {

pub fn add_native_program(&self, name: &str, program_id: &Pubkey) {
let account = native_loader::create_program_account(name);
self.accounts()
.store_slow(self.id, self.is_root(), program_id, &account);
self.accounts().store_slow(self.id, program_id, &account);
}

fn add_builtin_programs(&self) {
Expand Down Expand Up @@ -548,7 +546,7 @@ impl Bank {
// assert!(!self.is_frozen());
let now = Instant::now();
self.accounts()
.store_accounts(self.id, self.is_root(), txs, executed, loaded_accounts);
.store_accounts(self.id, txs, executed, loaded_accounts);

// once committed there is no way to unroll
let write_elapsed = now.elapsed();
Expand Down Expand Up @@ -633,7 +631,7 @@ impl Bank {
}

account.tokens -= tokens;
self.accounts().store_slow(self.id, true, pubkey, &account);
self.accounts().store_slow(self.id, pubkey, &account);
Ok(())
}
None => Err(BankError::AccountNotFound),
Expand All @@ -643,8 +641,7 @@ impl Bank {
pub fn deposit(&self, pubkey: &Pubkey, tokens: u64) {
let mut account = self.get_account(pubkey).unwrap_or_default();
account.tokens += tokens;
self.accounts()
.store_slow(self.id, self.is_root(), pubkey, &account);
self.accounts().store_slow(self.id, pubkey, &account);
}

fn accounts(&self) -> Arc<Accounts> {
Expand Down