Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change scheduler to use Lua-based script #5044

Merged
merged 1 commit into from Nov 5, 2021

Conversation

engwan
Copy link
Contributor

@engwan engwan commented Nov 2, 2021

This allows us to pop jobs atomically so that we remove redundant ZREMs
from multiple processes trying to process the same job.

When we (GitLab) upgraded to Sidekiq 6, we saw that the number of ZREM operations on our Redis server increased significantly. This had a negative effect on Redis CPU.

We tracked this down to 9b75467. That change fetches 100 jobs, then tries to remove them one at a time. This change makes sense for setups with a few number of processes since the maximum number of overlapping processes would be low.

In our case, we added logging and found out that ~44% of ZREM operations were redundant. We reverted to the Sidekiq 5 scheduler, but later on improved it further by introducing this patch.

It removed the redundant ZREM operations which restored Redis CPU back to Sidekiq 5 levels and reduced enqueue latency at the same time.

In some cases, the current Sidekiq 6 scheduler may be faster. Like when there's a single process and there are a lot of scheduled jobs. But I think it is safe to assume that as instances process more scheduled jobs, there would also be more processes. So overall, I think this is still an improvement over the current scheduler.

lib/sidekiq/scheduled.rb Outdated Show resolved Hide resolved
@mperham
Copy link
Collaborator

mperham commented Nov 2, 2021

I believe Gitlab has a Sidekiq Pro license. Have you tried the reliable scheduler in Pro? It uses Lua and does the job enqueue in Lua too.

@engwan
Copy link
Contributor Author

engwan commented Nov 2, 2021

I believe Gitlab has a Sidekiq Pro license. Have you tried the reliable scheduler in Pro? It uses Lua and does the job enqueue in Lua too.

We can't use it due to licensing issues. And even if we could, we need some client middleware to run on the actual enqueue.

This allows us to pop jobs atomically so that we remove redundant ZREMs
from multiple processes trying to process the same job.
@mperham
Copy link
Collaborator

mperham commented Nov 5, 2021

I'm fine with the code as is: compact and self-contained. Lua was originally Pro-only as it required newer versions of Redis but Lua has been in Redis for many years now; I think Sidekiq can start to use it where useful.

Once we have multiple Lua call points, we'll extract a utility method at that point in time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants