Add a more performant client to bench-tps #544

KirillLykov · 2024-04-02T16:12:10Z

Problem

Motivation is described in #415

Don't like the name HighTpsClient, but for now use it as working name.

It is also possible to modify TpuClient to be more performant by using runtime::spawn instead of runtime::invoke (or equivalent that we use there which is tokio::task::block_in_place(move || self.rpc_client.runtime().block_on(f))).
But in this case we cannot handle errors. Another way, might be to implement another method TpuClient::send_batch_xxx which will use spawn internally. see discussion

Summary of Changes

Add a new client which is supposed to be more performant than TpuClient for bench-tps needs.

Test runs results

With private cluster (distributed among 2 data centers, 10 nodes).

client	TPS+-std
TpuClient	5407+-3k
UDP connection	21823+-4.8k
HighTpsClient	10292+-6.2k

The identity of the node we use has 10% of total stake.
Would like to check if we can achieve better result with several identities to be close to UDP case.

TpuClient:

HighTpsClient:

With testnet:

#415 (comment)

Experiments setup

-u http://$IP:8899 --identity dos-funder.json --read-client-keys keypairs.yaml --duration 600 --tx_count 5000 --thread-batch-sleep-ms 10 --client-node-id invalidator/identity.json --bind-address $IP --block-data-file block.csv --threads 4 --tpu-connection-pool-size 4 --use-high-tps-client --sustained

codecov-commenter · 2024-04-02T17:41:54Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.8%. Comparing base (55c05c5) to head (4b45bb8).
Report is 2 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##           master     #544     +/-   ##
=========================================
- Coverage    81.8%    81.8%   -0.1%     
=========================================
  Files         851      851             
  Lines      230165   230172      +7     
=========================================
- Hits       188464   188460      -4     
- Misses      41701    41712     +11

bw-solana · 2024-04-05T16:24:32Z

bench-tps/src/cli.rs

