Add request pool to improve delivery performance

[Gargron]

Fix #7909

Use keep-alive connections to each site, in connection pools, to reduce DNS/TCP/SSL handshake overhead. Keep connections for 300 idle seconds, then clean them up.

Benchmark: 400 ActivityPub::DeliveryWorker jobs addressed to the same inbox on 5 Sidekiq threads

Before	After
18 s	8 s

[zunda]

Such a cool new feature! I've cherry-picked feature-request-pool at ac6e32a to my single user instance and saw increase in peak delivery speed from about 230 toots/min to 290 toots/min on a Sidekiq with 3 threads.

When the app is starting, I noticed a single occurrence of NoMethodError: undefined method 'call' for nil:NilClass at app/lib/request_pool.rb:26:in 'try_create' (full backtrace at https://gist.github.com/zunda/19162442ae84256633becb1f273795ee ). Might this be a code to leap unused pool?

I'll keep the server running the code for a while and see what happens.

[ClearlyClaire]

From https://github.com/mperham/connection_pool

WARNING: Don't ever use Timeout.timeout in your Ruby code or you will see occasional silent corruption and mysterious errors

Both http.rb and our own Request class do make use of Timeout.timeout

[Gargron]

When the app is starting, I noticed a single occurrence of NoMethodError: undefined method 'call' for nil:NilClass at app/lib/request_pool.rb:26:in 'try_create'

@zunda I have noticed this too. It seems to disappear when I remove the monkey-patch of the try_create method. The only difference between original try_create and monkey-patch is unless @created == @max.

The intent of the monkey-patch is to remove fixed size of the pool. If more threads try to access the same URL than the pool size, they will wait up to 5 seconds (and then fail), which would be a performance regression. While without fixed size, even if all those threads won't benefit from keep-alive on first call, performance will improve over repeated calls, and the reaper will remove unused connections over time.

There should be no way that @create_block is nil in try_create. It is set once, during initialize, nothing changes it. I assume this is some kind of thread-safety issue with monkey-patching.

WARNING: Don't ever use Timeout.timeout in your Ruby code or you will see occasional silent corruption and mysterious errors

@ThibG This reminds me of the warning not to use rack-timeout. I am not sure if this is truly a deal-breaker, or the timeouts are encapsulated enough not to break connection pooling.

[Gargron]

I have resolved the try_create issue. I shouldn't have been monkey-patching these classes, as other code (Sidekiq itself) uses them. I just created my own classes inheriting from them.

[zunda]

Coool! I've merged 3f8df86 and confirmed that the error is gone.

[Gargron]

Before/after using request pools (MAX_REQUEST_POOL_SIZE=60) when push queue is backlogged

[Gargron]

I could use some help debugging "too many open files" errors that seem to appear in this PR after some use

[Gargron]

Testing in production reveals a lot of timeout errors due to full pools, so it's not ready yet.

[Gargron]

I rewrote the code so that if the maximum pool size is reached, and we try to get a connection for a host that we don't have idle connections ready for, it removes one of the idle connections for a different host and creates a new one in its place.