bitflicker64 commented on code in PR #3185:
URL: https://github.com/apache/hugegraph/pull/3185#discussion_r3923362919
##########
hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/StoreAPI.java:
##########
@@ -386,4 +396,30 @@ class StoreStatistics {
public Serializable checkHealthy() {
return "";
}
+
+ /**
+ * Check Service Readiness
+ * Answers 200 only when this PD is part of a raft quorum, that is, the
raft node is active
+ * and knows the current leader. Otherwise answers 503 so that anything
gating on PD
+ * (the compose healthcheck in front of Stores, a Kubernetes readiness
probe)
+ * is held back until the PD can serve. Like /health this endpoint needs
no authentication.
+ *
+ * @return JSON with the readiness flag, the local raft state and the raft
address of the
+ * leader (null when there is none)
+ */
+ @GetMapping(value = "/ready", produces = MediaType.APPLICATION_JSON_VALUE)
+ public ResponseEntity<Map<String, Object>> checkReady() {
+ RaftEngine raft = RaftEngine.getInstance();
+ boolean ready = raft.isReady();
+ State state = raft.getNodeState();
+ PeerId leader = raft.getLeader();
Review Comment:
Fixed in c8adc85. Added `RaftEngine.getRaftStatus()`, which derives every
field from one `Node` reference and a single `getLeaderId()` read via the
`hasLeader(Node)` overload, and returns an immutable `RaftStatus`.
`checkReady()` now builds the body from that one snapshot and no longer imports
any jraft type, so the raft logic stays in the engine. `isReady()` delegates to
it, and the now-redundant `getNodeState()` is gone.
The specific inconsistency you named is also structurally impossible now,
since the leader address left the body (see the `AuthenticationConfigurer`
thread). New unit test `testStatusNeverReportsReadyWithoutALeader` covers the
invariant; 10/10 pass on JDK 11.
##########
hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/interceptor/AuthenticationConfigurer.java:
##########
@@ -32,6 +32,7 @@ public class AuthenticationConfigurer implements
WebMvcConfigurer {
public void addInterceptors(InterceptorRegistry registry) {
registry.addInterceptor(restAuthentication)
.addPathPatterns("/**")
- .excludePathPatterns("/actuator/*", "/v1/health",
"/v1/prom/targets/*");
+ .excludePathPatterns("/actuator/*", "/v1/health", "/v1/ready",
+ "/v1/prom/targets/*");
Review Comment:
Partly taken, in c8adc85. Dropped `leader` from the anonymous body, which
was the one genuinely new disclosure: it pointed at a specific node, and
nothing anonymous exposed a raft address before. The leader address stays on
the authenticated `/v1/members`, which the issue already noted carries
`pdLeader` with role and state.
Kept `ready`, `state` and `isLeader`. `ready` is load-bearing for probes,
because the `200`-on-rejection behaviour means a status-only check cannot gate.
`isLeader` reveals nothing beyond the `hg_raft_leader` gauge, which is
anonymous on `/actuator/prometheus` by design (option 3 of the issue), and
`state` is what makes a `503` diagnosable.
Body is now `{"ready":true,"state":"STATE_LEADER","isLeader":true}`. Happy
to trim further if you would rather it were only `ready`.
--
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]
