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:
@strelok89, `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]
