-
Notifications
You must be signed in to change notification settings - Fork 389
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
Change DEFAULT_TPU_CONNECTION_POOL_SIZE to 1 #1014
Change DEFAULT_TPU_CONNECTION_POOL_SIZE to 1 #1014
Conversation
I used --tpu-connection-pool-size parameter when running bench-tps to test the difference between 1 and 4. Actually I am not seeing a lot of difference. Our QOS stuff change, Quinn change and tokio change probably make the connection count less critical in the performance now as compared like 2 years ago. Pool size = 4 [2024-04-19T20:56:28.038376184Z INFO solana_metrics::metrics] datapoint: bench-tps-lamport_balance balance=30638705150i [2024-04-19T21:06:32.653674773Z INFO solana_bench_tps::bench] Node address | Max TPS | Total Transactions size=1 [2024-04-19T22:12:44.815093180Z INFO solana_bench_tps::bench] http://35.233.177.221:8899/ | 62737.48 | 198348 [2024-04-19T22:19:32.622039191Z INFO solana_bench_tps::bench] ---------------------+---------------+-------------------- |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1014 +/- ##
=======================================
Coverage 81.9% 81.9%
=======================================
Files 855 855
Lines 232141 232141
=======================================
+ Hits 190168 190171 +3
+ Misses 41973 41970 -3 |
/// The default connection count is set to 1 -- it should | ||
/// be sufficient for most use cases. Validators can use | ||
/// --tpu-connection-pool-size to override this default value. | ||
pub const DEFAULT_TPU_CONNECTION_POOL_SIZE: usize = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we wanted to keep this as before for RPC nodes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We decide to turn this to 1 by default. RPC operator can use --tpu-connection-pool-size to overwrite if really necessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iirc the justification for choosing four initially was already on shaky ground. one doesn't appear to be an overt regression
Backports to the stable branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. |
Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis. |
(cherry picked from commit 15f2dad)
(cherry picked from commit 15f2dad)
…-xyz#1014) (anza-xyz#1043) Change DEFAULT_TPU_CONNECTION_POOL_SIZE to 1 (anza-xyz#1014) Co-authored-by: Lijun Wang <83639177+lijunwangs@users.noreply.github.com>
Problem
DEFAULT_TPU_CONNECTION_POOL_SIZE was set to 4 to maximize the performance when we do local performance testing with bench-tps. With the server side QOS restrictions per stake node and unstaked node, the benefit of more connections is less obvious while having more connections increase the load on the server especially during leader change.
This only change the default to 1. Validators still can use existing --tpu-connection-pool-size to override the default value.
Summary of Changes
Change DEFAULT_TPU_CONNECTION_POOL_SIZE to 1.
Fixes #