SteNicholas commented on code in PR #3832:
URL: https://github.com/apache/celeborn/pull/3832#discussion_r3960085019
##########
worker/src/main/scala/org/apache/celeborn/service/deploy/worker/Worker.scala:
##########
@@ -935,6 +946,19 @@ private[celeborn] class Worker(
sb.toString()
}
+ override def healthCheck(): HandleResponse = {
+ val state = workerStatusManager.currentWorkerStatus.getState
+ if (!registered.get() || !registeredInMasterView.get()) {
Review Comment:
[P2] Wait for worker initialization before reporting readiness
`initialize()` starts the HTTP server and calls `registerWithMaster()`
before initializing the push/fetch handlers and registering the controller
endpoint. Successful registration sets both flags checked here to `true`, and
the state is already `Normal`, so a probe in that interval receives 200 even
though the handlers still reject traffic as "Worker Not Registered!". This can
mark the pod Ready and let a rolling update advance before the worker can serve.
I reproduced this by observing the real `/healthz` response at the
`cleaner.submit(...)` call in `initialize()`, without changing either
registration flag: HTTP 200 while both `pushDataHandler.checkRegistered()` and
`fetchHandler.checkRegistered` were false.
Could we also gate readiness on a safely published initialization-complete
flag, set after handler initialization and `rpcEnv.setupEndpoint(...)`, and add
a startup test covering this interval?
--
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]
