Use dedicated capsule for processing sidekiq-alive queue #96
Conversation
| @@ -28,7 +28,7 @@ def delete(key) | |||
| end | |||
|
|
|||
| def ttl(...) | |||
| Sidekiq.redis { |redis| redis.ttl(...) } | |||
| redis { |r| r.ttl(...) } | |||
There was a problem hiding this comment.
Updated both implementations to work same way
|
|
||
| def redis(&block) | ||
| # Default to Sidekiq.redis if capsule is not configured yet but redis adapter is accessed | ||
| (@capsule || Sidekiq).redis(&block) |
There was a problem hiding this comment.
Sidekiq documentation states that capsule specific redis pool should be used. We need this fallback because method returning redis adapter is a class method and can be accessed before actual configuration has been performed (it is used in tests like that currently).
|
|
||
| before do | ||
| allow(SidekiqAlive).to(receive(:fork) { 1 }) | ||
| allow(sq_config).to(receive(:on).with(:startup) { |&arg| arg.call }) | ||
|
|
||
| SidekiqAlive.instance_variable_set(:@redis, nil) |
There was a problem hiding this comment.
redis adapter is memoized on the class level and because it is created in tests, we reset it just to make sure that everything still works correctly for scenario where full configuration cycle was performed and adapter should return capsule specific redis pool
|
@arturictus Could You please review this one and let me know what You think? |
|
Did some testing in my app, and it seems we do need more than 1 connection in a pool. This is because we can end up calling I ended up getting |
|
This looks great!, thanks @andrcuns for this improvement. |
This introduces a separate capsule for handling sidekiq alive queue and makes it sequential because we don't really need to process multiple alive jobs in parallel.