Follow-up for
commit 809041b
Author: Illia Bobyr <illia.bobyr@solana.com>
Date: Wed Mar 22 11:03:30 2023 -0700
poh_verify => run_verification: Rename to be more accurate (#30811)
`poh_verify` actually disables transaction signature, tick count and
built in program argument verifications as well. It is somewhat
confusing to call it `poh_verify`.
* Add --{replaying,banking}-backend flags (noop atm)
* Greatly simplify enums with strum
* Update programs/sbf/Cargo.lock...
* Rely on Display, removing Debug
* constify cli_names()
* Don't allow omitting bankend value
* Rename to --block-{verification,production}-method
* Use more specific name
* Actually support missing value....
* Remove strictly-unnecessary flags
* Use lazy_static! instead of abusing DefaultArgs...
`poh_verify` actually disables transaction signature, tick count and
built in program argument verifications as well. It is somewhat
confusing to call it `poh_verify`.
* Clean up orphaned account snapshot hardlink dirs
* fix compilation issues
* debugged, now working. seeing the orphaned directories deleted
* change back to eprintln + exit for account_path error
* changed eprintln to panic for now
* add test_clean_orphaned_account_snapshot_dirs for codecov check
* address a few comments and nit isseus
* directly unzip, skipped the intermediate array of tuples
* let set_up_account_run_and_snapshot_paths return Result
* 'proper' typo, and comment on return
* use map_err
* use for loop in clean_orphaned_account_snapshot_dirs, removed panic
* add test_set_up_account_run_and_snapshot_paths
* minor, replace .for_each with .all
* rename set_up_account_run_and_snapshot_paths to create_all_accounts_run_and_snapshot_dirs
* remove unnecessary closure return type
* change to for loop
* change match to unwrap_or_else
* remove create_dir_all(&account_path) in create_all
* minor comment cleanup
* Move AdminRpcRequestMetadataPostInit to solana-core
* Move AdminRpcRequestMetadataPostInit write to just before wait_for_supermajority
* Pass AdminRpcRequestMetadataPostInit in TestValidatorGenesis
* Fixup local-cluster
tpu-client/tpu_connection_cache is refactored out the module and moved to connection-cache/connection_cache and the logic in client/connection_cache is consolidated to connection-cache/connection_cache as well. client/connection_cache only has a thin wrapper which forward calls to connection-cache/connection_cache and deal with constructions of quic/udp connection cache for clients using them both.2.
The TpuConnection is refactored to ClientConnection to make it generic and functions renamed to be proper for other workflows. eg. tpu_addr -> server_addr, send_transaction --> send_data and etc...
The enum dispatch is removed so that we can make the bulk of code of quic and udp agnostic of each other. The client is possible to load quic or udp only into its runtime.
The generic type parameter in the tpu-client/tpu_connection_cache is removed in order to create both quic and udp connection cache and use the object to send transactions with multiple branching when sending data. The generic type parameters and associated types are dropped in other types in order to make the trait "object safe" for this purpose.
I have annotated the code explaining the reasoning and the refactoring source -> destination.
There is no functional changes
bench-tps has been performed for rpc-client, thin-client and tpu-client. And it is found the performance number largely match the ones before the refactoring.
* Add run parent directory for accounts files
* fix test test_concurrent_snapshot_packaging
* review comments. renamed the path setup function
* Addressed most of the review comments
* remove explict type def for map result
* handle create_accounts_run_and_snapshot_dirs error with expect
* update with more review comments
* minor fixes from review comments
* simplify account_filename option assignment
* handle error from create_accounts_run_and_snapshot_dirs
* use then instead of then_some for lazy evaluation
* Clean up files in the old account_path before trasitioning to the new run path
* try_exist and accounts_dir removing extra
* sync rmdir, is_dir check
* handle the account_path not deletable case
* Add fully-reproducible online tracer for banking
* Don't use eprintln!()...
* Update programs/sbf/Cargo.lock...
* Remove meaningless assert_eq
* Group test-only code under aptly named mod
* Remove needless overflow handling in receive_until
* Delay stat aggregation as it's possible now
* Use Cow to avoid needless heap allocs
* Properly consume metrics action as soon as hold
* Trace UnprocessedTransactionStorage::len() instead
* Loosen joining api over type safety for replaystage
* Introce hash event to override these when simulating
* Use serde_with/serde_as instead of hacky workaround
* Update another Cargo.lock...
* Add detailed comment for Packet::buffer serialize
* Rename sender_overhead_minimized_receiver_loop()
* Use type interference for TraceError
* Another minor rename
* Retire now useless ForEach to simplify code
* Use type alias as much as possible
* Properly translate and propagate tracing errors
* Clarify --enable-banking-trace with better naming
* Consider unclean (signal-based) node restarts..
* Tweak logging and cli
* Remove Bank events as it's not needed anymore
* Make tpu own banking tracer thread
* Reduce diff a bit..
* Use latest serde_with
* Finally use the published rolling-file crate
* Make test code change more consistent
* Revive dead and non-terminating test code path...
* Dispose batches early now that possible
* Split off thread handle very early at ::new()
* Tweak message for TooSmallDirByteLimitl
* Remove too much of indirection
* Remove needless pub from ::channel()
* Clarify test comments
* Avoid needless event creation if tracer is disabled
* Write tests around file rotation and spill-over
* Remove unneeded PathBuf::clone()s...
* Introduce inner struct instead of tuple...
* Remove unused enum BankStatus...
* Avoid .unwrap() for the case of disabled tracer...
* Add run parent directory for accounts files
* fix test test_concurrent_snapshot_packaging
* review comments. renamed the path setup function
* Addressed most of the review comments
* remove explict type def for map result
* handle create_accounts_run_and_snapshot_dirs error with expect
* update with more review comments
* minor fixes from review comments
* simplify account_filename option assignment
* handle error from create_accounts_run_and_snapshot_dirs
* use then instead of then_some for lazy evaluation
* Refactor, move validator CLI related stuff to its own module(cli.rs)
The main function of the validator is getting big. Improve readability by moving away the CLI setup stuff to its own module.
* Restore help cli line
* Refactor, move test validator CLI config to cli.rs module
* Fix imports (cargo fmt)
* Move ConnectionCache back to solana-client, and duplicate ThinClient, TpuClient there
* Dedupe thin_client modules
* Dedupe tpu_client modules
* Move TpuClient to TpuConnectionCache
* Move ThinClient to TpuConnectionCache
* Move TpuConnection and quic/udp trait implementations back to solana-client
* Remove enum_dispatch from solana-tpu-client
* Move udp-client to its own crate
* Move quic-client to its own crate
The manual Blockstore compaction that was being initiated from
LedgerCleanupService has been disabled for quite some time in favor of
several optimizations.
Co-authored-by: Ryo Onodera <ryoqun@gmail.com>
#### Summary of Changes
This PR makes two FIFO-related validator arguments public:
--rocksdb_shred_compaction and --rocksdb_fifo_shred_storage_size.
#### Test Plan
* There're already ~26 validators running FIFO in mainnet-beta for more than 30 days,
no issues were reported so far.
* Ran a validator with FIFO for days and observed it's able to consistently catch up
and create new roots.
A fifo rocksdb instance must be opened with max size parameter on the
fifo columns. To support this, we previously plumbed a constant up to
callers that provided a default if unbounded growth desired.
This change attempts to be more rusty by exposing an option for this
value, and converting the option to a constant at the lowest level
possible.
### Problem
When FIFO compaction is used while --rocksdb_fifo_shred_storage_size
is unspecified, the FIFO shred storage size is set to a const default based
on the default `--limit-ledger-size`.
### Summary of the Change
When --rocksdb_fifo_shred_storage_size is unspecified, it is now
derived from `--limit-ledger-size` by reserving 1500 bytes for each
shred.
--tpu-enable-udp is introduced. And when this is on, the transaction receive and transaction forward is enabled using udp.
Except for a few tests which was hard-coded sending transactions using udp, most tests are being done with udp based tpu disabled.
* Add ability to use a non-default app profile id in bigtable requests
* Only run subcommand once when getting global configs
* Remove unneded scoping on option type
* Connection pool in connection cache and handle connection errors
1. The connection not has a pool of connections per address, configurable, default 4
2. The connections per address share a lazy initialized endpoint
3. Handle connection issues better, avoid race conditions
4. Various log improvement for help debug connection issues
* client: Remove static connection cache, plumb it instead
* Add TpuClient::new_with_connection_cache to not break downstream
* Refactor get_connection and RwLock into ConnectionCache
* Fix merge conflicts from new async TpuClient
* Remove `ConnectionCache::set_use_quic`
* Move DEFAULT_TPU_USE_QUIC to client, use ConnectionCache::default()
#### Problem
Currently, the creation of ShredStorageType::RocksFifo is hard coded in validator/src/main.rs.
But this common code will also need to be used in other places like ledger-tool.
#### Summary of Changes
This PR creates a helper functionShredStorageType::rocks_fifo that takes a total shred_storage_size
and equally allocates to data-shred and coding-shred storage.
Add in some CPU utilization metrics such as: number of vCPUs, clock frequency, average load across different time intervals, and number of total threads
#### Problem
When FIFO compaction is used, the size ratio between data shred and coding
shred is set to 1:1 based on the `--rocksdb_fifo_shred_storage_size` arg.
However, BlockstoreRocksFifoOptions::default() uses a slightly optimized
5:4 ratio instead, and the default() function is only used in benchmarks.
#### Summary of Changes
This PR makes both validator argument and BlockstoreRocksFifoOptions::default()
to use 1:1 ratio between data and coding shred size.
#### Problem
blockstore_db.rs has a mutual dependency between blockstore_metrics.rs.
#### Summary of Changes
This PR removes the mutual dependency by moving the option-related stuff
out from blockstore_db.rs to its new home --- blockstore_options.rs.
By doing this, we address the mutual dependency and also make the code cleaner.
#### Problem
LedgerColumnOptions contain two fields, perf_read_counter and perf_write_counter,
that are not really options but internal counters.
#### Summary of Changes
This PR introduces BlockstoreRocksDbPerfSamplingStatus, a struct that holds internal
status for RocksDB perf sampling and moves perf_read_counter and perf_write_counter
out from LedgerColumnOptions.
#### Summary of Changes
This PR replaces the use of thread_rng in RocksDB perf metric samples by
AtomicU32 with Ordering::Relaxed to improve the performance of determining
whether to sample the current RocksDB's read/write perf metric.
#### Problem
Currently, the number of RocksDB perf samples is controlled by an env arg
which is later handled using a lazy_static variable. However, there is a known
performance overhead of using lazy_static as mentioned in
https://github.com/solana-labs/solana/pull/6472.
#### Summary of Changes
Instead, this PR uses a hidden validator argument, --rocksdb-perf-sample-interval,
for controlling how often RocksDB read/write performance sample is collected.
Introduced flag --tpu-do-batch2.
Introduced flag to control the batch size-- by default 100
The default batch timeout is 200ms -- configurable. If either it time out or the batch size is filled, a new batch is sent
The batch honor the retry rate on the transaction already sent before.
Introduced two threads in STS: one for receiving new transactions and doing batch send and one for retrying old transactions and doing batch.6.
Fixes #
* transaction-status: Add return data to meta
* Add return data to simulation results
* Use pretty-hex for printing return data
* Update arg name, make TransactionRecord struct
* Rename TransactionRecord -> ExecutionRecord
This PR adds `--rocksdb-ledger-compression` as a hidden argument to the validator
for specifying the compression algorithm for TransactionStatus. Available compression
algorithms include `lz4`, `snappy`, `zlib`. The default value is `none`.
Experimental results show that with lz4 compression, we can achieve ~37% size-reduction
on the TransactionStatus column family, or ~8% size-reduction of the ledger store size.
Illia Bobyr
Ryo Onodera
Xiang Zhu
cavemanloverboy
Jeff Washington (jwash)
Tyera
Trent Nelson
behzad nouri
Lijun Wang
steviez
Kevin Ji
Brooks
Jeff Biseda
HaoranYi
eloylp
Dmitri Makarov
Yueh-Hsuan Chiang
Michael Vines
apfitzge
Jeff Washington (jwash)
Brennan Watt
Will Hickey
janlegner
Ahmad
sakridge
Nicholas Clarke
Ivan Mironov
Dan Coombs
Jon Cinque
DimAn
Pankaj Garg
Code Monkey
Giorgio Gambino
ryleung-solana