Use shared SshClient to prevent sshd-SshClient thread leak, to prevent excessive number of threads created#2008
Conversation
Signed-off-by: Olivier Lamy <olamy@apache.org>
|
this PR is a bit more aggressive especially applying some performance changes (so there is a conservative option to at least fix the thread leak bug #2007) |
|
Are you running this in your own environment? Has it solve your problems |
|
@nevingeorgesunny you did some changes in this plugin not long ago; perhaps you want to review this and/or #2007? @olamy there is no information on testing here. Was it tested at scale somehow, and confirmed to fix the reported problem? |
|
Well the other #2007 is just fixing the symptom but not the root (performance bad pattern) cause. While this PR will really fix the performance root cause and obviously the associated leak fixed by #2007 because not re creating client for each connection tentative. Some figuresA default NIO2 client creates three pools on
So on 8 cores that's 19 threads. For a single client and so per slave created by this plugin. One client for many serversThe The pools above are paid once per client, not once per connection. Sessions are cheap: each one registers its socket with the existing The CPU side follows the same bad pattern. Each client runs his own Maybe the reason to spin up a client per server was per-server behavior: different host key, different identity, different cipher preferences. You don't need a new client for any of that. See the code in the PR which is customizing this via session context. Re (nothing urgent though) Why using a nio library because the per-connection SshClient is a worst-of-both-worlds pattern (nio vs bio). |
|
@olamy all sensible in principle, it is just not apparent from the PR description whether the purported problem was reproduced in a test controller, and this PR confirmed to solve it. |
mirrors the pattern used in the cloudbees-ssh-slaves-plugin.
move host key verification from per-client setServerKeyVerifier() to per-session DelegatingServerKeyVerifier with session metadata.
registered via a Channel.Listener callback that was never reached on exception paths.
Signed-off-by: Olivier Lamy olamy@apache.org
Testing done
Submitter checklist