Skip to content

Commit

Permalink
ScanConfig defaults no longer sort results (anza-xyz#1539)
Browse files Browse the repository at this point in the history
  • Loading branch information
godmodegalactus authored and neutrinoks committed Jul 17, 2024
1 parent fdbfe54 commit b123269
Show file tree
Hide file tree
Showing 12 changed files with 91 additions and 41 deletions.
1 change: 1 addition & 0 deletions accounts-db/benches/accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,7 @@ fn bench_load_largest_accounts(b: &mut Bencher) {
20,
&HashSet::new(),
AccountAddressFilter::Exclude,
false,
)
});
}
58 changes: 40 additions & 18 deletions accounts-db/src/accounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ impl Accounts {
num: usize,
filter_by_address: &HashSet<Pubkey>,
filter: AccountAddressFilter,
sort_results: bool,
) -> ScanResult<Vec<(Pubkey, u64)>> {
if num == 0 {
return Ok(vec![]);
Expand Down Expand Up @@ -287,7 +288,7 @@ impl Accounts {
account_balances.push(Reverse((account.lamports(), *pubkey)));
}
},
&ScanConfig::default(),
&ScanConfig::new(!sort_results),
)?;
Ok(account_balances
.into_sorted_vec()
Expand Down Expand Up @@ -480,6 +481,7 @@ impl Accounts {
&self,
ancestors: &Ancestors,
bank_id: BankId,
sort_results: bool,
) -> ScanResult<Vec<PubkeyAccountSlot>> {
let mut collector = Vec::new();
self.accounts_db
Expand All @@ -493,7 +495,7 @@ impl Accounts {
collector.push((*pubkey, account, slot))
}
},
&ScanConfig::default(),
&ScanConfig::new(!sort_results),
)
.map(|_| collector)
}
Expand All @@ -503,12 +505,17 @@ impl Accounts {
ancestors: &Ancestors,
bank_id: BankId,
scan_func: F,
sort_results: bool,
) -> ScanResult<()>
where
F: FnMut(Option<(&Pubkey, AccountSharedData, Slot)>),
{
self.accounts_db
.scan_accounts(ancestors, bank_id, scan_func, &ScanConfig::default())
self.accounts_db.scan_accounts(
ancestors,
bank_id,
scan_func,
&ScanConfig::new(!sort_results),
)
}

