From 4dd7e71c978e7c11f29be8bcad31b5d154da614c Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Wed, 2 Sep 2026 19:30:16 +0530 Subject: [PATCH 1/9] feat(pd): add quorum-aware /v1/ready endpoint and raft gauges /v1/health answers 200 as soon as the Spring listener is up and never consults the raft state, so a PD that has lost its leader keeps reporting healthy to every consumer that gates on it (compose healthchecks, the Store's wait for PD, Kubernetes probes, wait-storage.sh). Keep /v1/health as pure liveness and add an unauthenticated /v1/ready that answers 200 only while the raft node is active and sees a leader, and 503 otherwise. A follower drops its leader id once heartbeats stop inside the election timeout and a leader steps down when it cannot reach a quorum, so "sees a leader" is the local view of being inside a quorum. Export three gauges next to hg_up so operators can alert on quorum loss: hg_raft_leader (1 on the leader), hg_raft_has_leader (1 while a leader is known) and hg_raft_alive_peers (peers the leader heard from inside the election timeout, NaN on non-leaders). Point the compose PD healthchecks at /v1/ready so Stores are no longer released against a leaderless PD, and document both endpoints. The PD startup CI test now also waits for /v1/ready on the live single-node PD, and the REST suite checks the endpoint and the gauges against it. Fixes #3183 --- docker/README.md | 12 +- docker/docker-compose-3pd-3store-3server.yml | 2 +- docker/docker-compose-hstore.yml | 2 +- hugegraph-pd/README.md | 2 + hugegraph-pd/docs/api-reference.md | 42 +++++ .../apache/hugegraph/pd/raft/RaftEngine.java | 66 +++++++- .../hugegraph/pd/metrics/PDMetrics.java | 22 +++ .../apache/hugegraph/pd/rest/StoreAPI.java | 36 ++++ .../interceptor/AuthenticationConfigurer.java | 3 +- .../hugegraph/pd/core/PDCoreSuiteTest.java | 2 + .../pd/raft/RaftEngineReadinessTest.java | 154 ++++++++++++++++++ .../apache/hugegraph/pd/rest/RestApiTest.java | 45 +++++ .../travis/test-start-hugegraph-pd.sh | 25 +++ hugegraph-store/docs/deployment-guide.md | 7 +- 14 files changed, 411 insertions(+), 9 deletions(-) create mode 100644 hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/raft/RaftEngineReadinessTest.java diff --git a/docker/README.md b/docker/README.md index 0bb74cf81f..38c016d498 100644 --- a/docker/README.md +++ b/docker/README.md @@ -131,7 +131,7 @@ docker compose -f docker-compose-hstore.yml ps Verify PD, Store, Server authentication, and Hubble: ```bash -curl -fsS http://localhost:8620/v1/health +curl -fsS http://localhost:8620/v1/ready curl -fsS http://localhost:8520/v1/health curl -fsS http://localhost:8080/versions test "$(curl -sS -o /dev/null -w '%{http_code}' \ @@ -186,7 +186,7 @@ and Hubble: ```bash for port in 8620 8621 8622; do - curl -fsS "http://localhost:${port}/v1/health" + curl -fsS "http://localhost:${port}/v1/ready" done for port in 8520 8521 8522; do curl -fsS "http://localhost:${port}/v1/health" @@ -202,6 +202,14 @@ 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 leader and +`503` otherwise, so the compose healthchecks gate Stores on `/v1/ready`. A +single PD elects itself; three PDs become ready once two can talk to each +other. + + Open `http://localhost:8088` and sign in as `admin` with the password from `.env`. diff --git a/docker/docker-compose-3pd-3store-3server.yml b/docker/docker-compose-3pd-3store-3server.yml index 6f599c6870..87b37dd2ed 100644 --- a/docker/docker-compose-3pd-3store-3server.yml +++ b/docker/docker-compose-3pd-3store-3server.yml @@ -37,7 +37,7 @@ x-pd-common: &pd-common restart: unless-stopped networks: [hg-net] healthcheck: - test: ["CMD-SHELL", "curl -fsS http://localhost:8620/v1/health >/dev/null || exit 1"] + test: ["CMD-SHELL", "curl -fsS http://localhost:8620/v1/ready >/dev/null || exit 1"] interval: 15s timeout: 10s retries: 30 diff --git a/docker/docker-compose-hstore.yml b/docker/docker-compose-hstore.yml index d201430692..3fbc92bc75 100644 --- a/docker/docker-compose-hstore.yml +++ b/docker/docker-compose-hstore.yml @@ -44,7 +44,7 @@ services: volumes: - pd-data:/hugegraph-pd/pd_data healthcheck: - test: ["CMD-SHELL", "curl -fsS http://localhost:8620/v1/health >/dev/null"] + test: ["CMD-SHELL", "curl -fsS http://localhost:8620/v1/ready >/dev/null"] interval: 10s timeout: 5s retries: 12 diff --git a/hugegraph-pd/README.md b/hugegraph-pd/README.md index 794dba9b98..c940dae4c9 100644 --- a/hugegraph-pd/README.md +++ b/hugegraph-pd/README.md @@ -284,6 +284,8 @@ docker/docker-compose-3pd-3store-3server.yml PD exposes metrics via REST API at: - Health check: `http://:8620/actuator/health` +- Liveness: `http://:8620/v1/health` (REST listener is up) +- Readiness: `http://:8620/v1/ready` (`200` only while the PD sees a raft leader) - Metrics: `http://:8620/actuator/metrics` ## Community diff --git a/hugegraph-pd/docs/api-reference.md b/hugegraph-pd/docs/api-reference.md index aa8cce8473..bad685a03b 100644 --- a/hugegraph-pd/docs/api-reference.md +++ b/hugegraph-pd/docs/api-reference.md @@ -774,12 +774,54 @@ 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", + "leader": "192.168.1.1:8610", + "isLeader": true +} +``` + +A follower reports `"state": "STATE_FOLLOWER"` with the leader's raft address. +When the quorum is lost the PD keeps answering `/v1/health` with `200` but +`/v1/ready` turns into `503` with `"ready": false` and `"leader": null`. +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. + ### Metrics ```bash curl http://localhost:8620/actuator/metrics ``` +Raft membership gauges (Prometheus names, scraped from `/actuator/prometheus`) +for alerting on quorum loss: + +| Gauge | Value | +|-------|-------| +| `hg_raft_leader` | `1` on the raft leader, `0` elsewhere | +| `hg_raft_has_leader` | `1` while this PD sees a leader (is inside a quorum), `0` otherwise | +| `hg_raft_alive_peers` | On the leader, the number of peers (itself included) heard from within the election timeout; `NaN` on other nodes | + +A cluster has lost its quorum when `sum(hg_raft_leader) == 0` or when +`hg_raft_has_leader == 0` on every member. + **Response** (Prometheus format): ``` # HELP pd_raft_state Raft state (0=Follower, 1=Candidate, 2=Leader) diff --git a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java index 314c9e57ef..7b1ed926d5 100644 --- a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java +++ b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java @@ -46,6 +46,7 @@ import com.alipay.sofa.jraft.Status; import com.alipay.sofa.jraft.conf.Configuration; import com.alipay.sofa.jraft.core.Replicator; +import com.alipay.sofa.jraft.core.State; import com.alipay.sofa.jraft.entity.PeerId; import com.alipay.sofa.jraft.entity.Task; import com.alipay.sofa.jraft.error.RaftError; @@ -203,7 +204,67 @@ 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. + *

+ * 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() { + Node node = this.raftNode; + 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. + * Unlike a plain liveness check this turns false as soon as the quorum is lost. + */ + public boolean isReady() { + Node node = this.raftNode; + if (node == null) { + return false; + } + State state = node.getNodeState(); + if (state == null || !state.isActive()) { + return false; + } + return hasLeader(); + } + + /** + * @return the jraft node state, or null before the raft node has been started + */ + public State getNodeState() { + Node node = this.raftNode; + return node == null ? null : node.getNodeState(); + } + + /** + * Number of raft peers, this node included, that the leader has heard from within the + * election timeout. Only the leader tracks replication state, so any other node returns -1. + */ + public int getAlivePeerCount() { + Node node = this.raftNode; + if (node == null || !node.isLeader(true)) { + return -1; + } + try { + return node.listAlivePeers().size(); + } catch (IllegalStateException e) { + // Lost leadership between the check and the call + return -1; + } } /** @@ -232,7 +293,8 @@ public PDConfig.Raft getConfig() { } public PeerId getLeader() { - return raftNode.getLeaderId(); + Node node = this.raftNode; + return node == null ? null : node.getLeaderId(); } /** diff --git a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/metrics/PDMetrics.java b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/metrics/PDMetrics.java index 483974a016..6151eade29 100644 --- a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/metrics/PDMetrics.java +++ b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/metrics/PDMetrics.java @@ -33,6 +33,7 @@ import org.apache.hugegraph.pd.grpc.Metapb; import org.apache.hugegraph.pd.grpc.Metapb.ShardGroup; import org.apache.hugegraph.pd.model.GraphStatistics; +import org.apache.hugegraph.pd.raft.RaftEngine; import org.apache.hugegraph.pd.service.PDRestService; import org.apache.hugegraph.pd.service.PDService; @@ -76,7 +77,28 @@ private void registerMeters() { Gauge.builder(PREFIX + ".terms", () -> setTerms()) .description("term of partitions in PD") .register(registry); + registerRaftMeters(); + } + /** + * Raft membership gauges so operators can alert on quorum loss. They mirror what + * {@code GET /v1/ready} answers: a PD that sees no leader is outside a quorum. + */ + private void registerRaftMeters() { + RaftEngine raft = RaftEngine.getInstance(); + Gauge.builder(PREFIX + ".raft.leader", () -> raft.isLeader() ? 1 : 0) + .description("1 if this PD is the raft leader, 0 otherwise") + .register(registry); + Gauge.builder(PREFIX + ".raft.has_leader", () -> raft.hasLeader() ? 1 : 0) + .description("1 if this PD sees a raft leader, i.e. is part of a quorum, 0 otherwise") + .register(registry); + Gauge.builder(PREFIX + ".raft.alive_peers", () -> { + int alive = raft.getAlivePeerCount(); + return alive < 0 ? Double.NaN : alive; + }) + .description("Number of raft peers, itself included, the leader has heard from " + + "within the election timeout; NaN on non-leader nodes") + .register(registry); } private long updateGraphs() { diff --git a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/StoreAPI.java b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/StoreAPI.java index 2cddb29feb..8f943d9db9 100644 --- a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/StoreAPI.java +++ b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/StoreAPI.java @@ -22,6 +22,7 @@ import java.util.Date; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -31,11 +32,14 @@ import org.apache.hugegraph.pd.model.RestApiResponse; import org.apache.hugegraph.pd.model.StoreRestRequest; import org.apache.hugegraph.pd.model.TimeRangeRequest; +import org.apache.hugegraph.pd.raft.RaftEngine; import org.apache.hugegraph.pd.service.PDRestService; import org.apache.hugegraph.pd.util.DateUtil; import org.apache.hugegraph.pd.util.StoreRestAddressUtil; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; +import org.springframework.http.ResponseEntity; import org.springframework.web.bind.annotation.DeleteMapping; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PathVariable; @@ -45,6 +49,8 @@ import org.springframework.web.bind.annotation.ResponseBody; import org.springframework.web.bind.annotation.RestController; +import com.alipay.sofa.jraft.core.State; +import com.alipay.sofa.jraft.entity.PeerId; import com.google.protobuf.util.JsonFormat; import lombok.Data; @@ -378,6 +384,10 @@ class StoreStatistics { * Check Service Health Status * This interface is used to check the health status of the service by accessing the /health * path via a GET request. + *

+ * This is a liveness signal only: it answers 200 as soon as the REST listener is up and + * does not consult the raft state. Use {@link #checkReady()} to find out whether this PD + * can actually serve. * * @return Returns a string indicating the service's health status. Typically, an empty * string indicates the service is healthy. @@ -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 + * (a Store waiting to register, the Server's wait-storage.sh, 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> checkReady() { + RaftEngine raft = RaftEngine.getInstance(); + boolean ready = raft.isReady(); + State state = raft.getNodeState(); + PeerId leader = raft.getLeader(); + + Map body = new LinkedHashMap<>(); + body.put("ready", ready); + body.put("state", state == null ? State.STATE_UNINITIALIZED.name() : state.name()); + body.put("leader", leader == null || leader.isEmpty() ? null : leader.toString()); + body.put("isLeader", raft.isLeader()); + HttpStatus status = ready ? HttpStatus.OK : HttpStatus.SERVICE_UNAVAILABLE; + return ResponseEntity.status(status).body(body); + } } diff --git a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/interceptor/AuthenticationConfigurer.java b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/interceptor/AuthenticationConfigurer.java index 7d10416967..d4b1e026d3 100644 --- a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/interceptor/AuthenticationConfigurer.java +++ b/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/*"); } } diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java index 95b044c76b..bdacf7d371 100644 --- a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java +++ b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java @@ -22,6 +22,7 @@ import org.apache.hugegraph.pd.raft.IpAuthHandlerTest; import org.apache.hugegraph.pd.raft.RaftEngineIpAuthIntegrationTest; import org.apache.hugegraph.pd.raft.RaftEngineLeaderAddressTest; +import org.apache.hugegraph.pd.raft.RaftEngineReadinessTest; import org.junit.runner.RunWith; import org.junit.runners.Suite; @@ -43,6 +44,7 @@ IpAuthHandlerTest.class, RaftEngineIpAuthIntegrationTest.class, RaftEngineLeaderAddressTest.class, + RaftEngineReadinessTest.class, // StoreNodeServiceTest.class, }) @Slf4j diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/raft/RaftEngineReadinessTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/raft/RaftEngineReadinessTest.java new file mode 100644 index 0000000000..72f3ba8617 --- /dev/null +++ b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/raft/RaftEngineReadinessTest.java @@ -0,0 +1,154 @@ +/* + * 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.hugegraph.pd.raft; + +import java.util.Arrays; + +import org.apache.hugegraph.testutil.Whitebox; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import com.alipay.sofa.jraft.Node; +import com.alipay.sofa.jraft.core.State; +import com.alipay.sofa.jraft.entity.PeerId; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * Covers the raft-aware readiness signal behind {@code GET /v1/ready} and the + * {@code hg.raft.*} gauges: a PD is ready only while it sees a raft leader. + */ +public class RaftEngineReadinessTest { + + private static final PeerId LEADER = new PeerId("10.0.0.1", 8610); + private static final PeerId SELF = new PeerId("10.0.0.2", 8610); + private static final PeerId OTHER = new PeerId("10.0.0.3", 8610); + + private Node originalRaftNode; + private Node mockNode; + + @Before + public void setUp() { + RaftEngine engine = RaftEngine.getInstance(); + originalRaftNode = engine.getRaftNode(); + mockNode = mock(Node.class); + Whitebox.setInternalState(engine, "raftNode", mockNode); + } + + @After + public void tearDown() { + Whitebox.setInternalState(RaftEngine.getInstance(), "raftNode", originalRaftNode); + } + + private void stub(State state, PeerId leader, boolean isLeader) { + when(mockNode.getNodeState()).thenReturn(state); + when(mockNode.getLeaderId()).thenReturn(leader); + when(mockNode.isLeader(true)).thenReturn(isLeader); + } + + @Test + public void testNotReadyBeforeRaftNodeStarts() { + Whitebox.setInternalState(RaftEngine.getInstance(), "raftNode", null); + RaftEngine engine = RaftEngine.getInstance(); + + Assert.assertFalse(engine.isReady()); + Assert.assertFalse(engine.hasLeader()); + Assert.assertFalse(engine.isLeader()); + Assert.assertNull(engine.getNodeState()); + Assert.assertNull(engine.getLeader()); + Assert.assertEquals(-1, engine.getAlivePeerCount()); + } + + @Test + public void testLeaderIsReady() { + stub(State.STATE_LEADER, SELF, true); + when(mockNode.listAlivePeers()).thenReturn(Arrays.asList(SELF, LEADER, OTHER)); + RaftEngine engine = RaftEngine.getInstance(); + + Assert.assertTrue(engine.isReady()); + Assert.assertTrue(engine.hasLeader()); + Assert.assertTrue(engine.isLeader()); + Assert.assertEquals(State.STATE_LEADER, engine.getNodeState()); + Assert.assertEquals(3, engine.getAlivePeerCount()); + } + + @Test + public void testFollowerWithLeaderIsReady() { + stub(State.STATE_FOLLOWER, LEADER, false); + RaftEngine engine = RaftEngine.getInstance(); + + Assert.assertTrue(engine.isReady()); + Assert.assertTrue(engine.hasLeader()); + Assert.assertFalse(engine.isLeader()); + Assert.assertEquals(LEADER, engine.getLeader()); + // Only the leader tracks replication, followers cannot count alive peers + Assert.assertEquals(-1, engine.getAlivePeerCount()); + } + + @Test + public void testFollowerWithoutLeaderIsNotReady() { + // jraft resets the leader id once heartbeats stop arriving inside the election timeout + stub(State.STATE_FOLLOWER, null, false); + RaftEngine engine = RaftEngine.getInstance(); + + Assert.assertFalse(engine.isReady()); + Assert.assertFalse(engine.hasLeader()); + } + + @Test + public void testEmptyLeaderIdIsNotReady() { + stub(State.STATE_FOLLOWER, PeerId.emptyPeer(), false); + RaftEngine engine = RaftEngine.getInstance(); + + Assert.assertFalse(engine.isReady()); + Assert.assertFalse(engine.hasLeader()); + } + + @Test + public void testCandidateIsNotReady() { + stub(State.STATE_CANDIDATE, null, false); + Assert.assertFalse(RaftEngine.getInstance().isReady()); + } + + @Test + public void testTransferringLeaderIsReady() { + stub(State.STATE_TRANSFERRING, SELF, true); + Assert.assertTrue(RaftEngine.getInstance().isReady()); + } + + @Test + public void testInactiveStatesAreNotReadyEvenWithLeaderId() { + for (State state : new State[]{State.STATE_ERROR, State.STATE_UNINITIALIZED, + State.STATE_SHUTTING, State.STATE_SHUTDOWN}) { + stub(state, LEADER, false); + Assert.assertFalse("state " + state + " must not be ready", + RaftEngine.getInstance().isReady()); + } + } + + @Test + public void testAlivePeerCountSurvivesLeadershipLossRace() { + stub(State.STATE_LEADER, SELF, true); + when(mockNode.listAlivePeers()).thenThrow(new IllegalStateException("Not leader")); + + Assert.assertEquals(-1, RaftEngine.getInstance().getAlivePeerCount()); + } +} diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/RestApiTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/RestApiTest.java index fb2b71d480..d02b9df54d 100644 --- a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/RestApiTest.java +++ b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/RestApiTest.java @@ -62,6 +62,51 @@ public void testQueryClusterInfo() throws URISyntaxException, IOException, Inter assert obj.getInt("status") == 0; } + @Test + public void testHealthNeedsNoAuth() throws URISyntaxException, IOException, + InterruptedException { + String url = pdRestAddr + "/v1/health"; + HttpRequest request = HttpRequest.newBuilder().uri(new URI(url)).GET().build(); + HttpResponse response = client.send(request, HttpResponse.BodyHandlers.ofString()); + assert response.statusCode() == 200; + } + + @Test + public void testReadyNeedsNoAuthAndReflectsRaft() throws URISyntaxException, IOException, + InterruptedException, JSONException { + // The CI PD is a single-node raft group, so it is its own leader and must be ready + String url = pdRestAddr + "/v1/ready"; + HttpRequest request = HttpRequest.newBuilder().uri(new URI(url)).GET().build(); + HttpResponse response = client.send(request, HttpResponse.BodyHandlers.ofString()); + assert response.statusCode() == 200 : "expected 200, got " + response.statusCode() + + " body=" + response.body(); + JSONObject obj = new JSONObject(response.body()); + assert obj.getBoolean("ready"); + assert obj.getBoolean("isLeader"); + assert "STATE_LEADER".equals(obj.getString("state")); + assert !obj.isNull("leader") && !obj.getString("leader").isEmpty(); + } + + @Test + public void testRaftGaugesExported() throws URISyntaxException, IOException, + InterruptedException { + String url = pdRestAddr + "/actuator/prometheus"; + HttpRequest request = HttpRequest.newBuilder().uri(new URI(url)).GET().build(); + HttpResponse response = client.send(request, HttpResponse.BodyHandlers.ofString()); + assert response.statusCode() == 200; + String body = response.body(); + assert body.contains("hg_raft_leader{") : "missing hg_raft_leader gauge"; + assert body.contains("hg_raft_has_leader{") : "missing hg_raft_has_leader gauge"; + assert body.contains("hg_raft_alive_peers{") : "missing hg_raft_alive_peers gauge"; + // Single-node CI cluster: this PD is the leader and hears from itself + assert body.matches("(?s).*hg_raft_leader\\{[^}]*\\} 1\\.0.*") : + "hg_raft_leader should be 1 on a single-node leader"; + assert body.matches("(?s).*hg_raft_has_leader\\{[^}]*\\} 1\\.0.*") : + "hg_raft_has_leader should be 1 on a single-node leader"; + assert body.matches("(?s).*hg_raft_alive_peers\\{[^}]*\\} 1\\.0.*") : + "hg_raft_alive_peers should be 1 on a single-node leader"; + } + @Test public void testQueryClusterMembers() throws URISyntaxException, IOException, InterruptedException, JSONException { diff --git a/hugegraph-server/hugegraph-dist/src/assembly/travis/test-start-hugegraph-pd.sh b/hugegraph-server/hugegraph-dist/src/assembly/travis/test-start-hugegraph-pd.sh index ab73255b8c..3c62816e1b 100755 --- a/hugegraph-server/hugegraph-dist/src/assembly/travis/test-start-hugegraph-pd.sh +++ b/hugegraph-server/hugegraph-dist/src/assembly/travis/test-start-hugegraph-pd.sh @@ -111,6 +111,24 @@ wait_for_pd() { return 1 } +# Wait until PD readiness endpoint answers 200 with ready=true, or timeout. +# /v1/ready must need no credentials and must reflect raft: a single-node +# raft group elects itself, so it has to become ready shortly after /v1/health. +wait_for_pd_ready() { + local elapsed=0 + while (( elapsed < STARTUP_WAIT )); do + local body status + body=$(curl -s -w '\n%{http_code}' "$PD_URL/v1/ready" 2>/dev/null || echo "000") + status=${body##*$'\n'} + if [[ "$status" == "200" ]] && [[ "$body" == *'"ready":true'* ]]; then + return 0 + fi + sleep 2 + elapsed=$((elapsed + 2)) + done + return 1 +} + # Wait until bin/pid is non-empty or timeout wait_for_pid_file() { local elapsed=0 @@ -199,6 +217,13 @@ else fail "PD health endpoint not responding after ${STARTUP_WAIT}s" fi +info "Waiting up to ${STARTUP_WAIT}s for PD readiness endpoint..." +if wait_for_pd_ready; then + pass "PD readiness endpoint reports a raft leader at $PD_URL/v1/ready" +else + fail "PD readiness endpoint did not report ready=true after ${STARTUP_WAIT}s" +fi + cleanup # ── test 2: foreground mode blocks ─────────────────────────────────────────── diff --git a/hugegraph-store/docs/deployment-guide.md b/hugegraph-store/docs/deployment-guide.md index de07904d64..148dbb5ceb 100644 --- a/hugegraph-store/docs/deployment-guide.md +++ b/hugegraph-store/docs/deployment-guide.md @@ -719,7 +719,7 @@ environment: ``` **Startup ordering** is enforced via `depends_on` with `condition: service_healthy`: -1. PD nodes start first and must pass healthchecks (`/v1/health`) +1. PD nodes start first and must pass healthchecks (`/v1/ready`, answered `200` only once the PD sees a raft leader) 2. Store nodes start after all PD nodes are healthy 3. Server nodes start after all Store nodes are healthy @@ -855,9 +855,12 @@ kubectl port-forward svc/hugegraph-store 8500:8500 -n hugegraph ### Health Check ```bash -# PD health +# PD liveness (REST listener up) curl http://192.168.1.10:8620/v1/health +# PD readiness (200 only while the PD sees a raft leader, 503 otherwise) +curl -i http://192.168.1.10:8620/v1/ready + # Store health curl http://192.168.1.20:8520/v1/health ``` From 5bd1b964bc0e646dbf2aacb68b68f21e2fda3ec6 Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Wed, 2 Sep 2026 20:02:05 +0530 Subject: [PATCH 2/9] fix(pd): address review on the readiness endpoint Make RaftEngine.raftNode volatile so /v1/ready and the hg_raft_* gauges, which read it from request and scrape threads, do not rely on the @PostConstruct ordering for safe publication, and let isReady() reuse the node it already snapshotted instead of re-reading the field. Drop the wait-storage.sh mention from the /v1/ready javadoc: that script polls /v1/stores and Stores register over gRPC, so the compose healthcheck and Kubernetes probes are the real consumers. Move the raft gauge table below the existing /actuator/metrics example so the example still reads as that command's response, and note that both quorum-loss expressions are briefly true during a normal election and need a for: clause longer than the election timeout. State in the docker README that /v1/ready first ships in 1.8.0, since an older HUGEGRAPH_VERSION would leave the PD healthcheck failing and the Stores never starting, and drop a doubled blank line. --- docker/README.md | 5 ++-- hugegraph-pd/docs/api-reference.md | 27 ++++++++++--------- .../apache/hugegraph/pd/raft/RaftEngine.java | 9 ++++--- .../apache/hugegraph/pd/rest/StoreAPI.java | 2 +- 4 files changed, 25 insertions(+), 18 deletions(-) diff --git a/docker/README.md b/docker/README.md index 38c016d498..f7e7860b5f 100644 --- a/docker/README.md +++ b/docker/README.md @@ -207,8 +207,9 @@ 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 leader and `503` otherwise, so the compose healthchecks gate Stores on `/v1/ready`. A single PD elects itself; three PDs become ready once two can talk to each -other. - +other. `/v1/ready` first ships in 1.8.0: with an older `HUGEGRAPH_VERSION` +the PD healthcheck never passes and the Stores never start, so pin 1.8.0 +or newer, or build the images from source with `docker-compose.dev.yml`. Open `http://localhost:8088` and sign in as `admin` with the password from `.env`. diff --git a/hugegraph-pd/docs/api-reference.md b/hugegraph-pd/docs/api-reference.md index bad685a03b..a3814e3c9c 100644 --- a/hugegraph-pd/docs/api-reference.md +++ b/hugegraph-pd/docs/api-reference.md @@ -810,18 +810,6 @@ so a PD that merely lost its leader is not restarted. curl http://localhost:8620/actuator/metrics ``` -Raft membership gauges (Prometheus names, scraped from `/actuator/prometheus`) -for alerting on quorum loss: - -| Gauge | Value | -|-------|-------| -| `hg_raft_leader` | `1` on the raft leader, `0` elsewhere | -| `hg_raft_has_leader` | `1` while this PD sees a leader (is inside a quorum), `0` otherwise | -| `hg_raft_alive_peers` | On the leader, the number of peers (itself included) heard from within the election timeout; `NaN` on other nodes | - -A cluster has lost its quorum when `sum(hg_raft_leader) == 0` or when -`hg_raft_has_leader == 0` on every member. - **Response** (Prometheus format): ``` # HELP pd_raft_state Raft state (0=Follower, 1=Candidate, 2=Leader) @@ -838,6 +826,21 @@ pd_store_count{state="Offline"} 0.0 pd_partition_count 36.0 ``` +#### Raft membership gauges + +Exported on `/actuator/prometheus` for alerting on quorum loss: + +| Gauge | Value | +|-------|-------| +| `hg_raft_leader` | `1` on the raft leader, `0` elsewhere | +| `hg_raft_has_leader` | `1` while this PD sees a leader (is inside a quorum), `0` otherwise | +| `hg_raft_alive_peers` | On the leader, the number of peers (itself included) heard from within the election timeout; `NaN` on other nodes | + +A cluster has lost its quorum when `sum(hg_raft_leader) == 0` or when +`hg_raft_has_leader == 0` on every member. Both are briefly true during a +normal election, so alert on them with a `for:` clause longer than the +election timeout rather than on the instantaneous value. + ### Partition API #### List Partitions diff --git a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java index 7b1ed926d5..215020f901 100644 --- a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java +++ b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java @@ -70,7 +70,7 @@ public class RaftEngine { private PDConfig.Raft config; private RaftGroupService raftGroupService; private RpcServer rpcServer; - private Node raftNode; + private volatile Node raftNode; private RaftRpcClient raftRpcClient; public RaftEngine() { @@ -217,7 +217,10 @@ public boolean isLeader() { * signal a readiness probe needs. */ public boolean hasLeader() { - Node node = this.raftNode; + return hasLeader(this.raftNode); + } + + private static boolean hasLeader(Node node) { if (node == null) { return false; } @@ -239,7 +242,7 @@ public boolean isReady() { if (state == null || !state.isActive()) { return false; } - return hasLeader(); + return hasLeader(node); } /** diff --git a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/StoreAPI.java b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/StoreAPI.java index 8f943d9db9..2ee5fc307b 100644 --- a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/StoreAPI.java +++ b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/StoreAPI.java @@ -401,7 +401,7 @@ public Serializable checkHealthy() { * 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 - * (a Store waiting to register, the Server's wait-storage.sh, a Kubernetes readiness probe) + * (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 From c8adc856e299bec4e2bc38202976057f87a3798d Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Thu, 3 Sep 2026 15:31:55 +0530 Subject: [PATCH 3/9] fix(pd): keep compose on liveness, tighten the readiness contract PD's auth interceptor rejects a request by writing an error envelope without setting a status, so every path it does not exclude, including a path that does not exist, answers 200. A healthcheck that only inspects the status code therefore reads a PD too old to carry /v1/ready as ready, which is the same "healthy without a quorum" shape this PR set out to fix. The compose files run published images, so revert their PD healthchecks and the manual verification calls to /v1/health and document what switching them over needs: a body match on "ready":true, and an image that carries the endpoint. Build the /v1/ready body from one RaftEngine.getRaftStatus() snapshot, taken from a single Node reference and a single getLeaderId() read, so a step-down midway cannot report a ready node that knows no leader. Drop the leader's raft address from the body. The endpoint is unauthenticated and the address was the one new disclosure; leadership itself is already published by the hg_raft_leader gauge, and the address stays on the authenticated /v1/members. Call the window in the hg_raft_alive_peers description what jraft measures, the leader lease timeout, which it derives as 90% of the election timeout by default, rather than the election timeout. Assert the empty body in testHealthNeedsNoAuth, since a 200 alone cannot tell an anonymous path from a rejected one. --- docker/README.md | 25 ++++++--- docker/docker-compose-3pd-3store-3server.yml | 2 +- docker/docker-compose-hstore.yml | 2 +- hugegraph-pd/docs/api-reference.md | 20 +++++-- .../apache/hugegraph/pd/raft/RaftEngine.java | 55 +++++++++++++++---- .../hugegraph/pd/metrics/PDMetrics.java | 2 +- .../apache/hugegraph/pd/rest/StoreAPI.java | 23 +++----- .../pd/raft/RaftEngineReadinessTest.java | 28 +++++++++- .../apache/hugegraph/pd/rest/RestApiTest.java | 6 +- 9 files changed, 120 insertions(+), 43 deletions(-) diff --git a/docker/README.md b/docker/README.md index f7e7860b5f..ec6e87119f 100644 --- a/docker/README.md +++ b/docker/README.md @@ -131,7 +131,7 @@ docker compose -f docker-compose-hstore.yml ps Verify PD, Store, Server authentication, and Hubble: ```bash -curl -fsS http://localhost:8620/v1/ready +curl -fsS http://localhost:8620/v1/health curl -fsS http://localhost:8520/v1/health curl -fsS http://localhost:8080/versions test "$(curl -sS -o /dev/null -w '%{http_code}' \ @@ -186,7 +186,7 @@ and Hubble: ```bash for port in 8620 8621 8622; do - curl -fsS "http://localhost:${port}/v1/ready" + curl -fsS "http://localhost:${port}/v1/health" done for port in 8520 8521 8522; do curl -fsS "http://localhost:${port}/v1/health" @@ -204,12 +204,21 @@ 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 leader and -`503` otherwise, so the compose healthchecks gate Stores on `/v1/ready`. A -single PD elects itself; three PDs become ready once two can talk to each -other. `/v1/ready` first ships in 1.8.0: with an older `HUGEGRAPH_VERSION` -the PD healthcheck never passes and the Stores never start, so pin 1.8.0 -or newer, or build the images from source with `docker-compose.dev.yml`. +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 +`/v1/ready` ships from the next release onwards while the files run published +images. Two things to know before pointing them at readiness: + +- Match on the body, not the status code. PD answers `200` with + `{"status":-1,"error":"Unauthorized!"}` on every path its auth interceptor + does not exclude, a path that does not exist included, so a status-only + probe reads a PD too old to have `/v1/ready` as ready. Gate with + `curl -fsS http://localhost:8620/v1/ready | grep -q '"ready":true'` instead. +- Pin `HUGEGRAPH_VERSION` to a release that carries the endpoint, or build the + images from source with `docker-compose.dev.yml`. Open `http://localhost:8088` and sign in as `admin` with the password from `.env`. diff --git a/docker/docker-compose-3pd-3store-3server.yml b/docker/docker-compose-3pd-3store-3server.yml index 87b37dd2ed..6f599c6870 100644 --- a/docker/docker-compose-3pd-3store-3server.yml +++ b/docker/docker-compose-3pd-3store-3server.yml @@ -37,7 +37,7 @@ x-pd-common: &pd-common restart: unless-stopped networks: [hg-net] healthcheck: - test: ["CMD-SHELL", "curl -fsS http://localhost:8620/v1/ready >/dev/null || exit 1"] + test: ["CMD-SHELL", "curl -fsS http://localhost:8620/v1/health >/dev/null || exit 1"] interval: 15s timeout: 10s retries: 30 diff --git a/docker/docker-compose-hstore.yml b/docker/docker-compose-hstore.yml index 3fbc92bc75..d201430692 100644 --- a/docker/docker-compose-hstore.yml +++ b/docker/docker-compose-hstore.yml @@ -44,7 +44,7 @@ services: volumes: - pd-data:/hugegraph-pd/pd_data healthcheck: - test: ["CMD-SHELL", "curl -fsS http://localhost:8620/v1/ready >/dev/null"] + test: ["CMD-SHELL", "curl -fsS http://localhost:8620/v1/health >/dev/null"] interval: 10s timeout: 5s retries: 12 diff --git a/hugegraph-pd/docs/api-reference.md b/hugegraph-pd/docs/api-reference.md index a3814e3c9c..f09ffb61e6 100644 --- a/hugegraph-pd/docs/api-reference.md +++ b/hugegraph-pd/docs/api-reference.md @@ -792,18 +792,28 @@ curl -i http://localhost:8620/v1/ready { "ready": true, "state": "STATE_LEADER", - "leader": "192.168.1.1:8610", "isLeader": true } ``` -A follower reports `"state": "STATE_FOLLOWER"` with the leader's raft address. -When the quorum is lost the PD keeps answering `/v1/health` with `200` but -`/v1/ready` turns into `503` with `"ready": false` and `"leader": null`. +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 +setting a status, so any unknown path answers `200` with +`{"status":-1,"error":"Unauthorized!"}`. A status-only probe therefore reads a +PD older than this endpoint as ready. A shell gate should use +`curl -fsS http://:8620/v1/ready | grep -q '"ready":true'`, and a +Kubernetes `httpGet` probe should be paired with a PD image that carries the +endpoint. + ### Metrics ```bash @@ -834,7 +844,7 @@ Exported on `/actuator/prometheus` for alerting on quorum loss: |-------|-------| | `hg_raft_leader` | `1` on the raft leader, `0` elsewhere | | `hg_raft_has_leader` | `1` while this PD sees a leader (is inside a quorum), `0` otherwise | -| `hg_raft_alive_peers` | On the leader, the number of peers (itself included) heard from within the election timeout; `NaN` on other nodes | +| `hg_raft_alive_peers` | On the leader, the number of peers (itself included) heard from within the leader lease timeout (90% of the election timeout by default); `NaN` on other nodes | A cluster has lost its quorum when `sum(hg_raft_leader) == 0` or when `hg_raft_has_leader == 0` on every member. Both are briefly true during a diff --git a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java index 215020f901..bad5a8ef3b 100644 --- a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java +++ b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java @@ -234,28 +234,63 @@ private static boolean hasLeader(Node node) { * Unlike a plain liveness check this turns false as soon as the quorum is lost. */ public boolean isReady() { + return getRaftStatus().isReady(); + } + + /** + * Take a consistent view of the local raft state. Every field is derived from one + * {@link Node} reference and a single {@code getLeaderId()} read, so a step-down while + * the view is being built cannot report a ready node that knows no leader. + */ + public RaftStatus getRaftStatus() { Node node = this.raftNode; if (node == null) { - return false; + return new RaftStatus(false, State.STATE_UNINITIALIZED.name(), false); } State state = node.getNodeState(); - if (state == null || !state.isActive()) { - return false; - } - return hasLeader(node); + boolean active = state != null && state.isActive(); + return new RaftStatus(active && hasLeader(node), + state == null ? State.STATE_UNINITIALIZED.name() : state.name(), + node.isLeader(true)); } /** - * @return the jraft node state, or null before the raft node has been started + * Immutable view of the raft state behind {@code GET /v1/ready}. It carries no cluster + * addresses: the endpoint is unauthenticated, and the leader's address stays on the + * authenticated {@code /v1/members}. */ - public State getNodeState() { - Node node = this.raftNode; - return node == null ? null : node.getNodeState(); + public static final class RaftStatus { + + private final boolean ready; + private final String state; + private final boolean localLeader; + + RaftStatus(boolean ready, String state, boolean localLeader) { + this.ready = ready; + this.state = state; + this.localLeader = localLeader; + } + + public boolean isReady() { + return this.ready; + } + + /** + * @return the jraft node state name, never null + */ + public String getState() { + return this.state; + } + + public boolean isLocalLeader() { + return this.localLeader; + } } /** * Number of raft peers, this node included, that the leader has heard from within the - * election timeout. Only the leader tracks replication state, so any other node returns -1. + * leader lease timeout, which jraft derives as 90% of the election timeout by default. + * Only the leader tracks replication state, so any other node returns -1. */ public int getAlivePeerCount() { Node node = this.raftNode; diff --git a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/metrics/PDMetrics.java b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/metrics/PDMetrics.java index 6151eade29..54ad2a81db 100644 --- a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/metrics/PDMetrics.java +++ b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/metrics/PDMetrics.java @@ -97,7 +97,7 @@ private void registerRaftMeters() { return alive < 0 ? Double.NaN : alive; }) .description("Number of raft peers, itself included, the leader has heard from " + - "within the election timeout; NaN on non-leader nodes") + "within the leader lease timeout; NaN on non-leader nodes") .register(registry); } diff --git a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/StoreAPI.java b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/StoreAPI.java index 2ee5fc307b..b3c30ca9e1 100644 --- a/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/StoreAPI.java +++ b/hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/StoreAPI.java @@ -49,8 +49,6 @@ import org.springframework.web.bind.annotation.ResponseBody; import org.springframework.web.bind.annotation.RestController; -import com.alipay.sofa.jraft.core.State; -import com.alipay.sofa.jraft.entity.PeerId; import com.google.protobuf.util.JsonFormat; import lombok.Data; @@ -404,22 +402,19 @@ public Serializable checkHealthy() { * (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) + * @return JSON with the readiness flag, the local raft state and whether this node is the + * leader. It carries no cluster addresses, since the endpoint is unauthenticated. */ @GetMapping(value = "/ready", produces = MediaType.APPLICATION_JSON_VALUE) public ResponseEntity> checkReady() { - RaftEngine raft = RaftEngine.getInstance(); - boolean ready = raft.isReady(); - State state = raft.getNodeState(); - PeerId leader = raft.getLeader(); + RaftEngine.RaftStatus status = RaftEngine.getInstance().getRaftStatus(); Map body = new LinkedHashMap<>(); - body.put("ready", ready); - body.put("state", state == null ? State.STATE_UNINITIALIZED.name() : state.name()); - body.put("leader", leader == null || leader.isEmpty() ? null : leader.toString()); - body.put("isLeader", raft.isLeader()); - HttpStatus status = ready ? HttpStatus.OK : HttpStatus.SERVICE_UNAVAILABLE; - return ResponseEntity.status(status).body(body); + body.put("ready", status.isReady()); + body.put("state", status.getState()); + body.put("isLeader", status.isLocalLeader()); + HttpStatus httpStatus = status.isReady() ? HttpStatus.OK + : HttpStatus.SERVICE_UNAVAILABLE; + return ResponseEntity.status(httpStatus).body(body); } } diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/raft/RaftEngineReadinessTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/raft/RaftEngineReadinessTest.java index 72f3ba8617..42ac156688 100644 --- a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/raft/RaftEngineReadinessTest.java +++ b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/raft/RaftEngineReadinessTest.java @@ -72,9 +72,13 @@ public void testNotReadyBeforeRaftNodeStarts() { Assert.assertFalse(engine.isReady()); Assert.assertFalse(engine.hasLeader()); Assert.assertFalse(engine.isLeader()); - Assert.assertNull(engine.getNodeState()); Assert.assertNull(engine.getLeader()); Assert.assertEquals(-1, engine.getAlivePeerCount()); + + RaftEngine.RaftStatus status = engine.getRaftStatus(); + Assert.assertFalse(status.isReady()); + Assert.assertFalse(status.isLocalLeader()); + Assert.assertEquals(State.STATE_UNINITIALIZED.name(), status.getState()); } @Test @@ -86,8 +90,12 @@ public void testLeaderIsReady() { Assert.assertTrue(engine.isReady()); Assert.assertTrue(engine.hasLeader()); Assert.assertTrue(engine.isLeader()); - Assert.assertEquals(State.STATE_LEADER, engine.getNodeState()); Assert.assertEquals(3, engine.getAlivePeerCount()); + + RaftEngine.RaftStatus status = engine.getRaftStatus(); + Assert.assertTrue(status.isReady()); + Assert.assertTrue(status.isLocalLeader()); + Assert.assertEquals(State.STATE_LEADER.name(), status.getState()); } @Test @@ -144,6 +152,22 @@ public void testInactiveStatesAreNotReadyEvenWithLeaderId() { } } + @Test + public void testStatusNeverReportsReadyWithoutALeader() { + // One snapshot, one getLeaderId() read: a step-down cannot yield ready with no leader + stub(State.STATE_FOLLOWER, null, false); + RaftEngine.RaftStatus status = RaftEngine.getInstance().getRaftStatus(); + Assert.assertFalse(status.isReady()); + Assert.assertEquals(State.STATE_FOLLOWER.name(), status.getState()); + Assert.assertFalse(status.isLocalLeader()); + + stub(State.STATE_FOLLOWER, LEADER, false); + status = RaftEngine.getInstance().getRaftStatus(); + Assert.assertTrue(status.isReady()); + Assert.assertEquals(State.STATE_FOLLOWER.name(), status.getState()); + Assert.assertFalse(status.isLocalLeader()); + } + @Test public void testAlivePeerCountSurvivesLeadershipLossRace() { stub(State.STATE_LEADER, SELF, true); diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/RestApiTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/RestApiTest.java index d02b9df54d..76a610dc17 100644 --- a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/RestApiTest.java +++ b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/RestApiTest.java @@ -69,6 +69,9 @@ public void testHealthNeedsNoAuth() throws URISyntaxException, IOException, HttpRequest request = HttpRequest.newBuilder().uri(new URI(url)).GET().build(); HttpResponse response = client.send(request, HttpResponse.BodyHandlers.ofString()); assert response.statusCode() == 200; + // The auth interceptor rejects with 200 and an error envelope, so the status alone + // cannot tell "anonymous" from "rejected". checkHealthy() returns an empty body. + assert response.body().isEmpty() : "expected an empty body, got " + response.body(); } @Test @@ -84,7 +87,8 @@ public void testReadyNeedsNoAuthAndReflectsRaft() throws URISyntaxException, IOE assert obj.getBoolean("ready"); assert obj.getBoolean("isLeader"); assert "STATE_LEADER".equals(obj.getString("state")); - assert !obj.isNull("leader") && !obj.getString("leader").isEmpty(); + // Unauthenticated, so it must not disclose cluster addresses + assert !obj.has("leader") : "the anonymous body must not carry the leader address"; } @Test From ffa13f9857892e8a65e104b2d24a0dfb4fc855fa Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Fri, 4 Sep 2026 01:17:30 +0530 Subject: [PATCH 4/9] docs(store): stop promising a readiness gate the compose files lack The startup ordering list said PD healthchecks probe /v1/ready, but c8adc85 put both compose files back on /v1/health and this line was missed, so the guide described a quorum gate that does not exist. Name /v1/health, say it is liveness only, and point at docker/README.md for what pointing the healthchecks at /v1/ready would require. --- hugegraph-store/docs/deployment-guide.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/hugegraph-store/docs/deployment-guide.md b/hugegraph-store/docs/deployment-guide.md index 148dbb5ceb..f002046927 100644 --- a/hugegraph-store/docs/deployment-guide.md +++ b/hugegraph-store/docs/deployment-guide.md @@ -719,10 +719,15 @@ environment: ``` **Startup ordering** is enforced via `depends_on` with `condition: service_healthy`: -1. PD nodes start first and must pass healthchecks (`/v1/ready`, answered `200` only once the PD sees a raft leader) +1. PD nodes start first and must pass healthchecks (`/v1/health`, liveness only) 2. Store nodes start after all PD nodes are healthy 3. Server nodes start after all Store nodes are healthy +`/v1/health` answers `200` as soon as the PD REST listener is up, so step 1 does +not wait for a raft quorum to form. PD also serves `/v1/ready`, which answers +`200` only while the PD sees a raft leader; `docker/README.md` covers what +pointing the healthchecks at it requires. + > **Note**: The deprecated env var names (`GRPC_HOST`, `RAFT_ADDRESS`, `RAFT_PEERS`, `PD_ADDRESS`, `BACKEND`, `PD_PEERS`) still work but log a warning. Use the `HG_*` prefixed names for new deployments. **Deploy**: From 7a6dbb8728221524594a78d04dd1990b917ef78d Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Fri, 4 Sep 2026 10:08:09 +0530 Subject: [PATCH 5/9] docs(pd): correct the active-state set and date the auth claim State.isActive() is ordinal() < STATE_ERROR over LEADER, TRANSFERRING, CANDIDATE, FOLLOWER, so a candidate is active too and the isReady() javadoc was a state short. Name the set jraft actually uses. Say what the candidate test exercises. jraft clears the leader id before starting an election, so testCandidateWithoutLeaderIsNotReady passes on the missing leader rather than on the state, and a second case records that a candidate does count as active. Mark the empty-peer test as guarding the Node contract, since NodeImpl maps an empty peer to null. Date the interceptor behaviour instead of asserting it as a property of PD. As of 1.7.0 a refusal carries 200 and an error envelope, which is what makes a status-only probe read an older PD as ready, but the body match holds whichever status a refusal carries. Same wording in the docker README and in testHealthNeedsNoAuth. Note that the HEALTHCHECK baked into hugegraph-pd/Dockerfile is on liveness as well. Both compose files override it, so it governs docker run and anything else inheriting the image probe. --- docker/README.md | 10 ++++++++-- hugegraph-pd/docs/api-reference.md | 12 +++++++----- .../org/apache/hugegraph/pd/raft/RaftEngine.java | 3 ++- .../hugegraph/pd/raft/RaftEngineReadinessTest.java | 14 +++++++++++++- .../org/apache/hugegraph/pd/rest/RestApiTest.java | 5 +++-- 5 files changed, 33 insertions(+), 11 deletions(-) diff --git a/docker/README.md b/docker/README.md index ec6e87119f..5adff0f72f 100644 --- a/docker/README.md +++ b/docker/README.md @@ -212,14 +212,20 @@ The healthchecks in these files still gate on `/v1/health`, because `/v1/ready` ships from the next release onwards while the files run published images. Two things to know before pointing them at readiness: -- Match on the body, not the status code. PD answers `200` with +- Match on the body, not the status code. As of 1.7.0 PD answers `200` with `{"status":-1,"error":"Unauthorized!"}` on every path its auth interceptor does not exclude, a path that does not exist included, so a status-only - probe reads a PD too old to have `/v1/ready` as ready. Gate with + probe reads a PD too old to have `/v1/ready` as ready. The body match holds + whichever status a refusal carries. Gate with `curl -fsS http://localhost:8620/v1/ready | grep -q '"ready":true'` instead. - Pin `HUGEGRAPH_VERSION` to a release that carries the endpoint, or build the images from source with `docker-compose.dev.yml`. +The `HEALTHCHECK` baked into `hugegraph-pd/Dockerfile` is `/v1/health` as well. +Both compose files override it, so it governs `docker run` and anything else +inheriting the image probe, and those keep reading a PD without a quorum as +healthy. + Open `http://localhost:8088` and sign in as `admin` with the password from `.env`. diff --git a/hugegraph-pd/docs/api-reference.md b/hugegraph-pd/docs/api-reference.md index f09ffb61e6..33750c7ad8 100644 --- a/hugegraph-pd/docs/api-reference.md +++ b/hugegraph-pd/docs/api-reference.md @@ -805,11 +805,13 @@ 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 -setting a status, so any unknown path answers `200` with -`{"status":-1,"error":"Unauthorized!"}`. A status-only probe therefore reads a -PD older than this endpoint as ready. A shell gate should use +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://:8620/v1/ready | grep -q '"ready":true'`, and a Kubernetes `httpGet` probe should be paired with a PD image that carries the endpoint. diff --git a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java index bad5a8ef3b..bd094fbafe 100644 --- a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java +++ b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java @@ -230,7 +230,8 @@ private static boolean hasLeader(Node node) { /** * 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. + * is in an active state, which jraft's {@code State.isActive()} takes to mean leader, + * transferring, candidate or follower, and sees a leader. * Unlike a plain liveness check this turns false as soon as the quorum is lost. */ public boolean isReady() { diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/raft/RaftEngineReadinessTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/raft/RaftEngineReadinessTest.java index 42ac156688..8f02207c1f 100644 --- a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/raft/RaftEngineReadinessTest.java +++ b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/raft/RaftEngineReadinessTest.java @@ -123,6 +123,8 @@ public void testFollowerWithoutLeaderIsNotReady() { @Test public void testEmptyLeaderIdIsNotReady() { + // Defensive: NodeImpl.getLeaderId() maps an empty peer to null, so this guards the + // Node contract rather than a value the real implementation returns stub(State.STATE_FOLLOWER, PeerId.emptyPeer(), false); RaftEngine engine = RaftEngine.getInstance(); @@ -131,11 +133,21 @@ public void testEmptyLeaderIdIsNotReady() { } @Test - public void testCandidateIsNotReady() { + public void testCandidateWithoutLeaderIsNotReady() { + // The only candidate shape jraft reaches: NodeImpl clears the leader id before it + // starts an election, so it is the missing leader, not the state, that holds it back stub(State.STATE_CANDIDATE, null, false); Assert.assertFalse(RaftEngine.getInstance().isReady()); } + @Test + public void testCandidateCountsAsActive() { + // Records the scope of the state check: State.isActive() is ordinal() < STATE_ERROR, + // so a candidate is active and would read as ready if it still knew a leader + stub(State.STATE_CANDIDATE, LEADER, false); + Assert.assertTrue(RaftEngine.getInstance().isReady()); + } + @Test public void testTransferringLeaderIsReady() { stub(State.STATE_TRANSFERRING, SELF, true); diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/RestApiTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/RestApiTest.java index 76a610dc17..a145b788a2 100644 --- a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/RestApiTest.java +++ b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/RestApiTest.java @@ -69,8 +69,9 @@ public void testHealthNeedsNoAuth() throws URISyntaxException, IOException, HttpRequest request = HttpRequest.newBuilder().uri(new URI(url)).GET().build(); HttpResponse response = client.send(request, HttpResponse.BodyHandlers.ofString()); assert response.statusCode() == 200; - // The auth interceptor rejects with 200 and an error envelope, so the status alone - // cannot tell "anonymous" from "rejected". checkHealthy() returns an empty body. + // A 200 alone does not prove the path is anonymous: as of 1.7.0 the auth interceptor + // refuses with 200 and an error envelope. checkHealthy() returns an empty body, which + // separates the two whichever status a refusal carries. assert response.body().isEmpty() : "expected an empty body, got " + response.body(); } From 7354c219528bf00ab8e246fd1ae081c02f014a1f Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Fri, 4 Sep 2026 10:15:36 +0530 Subject: [PATCH 6/9] refactor(pd): drop readiness surface nothing calls RaftEngine.isReady() had no caller outside its own tests: the endpoint reads getRaftStatus() and the gauges read isLeader() and hasLeader(). Remove it and keep its note on the active-state set where isActive() is actually called. testStatusNeverReportsReadyWithoutALeader only repeated the two follower shapes the tests either side of it already cover, so drop it and let the rest assert through the snapshot, which is the path production takes. Reduce wait_for_pd_ready to the gate the docs recommend, curl -f piped into grep. -f rejects the 503 and the body match rejects a 200 that is an auth envelope, so the hand-rolled status parsing bought nothing. --- .../apache/hugegraph/pd/raft/RaftEngine.java | 15 +++------ .../pd/raft/RaftEngineReadinessTest.java | 32 ++++--------------- .../travis/test-start-hugegraph-pd.sh | 13 +++----- 3 files changed, 16 insertions(+), 44 deletions(-) diff --git a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java index bd094fbafe..6b744a7003 100644 --- a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java +++ b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java @@ -228,20 +228,15 @@ private static boolean hasLeader(Node node) { 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, which jraft's {@code State.isActive()} takes to mean leader, - * transferring, candidate or follower, and sees a leader. - * Unlike a plain liveness check this turns false as soon as the quorum is lost. - */ - public boolean isReady() { - return getRaftStatus().isReady(); - } - /** * Take a consistent view of the local raft state. Every field is derived from one * {@link Node} reference and a single {@code getLeaderId()} read, so a step-down while * the view is being built cannot report a ready node that knows no leader. + *

+ * A node is ready when it has been started, is in an active state, which jraft's + * {@code State.isActive()} takes to mean leader, transferring, candidate or follower, + * and sees a leader. Unlike a plain liveness check this turns false as soon as the + * quorum is lost. */ public RaftStatus getRaftStatus() { Node node = this.raftNode; diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/raft/RaftEngineReadinessTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/raft/RaftEngineReadinessTest.java index 8f02207c1f..565d017f45 100644 --- a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/raft/RaftEngineReadinessTest.java +++ b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/raft/RaftEngineReadinessTest.java @@ -69,7 +69,6 @@ public void testNotReadyBeforeRaftNodeStarts() { Whitebox.setInternalState(RaftEngine.getInstance(), "raftNode", null); RaftEngine engine = RaftEngine.getInstance(); - Assert.assertFalse(engine.isReady()); Assert.assertFalse(engine.hasLeader()); Assert.assertFalse(engine.isLeader()); Assert.assertNull(engine.getLeader()); @@ -87,7 +86,6 @@ public void testLeaderIsReady() { when(mockNode.listAlivePeers()).thenReturn(Arrays.asList(SELF, LEADER, OTHER)); RaftEngine engine = RaftEngine.getInstance(); - Assert.assertTrue(engine.isReady()); Assert.assertTrue(engine.hasLeader()); Assert.assertTrue(engine.isLeader()); Assert.assertEquals(3, engine.getAlivePeerCount()); @@ -103,7 +101,7 @@ public void testFollowerWithLeaderIsReady() { stub(State.STATE_FOLLOWER, LEADER, false); RaftEngine engine = RaftEngine.getInstance(); - Assert.assertTrue(engine.isReady()); + Assert.assertTrue(engine.getRaftStatus().isReady()); Assert.assertTrue(engine.hasLeader()); Assert.assertFalse(engine.isLeader()); Assert.assertEquals(LEADER, engine.getLeader()); @@ -117,7 +115,7 @@ public void testFollowerWithoutLeaderIsNotReady() { stub(State.STATE_FOLLOWER, null, false); RaftEngine engine = RaftEngine.getInstance(); - Assert.assertFalse(engine.isReady()); + Assert.assertFalse(engine.getRaftStatus().isReady()); Assert.assertFalse(engine.hasLeader()); } @@ -128,7 +126,7 @@ public void testEmptyLeaderIdIsNotReady() { stub(State.STATE_FOLLOWER, PeerId.emptyPeer(), false); RaftEngine engine = RaftEngine.getInstance(); - Assert.assertFalse(engine.isReady()); + Assert.assertFalse(engine.getRaftStatus().isReady()); Assert.assertFalse(engine.hasLeader()); } @@ -137,7 +135,7 @@ public void testCandidateWithoutLeaderIsNotReady() { // The only candidate shape jraft reaches: NodeImpl clears the leader id before it // starts an election, so it is the missing leader, not the state, that holds it back stub(State.STATE_CANDIDATE, null, false); - Assert.assertFalse(RaftEngine.getInstance().isReady()); + Assert.assertFalse(RaftEngine.getInstance().getRaftStatus().isReady()); } @Test @@ -145,13 +143,13 @@ public void testCandidateCountsAsActive() { // Records the scope of the state check: State.isActive() is ordinal() < STATE_ERROR, // so a candidate is active and would read as ready if it still knew a leader stub(State.STATE_CANDIDATE, LEADER, false); - Assert.assertTrue(RaftEngine.getInstance().isReady()); + Assert.assertTrue(RaftEngine.getInstance().getRaftStatus().isReady()); } @Test public void testTransferringLeaderIsReady() { stub(State.STATE_TRANSFERRING, SELF, true); - Assert.assertTrue(RaftEngine.getInstance().isReady()); + Assert.assertTrue(RaftEngine.getInstance().getRaftStatus().isReady()); } @Test @@ -160,26 +158,10 @@ public void testInactiveStatesAreNotReadyEvenWithLeaderId() { State.STATE_SHUTTING, State.STATE_SHUTDOWN}) { stub(state, LEADER, false); Assert.assertFalse("state " + state + " must not be ready", - RaftEngine.getInstance().isReady()); + RaftEngine.getInstance().getRaftStatus().isReady()); } } - @Test - public void testStatusNeverReportsReadyWithoutALeader() { - // One snapshot, one getLeaderId() read: a step-down cannot yield ready with no leader - stub(State.STATE_FOLLOWER, null, false); - RaftEngine.RaftStatus status = RaftEngine.getInstance().getRaftStatus(); - Assert.assertFalse(status.isReady()); - Assert.assertEquals(State.STATE_FOLLOWER.name(), status.getState()); - Assert.assertFalse(status.isLocalLeader()); - - stub(State.STATE_FOLLOWER, LEADER, false); - status = RaftEngine.getInstance().getRaftStatus(); - Assert.assertTrue(status.isReady()); - Assert.assertEquals(State.STATE_FOLLOWER.name(), status.getState()); - Assert.assertFalse(status.isLocalLeader()); - } - @Test public void testAlivePeerCountSurvivesLeadershipLossRace() { stub(State.STATE_LEADER, SELF, true); diff --git a/hugegraph-server/hugegraph-dist/src/assembly/travis/test-start-hugegraph-pd.sh b/hugegraph-server/hugegraph-dist/src/assembly/travis/test-start-hugegraph-pd.sh index 3c62816e1b..0a78c30da2 100755 --- a/hugegraph-server/hugegraph-dist/src/assembly/travis/test-start-hugegraph-pd.sh +++ b/hugegraph-server/hugegraph-dist/src/assembly/travis/test-start-hugegraph-pd.sh @@ -111,18 +111,13 @@ wait_for_pd() { return 1 } -# Wait until PD readiness endpoint answers 200 with ready=true, or timeout. -# /v1/ready must need no credentials and must reflect raft: a single-node -# raft group elects itself, so it has to become ready shortly after /v1/health. +# Wait until PD readiness answers, or timeout. This is the gate the docs recommend: +# -f rejects the 503, the body match rejects a 200 that is an auth envelope rather +# than a readiness answer. No credentials, and a single-node group elects itself. wait_for_pd_ready() { local elapsed=0 while (( elapsed < STARTUP_WAIT )); do - local body status - body=$(curl -s -w '\n%{http_code}' "$PD_URL/v1/ready" 2>/dev/null || echo "000") - status=${body##*$'\n'} - if [[ "$status" == "200" ]] && [[ "$body" == *'"ready":true'* ]]; then - return 0 - fi + curl -fsS "$PD_URL/v1/ready" 2>/dev/null | grep -q '"ready":true' && return 0 sleep 2 elapsed=$((elapsed + 2)) done From 2b13aaa7cac9c155f498e960879b83ea2645a538 Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Fri, 4 Sep 2026 10:38:51 +0530 Subject: [PATCH 7/9] test(pd): pin the probe endpoints outside the auth interceptor The exclusion list was the one line this change left uncovered, and it carries a contract worth holding: if /v1/ready slips back behind the interceptor, PD answers a probe with 200 and an auth envelope instead of a readiness answer, so every healthcheck matching on the body holds forever while the status still looks healthy. Drive AuthenticationConfigurer with a real InterceptorRegistry and assert through MappedInterceptor.matches(), so the test states the behaviour, that these paths are not intercepted, rather than the literal patterns. /v1/members and friends stay intercepted in the same test. Verified by mutation: dropping /v1/ready from the list fails testProbeEndpointsAreAnonymous. --- .../hugegraph/pd/core/PDCoreSuiteTest.java | 2 + .../AuthenticationConfigurerTest.java | 84 +++++++++++++++++++ 2 files changed, 86 insertions(+) create mode 100644 hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/interceptor/AuthenticationConfigurerTest.java diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java index bdacf7d371..4f6d3af386 100644 --- a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java +++ b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java @@ -23,6 +23,7 @@ import org.apache.hugegraph.pd.raft.RaftEngineIpAuthIntegrationTest; import org.apache.hugegraph.pd.raft.RaftEngineLeaderAddressTest; import org.apache.hugegraph.pd.raft.RaftEngineReadinessTest; +import org.apache.hugegraph.pd.rest.interceptor.AuthenticationConfigurerTest; import org.junit.runner.RunWith; import org.junit.runners.Suite; @@ -45,6 +46,7 @@ RaftEngineIpAuthIntegrationTest.class, RaftEngineLeaderAddressTest.class, RaftEngineReadinessTest.class, + AuthenticationConfigurerTest.class, // StoreNodeServiceTest.class, }) @Slf4j diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/interceptor/AuthenticationConfigurerTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/interceptor/AuthenticationConfigurerTest.java new file mode 100644 index 0000000000..726b7df816 --- /dev/null +++ b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/interceptor/AuthenticationConfigurerTest.java @@ -0,0 +1,84 @@ +/* + * 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.hugegraph.pd.rest.interceptor; + +import java.util.List; + +import org.junit.Assert; +import org.junit.Test; +import org.springframework.util.AntPathMatcher; +import org.springframework.util.PathMatcher; +import org.springframework.web.servlet.config.annotation.InterceptorRegistry; +import org.springframework.web.servlet.handler.MappedInterceptor; + +import static org.mockito.Mockito.mock; + +/** + * The probe endpoints have to stay outside the auth interceptor. If one of them slips back + * behind it, PD answers a probe with 200 and an error envelope instead of a readiness + * answer, so every healthcheck gating on the body holds forever while the status still + * looks fine. That failure is silent, hence a check here rather than only in the live + * REST suite. + */ +public class AuthenticationConfigurerTest { + + private static final PathMatcher MATCHER = new AntPathMatcher(); + + /** + * {@code InterceptorRegistry.getInterceptors()} is protected, so read it from a subclass. + */ + private static final class TestRegistry extends InterceptorRegistry { + + List registered() { + return getInterceptors(); + } + } + + private static MappedInterceptor authInterceptor() { + AuthenticationConfigurer configurer = new AuthenticationConfigurer(); + configurer.restAuthentication = mock(RestAuthentication.class); + + TestRegistry registry = new TestRegistry(); + configurer.addInterceptors(registry); + + List registered = registry.registered(); + Assert.assertEquals(1, registered.size()); + return (MappedInterceptor) registered.get(0); + } + + @Test + public void testProbeEndpointsAreAnonymous() { + // A kubelet probe and a compose healthcheck cannot present credentials + MappedInterceptor auth = authInterceptor(); + Assert.assertFalse("/v1/ready must not be intercepted", + auth.matches("/v1/ready", MATCHER)); + Assert.assertFalse("/v1/health must not be intercepted", + auth.matches("/v1/health", MATCHER)); + Assert.assertFalse("/actuator/prometheus must not be intercepted", + auth.matches("/actuator/prometheus", MATCHER)); + } + + @Test + public void testEverythingElseStaysAuthenticated() { + MappedInterceptor auth = authInterceptor(); + Assert.assertTrue("/v1/members carries cluster addresses and must stay authenticated", + auth.matches("/v1/members", MATCHER)); + Assert.assertTrue(auth.matches("/v1/stores", MATCHER)); + Assert.assertTrue(auth.matches("/v1/members/change", MATCHER)); + } +} From 27f700950a5cde3f137ee54b4bef586fe7705908 Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Fri, 4 Sep 2026 10:41:10 +0530 Subject: [PATCH 8/9] test(pd): run the interceptor check in the rest suite The pd job runs mvn clean package between the core tests and the codecov upload, which wipes the exec file the core run appended to, so only the client and rest profiles reach the report. Move the check to PDRestSuiteTest, where it also sits closer to the REST layer it covers. --- .../main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java | 2 -- .../main/java/org/apache/hugegraph/pd/rest/PDRestSuiteTest.java | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java index 4f6d3af386..bdacf7d371 100644 --- a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java +++ b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java @@ -23,7 +23,6 @@ import org.apache.hugegraph.pd.raft.RaftEngineIpAuthIntegrationTest; import org.apache.hugegraph.pd.raft.RaftEngineLeaderAddressTest; import org.apache.hugegraph.pd.raft.RaftEngineReadinessTest; -import org.apache.hugegraph.pd.rest.interceptor.AuthenticationConfigurerTest; import org.junit.runner.RunWith; import org.junit.runners.Suite; @@ -46,7 +45,6 @@ RaftEngineIpAuthIntegrationTest.class, RaftEngineLeaderAddressTest.class, RaftEngineReadinessTest.class, - AuthenticationConfigurerTest.class, // StoreNodeServiceTest.class, }) @Slf4j diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/PDRestSuiteTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/PDRestSuiteTest.java index 5dba561948..ab56b6b064 100644 --- a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/PDRestSuiteTest.java +++ b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/PDRestSuiteTest.java @@ -17,6 +17,7 @@ package org.apache.hugegraph.pd.rest; +import org.apache.hugegraph.pd.rest.interceptor.AuthenticationConfigurerTest; import org.apache.hugegraph.pd.util.StoreRestAddressUtilTest; import org.junit.runner.RunWith; import org.junit.runners.Suite; @@ -26,6 +27,7 @@ @RunWith(Suite.class) @Suite.SuiteClasses({ RestApiTest.class, + AuthenticationConfigurerTest.class, StoreRestAddressUtilTest.class, }) @Slf4j From b1d07b4cc004d95463fbbb3751a5c0f442581dfb Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Fri, 4 Sep 2026 10:44:19 +0530 Subject: [PATCH 9/9] Revert "test(pd): run the interceptor check in the rest suite" This reverts commit 27f7009. Its reason was wrong: I read the pd job from a stale checkout, where mvn clean package sat between the core tests and the upload. On this branch Package runs first, then the four test profiles append to one exec, and the aggregate report is generated after the rest test, so core-test coverage reaches Codecov either way. With that settled the core suite is the better home. The check is a pure unit test, and the rest profile needs a live PD for the rest of its suite. --- .../main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java | 2 ++ .../main/java/org/apache/hugegraph/pd/rest/PDRestSuiteTest.java | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java index bdacf7d371..4f6d3af386 100644 --- a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java +++ b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java @@ -23,6 +23,7 @@ import org.apache.hugegraph.pd.raft.RaftEngineIpAuthIntegrationTest; import org.apache.hugegraph.pd.raft.RaftEngineLeaderAddressTest; import org.apache.hugegraph.pd.raft.RaftEngineReadinessTest; +import org.apache.hugegraph.pd.rest.interceptor.AuthenticationConfigurerTest; import org.junit.runner.RunWith; import org.junit.runners.Suite; @@ -45,6 +46,7 @@ RaftEngineIpAuthIntegrationTest.class, RaftEngineLeaderAddressTest.class, RaftEngineReadinessTest.class, + AuthenticationConfigurerTest.class, // StoreNodeServiceTest.class, }) @Slf4j diff --git a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/PDRestSuiteTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/PDRestSuiteTest.java index ab56b6b064..5dba561948 100644 --- a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/PDRestSuiteTest.java +++ b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/PDRestSuiteTest.java @@ -17,7 +17,6 @@ package org.apache.hugegraph.pd.rest; -import org.apache.hugegraph.pd.rest.interceptor.AuthenticationConfigurerTest; import org.apache.hugegraph.pd.util.StoreRestAddressUtilTest; import org.junit.runner.RunWith; import org.junit.runners.Suite; @@ -27,7 +26,6 @@ @RunWith(Suite.class) @Suite.SuiteClasses({ RestApiTest.class, - AuthenticationConfigurerTest.class, StoreRestAddressUtilTest.class, }) @Slf4j