Thread manager instantiation in the validator #4603
Conversation
alexpyattaev
force-pushed
the
tm_validator
branch
3 times, most recently
from
7dc07f7 to
ed1cb50
Compare
alexpyattaev
force-pushed
the
tm_validator
branch
2 times, most recently
from
4722843 to
faace10
Compare
alexpyattaev
force-pushed
the
tm_validator
branch
from
faace10 to
53f97fd
Compare
| max_threads = 8 | ||
|
|
||
| [rayon_configs.solRetransmit] # runs turbine protocol retransmit logic | ||
| max_threads = 4 |
There was a problem hiding this comment.
I thought current Agave set this to 8
There was a problem hiding this comment.
Yes, these are all from optimized config. I have pulled them all from the PR for now and will initially use current values (even though these are "better").
| worker_threads = 4 | ||
|
|
||
| [rayon_configs.solWinInsert] | ||
| worker_threads = 2 |
There was a problem hiding this comment.
I think this one was set to 8 as well
|
|
||
| [rayon_configs.solAccounts] # AccountsDb foreground threads | ||
| worker_threads = 6 | ||
|
|
| [tokio_configs.solQuicTpuFwd] | ||
| worker_threads = 4 | ||
|
|
||
| [tokio_configs.solRepairQuic] # this is not used on MNB |
There was a problem hiding this comment.
these comments are getting orphaned for sure if/when we turn on QUIC for these protocols 😆
There was a problem hiding this comment.
Yes:) But I will re add stuff to the file as I run integration and make sure they are all correct.
| let active_cnt = runtime.running_count.load(Ordering::SeqCst); | ||
| match active_cnt { | ||
| 0 => debug!("Shutting down Native thread pool {name}"), | ||
| _ => warn!("Native pool {name} has active threads during shutdown!"), |
There was a problem hiding this comment.
might be useful for debug to include number of threads still active
| impl ThreadManagerConfig { | ||
| /// Returns a valid "built-in" configuration for Agave that is safe to use in most cases. | ||
| /// Most operators will likely use this. | ||
| pub fn default_for_agave() -> Self { |
There was a problem hiding this comment.
could we just make this the default?
There was a problem hiding this comment.
No as "default" config should be effectively empty (as it is used by Deserialize). Since this functionessentially parses Toml it would create an infinite loop.
| #[derive(Clone, Debug, Serialize, Deserialize)] | ||
| #[serde(default)] | ||
| pub struct ThreadManagerConfig { |
There was a problem hiding this comment.
would add just a quick comment here about what ThreadManager is, what it does, and why it's needed.
There was a problem hiding this comment.
That is already present in https://github.com/alexpyattaev/agave/blob/tm_validator/thread-manager/README.md I did not want to also pollute the code with detailed info that would duplicate the readme. As for the struct you're highlighting I figured ThreadManagerConfig is pretty self-explanatory.
Problem
Starting the integration of thread manager into agave. This PR is just to make sure thread manager can be instantiated at startup. Separate PRs will be made to add CLI arguments, and to actually patch it in where it is needed.
Summary of Changes