bitflicker64 commented on code in PR #3185:
URL: https://github.com/apache/hugegraph/pull/3185#discussion_r3930909486
##########
hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java:
##########
@@ -203,7 +204,105 @@ public void shutDown() {
}
public boolean isLeader() {
- return this.raftNode.isLeader(true);
+ Node node = this.raftNode;
+ return node != null && node.isLeader(true);
+ }
+
+ /**
+ * Whether this node currently knows a raft leader.
+ * <p>
+ * A follower only keeps its leader id while heartbeats keep arriving
inside the election
+ * timeout, and a leader only keeps its role while it can reach a quorum.
A non-null leader
+ * therefore means this node is part of a quorum from its own point of
view, which is the
+ * signal a readiness probe needs.
+ */
+ public boolean hasLeader() {
+ return hasLeader(this.raftNode);
+ }
+
+ private static boolean hasLeader(Node node) {
+ if (node == null) {
+ return false;
+ }
+ PeerId leader = node.getLeaderId();
+ return leader != null && !leader.isEmpty();
+ }
+
+ /**
+ * Whether this node can take part in serving requests: the raft node has
been started,
+ * is in an active state (leader, follower or transferring leadership) and
sees a leader.
Review Comment:
Fixed in 7a6dbb8 with the suggested wording. Confirmed the enum order
against jraft-core 1.3.13: `LEADER, TRANSFERRING, CANDIDATE, FOLLOWER, ERROR,
...` with `isActive()` as `ordinal() < STATE_ERROR.ordinal()`, so the javadoc
was a state short.
Took both halves on the tests. Renamed to
`testCandidateWithoutLeaderIsNotReady`, with a comment that jraft clears the
leader id before starting an election so this is the only candidate shape it
reaches, and added `testCandidateCountsAsActive` covering `STATE_CANDIDATE`
with a leader id, which does read as ready and records the scope of the state
check. Marked `testEmptyLeaderIdIsNotReady` as guarding the `Node` contract
rather than a value `NodeImpl` returns. 11/11 pass on JDK 11.
##########
docker/README.md:
##########
@@ -202,6 +202,24 @@ done
curl -fsS http://localhost:8088/about
```
+PD answers two unauthenticated probe endpoints. `/v1/health` is liveness only:
+it returns `200` as soon as the REST listener is up, even when the PD has no
+raft leader. `/v1/ready` returns `200` only while the PD sees a raft leader,
+and `503` otherwise. A single PD elects itself; three PDs become ready once
+two of them can talk to each other.
+
+The healthchecks in these files still gate on `/v1/health`, because
Review Comment:
Fixed in 7a6dbb8. Added a paragraph after the two bullets saying the
`HEALTHCHECK` baked into `hugegraph-pd/Dockerfile` is `/v1/health` as well,
that both compose files override it, and that it therefore governs `docker run`
and anything else inheriting the image probe, which keeps reading a quorum-less
PD as healthy.
Agreed the version-skew argument does not cover it, since that probe ships
with the image. Left the Dockerfile alone to keep this diff to one decision;
happy to file the follow-up, or fold it in here if you would rather it landed
together.
##########
hugegraph-pd/docs/api-reference.md:
##########
@@ -774,6 +774,46 @@ curl http://localhost:8620/actuator/health
}
```
+### Liveness and Readiness
+
+Two unauthenticated endpoints are meant for probes and startup gates:
+
+| Endpoint | Meaning | Status |
+|----------|---------|--------|
+| `GET /v1/health` | Liveness: the REST listener is up. Does not consult raft.
| always `200` |
+| `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 |
+
+```bash
+curl -i http://localhost:8620/v1/ready
+```
+
+**Response** (leader of a healthy cluster):
+```json
+{
+ "ready": true,
+ "state": "STATE_LEADER",
+ "isLeader": true
+}
+```
+
+A follower reports `"state": "STATE_FOLLOWER"` with `"isLeader": false`. When
+the quorum is lost the PD keeps answering `/v1/health` with `200` but
+`/v1/ready` turns into `503` with `"ready": false`. Being unauthenticated, the
+body carries no cluster addresses; the leader's address stays on `/v1/members`.
+
+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.
+
+Match on the body rather than on the status code alone. PD's auth interceptor
+rejects a request it does not exclude by writing an error envelope without
Review Comment:
Fixed in 7a6dbb8, in all three places. Each now dates the behaviour rather
than asserting it: "as of 1.7.0" a refusal carries `200` and an error envelope,
followed by the point that the body match holds whichever status a refusal
carries. `api-reference.md` also attributes it to `RestAuthentication` rather
than to PD in general.
So if #3189 lands first, the three copies read as history rather than as a
wrong claim, and the advice they exist to give survives unchanged. Checked
#3189 at 5c339c1: it touches `docker/README.md`, `api-reference.md` and
`RestApiTest.java` too, so whichever order they merge in there will be textual
conflicts in those files regardless.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