pub fn hold_range_in_memory<R>(
Expand All @@ -534,7 +541,7 @@ impl Accounts {
"", // disable logging of this. We now parallelize it and this results in multiple parallel logs
ancestors,
range,
&ScanConfig::new(true),
&ScanConfig::default(),
|option| Self::load_with_slot(&mut collector, option),
);
collector
Expand Down Expand Up @@ -2046,7 +2053,8 @@ mod tests {
bank_id,
0,
&HashSet::new(),
AccountAddressFilter::Exclude
AccountAddressFilter::Exclude,
false
)
.unwrap(),
vec![]
Expand All @@ -2058,7 +2066,8 @@ mod tests {
bank_id,
0,
&all_pubkeys,
AccountAddressFilter::Include
AccountAddressFilter::Include,
false
)
.unwrap(),
vec![]
Expand All @@ -2073,7 +2082,8 @@ mod tests {
bank_id,
1,
&HashSet::new(),
AccountAddressFilter::Exclude
AccountAddressFilter::Exclude,
false
)
.unwrap(),
vec![(pubkey1, 42)]
Expand All @@ -2085,7 +2095,8 @@ mod tests {
bank_id,
2,
&HashSet::new(),
AccountAddressFilter::Exclude
AccountAddressFilter::Exclude,
false
)
.unwrap(),
vec![(pubkey1, 42), (pubkey0, 42)]
Expand All @@ -2097,7 +2108,8 @@ mod tests {
bank_id,
3,
&HashSet::new(),
AccountAddressFilter::Exclude
AccountAddressFilter::Exclude,
false
)
.unwrap(),
vec![(pubkey1, 42), (pubkey0, 42), (pubkey2, 41)]
Expand All @@ -2111,7 +2123,8 @@ mod tests {
bank_id,
6,
&HashSet::new(),
AccountAddressFilter::Exclude
AccountAddressFilter::Exclude,
false
)
.unwrap(),
vec![(pubkey1, 42), (pubkey0, 42), (pubkey2, 41)]
Expand All @@ -2126,7 +2139,8 @@ mod tests {
bank_id,
1,
&exclude1,
AccountAddressFilter::Exclude
AccountAddressFilter::Exclude,
false
)
.unwrap(),
vec![(pubkey0, 42)]
Expand All @@ -2138,7 +2152,8 @@ mod tests {
bank_id,
2,
&exclude1,
AccountAddressFilter::Exclude
AccountAddressFilter::Exclude,
false
)
.unwrap(),
vec![(pubkey0, 42), (pubkey2, 41)]
Expand All @@ -2150,7 +2165,8 @@ mod tests {
bank_id,
3,
&exclude1,
AccountAddressFilter::Exclude
AccountAddressFilter::Exclude,
false
)
.unwrap(),
vec![(pubkey0, 42), (pubkey2, 41)]
Expand All @@ -2165,7 +2181,8 @@ mod tests {
bank_id,
1,
&include1_2,
AccountAddressFilter::Include
AccountAddressFilter::Include,
false
)
.unwrap(),
vec![(pubkey1, 42)]
Expand All @@ -2177,7 +2194,8 @@ mod tests {
bank_id,
2,
&include1_2,
AccountAddressFilter::Include
AccountAddressFilter::Include,
false
)
.unwrap(),
vec![(pubkey1, 42), (pubkey2, 41)]
Expand All @@ -2189,7 +2207,8 @@ mod tests {
bank_id,
3,
&include1_2,
AccountAddressFilter::Include
AccountAddressFilter::Include,
false
)
.unwrap(),
vec![(pubkey1, 42), (pubkey2, 41)]
Expand Down Expand Up @@ -2217,7 +2236,10 @@ mod tests {
#[test]
fn test_maybe_abort_scan() {
assert!(Accounts::maybe_abort_scan(ScanResult::Ok(vec![]), &ScanConfig::default()).is_ok());
let config = ScanConfig::default().recreate_with_abort();
assert!(
Accounts::maybe_abort_scan(ScanResult::Ok(vec![]), &ScanConfig::new(false)).is_ok()
);
let config = ScanConfig::new(false).recreate_with_abort();
assert!(Accounts::maybe_abort_scan(ScanResult::Ok(vec![]), &config).is_ok());
config.abort();
assert!(Accounts::maybe_abort_scan(ScanResult::Ok(vec![]), &config).is_err());
Expand Down
19 changes: 16 additions & 3 deletions accounts-db/src/accounts_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ pub enum UpsertReclaim {
IgnoreReclaims,
}

#[derive(Debug, Default)]
#[derive(Debug)]
pub struct ScanConfig {
/// checked by the scan. When true, abort scan.
pub abort: Option<Arc<AtomicBool>>,
Expand All @@ -100,11 +100,20 @@ pub struct ScanConfig {
pub collect_all_unsorted: bool,
}

impl Default for ScanConfig {
fn default() -> Self {
Self {
abort: None,
collect_all_unsorted: true,
}
}
}

impl ScanConfig {
pub fn new(collect_all_unsorted: bool) -> Self {
Self {
collect_all_unsorted,
..ScanConfig::default()
..Default::default()
}
}

Expand Down Expand Up @@ -4210,10 +4219,14 @@ pub mod tests {
assert!(!config.is_aborted());
}

let config = ScanConfig::default();
let config = ScanConfig::new(false);
assert!(!config.collect_all_unsorted);
assert!(config.abort.is_none());

let config = ScanConfig::default();
assert!(config.collect_all_unsorted);
assert!(config.abort.is_none());

let config = config.recreate_with_abort();
assert!(config.abort.is_some());
assert!(!config.is_aborted());
Expand Down
1 change: 1 addition & 0 deletions cli/src/cluster_query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1444,6 +1444,7 @@ pub fn process_largest_accounts(
.get_largest_accounts_with_config(RpcLargestAccountsConfig {
commitment: Some(config.commitment),
filter,
sort_results: None,
})?
.value;
let largest_accounts = CliAccountBalances { accounts };
Expand Down
6 changes: 3 additions & 3 deletions ledger-tool/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2091,7 +2091,7 @@ fn main() {

if remove_stake_accounts {
for (address, mut account) in bank
.get_program_accounts(&stake::program::id(), &ScanConfig::default())
.get_program_accounts(&stake::program::id(), &ScanConfig::new(false))
.unwrap()
.into_iter()
{
Expand Down Expand Up @@ -2141,7 +2141,7 @@ fn main() {

if !vote_accounts_to_destake.is_empty() {
for (address, mut account) in bank
.get_program_accounts(&stake::program::id(), &ScanConfig::default())
.get_program_accounts(&stake::program::id(), &ScanConfig::new(false))
.unwrap()
.into_iter()
{
Expand Down Expand Up @@ -2181,7 +2181,7 @@ fn main() {
for (address, mut account) in bank
.get_program_accounts(
&solana_vote_program::id(),
&ScanConfig::default(),
&ScanConfig::new(false),
)
.unwrap()
.into_iter()
Expand Down
4 changes: 2 additions & 2 deletions ledger-tool/src/output.rs
Original file line number Diff line number Diff line change
Expand Up @@ -656,7 +656,7 @@ impl AccountsScanner {

match &self.config.mode {
AccountsOutputMode::All => {
self.bank.scan_all_accounts(scan_func).unwrap();
self.bank.scan_all_accounts(scan_func, true).unwrap();
}
AccountsOutputMode::Individual(pubkeys) => pubkeys.iter().for_each(|pubkey| {
if let Some((account, slot)) = self
Expand All @@ -676,7 +676,7 @@ impl AccountsScanner {
}),
AccountsOutputMode::Program(program_pubkey) => self
.bank
.get_program_accounts(program_pubkey, &ScanConfig::default())
.get_program_accounts(program_pubkey, &ScanConfig::new(false))
.unwrap()
.iter()
.filter(|(_, account)| self.should_process_account(account))
Expand Down
1 change: 1 addition & 0 deletions rpc-client-api/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ pub struct RpcLargestAccountsConfig {
#[serde(flatten)]
pub commitment: Option<CommitmentConfig>,
pub filter: Option<RpcLargestAccountsFilter>,
pub sort_results: Option<bool>,
}

#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)]
Expand Down
1 change: 1 addition & 0 deletions rpc-client/src/nonblocking/rpc_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2257,6 +2257,7 @@ impl RpcClient {
/// let config = RpcLargestAccountsConfig {
/// commitment: Some(commitment_config),
/// filter: Some(RpcLargestAccountsFilter::Circulating),
/// sort_results: None,
/// };
/// let accounts = rpc_client.get_largest_accounts_with_config(
/// config,
Expand Down
1 change: 1 addition & 0 deletions rpc-client/src/rpc_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1872,6 +1872,7 @@ impl RpcClient {
/// let config = RpcLargestAccountsConfig {
/// commitment: Some(commitment_config),
/// filter: Some(RpcLargestAccountsFilter::Circulating),
/// sort_results: None,
/// };
/// let accounts = rpc_client.get_largest_accounts_with_config(
/// config,
Expand Down
8 changes: 7 additions & 1 deletion rpc/src/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,7 @@ impl JsonRpcRequestProcessor {
) -> RpcCustomResult<RpcResponse<Vec<RpcAccountBalance>>> {
let config = config.unwrap_or_default();
let bank = self.bank(config.commitment);
let sort_results = config.sort_results.unwrap_or(true);

if let Some((slot, accounts)) = self.get_cached_largest_accounts(&config.filter) {
Ok(RpcResponse {
Expand All @@ -900,7 +901,12 @@ impl JsonRpcRequestProcessor {
(HashSet::new(), AccountAddressFilter::Exclude)
};
let accounts = bank
.get_largest_accounts(NUM_LARGEST_ACCOUNTS, &addresses, address_filter)
.get_largest_accounts(
NUM_LARGEST_ACCOUNTS,
&addresses,
address_filter,
sort_results,
)
.map_err(|e| RpcCustomError::ScanError {
message: e.to_string(),
})?
Expand Down
14 changes: 9 additions & 5 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5377,18 +5377,20 @@ impl Bank {
}

/// Returns all the accounts this bank can load
pub fn get_all_accounts(&self) -> ScanResult<Vec<PubkeyAccountSlot>> {
self.rc.accounts.load_all(&self.ancestors, self.bank_id)
pub fn get_all_accounts(&self, sort_results: bool) -> ScanResult<Vec<PubkeyAccountSlot>> {
self.rc
.accounts
.load_all(&self.ancestors, self.bank_id, sort_results)
}

// Scans all the accounts this bank can load, applying `scan_func`
pub fn scan_all_accounts<F>(&self, scan_func: F) -> ScanResult<()>
pub fn scan_all_accounts<F>(&self, scan_func: F, sort_results: bool) -> ScanResult<()>
where
F: FnMut(Option<(&Pubkey, AccountSharedData, Slot)>),
{
self.rc
.accounts
.scan_all(&self.ancestors, self.bank_id, scan_func)
.scan_all(&self.ancestors, self.bank_id, scan_func, sort_results)
}

pub fn get_program_accounts_modified_since_parent(
Expand Down Expand Up @@ -5434,13 +5436,15 @@ impl Bank {
num: usize,
filter_by_address: &HashSet<Pubkey>,
filter: AccountAddressFilter,
sort_results: bool,
) -> ScanResult<Vec<(Pubkey, u64)>> {
self.rc.accounts.load_largest_accounts(
&self.ancestors,
self.bank_id,
num,
filter_by_address,
filter,
sort_results,
)
}

Expand Down Expand Up @@ -6723,7 +6727,7 @@ impl Bank {

/// Get all the accounts for this bank and calculate stats
pub fn get_total_accounts_stats(&self) -> ScanResult<TotalAccountsStats> {
let accounts = self.get_all_accounts()?;
let accounts = self.get_all_accounts(false)?;
Ok(self.calculate_total_accounts_stats(
accounts
.iter()
Expand Down
Loading

0 comments on commit b123269

Please sign in to comment.