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

ScanConfig defaults no longer sort results #1539

Merged
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 @@ -2153,7 +2160,8 @@ mod tests {
bank_id,
0,
&HashSet::new(),
AccountAddressFilter::Exclude
AccountAddressFilter::Exclude,
false
)
.unwrap(),
vec![]
Expand All @@ -2165,7 +2173,8 @@ mod tests {
bank_id,
0,
&all_pubkeys,
AccountAddressFilter::Include
AccountAddressFilter::Include,
false
)
.unwrap(),
vec![]
Expand All @@ -2180,7 +2189,8 @@ mod tests {
bank_id,
1,
&HashSet::new(),
AccountAddressFilter::Exclude
AccountAddressFilter::Exclude,
false
)
.unwrap(),
vec![(pubkey1, 42)]
Expand All @@ -2192,7 +2202,8 @@ mod tests {
bank_id,
2,
&HashSet::new(),
AccountAddressFilter::Exclude
AccountAddressFilter::Exclude,
false
)
.unwrap(),
vec![(pubkey1, 42), (pubkey0, 42)]
Expand All @@ -2204,7 +2215,8 @@ mod tests {
bank_id,
3,
&HashSet::new(),
AccountAddressFilter::Exclude
AccountAddressFilter::Exclude,
false
)
.unwrap(),
vec![(pubkey1, 42), (pubkey0, 42), (pubkey2, 41)]
Expand All @@ -2218,7 +2230,8 @@ mod tests {
bank_id,
6,
&HashSet::new(),
AccountAddressFilter::Exclude
AccountAddressFilter::Exclude,
false
)
.unwrap(),
vec![(pubkey1, 42), (pubkey0, 42), (pubkey2, 41)]
Expand All @@ -2233,7 +2246,8 @@ mod tests {
bank_id,
1,
&exclude1,
AccountAddressFilter::Exclude
AccountAddressFilter::Exclude,
false
)
.unwrap(),
vec![(pubkey0, 42)]
Expand All @@ -2245,7 +2259,8 @@ mod tests {
bank_id,
2,
&exclude1,
AccountAddressFilter::Exclude
AccountAddressFilter::Exclude,
false
)
.unwrap(),
vec![(pubkey0, 42), (pubkey2, 41)]
Expand All @@ -2257,7 +2272,8 @@ mod tests {
bank_id,
3,
&exclude1,
AccountAddressFilter::Exclude
AccountAddressFilter::Exclude,
false
)
.unwrap(),
vec![(pubkey0, 42), (pubkey2, 41)]
Expand All @@ -2272,7 +2288,8 @@ mod tests {
bank_id,
1,
&include1_2,
AccountAddressFilter::Include
AccountAddressFilter::Include,
false
)
.unwrap(),
vec![(pubkey1, 42)]
Expand All @@ -2284,7 +2301,8 @@ mod tests {
bank_id,
2,
&include1_2,
AccountAddressFilter::Include
AccountAddressFilter::Include,
false
)
.unwrap(),
vec![(pubkey1, 42), (pubkey2, 41)]
Expand All @@ -2296,7 +2314,8 @@ mod tests {
bank_id,
3,
&include1_2,
AccountAddressFilter::Include
AccountAddressFilter::Include,
false
)
.unwrap(),
vec![(pubkey1, 42), (pubkey2, 41)]
Expand Down Expand Up @@ -2324,7 +2343,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)]
brooksprumo marked this conversation as resolved.
Show resolved Hide resolved
#[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 @@ -4214,10 +4223,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 @@ -1443,6 +1443,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 @@ -2032,7 +2032,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 @@ -2082,7 +2082,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 @@ -2122,7 +2122,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 @@ -872,6 +872,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 @@ -896,7 +897,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 @@ -5304,18 +5304,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 @@ -5361,13 +5363,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 @@ -6643,7 +6647,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