arturictus / sidekiq_alive

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

Question: What difference makes "sidekiq quite" as preStop hook? #53

Closed evgeniy-b closed 3 years ago

evgeniy-b commented 3 years ago

The README recommends sending TSTP signal to Sidekiq as Kubernetes preStop hook. Looking at k8s documentation, this hook executed before TERM signal is sent. So what difference does actually make sending TSTP + TERM comparing to only TERM? It feels like TERM should be good enough on its own, unless I'm missing something here.

PS. Thanks for the great gem! It is good to have it.

arturictus commented 3 years ago

https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle/#pod-termination

The signal TERM will kill the process and all the jobs running without waiting for them to finish ending up in an inconsistent state.

The TSTP will stop enqueuing jobs and the terminationGracePeriodSeconds to finish the already enqueued.

https://github.com/mperham/sidekiq/wiki/Signals

I hope that clarifies

evgeniy-b commented 3 years ago

The signal TERM will kill the process and all the jobs running without waiting for them to finish ending up in an inconsistent state.

This is not how it is mentioned in the Sidekiq docs:

TERM signals that Sidekiq should shut down within the -t timeout option given at start-up. It will stop accepting new work, but continue working on current messages (as with TSTP). https://github.com/mperham/sidekiq/wiki/Signals#term

Side note: using the same check for liveness and readiness probes could be considered sub-optimal because they are expected to behave differently. When the process is terminating it should not accept new workload (readiness should start failing), but should continue reporting its liveness (it is still alive despite being terminated). Because Sidekiq doesn't accept connections directly, readiness probe makes no difference (that I'm aware of) so I guess it could be skipped. And as I see it, failing liveness probe also changes nothing on shutdown because the process is already terminating.

arturictus commented 3 years ago

This is not how it is mentioned in the Sidekiq docs:

Best practice is to send TSTP at the start of a deploy and TERM at the end of a deploy.

https://github.com/mperham/sidekiq/wiki/Signals#tstp

evgeniy-b commented 3 years ago

Best practice is to send TSTP at the start of a deploy and TERM at the end of a deploy.

It is a very good practice indeed. But I do not see how it applies to k8s pod lifecycle where preStop hook is executed right before the TERM signal is sent.

arturictus commented 3 years ago

Note: If the preStop hook needs longer to complete than the default grace period allows, you must modify terminationGracePeriodSeconds to suit this.

https://kubernetes.io/docs/concepts/workloads/pods/pod-lifecycle/#pod-termination

evgeniy-b commented 3 years ago

preStop hook in this case does not take long to execute because it only sends TSTP signal and that is quick. Right after that k8s sends TERM to Sidekiq. So it doesn't really makes a difference to send TSTP a few milliseconds before TERM.