Jelajahi Sumber

Deprecate `ThinClient` and remove `ThinClient` from `bench-tps` (#35365)

* deprecate ThinClient

* switch localcluster bench test to use tpuclient

add back in command line args for thinclient. add thin-client deprecation README

refactor TpuClient connection

* remove thin-client from net/

* change 2.0.0 to 1.19.0
Greg Cusack 1 tahun lalu
induk
melakukan
5f6d66e87b

+ 28 - 58
bench-tps/src/cli.rs

@@ -13,7 +13,7 @@ use {
     },
     solana_tpu_client::tpu_client::{DEFAULT_TPU_CONNECTION_POOL_SIZE, DEFAULT_TPU_USE_QUIC},
     std::{
-        net::{IpAddr, Ipv4Addr, SocketAddr},
+        net::{IpAddr, Ipv4Addr},
         time::Duration,
     },
 };
@@ -24,9 +24,6 @@ const NUM_LAMPORTS_PER_ACCOUNT_DEFAULT: u64 = solana_sdk::native_token::LAMPORTS
 pub enum ExternalClientType {
     // Submits transactions to an Rpc node using an RpcClient
     RpcClient,
-    // Submits transactions directly to leaders using a ThinClient, broadcasting to multiple
-    // leaders when num_nodes > 1
-    ThinClient,
     // Submits transactions directly to leaders using a TpuClient, broadcasting to upcoming leaders
     // via TpuClient default configuration
     TpuClient,
@@ -53,12 +50,10 @@ pub enum ComputeUnitPrice {
 /// Holds the configuration for a single run of the benchmark
 #[derive(PartialEq, Debug)]
 pub struct Config {
-    pub entrypoint_addr: SocketAddr,
     pub json_rpc_url: String,
     pub websocket_url: String,
     pub id: Keypair,
     pub threads: usize,
-    pub num_nodes: usize,
     pub duration: Duration,
     pub tx_count: usize,
     pub keypair_multiplier: usize,
@@ -68,10 +63,8 @@ pub struct Config {
     pub write_to_client_file: bool,
     pub read_from_client_file: bool,
     pub target_lamports_per_signature: u64,
-    pub multi_client: bool,
     pub num_lamports_per_account: u64,
     pub target_slots_per_epoch: u64,
-    pub target_node: Option<Pubkey>,
     pub external_client_type: ExternalClientType,
     pub use_quic: bool,
     pub tpu_connection_pool_size: usize,
@@ -89,12 +82,10 @@ impl Eq for Config {}
 impl Default for Config {
     fn default() -> Config {
         Config {
-            entrypoint_addr: SocketAddr::from((Ipv4Addr::LOCALHOST, 8001)),
             json_rpc_url: ConfigInput::default().json_rpc_url,
             websocket_url: ConfigInput::default().websocket_url,
             id: Keypair::new(),
             threads: 4,
-            num_nodes: 1,
             duration: Duration::new(std::u64::MAX, 0),
             tx_count: 50_000,
             keypair_multiplier: 8,
@@ -104,10 +95,8 @@ impl Default for Config {
             write_to_client_file: false,
             read_from_client_file: false,
             target_lamports_per_signature: FeeRateGovernor::default().target_lamports_per_signature,
-            multi_client: true,
             num_lamports_per_account: NUM_LAMPORTS_PER_ACCOUNT_DEFAULT,
             target_slots_per_epoch: 0,
-            target_node: None,
             external_client_type: ExternalClientType::default(),
             use_quic: DEFAULT_TPU_USE_QUIC,
             tpu_connection_pool_size: DEFAULT_TPU_CONNECTION_POOL_SIZE,
@@ -169,8 +158,10 @@ pub fn build_args<'a>(version: &'_ str) -> App<'a, '_> {
                 .takes_value(true)
                 .conflicts_with("rpc_client")
                 .requires("tpu_addr")
-                .requires("thin_client")
-                .help("Specify custom rpc_addr to create thin_client"),
+                .hidden(hidden_unless_forced())
+                .help("Specify custom rpc_addr to create thin_client. \
+                    Note: ThinClient is deprecated. Argument will not be used. \
+                    Use tpc_client or rpc_client instead"),
         )
         .arg(
             Arg::with_name("tpu_addr")
@@ -179,8 +170,10 @@ pub fn build_args<'a>(version: &'_ str) -> App<'a, '_> {
                 .conflicts_with("rpc_client")
                 .takes_value(true)
                 .requires("rpc_addr")
-                .requires("thin_client")
-                .help("Specify custom tpu_addr to create thin_client"),
+                .hidden(hidden_unless_forced())
+                .help("Specify custom tpu_addr to create thin_client. \
+                    Note: ThinClient is deprecated. Argument will not be used. \
+                    Use tpc_client or rpc_client instead"),
         )
         .arg(
             Arg::with_name("entrypoint")
@@ -188,7 +181,10 @@ pub fn build_args<'a>(version: &'_ str) -> App<'a, '_> {
                 .long("entrypoint")
                 .value_name("HOST:PORT")
                 .takes_value(true)
-                .help("Rendezvous with the cluster at this entry point; defaults to 127.0.0.1:8001"),
+                .hidden(hidden_unless_forced())
+                .help("Rendezvous with the cluster at this entry point; defaults to 127.0.0.1:8001. \
+                    Note: ThinClient is deprecated. Argument will not be used. \
+                    Use tpc_client or rpc_client instead"),
         )
         .arg(
             Arg::with_name("faucet")
@@ -213,7 +209,10 @@ pub fn build_args<'a>(version: &'_ str) -> App<'a, '_> {
                 .long("num-nodes")
                 .value_name("NUM")
                 .takes_value(true)
-                .help("Wait for NUM nodes to converge"),
+                .hidden(hidden_unless_forced())
+                .help("Wait for NUM nodes to converge. \
+                    Note: ThinClient is deprecated. Argument will not be used. \
+                    Use tpc_client or rpc_client instead"),
         )
         .arg(
             Arg::with_name("threads")
@@ -238,7 +237,10 @@ pub fn build_args<'a>(version: &'_ str) -> App<'a, '_> {
         .arg(
             Arg::with_name("no-multi-client")
                 .long("no-multi-client")
-                .help("Disable multi-client support, only transact with the entrypoint."),
+                .hidden(hidden_unless_forced())
+                .help("Disable multi-client support, only transact with the entrypoint. \
+                    Note: ThinClient is deprecated. Flag will not be used. \
+                    Use tpc_client or rpc_client instead"),
         )
         .arg(
             Arg::with_name("target_node")
@@ -246,7 +248,10 @@ pub fn build_args<'a>(version: &'_ str) -> App<'a, '_> {
                 .requires("no-multi-client")
                 .takes_value(true)
                 .value_name("PUBKEY")
-                .help("Specify an exact node to send transactions to."),
+                .hidden(hidden_unless_forced())
+                .help("Specify an exact node to send transactions to. \
+                    Note: ThinClient is deprecated. Argument will not be used. \
+                    Use tpc_client or rpc_client instead"),
         )
         .arg(
             Arg::with_name("tx_count")
@@ -316,7 +321,6 @@ pub fn build_args<'a>(version: &'_ str) -> App<'a, '_> {
         .arg(
             Arg::with_name("rpc_client")
                 .long("use-rpc-client")
-                .conflicts_with("thin_client")
                 .conflicts_with("tpu_client")
                 .takes_value(false)
                 .help("Submit transactions with a RpcClient")
@@ -325,33 +329,20 @@ pub fn build_args<'a>(version: &'_ str) -> App<'a, '_> {
             Arg::with_name("tpu_client")
                 .long("use-tpu-client")
                 .conflicts_with("rpc_client")
-                .conflicts_with("thin_client")
                 .takes_value(false)
                 .help("Submit transactions with a TpuClient")
         )
-        .arg(
-            Arg::with_name("thin_client")
-                .long("use-thin-client")
-                .conflicts_with("rpc_client")
-                .conflicts_with("tpu_client")
-                .takes_value(false)
-                .hidden(hidden_unless_forced())
-                .help("Submit transactions with a ThinClient. Note: usage is discouraged. \
-                    ThinClient will be deprecated.")
-        )
         .arg(
             Arg::with_name("tpu_disable_quic")
                 .long("tpu-disable-quic")
                 .takes_value(false)
-                .help("Do not submit transactions via QUIC; only affects ThinClient \
-                    or TpuClient (default) sends"),
+                .help("Do not submit transactions via QUIC; only affects TpuClient (default) sends"),
         )
         .arg(
             Arg::with_name("tpu_connection_pool_size")
                 .long("tpu-connection-pool-size")
                 .takes_value(true)
-                .help("Controls the connection pool size per remote address; only affects ThinClient  \
-                    or TpuClient (default) sends"),
+                .help("Controls the connection pool size per remote address; only affects TpuClient (default) sends"),
         )
         .arg(
             Arg::with_name("compute_unit_price")
@@ -456,8 +447,6 @@ pub fn parse_args(matches: &ArgMatches) -> Result<Config, &'static str> {
 
     if matches.is_present("rpc_client") {
         args.external_client_type = ExternalClientType::RpcClient;
-    } else if matches.is_present("thin_client") {
-        args.external_client_type = ExternalClientType::ThinClient;
     }
 
     if matches.is_present("tpu_disable_quic") {
@@ -471,19 +460,10 @@ pub fn parse_args(matches: &ArgMatches) -> Result<Config, &'static str> {
             .map_err(|_| "can't parse tpu-connection-pool-size")?;
     }
 
-    if let Some(addr) = matches.value_of("entrypoint") {
-        args.entrypoint_addr = solana_net_utils::parse_host_port(addr)
-            .map_err(|_| "failed to parse entrypoint address")?;
-    }
-
     if let Some(t) = matches.value_of("threads") {
         args.threads = t.to_string().parse().map_err(|_| "can't parse threads")?;
     }
 
-    if let Some(n) = matches.value_of("num-nodes") {
-        args.num_nodes = n.to_string().parse().map_err(|_| "can't parse num-nodes")?;
-    }
-
     if let Some(duration) = matches.value_of("duration") {
         let seconds = duration
             .to_string()
@@ -533,13 +513,6 @@ pub fn parse_args(matches: &ArgMatches) -> Result<Config, &'static str> {
             .map_err(|_| "can't parse target-lamports-per-signature")?;
     }
 
-    args.multi_client = !matches.is_present("no-multi-client");
-    args.target_node = matches
-        .value_of("target_node")
-        .map(|target_str| target_str.parse::<Pubkey>())
-        .transpose()
-        .map_err(|_| "Failed to parse target-node")?;
-
     if let Some(v) = matches.value_of("num_lamports_per_account") {
         args.num_lamports_per_account = v
             .to_string()
@@ -611,7 +584,7 @@ mod tests {
         super::*,
         solana_sdk::signature::{read_keypair_file, write_keypair_file, Keypair, Signer},
         std::{
-            net::{IpAddr, Ipv4Addr, SocketAddr},
+            net::{IpAddr, Ipv4Addr},
             time::Duration,
         },
         tempfile::{tempdir, TempDir},
@@ -671,8 +644,6 @@ mod tests {
             "4",
             "--read-client-keys",
             "./client-accounts.yml",
-            "--entrypoint",
-            "192.1.2.3:8001",
         ]);
         let actual = parse_args(&matches).unwrap();
         assert_eq!(
@@ -686,7 +657,6 @@ mod tests {
                 threads: 4,
                 read_from_client_file: true,
                 client_ids_and_stake_file: "./client-accounts.yml".to_string(),
-                entrypoint_addr: SocketAddr::from((Ipv4Addr::new(192, 1, 2, 3), 8001)),
                 ..Config::default()
             }
         );

+ 2 - 85
bench-tps/src/main.rs

@@ -1,6 +1,5 @@
 #![allow(clippy::arithmetic_side_effects)]
 use {
-    clap::value_t,
     log::*,
     solana_bench_tps::{
         bench::{do_bench_tps, max_lamports_for_prioritization},
@@ -11,11 +10,9 @@ use {
     },
     solana_client::{
         connection_cache::ConnectionCache,
-        thin_client::ThinClient,
         tpu_client::{TpuClient, TpuClientConfig},
     },
     solana_genesis::Base64Account,
-    solana_gossip::gossip_service::{discover_cluster, get_client, get_multi_client},
     solana_rpc_client::rpc_client::RpcClient,
     solana_sdk::{
         commitment_config::CommitmentConfig,
@@ -24,12 +21,12 @@ use {
         signature::{Keypair, Signer},
         system_program,
     },
-    solana_streamer::{socket::SocketAddrSpace, streamer::StakedNodes},
+    solana_streamer::streamer::StakedNodes,
     std::{
         collections::HashMap,
         fs::File,
         io::prelude::*,
-        net::{IpAddr, SocketAddr},
+        net::IpAddr,
         path::Path,
         process::exit,
         sync::{Arc, RwLock},
@@ -125,13 +122,8 @@ fn create_connection_cache(
 #[allow(clippy::too_many_arguments)]
 fn create_client(
     external_client_type: &ExternalClientType,
-    entrypoint_addr: &SocketAddr,
     json_rpc_url: &str,
     websocket_url: &str,
-    multi_client: bool,
-    rpc_tpu_sockets: Option<(SocketAddr, SocketAddr)>,
-    num_nodes: usize,
-    target_node: Option<Pubkey>,
     connection_cache: ConnectionCache,
     commitment_config: CommitmentConfig,
 ) -> Arc<dyn BenchTpsClient + Send + Sync> {
@@ -140,53 +132,6 @@ fn create_client(
             json_rpc_url.to_string(),
             commitment_config,
         )),
-        ExternalClientType::ThinClient => {
-            let connection_cache = Arc::new(connection_cache);
-            if let Some((rpc, tpu)) = rpc_tpu_sockets {
-                Arc::new(ThinClient::new(rpc, tpu, connection_cache))
-            } else {
-                let nodes =
-                    discover_cluster(entrypoint_addr, num_nodes, SocketAddrSpace::Unspecified)
-                        .unwrap_or_else(|err| {
-                            eprintln!("Failed to discover {num_nodes} nodes: {err:?}");
-                            exit(1);
-                        });
-                if multi_client {
-                    let (client, num_clients) =
-                        get_multi_client(&nodes, &SocketAddrSpace::Unspecified, connection_cache);
-                    if nodes.len() < num_clients {
-                        eprintln!(
-                            "Error: Insufficient nodes discovered.  Expecting {num_nodes} or more"
-                        );
-                        exit(1);
-                    }
-                    Arc::new(client)
-                } else if let Some(target_node) = target_node {
-                    info!("Searching for target_node: {:?}", target_node);
-                    let mut target_client = None;
-                    for node in nodes {
-                        if node.pubkey() == &target_node {
-                            target_client = Some(get_client(
-                                &[node],
-                                &SocketAddrSpace::Unspecified,
-                                connection_cache,
-                            ));
-                            break;
-                        }
-                    }
-                    Arc::new(target_client.unwrap_or_else(|| {
-                        eprintln!("Target node {target_node} not found");
-                        exit(1);
-                    }))
-                } else {
-                    Arc::new(get_client(
-                        &nodes,
-                        &SocketAddrSpace::Unspecified,
-                        connection_cache,
-                    ))
-                }
-            }
-        }
         ExternalClientType::TpuClient => {
             let rpc_client = Arc::new(RpcClient::new_with_commitment(
                 json_rpc_url.to_string(),
@@ -236,20 +181,16 @@ fn main() {
     };
 
     let cli::Config {
-        entrypoint_addr,
         json_rpc_url,
         websocket_url,
         id,
-        num_nodes,
         tx_count,
         keypair_multiplier,
         client_ids_and_stake_file,
         write_to_client_file,
         read_from_client_file,
         target_lamports_per_signature,
-        multi_client,
         num_lamports_per_account,
-        target_node,
         external_client_type,
         use_quic,
         tpu_connection_pool_size,
@@ -295,25 +236,6 @@ fn main() {
         return;
     }
 
-    info!("Connecting to the cluster");
-    let rpc_tpu_sockets: Option<(SocketAddr, SocketAddr)> =
-        if let Ok(rpc_addr) = value_t!(matches, "rpc_addr", String) {
-            let rpc = rpc_addr.parse().unwrap_or_else(|e| {
-                eprintln!("RPC address should parse as socketaddr {e:?}");
-                exit(1);
-            });
-            let tpu = value_t!(matches, "tpu_addr", String)
-                .unwrap()
-                .parse()
-                .unwrap_or_else(|e| {
-                    eprintln!("TPU address should parse to a socket: {e:?}");
-                    exit(1);
-                });
-            Some((rpc, tpu))
-        } else {
-            None
-        };
-
     let connection_cache = create_connection_cache(
         json_rpc_url,
         *tpu_connection_pool_size,
@@ -324,13 +246,8 @@ fn main() {
     );
     let client = create_client(
         external_client_type,
-        entrypoint_addr,
         json_rpc_url,
         websocket_url,
-        *multi_client,
-        rpc_tpu_sockets,
-        *num_nodes,
-        *target_node,
         connection_cache,
         *commitment_config,
     );

+ 0 - 1
bench-tps/src/perf_utils.rs

@@ -47,7 +47,6 @@ pub fn sample_txs<T>(
         let mut txs =
             match client.get_transaction_count_with_commitment(CommitmentConfig::processed()) {
                 Err(e) => {
-                    // ThinClient with multiple options should pick a better one now.
                     info!("Couldn't get transaction count {:?}", e);
                     sleep(Duration::from_secs(sample_period));
                     continue;

+ 19 - 9
bench-tps/tests/bench_tps.rs

@@ -8,7 +8,7 @@ use {
         send_batch::generate_durable_nonce_accounts,
     },
     solana_client::{
-        thin_client::ThinClient,
+        connection_cache::ConnectionCache,
         tpu_client::{TpuClient, TpuClientConfig},
     },
     solana_core::validator::ValidatorConfig,
@@ -78,14 +78,24 @@ fn test_bench_tps_local_cluster(config: Config) {
 
     cluster.transfer(&cluster.funding_keypair, &faucet_pubkey, 100_000_000);
 
-    let client = Arc::new(ThinClient::new(
-        cluster.entry_point_info.rpc().unwrap(),
-        cluster
-            .entry_point_info
-            .tpu(cluster.connection_cache.protocol())
-            .unwrap(),
-        cluster.connection_cache.clone(),
-    ));
+    let ConnectionCache::Quic(cache) = &*cluster.connection_cache else {
+        panic!("Expected a Quic ConnectionCache.");
+    };
+
+    let rpc_pubsub_url = format!("ws://{}/", cluster.entry_point_info.rpc_pubsub().unwrap());
+    let rpc_url = format!("http://{}", cluster.entry_point_info.rpc().unwrap());
+
+    let client = Arc::new(
+        TpuClient::new_with_connection_cache(
+            Arc::new(RpcClient::new(rpc_url)),
+            rpc_pubsub_url.as_str(),
+            TpuClientConfig::default(),
+            cache.clone(),
+        )
+        .unwrap_or_else(|err| {
+            panic!("Could not create TpuClient {err:?}");
+        }),
+    );
 
     let lamports_per_account = 100;
 

+ 3 - 1
client/src/thin_client.rs

@@ -2,7 +2,7 @@
 //! a server-side TPU.  Client code should use this object instead of writing
 //! messages to the network directly. The binary encoding of its messages are
 //! unstable and may change in future releases.
-
+#[allow(deprecated)]
 use {
     crate::connection_cache::{dispatch, ConnectionCache},
     solana_quic_client::{QuicConfig, QuicConnectionManager, QuicPool},
@@ -32,11 +32,13 @@ use {
 /// A thin wrapper over thin-client/ThinClient to ease
 /// construction of the ThinClient for code dealing both with udp and quic.
 /// For the scenario only using udp or quic, use thin-client/ThinClient directly.
+#[allow(deprecated)]
 pub enum ThinClient {
     Quic(BackendThinClient<QuicPool, QuicConnectionManager, QuicConfig>),
     Udp(BackendThinClient<UdpPool, UdpConnectionManager, UdpConfig>),
 }
 
+#[allow(deprecated)]
 impl ThinClient {
     /// Create a new ThinClient that will interface with the Rpc at `rpc_addr` using TCP
     /// and the Tpu at `tpu_addr` over `transactions_socket` using Quic or UDP

+ 2 - 2
net/net.sh

@@ -118,7 +118,7 @@ Operate a configured testnet
                                       - Enable UDP for tpu transactions
 
    --client-type
-                                      - Specify backend client type for bench-tps. Valid options are (thin-client|rpc-client|tpu-client), tpu-client is default
+                                      - Specify backend client type for bench-tps. Valid options are (rpc-client|tpu-client), tpu-client is default
 
  sanity/start-specific options:
    -F                   - Discard validator nodes that didn't bootup successfully
@@ -972,7 +972,7 @@ while [[ -n $1 ]]; do
     elif [[ $1 = --client-type ]]; then
       clientType=$2
       case "$clientType" in
-        thin-client|tpu-client|rpc-client)
+        tpu-client|rpc-client)
           ;;
         *)
           echo "Unexpected client type: \"$clientType\""

+ 1 - 11
net/remote/remote-client.sh

@@ -43,19 +43,12 @@ skip)
   exit 1
 esac
 
-THIN_CLIENT=false
 RPC_CLIENT=false
 case "$clientType" in
-  thin-client)
-    THIN_CLIENT=true
-    RPC_CLIENT=false
-    ;;
   tpu-client)
-    THIN_CLIENT=false
     RPC_CLIENT=false
     ;;
   rpc-client)
-    THIN_CLIENT=false
     RPC_CLIENT=true
     ;;
   *)
@@ -74,10 +67,7 @@ solana-bench-tps)
 
   args=()
 
-  if ${THIN_CLIENT}; then
-    args+=(--entrypoint "$entrypointIp:8001")
-    args+=(--use-thin-client)
-  elif ${RPC_CLIENT}; then
+  if ${RPC_CLIENT}; then
     args+=(--use-rpc-client)
   fi
 

+ 4 - 0
thin-client/README.md

@@ -0,0 +1,4 @@
+# thin-client
+This crate for `thin-client` is deprecated as of v1.19.0. It will receive no bugfixes or updates.
+
+Please use `tpu-client` or `rpc-client`.

+ 5 - 0
thin-client/src/thin_client.rs

@@ -111,6 +111,7 @@ impl ClientOptimizer {
 }
 
 /// An object for querying and sending transactions to the network.
+#[deprecated(since = "1.19.0", note = "Use [RpcClient] or [TpuClient] instead.")]
 pub struct ThinClient<
     P, // ConnectionPool
     M, // ConnectionManager
@@ -122,6 +123,7 @@ pub struct ThinClient<
     connection_cache: Arc<ConnectionCache<P, M, C>>,
 }
 
+#[allow(deprecated)]
 impl<P, M, C> ThinClient<P, M, C>
 where
     P: ConnectionPool<NewConnectionConfig = C>,
@@ -323,6 +325,7 @@ where
     }
 }
 
+#[allow(deprecated)]
 impl<P, M, C> Client for ThinClient<P, M, C>
 where
     P: ConnectionPool<NewConnectionConfig = C>,
@@ -334,6 +337,7 @@ where
     }
 }
 
+#[allow(deprecated)]
 impl<P, M, C> SyncClient for ThinClient<P, M, C>
 where
     P: ConnectionPool<NewConnectionConfig = C>,
@@ -619,6 +623,7 @@ where
     }
 }
 
+#[allow(deprecated)]
 impl<P, M, C> AsyncClient for ThinClient<P, M, C>
 where
     P: ConnectionPool<NewConnectionConfig = C>,