Reaper manager registration is subject to race conditions
leandrogoe opened this issue · 1 comments
leandrogoe commented
Describe the bug
The reaper manager registration is subject to race conditions, which means multiple reapers can potentially start at the same time causing extra load in redis. Bug found on version v7.1.30.
Expected behavior
There should be only one reaper manager, even when running multiple Sidekiq processes.
Current behavior
The bug can be easily replicated using the following snippet:
> 1000.times { |index| Thread.new { sleep(rand()); SidekiqUniqueJobs::Orphans::Manager.start } }
2023-08-04T12:22:56.128Z 86290 TID-oxfywtnt6 uniquejobs=orphan-reaper INFO: Starting Reaper
2023-08-04T12:22:56.129Z 86290 TID-oxfywuq7a uniquejobs=orphan-reaper INFO: Starting Reaper
2023-08-04T12:22:56.129Z 86290 TID-oxfywt5fm uniquejobs=orphan-reaper INFO: Starting Reaper
2023-08-04T12:22:56.129Z 86290 TID-oxfywqgxi uniquejobs=orphan-reaper INFO: Starting Reaper
2023-08-04T12:22:56.129Z 86290 TID-oxfywulj6 uniquejobs=orphan-reaper INFO: Starting Reaper
2023-08-04T12:22:56.129Z 86290 TID-oxfywt64y uniquejobs=orphan-reaper INFO: Starting Reaper
2023-08-04T12:22:57.141Z 86290 TID-oxfwnhp82 uniquejobs=orphan-reaper INFO: Nothing to delete; exiting.
2023-08-04T12:22:57.144Z 86290 TID-oxfwnhp82 uniquejobs=orphan-reaper INFO: Nothing to delete; exiting.
2023-08-04T12:22:56.612Z 86290 TID-oxfywtjim uniquejobs=orphan-reaper INFO: Starting Reaper
2023-08-04T12:22:56.613Z 86290 TID-oxfywqen6 uniquejobs=orphan-reaper INFO: Starting Reaper
2023-08-04T12:22:56.613Z 86290 TID-oxfywt5sq uniquejobs=orphan-reaper INFO: Starting Reaper
2023-08-04T12:22:56.613Z 86290 TID-oxfywtoq6 uniquejobs=orphan-reaper INFO: Starting Reaper
2023-08-04T12:22:56.613Z 86290 TID-oxfywqhpu uniquejobs=orphan-reaper INFO: Starting Reaper
2023-08-04T12:22:56.613Z 86290 TID-oxfywt28a uniquejobs=orphan-reaper INFO: Starting Reaper
1000
As you can see multiple managers were registered, but there should only be one.
On our deployment, we have frequent Sidekiq restarts, which exacerbates this problem, and we ended up running over 20+ reaper managers at the same time, what cripples Redis performance.