strelok89 commented on code in PR #3832:
URL: https://github.com/apache/celeborn/pull/3832#discussion_r3979875815
##########
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:
Good catch, thanks — you're right, and fixed in f753d0983.
`Worker` now has an `initialized` AtomicBoolean, set immediately after the
handler initialization and `rpcEnv.setupEndpoint(RpcNameConstants.WORKER_EP,
controller)`, and `healthCheck()` checks it *first*, before the registration
checks, returning `worker is still initializing`. The ordering matters: in the
interval you reproduced, `registered` and `registeredInMasterView` are already
true and the state is `Normal`, so the init check has to come first for the
reason to be accurate.
Test-wise, `ApiWorkerResourceSuite` gains a case that asserts the worker is
registered, master-visible and `Normal`, then clears `initialized` and asserts
`/healthz` returns 503 with that reason. It reproduces the window by clearing
the flag rather than racing a live `initialize()`, which would be flaky in CI.
I also made `MiniClusterFeature`'s startup wait block on `worker.initialized`
in addition to `worker.registered`, otherwise the existing suites could
intermittently hit the new 503 window.
Master is unchanged — its `healthCheck()` is still the default `(true, "")`
from `HttpService`.
--
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]
