arturictus / sidekiq_alive

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

why use 'redis keys command' ? #46

Closed theodore-park closed 3 years ago

theodore-park commented 3 years ago

image

I am trying to use sidekiq_alive to use the kubernetes probe, but I have a problem.

when starting sidekiq_alive, it executes the above code,

The redis keys command is dangerous, so it can't be executed by blocking it. Is there a way to replace it?

please help me. thx

arturictus commented 3 years ago

Hi @theodore-park, Can you elaborate on why do you think is dangerous? Do you mean because is blocking the thread on startup until the query returns? Right now this method is used at the end of startup and only for logging, then it gets used in the worker. https://github.com/arturictus/sidekiq_alive/blob/d30626ea53a15055f92e8257cabdae51eebc8aa1/lib/sidekiq_alive.rb#L23-L23

So it will be easy to delay this call just by removing it from the successful_startup_text banner: https://github.com/arturictus/sidekiq_alive/blob/d30626ea53a15055f92e8257cabdae51eebc8aa1/lib/sidekiq_alive.rb#L143-L143

Thanks

esalter commented 3 years ago

We ran into this too.

In our case, we have several million keys in our redis db, and even though this command is scoped, it still causes very high latency. Even though it's "only" a couple seconds, it seems to block all other commands until it completes, which means everything else backs up.

We fixed it by monkey-patching the banner to remove this call.

theodore-park commented 3 years ago

@arturictus In my case, I had a problem with someone else accidentally entering the'keys' command at the company. So, the company blocked the'keys' command and prevented anyone from entering even a mistake. Thanks for the answer, I solved it with Monkey Patching!

arturictus commented 3 years ago

Hi, This has been solved in the last release with this PR #52, hope it solves the problems with keys