ConnectionPool: idle_timeout of 0 causes 100% and busy looping

[tisba]

I recently noticed that our sidekiq is running at 100% CPU (see sidekiq/sidekiq#3918). We are using sshkit in our sidekiq workers.

We were using:

SSHKit::Backend::Netssh.pool.idle_timeout = 0

Which causes a busy loop in ConnectionPool#run_eviction_loop as [idle_timeout, 5].min will be 0:

sshkit/lib/sshkit/backends/connection_pool.rb

Lines 131 to 139 in 70d01df

	def run_eviction_loop
	loop do
	process_deferred_close
	# Periodically sweep all Caches to evict stale connections
	sleep([idle_timeout, 5].min)
	caches.values.each(&:evict)
	end
	end

Also, according to ConnectionPools documentation, idle_timeout can also be false or nil which will actually cause an exception ([nil, 5].min will raise).

I'm happy to propose a fix, but I'd like to have guidance how. My proposal would be to impose a lower limit on run_eviction_loop loop frequency like at most once every 5 seconds. On the other hand I don't understand why the eviction loop is required at all if connection caching is disabled (idle_timeout = 0, ConnectionPool#cache_enabled? -> false).

As far as I can tell the code was heavily changed/rewritten with #328 by @mattbrictson. Maybe he can provide some guidance?

In any case, we can work with increasing the idle_timeout to >0 for now. I still like to have this problem fixed though.

[mikegee]

No help for the underlying issue, but I don't think you would have seen this problem if capistrano weren't loaded in the Sidekiq process. The docs say to use require: false in the Gemfile. https://github.com/capistrano/capistrano#install-the-capistrano-gem

[tisba]

To clarify things: We do not load capistrano in sidekiq, only sshkit. We use sshkit directly to manage servers.

[tisba]

-bump-

I'm happy to provide a patch to the problem (outlined in the description above). Is this a viable way to solve this issue?

[leehambley]

Hi @tisba if you can provide a patch it would be most welcome. We are still actively maintaining this library when community patches are provided, but both @mattbrictson and myself are not spending a lot of time on bug fixes ourselves.

Please do submit a fix, if you can, and the doc issue:

Also, according to ConnectionPools documentation, idle_timeout can also be false or nil which will actually cause an exception ([nil, 5].min will raise).

would be well worth addressing as part of your submission. Thanks indeed.

ps. might I ask what's you need for disabling connection pooling all together? (have you deferred control to a controlmaster at the system level?

[tisba]

We are using sshkit via capistrano, but also stand-alone (mostly) inside sidekiq jobs. We talk to many hosts but most operations are one-off anyway, so there is no real need to pool connections. I don't recall exactly why we tried to disable connection pooling though; not sure if we had "a real issue", not only "we don't need it".

Anyway it caused a lot of searching to find out why we kept one core at 100% utilisation and it should be rather simple to fix. I'll try to come up with a patch and we'll see :)

💕 for maintaining sshkit. This gem was really helpful to us!