@@ -27,6 +27,8 @@ pub enum ExternalClientType {
    // Submits transactions directly to leaders using a TpuClient, broadcasting to upcoming leaders
    // via TpuClient default configuration
    TpuClient,
+


Would be nice to add comment here to detail where we send transactions (direct to leaders) and short summary of difference w/ respect to TpuClient

bw-solana

Overall, LGTM

KirillLykov · 2024-04-05T16:29:27Z

Overall, LGTM

i'm not sure if it is better to:

create a new client (as it is)
modify TpuClient to use spawn instead
in bencht-tps use async so that we can call spawn (might be more time consuming to implement)

KirillLykov · 2024-04-05T16:31:14Z

@ilya-bobyr suggestions about the name of the class? I don't really like HighTpsClient, would prefer BenchTpsClient which unfortunately is the name of the trait already. Using the same name for trait and struct might be not the best practice as far as I understood

gregcusack

looks good for the most part! the numbers speak for themselves! just a couple questions/nits

gregcusack · 2024-04-05T16:49:00Z

bench-tps/src/cli.rs

+        .arg(
+            Arg::with_name("high_tps_client")
+                .long("use-high-tps-client")
+                .conflicts_with("rpc_client")


need a .conflicts_with("tpu_client") here?

gregcusack · 2024-04-05T16:52:26Z

bench-tps/src/main.rs

+                        rpc_client,
+                        websocket_url,
+                        HighTpsClientConfig {
+                            fanout_slots: 1,


were your experiments run with fanout_slots: 1 and any reason you landed on 1? Does it make sense to make configurable via CLI? The code in high_tps_client.rs makes it seem like it should be configurable.

Wrote in the other comment my thoughts about that. Regarding configuration in general, I would try to decrease the number of configurable parameters to simplify the ux of the tool. If we know some optimal parameters, I would prefer to hardcode them to decrease the mental to understand how to configure them. Currently, we have: number of threads, tx count, size of CC, sustained/not, timeout, ... I think we should just find one optimal set of parameters because it most of these are never touched. And if they are changed, it might mean that the defaults are not the most performant.

gregcusack · 2024-04-05T17:03:39Z

bench-tps/src/bench_tps_client/high_tps_client.rs

+use {
+    crate::bench_tps_client::{BenchTpsClient, BenchTpsError, Result},
+    solana_client::{
+        connection_cache::Protocol, nonblocking::tpu_client::LeaderTpuService,


can you import this directly from solana_tpu_client::nonblocking::tpu_client::LeaderTpuService? sounds like we're eventually going to get rid of the solana_client according to Tyera

gregcusack · 2024-04-05T17:11:45Z

bench-tps/src/bench_tps_client/high_tps_client.rs

+
+    fn send_batch(&self, transactions: Vec<Transaction>) -> Result<()> {
+        let wire_transactions = transactions
+            .into_iter() //.into_par_iter() any effect of this?


in response to comment here: probably if Vec<Transaction> is long. tpu_client uses .into_par_iter() so may make sense to use into_par_iter() but i haven't tested it to see performance diff

gregcusack · 2024-04-05T17:16:18Z

bench-tps/src/bench_tps_client/high_tps_client.rs

+            .map(|tx| bincode::serialize(&tx).expect("transaction should be valid."))
+            .collect::<Vec<_>>();
+        for c in wire_transactions.chunks(self.config.send_batch_size) {
+            let tpu_addresses = self


possible rename tpu_addresses to leader_tpu_addresses??

actually context may clear enough.

gregcusack · 2024-04-05T17:32:15Z

bench-tps/src/bench_tps_client/high_tps_client.rs

+            let tpu_addresses = self
+                .leader_tpu_service
+                .leader_tpu_sockets(self.config.fanout_slots);
+            for tpu_address in &tpu_addresses {


only sending to one leader here right?

should be to fanout_slots leaders.
In configuration that I specify 1 for this parameter because I think that we don't care how many txs have been timed out but only interested in maximazing TPS. Due to that it makes sense to utilize NIC bandwidth to send as many datagrams to one leader as possible (although it might be that for particular cluster configuration we cannot fully utilize it due to staked congestion control)

lijunwangs

I am not sure we want to introduce a new interface to bench-tps. Can you spell out the major difference between tpu-client and this new HighTpsClient? If it is just performance improvement, why cannot we just improve the existing TpuClient?

gregcusack · 2024-04-29T23:01:24Z

I am not sure we want to introduce a new interface to bench-tps. Can you spell out the major difference between tpu-client and this new HighTpsClient? If it is just performance improvement, why cannot we just improve the existing TpuClient?

FWIW I also +1 this. I have to modify TpuClient to get it to work properly in LocalCluster and it seems the changes I've made are a small subset of the changes you've added into HighTpsClient.

KirillLykov · 2025-01-17T15:19:27Z

Drop this one because there is tpu-client-next with which I manage to achieve 12k+-8k txs per block (using transaction-bench instead).

KirillLykov mentioned this pull request Apr 3, 2024

add high tps client: multi stake #559

Closed

KirillLykov force-pushed the klykov/add-high-tps-client branch from b62e9fa to a86fb92 Compare April 5, 2024 15:19

KirillLykov added 3 commits April 5, 2024 15:55

add high-tps-client

10a2c9b

update Cargo.lock

f21fe34

make leader_tpu_sockets to be public method

4b45bb8

KirillLykov force-pushed the klykov/add-high-tps-client branch from a86fb92 to 4b45bb8 Compare April 5, 2024 15:55

KirillLykov marked this pull request as ready for review April 5, 2024 15:55

KirillLykov requested review from bw-solana, gregcusack and lijunwangs April 5, 2024 15:59

bw-solana reviewed Apr 5, 2024

View reviewed changes

bw-solana approved these changes Apr 5, 2024

View reviewed changes

gregcusack reviewed Apr 5, 2024

View reviewed changes

lijunwangs reviewed Apr 29, 2024

View reviewed changes

KirillLykov closed this Jan 17, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a more performant client to bench-tps #544

Add a more performant client to bench-tps #544

KirillLykov commented Apr 2, 2024 •

edited

Loading

codecov-commenter commented Apr 2, 2024 •

edited

Loading

bw-solana Apr 5, 2024

bw-solana left a comment

KirillLykov commented Apr 5, 2024

KirillLykov commented Apr 5, 2024

gregcusack left a comment

gregcusack Apr 5, 2024

gregcusack Apr 5, 2024

KirillLykov Apr 8, 2024

gregcusack Apr 5, 2024

gregcusack Apr 5, 2024

gregcusack Apr 5, 2024

gregcusack Apr 5, 2024

gregcusack Apr 5, 2024

KirillLykov Apr 8, 2024

lijunwangs left a comment

gregcusack commented Apr 29, 2024

KirillLykov commented Jan 17, 2025

Add a more performant client to bench-tps #544

Add a more performant client to bench-tps #544

Conversation

KirillLykov commented Apr 2, 2024 • edited Loading

Problem

Summary of Changes

Test runs results

Experiments setup

codecov-commenter commented Apr 2, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

bw-solana left a comment

Choose a reason for hiding this comment

KirillLykov commented Apr 5, 2024

KirillLykov commented Apr 5, 2024

gregcusack left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lijunwangs left a comment

Choose a reason for hiding this comment

gregcusack commented Apr 29, 2024

KirillLykov commented Jan 17, 2025

KirillLykov commented Apr 2, 2024 •

edited

Loading

codecov-commenter commented Apr 2, 2024 •

edited

Loading