arturictus / sidekiq_alive

Liveness probe for Sidekiq in Kubernetes deployments
MIT License
188 stars 57 forks source link

Replace scan and simple set methods with a Set #92

Closed lucas-aragno closed 1 year ago

lucas-aragno commented 1 year ago

Hi @arturictus !

Me again! We have been digging at this issue and although the method causing the issue itself:

 def hostname_registered?(hostname) 
   SidekiqAlive.registered_instances.any? do |ri| 
     /#{hostname}/ =~ ri 
   end 
 end 

was removed here we were still facing a similar issue. After some digging it's still the same problem we found here.

We still need to use the registered_instances method and that ultimately uses a SCAN that's O(N) and we have so many jobs that our sidekiq could have potentially millions of keys at the time so SCAN is not an option.

And since the goal of that method is just to return the registered keys I don't think SCAN is necessary. In this PR I'm just creating a set of keys (registered instances) and the score of each key is their TTL. This way we can retrieve them without having to go through everything on redis.

What do you think?

arturictus commented 1 year ago

Hi @lucas-aragno, Thanks for this PR, great work!

arturictus commented 1 year ago

Did some changes here: #93