gauravkm commented on code in PR #3854:
URL: https://github.com/apache/celeborn/pull/3854#discussion_r4056239424
##########
charts/celeborn/values.yaml:
##########
@@ -263,6 +304,33 @@ worker:
- name: disk4
mountPath: /mnt/disk4
+ # -- Startup probe for Celeborn worker containers. Disabled by default,
since no liveness probe
+ # is configured by default either.
+ startupProbe:
+ # httpGet:
+ # path: /healthz
+ # port: metrics
+
+ # -- Liveness probe for Celeborn worker containers. Disabled by default:
worker `/healthz`
+ # fails while the worker is not registered with the master, so a liveness
probe on it would
+ # restart every worker during a master outage. See README.md.
+ livenessProbe:
+ # httpGet:
+ # path: /healthz
+ # port: metrics
+
+ # -- Readiness probe for Celeborn worker containers. `/healthz` reports
healthy only once the
+ # worker is initialized and registered with the master, which keeps a
rolling update from
+ # advancing to the next worker too early. Set to `null` to disable.
+ readinessProbe:
+ httpGet:
+ path: /healthz
Review Comment:
**[P1] Align the probe defaults with the default image**
The chart still defaults to `docker.io/apache/celeborn:0.7.0` through
`Chart.yaml`'s `appVersion`, but the [0.7.0 HTTP
resources](https://github.com/apache/celeborn/tree/v0.7.0/service/src/main/scala/org/apache/celeborn/server/common/http/api)
do not implement `/healthz`. These default probes therefore cannot succeed
against that release: workers remain NotReady and the master startup probe
causes restarts, even once the peer-ordering issue is resolved. Please select a
compatible default image or keep the probes opt-in for unsupported releases,
and cover compatibility with the actual default image.
--
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]
