zaynt4606 commented on code in PR #3854: URL: https://github.com/apache/celeborn/pull/3854#discussion_r4434810910
########## charts/celeborn/README.md: ########## @@ -391,6 +391,98 @@ KEDA has settled, or pass `--set worker.replicas=<current>` when upgrading. For a continuous-delivery tool, tell it to ignore the field - in Argo CD, `ignoreDifferences` on `/spec/replicas` for the worker statefulsets, with `RespectIgnoreDifferences=true`. +## Health probes + +Both the master and the worker serve `GET /healthz` on their HTTP port, returning `200` when the +process can serve and `503` when it cannot. The chart wires probes to that endpoint through +`master.startupProbe`, `master.livenessProbe`, `master.readinessProbe` and the matching `worker.*` +values. Each value is a complete Kubernetes probe object, passed through as it is. + +All probes are off by default. `/healthz` was added after Celeborn 0.7.0, the release the chart +deploys by default, so probes pointed at it would never pass against that image: workers would +stay NotReady and masters would be restarted. Enable them only with an image that serves +`/healthz`. + +With such an image, a working setup looks like this: + +```yaml +master: + podManagementPolicy: Parallel # immutable - see "Readiness and pod management policy" + startupProbe: + httpGet: + path: /healthz + port: metrics + periodSeconds: 10 + timeoutSeconds: 5 + failureThreshold: 30 + livenessProbe: + httpGet: + path: /healthz + port: metrics + periodSeconds: 10 + timeoutSeconds: 5 + failureThreshold: 6 + +worker: + readinessProbe: + httpGet: + path: /healthz + port: metrics + initialDelaySeconds: 10 + periodSeconds: 10 + timeoutSeconds: 5 + failureThreshold: 3 +``` + +The reasoning behind that shape: + +- The master check is shallow - it reports healthy once the master HTTP service is up - so it is + safe to restart on. Pair the liveness probe with a startup probe, which holds it off while a + master waits for the DNS records of its peers; that can take a while if one replica is slow to + be scheduled. The startup probe allows `failureThreshold` x `periodSeconds` - 300s above. +- The worker readiness check is the reason the probes exist: it reports healthy only after the + worker has finished initializing and registered with the master, which stops a rolling update + from moving on to the next worker while the previous one still rejects traffic. +- Leave the worker liveness probe off. Worker `/healthz` fails while the worker is not registered + with the master, so a liveness probe on it would restart every worker in the cluster during a + master outage or failover. If one is wanted, use a `tcpSocket` check on the RPC port. + +### Parked workers and rolling updates + +A worker parked with `DecommissionThenIdle` reports state `Idle`, which `/healthz` treats as not +serving, because the master does not schedule to an `Idle` worker either. Its pod therefore stays +NotReady and a rolling update will not advance past that ordinal. Recommission parked workers +before upgrading, or set `worker.readinessProbe: null` for that rollout. Review Comment: Non-blocking documentation suggestion: With the default `OrderedReady` policy, an existing Idle/NotReady worker blocks the StatefulSet controller before it replaces any Pods. Setting `worker.readinessProbe: null` alone therefore leaves the old probe on that Pod and does not unblock the rollout. Could we keep recommissioning as the recommended recovery, and clarify that the alternative requires applying the probe change and then manually recreating the blocked Pod(s)? See the [Kubernetes recovery guidance](https://kubernetes.io/docs/concepts/workloads/controllers/statefulset/#forced-rollback). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
