Skip to content

Commit 2aebabd

Browse files
committed
fix(pd): report fatal raft errors through health probes
1 parent be68aa2 commit 2aebabd

6 files changed

Lines changed: 75 additions & 20 deletions

File tree

‎docker/README.md‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,20 @@ curl -fsS http://localhost:8088/about
172172

173173
</details>
174174

175+
### PD health and readiness
176+
177+
PD's `/v1/ready` returns HTTP 503 while it cannot serve through a Raft quorum.
178+
`/v1/health` remains HTTP 200 during an election or temporary quorum loss, but
179+
returns HTTP 503 after a fatal Raft error or state-machine shutdown. Both health
180+
responses retain the empty text body. Probes read cached state without waiting
181+
for the Raft node lock.
182+
183+
A failed snapshot, for example after the PD disk fills, leaves JRaft in a terminal
184+
error state. Free disk space before restarting the affected PD; freeing space
185+
alone does not reinitialize its Raft node. Configure liveness against `/v1/health`
186+
with an image containing this fix and gate traffic on `/v1/ready`. Older images
187+
still need the single-PD readiness-based liveness workaround supplied by the Helm chart.
188+
175189
### Stop or remove a deployment
176190

177191
`stop` keeps containers and data; `down` removes containers and the network but keeps data. **`down -v` also deletes all topology data.**

‎helm/hugegraph/values.yaml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,8 @@ pd:
177177
# election to lose and would otherwise pass /v1/health forever after losing
178178
# leadership for good (apache/hugegraph#3222). Startup follows liveness so
179179
# the boot-to-ready window sits inside the startup budget.
180+
# TODO: drop the single-PD readiness derivation once the minimum supported image
181+
# includes the /v1/health fatal Raft error fix for apache/hugegraph#3222.
180182
livenessPath: ""
181183
probes:
182184
startup:

‎hugegraph-pd/docs/api-reference.md‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -809,7 +809,7 @@ Two unauthenticated endpoints are meant for probes and startup gates:
809809

810810
| Endpoint | Meaning | Status |
811811
|----------|---------|--------|
812-
| `GET /v1/health` | Liveness: the REST listener is up. Does not consult raft. | always `200` |
812+
| `GET /v1/health` | Liveness: the REST listener is up and Raft has no terminal error. Empty text body. | `503` for `STATE_ERROR` or `STATE_SHUTDOWN`, `200` otherwise |
813813
| `GET /v1/ready` | Readiness: the raft node is active and sees a leader, so this PD is inside a quorum. | `200` when ready, `503` otherwise |
814814

815815
```bash
@@ -829,6 +829,11 @@ A follower reports `"state": "STATE_FOLLOWER"` with `"isLeader": false`. When th
829829

830830
The answer is served from state the raft callbacks maintain rather than from the raft node, so it stays prompt while an election is running and never waits on the node lock. `state` is therefore the last change raft announced. A PD reports `STATE_UNINITIALIZED` with `"ready": false` from process start until its first raft callback, which is the ordinary startup window before a quorum first forms, and jraft emits no callback for candidacy or leadership transfer, so a candidate reports `STATE_FOLLOWER` with `"ready": false`.
831831

832+
After a fatal snapshot error, `/v1/ready` retains `STATE_ERROR` even after the leader
833+
steps down, and `/v1/health` returns `503`. Fix the underlying disk or storage
834+
problem and restart the affected PD: freeing disk space alone does not restart
835+
JRaft's terminal state machine.
836+
832837
Point Kubernetes readiness probes, `depends_on` healthchecks and any "wait for PD" script at `/v1/ready`; keep liveness probes on `/v1/health` so a PD that merely lost its leader is not restarted.
833838

834839
Match on the body rather than on the status code alone. A PD that predates this endpoint does not reliably answer `404` for it: `RestAuthentication` refuses a request it does not exclude by writing an error envelope, and as of 1.7.0 it does so without setting a status, so an unknown path answers `200` with `{"status":-1,"error":"Unauthorized!"}`. A status-only probe therefore reads such a PD as ready. The body match holds whichever status a refusal carries: a shell gate should use `curl -fsS http://<pd-host>:8620/v1/ready | grep -q '"ready":true'`, and a Kubernetes `httpGet` probe should be paired with a PD image that carries the endpoint.

