[CELEBORN-2447] Add a /health endpoint to master and worker

[strelok89]

What changes were proposed in this pull request?

Add a GET /health endpoint served by both master and worker, whose HTTP status code reflects whether the service is able to serve: 200 when healthy and 503 when not.

• HttpService#healthCheck defaults to a shallow check that reports healthy once the HTTP service is available. This is what the master uses.
• Worker overrides it to report healthy only when the worker is registered with the master and its state is Normal.
• /health is added to the default HTTP authentication bypass paths.

The endpoint is mounted at the root rather than under /api/v1 so that a probe is not coupled to the API version, consistent with the other operational paths (/ping, /metrics/prometheus). The checked-in OpenAPI spec covers only /api/v1, so the generated client is unaffected.

Example:

$ curl -s -o /dev/null -w '%{http_code}\n' http://worker:9096/health
200

$ curl -s http://worker:9096/health
{"service":"worker","healthy":false,"reason":"worker is not registered with master"}

Why are the changes needed?

Celeborn currently exposes no endpoint whose status code reflects whether the process can serve, so a meaningful Kubernetes probe cannot be written. /ping always returns 200 regardless of state, and /api/v1/workers returns 200 even when isRegistered is false, so operators have to fall back to an exec probe that shells out and greps the JSON body.

The practical cost is rolling updates. Without a readiness signal, a StatefulSet marks a worker pod Ready as soon as its container process starts and immediately proceeds to the next ordinal, before the restarted worker has re-registered with the master. On a large cluster this can remove a meaningful fraction of workers from service before any of them rejoin.

Two design points worth calling out for review:

The worker check mirrors the master's own definition. AbstractMetaManager#isWorkerAvailable accepts only Normal, so a worker that is idle, decommissioning or exiting is already excluded from availableWorkers and never selected in offerSlots. Reporting such a worker as healthy would put /health at odds with the master. Whether Idle specifically should fail readiness is discussed on CELEBORN-2447 — it is reachable only through an explicit DecommissionThenIdle event, so the alternative (200 for Idle, 503 only for the decommission/exit states) is a reasonable variation if reviewers prefer it.

The master check is deliberately shallow. Masters are fronted by a headless Service that does not set publishNotReadyAddresses. A quorum-aware or leader-aware check would report every master as unhealthy during a cold start, withholding their DNS records and preventing them from discovering each other to form quorum. A follower is also a healthy replica. Quorum and leadership remain observable through /api/v1/ratis and /api/v1/masters.

/health bypasses authentication by default because a kubelet cannot present credentials, and celeborn.http.auth.bypass.api.paths defaults to empty (CELEBORN-2278).

Wiring probes into the Helm chart, which currently defines no livenessProbe, readinessProbe or startupProbe for either role, is intended as a follow-up so the two changes can be reviewed independently.

Does this PR resolve a correctness bug?

• Yes

Does this PR introduce any user-facing change?

• Yes

A new /health endpoint on master and worker, documented in docs/restapi.md.

How was this patch tested?

New tests:

• ApiBaseResourceSuite: /health returns 200 and reports the correct service name, for both master and worker.
• ApiBaseResourceAuthenticationSuite: /health is reachable without credentials.
• ApiWorkerResourceSuite: /health returns 503 when the worker is not registered.

Verified locally on JDK 11: celeborn-service/Test/compile and celeborn-worker/Test/compile pass, ApiMasterResourceSuite (19/19) and ApiMasterResourceAuthenticationSuite (9/9) pass, and spotless:check passes for the service and worker modules.

The worker-side suites were not run locally: they were developed on Windows, where any MiniClusterFeature test fails during setup because CelebornConf#workerBaseDirs splits each storage dir on : and rejects C:\... paths. That is pre-existing and unrelated to this change, and those suites are covered by CI.

[Copilot]

Pull request overview

This pull request adds a root-level GET /health endpoint on both Celeborn master and worker HTTP services, returning 200 when the process is considered able to serve and 503 otherwise, primarily to support Kubernetes readiness probing.

