yuqi1129 commented on code in PR #13067:
URL: https://github.com/apache/gravitino/pull/13067#discussion_r3978516428
##########
docs/gravitino-server-config.md:
##########
@@ -208,19 +208,20 @@ Gravitino exposes three health endpoints following
of them are exempt from authentication, so Kubernetes probes, load balancers,
and traffic managers
reach them without credentials.
-| Endpoint | Root Alias | Description
| HTTP Status |
-|-------------------------|---------------------|---------------------------------------------------------------------------------------------------------------------------------------------|-------------|
-| `GET /api/health/live` | `GET /health/live` | Liveness. Returns 200 as
long as an HTTP server thread can respond. Use it to decide whether to restart
a pod. | 200 |
-| `GET /api/health/ready` | `GET /health/ready` | Readiness. Returns 200 when
the entity store answers within the probe timeout, 503 when it is unavailable
or slow. Use it to route traffic. | 200 or 503 |
-| `GET /api/health` | `GET /health` | Aggregate. Returns 200 when
both of the above pass. Also aliased as `GET /health.html`.
| 200 or 503 |
+| Endpoint | Root Alias | Description
| HTTP Status |
+|-------------------------|---------------------|-----------------------------------------------------------------------------------------------------------------------------------------------------|-------------|
+| `GET /api/health/live` | `GET /health/live` | Liveness. Returns 200 if an
HTTP thread can respond and no OOM has been observed; otherwise 503. Use it to
decide whether to restart a pod. | 200 or 503 |
+| `GET /api/health/ready` | `GET /health/ready` | Readiness. Returns 200 when
no OOM has been observed and the entity store answers within the probe timeout;
otherwise 503. Use it to route traffic. | 200 or 503 |
+| `GET /api/health` | `GET /health` | Aggregate. Returns 200 when
both of the above pass. Also aliased as `GET /health.html`.
| 200 or 503 |
| Configuration Item | Description
| Default Value |
|------------------------------------------------------|---------------------------------------------------------------------|---------------|
| `gravitino.server.health.entityStore.probeTimeoutMs` | Timeout in
milliseconds for the entity store probe behind `/ready`. | `2000` |
Every endpoint returns the same JSON shape, but not the same checks. `code` is
always `0`,
`status` is `UP` or `DOWN`, and `checks` carries one entry per component
probed. `/live` reports
-`httpServer` alone, `/ready` reports `entityStore` alone, and the aggregate
endpoint reports both:
+`httpServer` alone, `/ready` reports `entityStore` alone, and the aggregate
endpoint reports both
+while no OOM has been observed:
Review Comment:
Fixed in 1762ae3. The main-server status description, JSON example, and
`jvm: down` text now use lowercase `up`/`down`.
##########
docs/health-and-readiness.md:
##########
@@ -54,7 +57,44 @@ endpoint rather than to a check of its own.
The response body carries an overall status and a list of individual checks.
Each check has a name,
a status of UP or DOWN, and a details map that explains a failure. On the
Gravitino server the two
-check names are `httpServer` and `entityStore`.
+normal check names are `httpServer` and `entityStore`. After an observed
out-of-memory error, all
Review Comment:
Fixed in 1762ae3. This section now uses lowercase `up`/`down` for the main
server, and the OOM section explicitly distinguishes Gravitino/Lance from
Iceberg REST, which retains uppercase status values.
##########
docs/iceberg-rest-service.md:
##########
@@ -724,7 +724,7 @@ The Iceberg REST server exposes three health check
endpoints following the same
| Endpoint | Description
|
HTTP status |
|-----------------------------|----------------------------------------------------------------------------------------------------------------------------|-------------|
-| `GET /iceberg/health/live` | Liveness probe. Returns 200 as long as the
HTTP server thread can respond.
| 200 |
+| `GET /iceberg/health/live` | Liveness probe. Returns 200 when the HTTP
thread can respond and no OOM has been observed.
| 200 / 503 |
Review Comment:
Updated in 1762ae3. The live endpoint description now explicitly says it
returns 503 after an observed OOM. Readiness also lists the no-OOM condition
alongside initialization, with 503 otherwise.
##########
server-common/src/main/java/org/apache/gravitino/server/web/JettyServer.java:
##########
@@ -484,6 +484,7 @@ public Thread run() {
thread.setName(getName() + "-" + thread.getId());
thread.setUncaughtExceptionHandler(
(t, throwable) -> {
+ ServerHealth.getInstance().recordFailure(throwable);
Review Comment:
Fixed in 1762ae3. `JettyServer` now installs an OOM-recording filter first
on `/*`; it records escaping failures and rethrows them. Both
`AuthenticationFilter` catch branches also record before calling
`sendAuthErrorResponse`, covering wrapped OOM consumed by authentication and
the Iceberg/Lance response overrides.
Added real HTTP tests for direct/wrapped OOM in downstream filters,
non-Jersey servlets, and authentication. The filter/servlet tests verify all
seven main-server health paths return 503 while a warm endpoint still succeeds;
the authentication tests verify recording precedes response conversion.
Ordinary failures do not mark the server unhealthy. The docs now describe the
worker handler as an additional fallback.
##########
server-common/src/main/java/org/apache/gravitino/server/web/ServerHealth.java:
##########
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.server.web;
+
+import javax.annotation.Nullable;
+
+/**
+ * Records out-of-memory failures observed by the server. Once recorded, the
failure remains until
+ * the process restarts; a successful request does not establish that the JVM
has recovered.
+ *
+ * <p>The failure path only sets a flag. It does not retain the error, format
its stack trace, or
+ * allocate a collection while walking its causes.
+ */
+public final class ServerHealth {
+ private static final ServerHealth INSTANCE = new ServerHealth();
+
+ private volatile boolean outOfMemory;
+
+ /** Creates an independent health state, initially healthy. */
+ public ServerHealth() {}
+
+ /**
+ * Returns the shared state used by the server's request and health-check
paths.
+ *
+ * @return the shared health state
+ */
+ public static ServerHealth getInstance() {
+ return INSTANCE;
+ }
+
+ /**
+ * Records an out-of-memory error, including one wrapped in another
throwable.
+ *
+ * @param failure the observed failure, or null
+ */
+ public void recordFailure(@Nullable Throwable failure) {
Review Comment:
I chose the documented-limitation option in 1762ae3. The method Javadoc,
health documentation, and PR description now explicitly say detection follows
the throwable and cause chain, excluding suppressed exceptions. This preserves
the current traversal without the defensive array copies from `getSuppressed()`.
##########
docs/health-and-readiness.md:
##########
@@ -54,7 +57,44 @@ endpoint rather than to a check of its own.
The response body carries an overall status and a list of individual checks.
Each check has a name,
a status of UP or DOWN, and a details map that explains a failure. On the
Gravitino server the two
-check names are `httpServer` and `entityStore`.
+normal check names are `httpServer` and `entityStore`. After an observed
out-of-memory error, all
+three endpoints instead report the `jvm` failure described below.
+
+## Out-of-memory Failures
+
+A Metaspace or heap `OutOfMemoryError` can leave already-loaded endpoints
responding successfully
+while other operations fail. A successful HTTP response or entity-store lookup
therefore does not
+prove recovery after OOM.
+
+The Gravitino, Iceberg REST, and Lance REST servers record OOM observed by
their Jersey exception
+listeners, error mappers, shared request execution/error-response helpers, and
Jetty worker
+uncaught-exception handlers. The main server also records failures in
health-probe tasks.
+Wrapped causes are checked too. Once recorded, the affected service’s health
endpoints and root
+aliases return HTTP 503 with this body (the main server uses the `/api/health`
prefix):
+
+```json
+{
+ "code": 0,
+ "status": "down",
+ "checks": [
+ {
+ "name": "jvm",
+ "status": "down",
+ "details": { "reason": "OutOfMemoryError; restart required" }
+ }
+ ]
+}
+```
+
+This state lasts until process restart, even if subsequent ordinary API
requests succeed. Health
+checks skip the entity-store probe once OOM is recorded. A database outage,
ordinary HTTP 500,
+`StackOverflowError`, or missing connector class alone does not set this state.
+
+Detection covers errors reaching these server boundaries; it cannot detect an
OOM swallowed
+entirely by a connector or unrelated background executor. This is not a
JVM-wide OOM trap. If the
+JVM cannot allocate enough memory to answer a probe, the probe may fail
without a JSON response.
+Each service tracks errors observed within its own runtime. Auxiliary services
with isolated
Review Comment:
Corrected in 1762ae3 and the PR description. Embedded services share the
marker through the default auxiliary classloaders; separate JVM processes track
OOM independently. The docs still recommend probing each service port because
HTTP availability and initialization checks remain service-specific.
--
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]