‎hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftStateMachine.java‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -214,13 +214,10 @@ public void onLeaderStart(final long term) {
214214
@Override
215215
public void onLeaderStop(final Status status) {
216216
this.leaderTerm.set(-1);
217-
// TODO: keep STATE_ERROR set by onError instead of overwriting it with STATE_FOLLOWER, so
218-
// the probe view, and with it /v1/ready, reports a PD that stepped down for good after a
219-
// snapshot failure. /v1/health does not read this view; making it report the state is the
220-
// separate TODO at StoreAPI.checkHealthy. The Helm chart (helm/hugegraph) works around both
221-
// by deriving a single PD's startup and liveness probes to /v1/ready.
222-
// https://github.com/apache/hugegraph/issues/3222
223-
this.probeView = new ProbeView(State.STATE_FOLLOWER, false);
217+
// JRaft steps down after reporting a fatal error. That callback does not recover the node.
218+
if (this.probeView.state != State.STATE_ERROR) {
219+
this.probeView = new ProbeView(State.STATE_FOLLOWER, false);
220+
}
224221
super.onLeaderStop(status);
225222
log.info("Raft lost leader ");
226223
}

‎hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/StoreAPI.java‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717

1818
package org.apache.hugegraph.pd.rest;
1919

20-
import java.io.Serializable;
2120
import java.util.ArrayList;
2221
import java.util.Date;
2322
import java.util.HashMap;
@@ -49,6 +48,7 @@
4948
import org.springframework.web.bind.annotation.ResponseBody;
5049
import org.springframework.web.bind.annotation.RestController;
5150

51+
import com.alipay.sofa.jraft.core.State;
5252
import com.google.protobuf.util.JsonFormat;
5353

5454
import lombok.Data;
@@ -388,22 +388,20 @@ class StoreStatistics {
388388
* This interface is used to check the health status of the service by accessing the /health
389389
* path via a GET request.
390390
* <p>
391-
* This is a liveness signal only: it answers 200 as soon as the REST listener is up and
392-
* does not consult the raft state. Use {@link #checkReady()} to find out whether this PD
393-
* can actually serve.
391+
* This liveness signal answers 503 for a fatal raft error or a stopped state machine.
392+
* Elections and temporary quorum loss remain healthy. Use {@link #checkReady()} to find
393+
* out whether this PD can actually serve.
394394
*
395395
* @return Returns a string indicating the service's health status. Typically, an empty
396396
* string indicates the service is healthy.
397397
*/
398-
// TODO: answer non-200 when the raft state machine is in STATE_ERROR (a failed snapshot) or
399-
// the KV store never opened (a held RocksDB LOCK): both leave this PD unable to recover while
400-
// /health stays 200 forever. The Helm chart (helm/hugegraph) derives a single PD's startup
401-
// and liveness probes to /v1/ready for this reason; drop that derivation once this reports it.
402-
// https://github.com/apache/hugegraph/issues/3222
403-
// https://github.com/apache/hugegraph/issues/3226
404398
@GetMapping(value = "/health", produces = MediaType.TEXT_PLAIN_VALUE)
405-
public Serializable checkHealthy() {
406-
return "";
399+
public ResponseEntity<String> checkHealthy() {
400+
String state = RaftEngine.getInstance().getRaftStatus().getState();
401+
boolean failed = State.STATE_ERROR.name().equals(state) ||
402+
State.STATE_SHUTDOWN.name().equals(state);
403+
return ResponseEntity.status(failed ? HttpStatus.SERVICE_UNAVAILABLE : HttpStatus.OK)
404+
.body("");
407405
}
408406

409407
/**

‎hugegraph-pd/hg-pd-service/src/test/java/org/apache/hugegraph/pd/rest/StoreAPIReadyTest.java‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import com.alipay.sofa.jraft.core.State;
3636
import com.alipay.sofa.jraft.entity.LeaderChangeContext;
3737
import com.alipay.sofa.jraft.entity.PeerId;
38+
import com.alipay.sofa.jraft.error.RaftException;
3839

3940
/**
4041
* Pins the status a readiness probe actually receives from {@code GET /v1/ready}: 503 while
@@ -120,6 +121,44 @@ private static LeaderChangeContext context() {
120121
return new LeaderChangeContext(LEADER, 5, Status.OK());
121122
}
122123

124+
@Test
125+
public void testFatalErrorSurvivesLeaderStopAndAnswers503() {
126+
Whitebox.setInternalState(RaftEngine.getInstance(), "raftNode", untouchableNode());
127+
this.stateMachine.onLeaderStart(5);
128+
this.stateMachine.onError(new RaftException());
129+
this.stateMachine.onLeaderStop(new Status(-1, "snapshot failed"));
130+
131+
Assert.assertEquals(HttpStatus.SERVICE_UNAVAILABLE,
132+
this.api.checkHealthy().getStatusCode());
133+
Assert.assertEquals("", this.api.checkHealthy().getBody());
134+
ResponseEntity<Map<String, Object>> response = this.api.checkReady();
135+
Assert.assertEquals(HttpStatus.SERVICE_UNAVAILABLE, response.getStatusCode());
136+
Assert.assertEquals(State.STATE_ERROR.name(), response.getBody().get("state"));
137+
Assert.assertEquals(Boolean.FALSE, response.getBody().get("isLeader"));
138+
}
139+
140+
@Test
141+
public void testQuorumLossRemainsLive() {
142+
Whitebox.setInternalState(RaftEngine.getInstance(), "raftNode", untouchableNode());
143+
this.stateMachine.onLeaderStart(5);
144+
Assert.assertEquals(HttpStatus.OK, this.api.checkHealthy().getStatusCode());
145+
this.stateMachine.onLeaderStop(Status.OK());
146+
Assert.assertEquals(HttpStatus.OK, this.api.checkHealthy().getStatusCode());
147+
this.stateMachine.onStartFollowing(context());
148+
Assert.assertEquals(HttpStatus.OK, this.api.checkHealthy().getStatusCode());
149+
this.stateMachine.onStopFollowing(context());
150+
Assert.assertEquals(HttpStatus.OK, this.api.checkHealthy().getStatusCode());
151+
Assert.assertEquals(HttpStatus.SERVICE_UNAVAILABLE,
152+
this.api.checkReady().getStatusCode());
153+
}
154+
155+
@Test
156+
public void testShutdownIsNotLive() {
157+
this.stateMachine.onShutdown();
158+
Assert.assertEquals(HttpStatus.SERVICE_UNAVAILABLE,
159+
this.api.checkHealthy().getStatusCode());
160+
}
161+
123162
/**
124163
* A started raft node that fails the test if the endpoint calls it. Readiness is served
125164
* from the state machine callbacks precisely so that it never waits on the node lock.

0 commit comments

Comments
 (0)