Changes:

• Introduces a new /health JAX-RS resource returning a structured JSON response and appropriate HTTP status codes.
• Adds a default HttpService#healthCheck implementation and a worker override that gates health on registration and Normal worker state.
• Extends HTTP auth bypass defaults to allow unauthenticated access to /health, and adds/extends tests plus REST API documentation.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
worker/src/test/scala/org/apache/celeborn/service/deploy/worker/http/api/ApiWorkerResourceSuite.scala	Adds a worker-specific test asserting /health returns 503 when the worker is not registered.
worker/src/main/scala/org/apache/celeborn/service/deploy/worker/Worker.scala	Implements worker-specific healthCheck() logic based on registration and worker state.
service/src/test/scala/org/apache/celeborn/server/common/http/ApiBaseResourceSuite.scala	Adds a base test validating /health returns 200 and includes the service name when healthy.
service/src/test/scala/org/apache/celeborn/server/common/http/ApiBaseResourceAuthenticationSuite.scala	Adds a test verifying /health is reachable without credentials.
service/src/main/scala/org/apache/celeborn/server/common/HttpService.scala	Adds a default healthCheck() hook for services to implement health semantics.
service/src/main/scala/org/apache/celeborn/server/common/http/authentication/AuthenticationFilter.scala	Adds /health to the default authentication bypass path set.
service/src/main/scala/org/apache/celeborn/server/common/http/api/HealthResource.scala	Adds the new /health endpoint and its response schema.
docs/restapi.md	Documents the new /health endpoint behavior and intent for readiness probing.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[zaynt4606]

Thanks @strelok89 for working on this. I found two cases where the worker readiness endpoint can report a false positive.

[zaynt4606]

Could we clear registered as soon as a heartbeat response explicitly reports registered == false, before calling registerWithMaster()? This flag is only set to true after a successful registration. As written, /health can continue returning 200 while re-registration retries, which can last up to celeborn.worker.register.timeout (180s by default). A StatefulSet can therefore proceed to the next worker even though this worker is absent from the Master registration state. Please also add a test that exercises this production path instead of only setting the flag directly.

[strelok89]

Fixed, though not by clearing registered — that flag also gates RPC serving. FetchHandler#checkRegistered and PushDataHandler#checkRegistered read it, and TransportRequestHandler#checkRegistered (TransportRequestHandler.java:169) fails every push RPC and chunk fetch with "Worker Not Registered!" while it is false. Clearing it would reject live traffic for the whole re-registration window, even though the worker still holds the data that clients have locations for.

Instead I added a separate registeredInMasterView flag, cleared as soon as a heartbeat response reports registered == false and restored on successful registration. Only /health reads it; registered keeps its current semantics.

To make the production path testable I extracted Worker#handleHeartbeatResponse, and the new test in WorkerSuite records the flag at the moment the re-registration RPC is issued, so it asserts the ordering rather than just the end state.

If the project would rather fence traffic when the master loses a worker, clearing registered is a defensible alternative — happy to switch, but that seems like a separate discussion from the health endpoint.

[zaynt4606]

currentWorkerStatus is a plain mutable var. It is replaced inside synchronized transitionState, but this HTTP read does not acquire the same monitor and the field is not volatile or atomic. There is no happens-before relationship, so the probe can observe stale Normal after the worker enters Idle, decommission, or exit and return 200 instead of the documented 503. Could we safely publish/read this state (for example, with @volatile, an atomic reference, or a synchronized accessor) and add a non-Normal state test?

[strelok89]

Good catch — currentWorkerStatus is now @volatile, and I added a test that transitions the worker to InDecommission and asserts /health returns 503.

[strelok89]

@zaynt4606 fixed the things waiting for you check

[pan3793]

maybe /healthz?

https://stackoverflow.com/questions/43380939/where-does-the-convention-of-using-healthz-for-application-health-checks-come-f

[strelok89]

Renamed to /healthz. Updated the JAX-RS path, the auth bypass set, the docs and the three test suites — behaviour is unchanged.