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

Wait for at least one account to have been created before starting the account benchmarks #3960

Merged
merged 1 commit into from
Dec 9, 2024

Conversation

steveluscher
Copy link

Problem

Consider a benchmark like the one in #3959. Presently, that PR causes a lot of this to be echoed when the benchmark first starts up:

[2024-12-05T22:22:43.421033375Z INFO  solana_accounts_cluster_bench] get_account_info: No accounts have yet been created; skipping
[2024-12-05T22:22:43.421044166Z INFO  solana_accounts_cluster_bench] get_account_info: No accounts have yet been created; skipping
[2024-12-05T22:22:43.421054506Z INFO  solana_accounts_cluster_bench] get_account_info: No accounts have yet been created; skipping
[2024-12-05T22:22:43.421065336Z INFO  solana_accounts_cluster_bench] get_account_info: No accounts have yet been created; skipping
[2024-12-05T22:22:43.421075706Z INFO  solana_accounts_cluster_bench] get_account_info: No accounts have yet been created; skipping

Summary of Changes

Wait on the creation of the first accounts before starting the benchmark.

Related: #3959.

let exit = exit.clone();
let max_closed = seed_tracker.max_closed.clone();
let max_created = seed_tracker.max_created.clone();
let mint = *mint;
Builder::new()
.name(format!("rpc-bench-{}", thread))
.spawn(move || {
start.recv().unwrap();
Copy link
Author

Choose a reason for hiding this comment

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

Prevents run_rpc_bench_loop from being executed until it receives the signal.

@steveluscher steveluscher added the automerge automerge Merge this Pull Request automatically once CI passes label Dec 6, 2024
Copy link

mergify bot commented Dec 6, 2024

automerge label removed due to a CI failure

@mergify mergify bot removed the automerge automerge Merge this Pull Request automatically once CI passes label Dec 6, 2024
@steveluscher steveluscher force-pushed the wait-to-start-bench branch 2 times, most recently from 2038b8a to 439fd26 Compare December 6, 2024 06:36
@steveluscher
Copy link
Author

Updated implementation to use Barrier for synchronization.

@steveluscher steveluscher merged commit 1c2dc20 into anza-xyz:master Dec 9, 2024
31 of 33 checks passed
@steveluscher steveluscher deleted the wait-to-start-bench branch December 9, 2024 08:07
@behzadnouri
Copy link

@steveluscher Can you please do not merge with red CI.
This has broken CI for others:
https://buildkite.com/anza/agave/builds/15530#0193ac9c-ff2e-4c03-a763-be27344672ab

@yihau didn't we have a check to prevent people to merge without a green CI.

@steveluscher
Copy link
Author

Can you please do not merge with red CI.

Wild. I'm really diligent about this. Let me see what happened here.

@steveluscher
Copy link
Author

I see what happened. I had a ton of PRs in a stack, none of which pushed this method over the limit itself, but the rebased version of this one (on top of one of the others) did.

I'll send a PR to fix that now.

@steveluscher
Copy link
Author

#4023.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants