From 4dd7e71c978e7c11f29be8bcad31b5d154da614c Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Wed, 2 Sep 2026 19:30:16 +0530 Subject: [PATCH 01/23] 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 02/23] 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 03/23] 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 04/23] 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 05/23] 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 06/23] 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 07/23] 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 08/23] 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 09/23] 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 From f4fb4ea532fa5d9139870d19eb78007d3b0970e0 Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Sat, 5 Sep 2026 11:06:48 +0530 Subject: [PATCH 10/23] fix(pd): drop the interceptor test a full install cannot compile hg-pd-service is repackaged by spring-boot-maven-plugin, so the artifact a full mvn install leaves behind is an executable jar with classes under BOOT-INF/classes, invisible to javac. A mvn test -am reactor compiles hg-pd-test against target/classes instead, which is why the test passed in the pd job and locally while build-commons and both macOS server jobs failed with cannot-find-symbol. Moving the test into hg-pd-service would not help: every CI package step runs -Dmaven.test.skip=true and the pd profiles only execute suites in hg-pd-test, so it would never run. The anonymity of /v1/health and /v1/ready stays pinned by the live REST tests, which asserted it before the unit test existed. --- .../hugegraph/pd/core/PDCoreSuiteTest.java | 2 - .../AuthenticationConfigurerTest.java | 84 ------------------- 2 files changed, 86 deletions(-) delete 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 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/interceptor/AuthenticationConfigurerTest.java b/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/interceptor/AuthenticationConfigurerTest.java deleted file mode 100644 index 726b7df816..0000000000 --- a/hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/interceptor/AuthenticationConfigurerTest.java +++ /dev/null @@ -1,84 +0,0 @@ -/* - * 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 70744406d48c4d7ccd3a535439d4f7f6747df6a1 Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Sat, 5 Sep 2026 11:08:14 +0530 Subject: [PATCH 11/23] fix(pd): make the raft snapshot single-read and portable Derive the snapshot's leader flag from the state read one line above instead of a third locked node call. jraft's isLeader(true) is exactly state == STATE_LEADER, so the value is unchanged while the fields can no longer contradict each other, which is what the javadoc promised. Name the gauges hg.raft.has.leader and hg.raft.alive.peers. Micrometer renders both to the same Prometheus names as before, but dot-separated segments keep other registries from mixing separators. Capture the readiness body in wait_for_pd_ready instead of piping into grep -q: under the script's pipefail a SIGPIPE-killed curl could misread a ready PD, and wait_for_pd already uses the capture shape. --- .../java/org/apache/hugegraph/pd/raft/RaftEngine.java | 11 +++++++---- .../org/apache/hugegraph/pd/metrics/PDMetrics.java | 4 ++-- .../src/assembly/travis/test-start-hugegraph-pd.sh | 8 ++++++-- 3 files changed, 15 insertions(+), 8 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 6b744a7003..b805ea75f6 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 @@ -229,9 +229,12 @@ private static boolean hasLeader(Node node) { } /** - * 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. + * Take a consistent view of the local raft state: one {@link Node} reference, one + * {@code getNodeState()} read and one {@code getLeaderId()} read, with the leader flag + * derived from that same state, so a step-down while the view is being built cannot + * report a ready node that knows no leader or a leader that is not in leader state. + * The derivation matches jraft, whose {@code isLeader(true)} is exactly + * {@code state == STATE_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, @@ -247,7 +250,7 @@ public RaftStatus getRaftStatus() { boolean active = state != null && state.isActive(); return new RaftStatus(active && hasLeader(node), state == null ? State.STATE_UNINITIALIZED.name() : state.name(), - node.isLeader(true)); + State.STATE_LEADER == state); } /** 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 54ad2a81db..6f7660bc12 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 @@ -89,10 +89,10 @@ private void registerRaftMeters() { 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) + 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", () -> { + Gauge.builder(PREFIX + ".raft.alive.peers", () -> { int alive = raft.getAlivePeerCount(); return alive < 0 ? Double.NaN : alive; }) 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 0a78c30da2..7a3f70b602 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,13 +111,17 @@ wait_for_pd() { return 1 } -# Wait until PD readiness answers, or timeout. This is the gate the docs recommend: +# Wait until PD readiness answers, or timeout. Same 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. +# Captured rather than piped: under pipefail a grep -q SIGPIPE could misread a +# ready PD, and wait_for_pd() above uses the same shape. wait_for_pd_ready() { local elapsed=0 while (( elapsed < STARTUP_WAIT )); do - curl -fsS "$PD_URL/v1/ready" 2>/dev/null | grep -q '"ready":true' && return 0 + local body + body=$(curl -fsS "$PD_URL/v1/ready" 2>/dev/null || true) + grep -q '"ready":true' <<<"$body" && return 0 sleep 2 elapsed=$((elapsed + 2)) done From a3b939525f00b5462ef4305edeb46ab7d62ca691 Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Sat, 5 Sep 2026 14:13:41 +0530 Subject: [PATCH 12/23] fix(pd): serve readiness from raft callbacks, not the node lock Tested on Kubernetes against three PDs, the first /v1/ready request after two pods were deleted took 9.79 s to return its 503, and with a 2 s client timeout every later sample inside the leaderless window timed out: while jraft runs an election it holds the node lock as it reconnects to peers that no longer answer, and the probe read getNodeState() and getLeaderId() under that lock. Keep a volatile copy of the last announced state and leader visibility in RaftStateMachine, written by onLeaderStart, onLeaderStop, onStartFollowing, onStopFollowing, onError and onShutdown, and serve getRaftStatus(), hasLeader() and the gauges from it. The probe path no longer touches the node at all, which the unit tests now pin with verifyNoInteractions; getAlivePeerCount() checks the lock-free term flag first and only calls listAlivePeers() on a settled leader. jraft emits no callback for candidacy or leadership transfer, so state reports the last announced role and a candidate reads as a follower without a leader, the same not-ready answer as before. Measured with two blackholed peers so every reconnect hangs, the shape of the Kubernetes fault: 15 consecutive /v1/ready samples during the perpetual election all answered 503 in under 21 ms, and a prometheus scrape took 51 ms. --- hugegraph-pd/docs/api-reference.md | 6 + .../apache/hugegraph/pd/raft/RaftEngine.java | 54 +++----- .../hugegraph/pd/raft/RaftStateMachine.java | 32 +++++ .../hugegraph/pd/metrics/PDMetrics.java | 2 +- .../pd/raft/RaftEngineReadinessTest.java | 128 ++++++++++-------- 5 files changed, 134 insertions(+), 88 deletions(-) diff --git a/hugegraph-pd/docs/api-reference.md b/hugegraph-pd/docs/api-reference.md index 33750c7ad8..2ad3aa49e5 100644 --- a/hugegraph-pd/docs/api-reference.md +++ b/hugegraph-pd/docs/api-reference.md @@ -801,6 +801,12 @@ 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`. +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: jraft +emits no callback for candidacy or leadership transfer, so a candidate reports +`STATE_FOLLOWER` with `"ready": false`. + 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. 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 b805ea75f6..6ac3d1c7ef 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 @@ -209,48 +209,36 @@ public boolean isLeader() { } /** - * Whether this node currently knows a raft leader. + * Whether this node currently sees 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 + * A follower only keeps its leader while heartbeats keep arriving inside the election + * timeout, and a leader only keeps its role while it can reach a quorum. Seeing a leader * therefore means this node is part of a quorum from its own point of view, which is the - * signal a readiness probe needs. + * signal a readiness probe needs. Served from the state machine callbacks, not from the + * raft node, so it never waits on the node lock. */ 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(); + return this.raftNode != null && this.stateMachine.seesLeader(); } /** - * Take a consistent view of the local raft state: one {@link Node} reference, one - * {@code getNodeState()} read and one {@code getLeaderId()} read, with the leader flag - * derived from that same state, so a step-down while the view is being built cannot - * report a ready node that knows no leader or a leader that is not in leader state. - * The derivation matches jraft, whose {@code isLeader(true)} is exactly - * {@code state == STATE_LEADER}. + * Take a view of the local raft state from the volatile copies the state machine + * callbacks maintain, never from the raft node itself. During an election jraft holds + * the node lock while it reconnects to peers, so a probe that read the node stalled for + * the connect timeout instead of answering its 503 promptly. All fields derive from the + * same callback-written values, so they cannot contradict each other. *

- * 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. + * The state reported is the last one a callback announced: leader, follower, error or + * shutdown. jraft emits no callback for candidacy or leadership transfer, so a candidate + * reads as a follower that sees no leader, which yields the same not-ready answer. */ public RaftStatus getRaftStatus() { - Node node = this.raftNode; - if (node == null) { + if (this.raftNode == null) { return new RaftStatus(false, State.STATE_UNINITIALIZED.name(), false); } - State state = node.getNodeState(); - boolean active = state != null && state.isActive(); - return new RaftStatus(active && hasLeader(node), - state == null ? State.STATE_UNINITIALIZED.name() : state.name(), - State.STATE_LEADER == state); + State state = this.stateMachine.getProbeState(); + return new RaftStatus(state.isActive() && this.stateMachine.seesLeader(), + state.name(), State.STATE_LEADER == state); } /** @@ -289,11 +277,13 @@ public boolean isLocalLeader() { /** * Number of raft peers, this node included, that the leader has heard from within the * 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. + * Only the leader tracks replication state, so any other node returns -1. The leader + * check is the state machine's lock-free term flag, so a scrape on a non-leader never + * waits on the node lock; {@code listAlivePeers} itself only runs on a settled leader. */ public int getAlivePeerCount() { Node node = this.raftNode; - if (node == null || !node.isLeader(true)) { + if (node == null || !this.stateMachine.isLeader()) { return -1; } try { diff --git a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftStateMachine.java b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftStateMachine.java index 6fad3347fa..1e7cedb2cc 100644 --- a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftStateMachine.java +++ b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftStateMachine.java @@ -36,6 +36,7 @@ import com.alipay.sofa.jraft.Iterator; import com.alipay.sofa.jraft.Status; import com.alipay.sofa.jraft.conf.Configuration; +import com.alipay.sofa.jraft.core.State; import com.alipay.sofa.jraft.core.StateMachineAdapter; import com.alipay.sofa.jraft.entity.LeaderChangeContext; import com.alipay.sofa.jraft.entity.LocalFileMetaOutter; @@ -56,6 +57,11 @@ public class RaftStateMachine extends StateMachineAdapter { private static final String SNAPSHOT_DIR_NAME = "snapshot"; private static final String SNAPSHOT_ARCHIVE_NAME = "snapshot.zip"; private final AtomicLong leaderTerm = new AtomicLong(-1); + // Kept for the readiness probe and the raft gauges, which must answer without + // touching the raft node: during an election jraft holds the node lock while it + // reconnects to peers, and a reader stalls for the connect timeout. + private volatile State probeState = State.STATE_UNINITIALIZED; + private volatile boolean seesLeader = false; private List taskHandlers; private List stateListeners; @@ -76,6 +82,21 @@ public boolean isLeader() { return this.leaderTerm.get() > 0; } + /** + * The last node state a raft callback announced. jraft emits no callback for candidacy + * or leadership transfer, so a candidate reads as a follower that sees no leader. + */ + public State getProbeState() { + return this.probeState; + } + + /** + * Whether the last raft callback left this node with a visible leader. + */ + public boolean seesLeader() { + return this.seesLeader; + } + @Override public void onApply(Iterator iter) { while (iter.hasNext()) { @@ -105,17 +126,23 @@ public void onApply(Iterator iter) { @Override public void onError(final RaftException e) { + this.probeState = State.STATE_ERROR; + this.seesLeader = false; log.error("Raft StateMachine on error {}", e); } @Override public void onShutdown() { + this.probeState = State.STATE_SHUTDOWN; + this.seesLeader = false; super.onShutdown(); } @Override public void onLeaderStart(final long term) { this.leaderTerm.set(term); + this.probeState = State.STATE_LEADER; + this.seesLeader = true; super.onLeaderStart(term); log.info("Raft becomes leader"); @@ -129,12 +156,16 @@ public void onLeaderStart(final long term) { @Override public void onLeaderStop(final Status status) { this.leaderTerm.set(-1); + this.probeState = State.STATE_FOLLOWER; + this.seesLeader = false; super.onLeaderStop(status); log.info("Raft lost leader "); } @Override public void onStartFollowing(final LeaderChangeContext ctx) { + this.probeState = State.STATE_FOLLOWER; + this.seesLeader = true; super.onStartFollowing(ctx); Utils.runInThread(() -> { if (!CollectionUtils.isEmpty(stateListeners)) { @@ -145,6 +176,7 @@ public void onStartFollowing(final LeaderChangeContext ctx) { @Override public void onStopFollowing(final LeaderChangeContext ctx) { + this.seesLeader = false; super.onStopFollowing(ctx); } 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 6f7660bc12..793aa24b05 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 @@ -86,7 +86,7 @@ private void registerMeters() { */ private void registerRaftMeters() { RaftEngine raft = RaftEngine.getInstance(); - Gauge.builder(PREFIX + ".raft.leader", () -> raft.isLeader() ? 1 : 0) + Gauge.builder(PREFIX + ".raft.leader", () -> raft.getRaftStatus().isLocalLeader() ? 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) 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 565d017f45..a011c3edef 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 @@ -26,42 +26,54 @@ import org.junit.Test; import com.alipay.sofa.jraft.Node; +import com.alipay.sofa.jraft.Status; import com.alipay.sofa.jraft.core.State; +import com.alipay.sofa.jraft.entity.LeaderChangeContext; import com.alipay.sofa.jraft.entity.PeerId; +import com.alipay.sofa.jraft.error.RaftException; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verifyNoInteractions; 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. + * {@code hg.raft.*} gauges. The signal is served from the volatile copies the state machine + * callbacks maintain, never from the raft node, so the probe stays prompt while an election + * holds the node lock; these tests drive the callbacks the way jraft does. */ 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 RaftStateMachine originalStateMachine; + private Node mockNode; + private RaftStateMachine stateMachine; @Before public void setUp() { RaftEngine engine = RaftEngine.getInstance(); originalRaftNode = engine.getRaftNode(); + originalStateMachine = Whitebox.getInternalState(engine, "stateMachine"); + + // A fresh machine so callbacks fired here reach no listeners other tests registered mockNode = mock(Node.class); + stateMachine = new RaftStateMachine(); Whitebox.setInternalState(engine, "raftNode", mockNode); + Whitebox.setInternalState(engine, "stateMachine", stateMachine); } @After public void tearDown() { - Whitebox.setInternalState(RaftEngine.getInstance(), "raftNode", originalRaftNode); + RaftEngine engine = RaftEngine.getInstance(); + Whitebox.setInternalState(engine, "raftNode", originalRaftNode); + Whitebox.setInternalState(engine, "stateMachine", originalStateMachine); } - 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); + private static LeaderChangeContext ctx() { + return new LeaderChangeContext(LEADER, 5, Status.OK()); } @Test @@ -69,102 +81,108 @@ public void testNotReadyBeforeRaftNodeStarts() { Whitebox.setInternalState(RaftEngine.getInstance(), "raftNode", null); RaftEngine engine = RaftEngine.getInstance(); + RaftEngine.RaftStatus status = engine.getRaftStatus(); + Assert.assertFalse(status.isReady()); + Assert.assertFalse(status.isLocalLeader()); + Assert.assertEquals(State.STATE_UNINITIALIZED.name(), status.getState()); Assert.assertFalse(engine.hasLeader()); - Assert.assertFalse(engine.isLeader()); - Assert.assertNull(engine.getLeader()); Assert.assertEquals(-1, engine.getAlivePeerCount()); + } - RaftEngine.RaftStatus status = engine.getRaftStatus(); + @Test + public void testStartedNodeWithoutAnyCallbackIsNotReady() { + RaftEngine.RaftStatus status = RaftEngine.getInstance().getRaftStatus(); Assert.assertFalse(status.isReady()); - Assert.assertFalse(status.isLocalLeader()); Assert.assertEquals(State.STATE_UNINITIALIZED.name(), status.getState()); } @Test public void testLeaderIsReady() { - stub(State.STATE_LEADER, SELF, true); - when(mockNode.listAlivePeers()).thenReturn(Arrays.asList(SELF, LEADER, OTHER)); + stateMachine.onLeaderStart(5); + when(mockNode.listAlivePeers()).thenReturn(Arrays.asList(LEADER, new PeerId("b", 1), + new PeerId("c", 1))); RaftEngine engine = RaftEngine.getInstance(); - Assert.assertTrue(engine.hasLeader()); - Assert.assertTrue(engine.isLeader()); - 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()); + Assert.assertTrue(engine.hasLeader()); + Assert.assertEquals(3, engine.getAlivePeerCount()); } @Test public void testFollowerWithLeaderIsReady() { - stub(State.STATE_FOLLOWER, LEADER, false); + stateMachine.onStartFollowing(ctx()); RaftEngine engine = RaftEngine.getInstance(); - Assert.assertTrue(engine.getRaftStatus().isReady()); + RaftEngine.RaftStatus status = engine.getRaftStatus(); + Assert.assertTrue(status.isReady()); + Assert.assertFalse(status.isLocalLeader()); + Assert.assertEquals(State.STATE_FOLLOWER.name(), status.getState()); 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); + public void testFollowerLosingItsLeaderTurnsNotReady() { + // The quorum-loss shape from the issue: heartbeats stop, jraft announces the loss + stateMachine.onStartFollowing(ctx()); + stateMachine.onStopFollowing(ctx()); RaftEngine engine = RaftEngine.getInstance(); - Assert.assertFalse(engine.getRaftStatus().isReady()); + RaftEngine.RaftStatus status = engine.getRaftStatus(); + Assert.assertFalse(status.isReady()); + Assert.assertEquals(State.STATE_FOLLOWER.name(), status.getState()); Assert.assertFalse(engine.hasLeader()); } @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); + public void testLeaderSteppingDownTurnsNotReady() { + stateMachine.onLeaderStart(5); + stateMachine.onLeaderStop(Status.OK()); RaftEngine engine = RaftEngine.getInstance(); - Assert.assertFalse(engine.getRaftStatus().isReady()); + RaftEngine.RaftStatus status = engine.getRaftStatus(); + Assert.assertFalse(status.isReady()); + Assert.assertFalse(status.isLocalLeader()); Assert.assertFalse(engine.hasLeader()); + Assert.assertEquals(-1, engine.getAlivePeerCount()); } @Test - 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); + public void testErrorAndShutdownAreNotReadyEvenAfterLeadership() { + stateMachine.onLeaderStart(5); + stateMachine.onError(mock(RaftException.class)); Assert.assertFalse(RaftEngine.getInstance().getRaftStatus().isReady()); - } + Assert.assertEquals(State.STATE_ERROR.name(), + RaftEngine.getInstance().getRaftStatus().getState()); - @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().getRaftStatus().isReady()); + stateMachine.onShutdown(); + Assert.assertFalse(RaftEngine.getInstance().getRaftStatus().isReady()); + Assert.assertEquals(State.STATE_SHUTDOWN.name(), + RaftEngine.getInstance().getRaftStatus().getState()); } @Test - public void testTransferringLeaderIsReady() { - stub(State.STATE_TRANSFERRING, SELF, true); - Assert.assertTrue(RaftEngine.getInstance().getRaftStatus().isReady()); - } + public void testProbeNeverTouchesTheRaftNode() { + // The point of serving from callbacks: an election holds the node lock while jraft + // reconnects to peers, so the probe and the leader gauges must not read the node + stateMachine.onStartFollowing(ctx()); + stateMachine.onStopFollowing(ctx()); + RaftEngine engine = RaftEngine.getInstance(); - @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().getRaftStatus().isReady()); - } + engine.getRaftStatus(); + engine.hasLeader(); + engine.getAlivePeerCount(); + + verifyNoInteractions(mockNode); } @Test public void testAlivePeerCountSurvivesLeadershipLossRace() { - stub(State.STATE_LEADER, SELF, true); + stateMachine.onLeaderStart(5); when(mockNode.listAlivePeers()).thenThrow(new IllegalStateException("Not leader")); Assert.assertEquals(-1, RaftEngine.getInstance().getAlivePeerCount()); From b6ad26c0d7506af233e435185f426de1955c5009 Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Sun, 6 Sep 2026 12:33:10 +0530 Subject: [PATCH 13/23] fix(pd): swap the probe view in as one volatile write getRaftStatus() read probeState and seesLeader as two volatile loads while its javadoc said the fields cannot contradict each other, so a reader landing between onLeaderStart's two stores could see ready:false with state:STATE_LEADER, and the mirror case after onLeaderStop. Hold both values in one immutable ProbeView, written once per callback on the single FSM thread and read once by getRaftStatus() and hasLeader(), the same single-snapshot shape 7074440 gave the node-based version. The javadoc now also says the view trails the node by the FSM queue instead of implying it is current. Document STATE_UNINITIALIZED as what a PD reports until its first raft callback, which is the ordinary startup window before a quorum first forms. --- hugegraph-pd/docs/api-reference.md | 8 ++- .../apache/hugegraph/pd/raft/RaftEngine.java | 17 +++--- .../hugegraph/pd/raft/RaftStateMachine.java | 56 ++++++++++--------- 3 files changed, 46 insertions(+), 35 deletions(-) diff --git a/hugegraph-pd/docs/api-reference.md b/hugegraph-pd/docs/api-reference.md index 2ad3aa49e5..2d52ec66fe 100644 --- a/hugegraph-pd/docs/api-reference.md +++ b/hugegraph-pd/docs/api-reference.md @@ -803,9 +803,11 @@ body carries no cluster addresses; the leader's address stays on `/v1/members`. 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: jraft -emits no callback for candidacy or leadership transfer, so a candidate reports -`STATE_FOLLOWER` with `"ready": false`. +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`. Point Kubernetes readiness probes, `depends_on` healthchecks and any "wait for PD" script at `/v1/ready`; keep liveness probes on `/v1/health` 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 6ac3d1c7ef..ad213019e8 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 @@ -218,27 +218,30 @@ public boolean isLeader() { * raft node, so it never waits on the node lock. */ public boolean hasLeader() { - return this.raftNode != null && this.stateMachine.seesLeader(); + return this.raftNode != null && this.stateMachine.getProbeView().seesLeader; } /** * Take a view of the local raft state from the volatile copies the state machine * callbacks maintain, never from the raft node itself. During an election jraft holds * the node lock while it reconnects to peers, so a probe that read the node stalled for - * the connect timeout instead of answering its 503 promptly. All fields derive from the - * same callback-written values, so they cannot contradict each other. + * the connect timeout instead of answering its 503 promptly. All fields derive from one + * read of one immutable view, swapped in whole per callback, so they cannot contradict + * each other. *

* The state reported is the last one a callback announced: leader, follower, error or * shutdown. jraft emits no callback for candidacy or leadership transfer, so a candidate - * reads as a follower that sees no leader, which yields the same not-ready answer. + * reads as a follower that sees no leader, which yields the same not-ready answer. The + * view trails the node by whatever sits in the FSM queue ahead of the announcement, + * which is the price of never waiting on the node lock. */ public RaftStatus getRaftStatus() { if (this.raftNode == null) { return new RaftStatus(false, State.STATE_UNINITIALIZED.name(), false); } - State state = this.stateMachine.getProbeState(); - return new RaftStatus(state.isActive() && this.stateMachine.seesLeader(), - state.name(), State.STATE_LEADER == state); + RaftStateMachine.ProbeView view = this.stateMachine.getProbeView(); + return new RaftStatus(view.state.isActive() && view.seesLeader, + view.state.name(), State.STATE_LEADER == view.state); } /** diff --git a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftStateMachine.java b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftStateMachine.java index 1e7cedb2cc..cbc6c81c5c 100644 --- a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftStateMachine.java +++ b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftStateMachine.java @@ -59,9 +59,25 @@ public class RaftStateMachine extends StateMachineAdapter { private final AtomicLong leaderTerm = new AtomicLong(-1); // Kept for the readiness probe and the raft gauges, which must answer without // touching the raft node: during an election jraft holds the node lock while it - // reconnects to peers, and a reader stalls for the connect timeout. - private volatile State probeState = State.STATE_UNINITIALIZED; - private volatile boolean seesLeader = false; + // reconnects to peers, and a reader stalls for the connect timeout. One volatile + // immutable pair, swapped in whole per callback, so a reader can never see the + // state of one announcement with the leader visibility of another. + private volatile ProbeView probeView = new ProbeView(State.STATE_UNINITIALIZED, false); + + /** + * Immutable pair of the last announced node state and leader visibility. Written only + * from the FSM callback thread; both fields always come from the same announcement. + */ + static final class ProbeView { + + final State state; + final boolean seesLeader; + + ProbeView(State state, boolean seesLeader) { + this.state = state; + this.seesLeader = seesLeader; + } + } private List taskHandlers; private List stateListeners; @@ -83,18 +99,12 @@ public boolean isLeader() { } /** - * The last node state a raft callback announced. jraft emits no callback for candidacy - * or leadership transfer, so a candidate reads as a follower that sees no leader. - */ - public State getProbeState() { - return this.probeState; - } - - /** - * Whether the last raft callback left this node with a visible leader. + * The last state and leader visibility a raft callback announced, as one immutable + * view. jraft emits no callback for candidacy or leadership transfer, so a candidate + * reads as a follower that sees no leader. */ - public boolean seesLeader() { - return this.seesLeader; + ProbeView getProbeView() { + return this.probeView; } @Override @@ -126,23 +136,20 @@ public void onApply(Iterator iter) { @Override public void onError(final RaftException e) { - this.probeState = State.STATE_ERROR; - this.seesLeader = false; + this.probeView = new ProbeView(State.STATE_ERROR, false); log.error("Raft StateMachine on error {}", e); } @Override public void onShutdown() { - this.probeState = State.STATE_SHUTDOWN; - this.seesLeader = false; + this.probeView = new ProbeView(State.STATE_SHUTDOWN, false); super.onShutdown(); } @Override public void onLeaderStart(final long term) { this.leaderTerm.set(term); - this.probeState = State.STATE_LEADER; - this.seesLeader = true; + this.probeView = new ProbeView(State.STATE_LEADER, true); super.onLeaderStart(term); log.info("Raft becomes leader"); @@ -156,16 +163,14 @@ public void onLeaderStart(final long term) { @Override public void onLeaderStop(final Status status) { this.leaderTerm.set(-1); - this.probeState = State.STATE_FOLLOWER; - this.seesLeader = false; + this.probeView = new ProbeView(State.STATE_FOLLOWER, false); super.onLeaderStop(status); log.info("Raft lost leader "); } @Override public void onStartFollowing(final LeaderChangeContext ctx) { - this.probeState = State.STATE_FOLLOWER; - this.seesLeader = true; + this.probeView = new ProbeView(State.STATE_FOLLOWER, true); super.onStartFollowing(ctx); Utils.runInThread(() -> { if (!CollectionUtils.isEmpty(stateListeners)) { @@ -176,7 +181,8 @@ public void onStartFollowing(final LeaderChangeContext ctx) { @Override public void onStopFollowing(final LeaderChangeContext ctx) { - this.seesLeader = false; + // Callbacks run on the single FSM thread, so reading our own field is safe + this.probeView = new ProbeView(this.probeView.state, false); super.onStopFollowing(ctx); } From 2df9a55a6faa130c51491c8000d7f6a721040b9a Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Sun, 6 Sep 2026 21:11:29 +0530 Subject: [PATCH 14/23] docs(pd): warn against aggregating hg_raft_alive_peers The gauge is NaN on every node but the leader, and a single NaN sample turns sum() or avg() into NaN, so an operator who graphs it across instances gets nothing back. Note that next to the quorum-loss alerts and give the leader-scoped query to use instead. --- hugegraph-pd/docs/api-reference.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/hugegraph-pd/docs/api-reference.md b/hugegraph-pd/docs/api-reference.md index 2d52ec66fe..79756afeb5 100644 --- a/hugegraph-pd/docs/api-reference.md +++ b/hugegraph-pd/docs/api-reference.md @@ -861,6 +861,11 @@ A cluster has lost its quorum when `sum(hg_raft_leader) == 0` or when normal election, so alert on them with a `for:` clause longer than the election timeout rather than on the instantaneous value. +Do not aggregate `hg_raft_alive_peers` across instances: it is `NaN` on every +node but the leader, and one `NaN` sample turns the result of `sum()` or +`avg()` into `NaN` as well. Select the leader's series instead, for example +`hg_raft_alive_peers and on(instance) (hg_raft_leader == 1)`. + ### Partition API #### List Partitions From 2ac8f6c9e5e82f5280b376d34d8bd15b683b9166 Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Tue, 8 Sep 2026 00:18:19 +0530 Subject: [PATCH 15/23] fix(pd): refresh the raft alive peer count off the request thread listAlivePeers takes the jraft node read lock before it checks for leadership, so a scrape that called it waited out whoever held the write lock. stepDown holds that lock while it writes raft metadata on a term bump, and preVote holds it across a reconnect to every peer, bounded only by raft.rpc-timeout. The gauge now reads a volatile count that a one second refresher publishes on its own daemon thread, and reports -1, so NaN, until the first refresh. jraft walks the same peer set in its own step down timer every half election timeout, so the poll costs nothing next to what the node already does. Drop the javadoc claim that the call only runs on a settled leader, which the lock order cannot support. --- .../apache/hugegraph/pd/raft/RaftEngine.java | 61 +++++++++++++++++-- .../pd/raft/RaftEngineReadinessTest.java | 27 +++++++- 2 files changed, 80 insertions(+), 8 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 ad213019e8..4e4b954585 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 @@ -27,6 +27,8 @@ import java.util.concurrent.CompletableFuture; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicReference; @@ -64,6 +66,14 @@ @Slf4j public class RaftEngine { + /** + * Refresh period of the alive peer count behind {@code hg_raft_alive_peers}. jraft's own + * step-down timer walks the same peer set every half election timeout, so a one second + * poll adds nothing next to the work the node already does, and it keeps the gauge fresh + * well inside any scrape interval. + */ + private static final long ALIVE_PEERS_REFRESH_MS = 1000L; + private volatile static RaftEngine instance = new RaftEngine(); private RaftStateMachine stateMachine; private String groupId = "pd_raft"; @@ -72,6 +82,8 @@ public class RaftEngine { private RpcServer rpcServer; private volatile Node raftNode; private RaftRpcClient raftRpcClient; + private volatile int alivePeerCount = -1; + private ScheduledExecutorService alivePeersRefresher; public RaftEngine() { this.stateMachine = new RaftStateMachine(); @@ -134,6 +146,7 @@ public synchronized boolean init(PDConfig.Raft config) { this.raftGroupService = new RaftGroupService(groupId, serverId, nodeOptions, rpcServer, true); this.raftNode = raftGroupService.start(false); + startAlivePeersRefresher(); log.info("RaftEngine start successfully: id = {}, peers list = {}", groupId, nodeOptions.getInitialConf().getPeers()); return this.raftNode != null; @@ -183,6 +196,11 @@ public List backChannelHandlers() { } public void shutDown() { + if (this.alivePeersRefresher != null) { + this.alivePeersRefresher.shutdownNow(); + this.alivePeersRefresher = null; + } + this.alivePeerCount = -1; if (this.raftGroupService != null) { this.raftGroupService.shutdown(); try { @@ -280,20 +298,51 @@ public boolean isLocalLeader() { /** * Number of raft peers, this node included, that the leader has heard from within the * 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. The leader - * check is the state machine's lock-free term flag, so a scrape on a non-leader never - * waits on the node lock; {@code listAlivePeers} itself only runs on a settled leader. + * Only the leader tracks replication state, so any other node reports -1. + *

+ * This is the value the refresher last published, at most one refresh period old, and -1 + * until the first refresh runs. The count cannot be read here: {@code listAlivePeers} + * takes the node read lock before it checks for leadership, so a caller that read the + * node would wait out whoever holds the write lock, which during an election is a + * per-peer reconnect bounded only by the rpc connect timeout. */ public int getAlivePeerCount() { + return this.alivePeerCount; + } + + private void startAlivePeersRefresher() { + ScheduledExecutorService refresher = + Executors.newSingleThreadScheduledExecutor(runnable -> { + Thread thread = new Thread(runnable, "pd-raft-alive-peers"); + thread.setDaemon(true); + return thread; + }); + refresher.scheduleWithFixedDelay(this::refreshAlivePeerCount, 0, + ALIVE_PEERS_REFRESH_MS, TimeUnit.MILLISECONDS); + this.alivePeersRefresher = refresher; + } + + /** + * Read the alive peer count from the raft node and publish it for the gauge. Runs on the + * refresher thread, never on a request thread, because the read can block for as long as + * an election holds the node write lock. A fixed delay schedule means a blocked refresh + * only delays the next one, and the gauge keeps reporting the last value meanwhile. + */ + void refreshAlivePeerCount() { Node node = this.raftNode; if (node == null || !this.stateMachine.isLeader()) { - return -1; + this.alivePeerCount = -1; + return; } try { - return node.listAlivePeers().size(); + this.alivePeerCount = node.listAlivePeers().size(); } catch (IllegalStateException e) { // Lost leadership between the check and the call - return -1; + this.alivePeerCount = -1; + } catch (Exception e) { + // Never let the refresh schedule die on an unexpected failure + log.warn("Failed to refresh the raft alive peer count", e); + this.alivePeerCount = -1; } } 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 a011c3edef..77b83ba8fb 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 @@ -40,7 +40,9 @@ * Covers the raft-aware readiness signal behind {@code GET /v1/ready} and the * {@code hg.raft.*} gauges. The signal is served from the volatile copies the state machine * callbacks maintain, never from the raft node, so the probe stays prompt while an election - * holds the node lock; these tests drive the callbacks the way jraft does. + * holds the node lock; these tests drive the callbacks the way jraft does. The alive peer + * count is the one value that has to come off the node, so it is refreshed explicitly here, + * the way the refresher thread does it in a running PD. */ public class RaftEngineReadinessTest { @@ -63,6 +65,8 @@ public void setUp() { stateMachine = new RaftStateMachine(); Whitebox.setInternalState(engine, "raftNode", mockNode); Whitebox.setInternalState(engine, "stateMachine", stateMachine); + // The count is cached on the singleton, so clear what an earlier test published + engine.refreshAlivePeerCount(); } @After @@ -80,6 +84,7 @@ private static LeaderChangeContext ctx() { public void testNotReadyBeforeRaftNodeStarts() { Whitebox.setInternalState(RaftEngine.getInstance(), "raftNode", null); RaftEngine engine = RaftEngine.getInstance(); + engine.refreshAlivePeerCount(); RaftEngine.RaftStatus status = engine.getRaftStatus(); Assert.assertFalse(status.isReady()); @@ -102,6 +107,7 @@ public void testLeaderIsReady() { when(mockNode.listAlivePeers()).thenReturn(Arrays.asList(LEADER, new PeerId("b", 1), new PeerId("c", 1))); RaftEngine engine = RaftEngine.getInstance(); + engine.refreshAlivePeerCount(); RaftEngine.RaftStatus status = engine.getRaftStatus(); Assert.assertTrue(status.isReady()); @@ -115,6 +121,7 @@ public void testLeaderIsReady() { public void testFollowerWithLeaderIsReady() { stateMachine.onStartFollowing(ctx()); RaftEngine engine = RaftEngine.getInstance(); + engine.refreshAlivePeerCount(); RaftEngine.RaftStatus status = engine.getRaftStatus(); Assert.assertTrue(status.isReady()); @@ -143,6 +150,7 @@ public void testLeaderSteppingDownTurnsNotReady() { stateMachine.onLeaderStart(5); stateMachine.onLeaderStop(Status.OK()); RaftEngine engine = RaftEngine.getInstance(); + engine.refreshAlivePeerCount(); RaftEngine.RaftStatus status = engine.getRaftStatus(); Assert.assertFalse(status.isReady()); @@ -168,7 +176,8 @@ public void testErrorAndShutdownAreNotReadyEvenAfterLeadership() { @Test public void testProbeNeverTouchesTheRaftNode() { // The point of serving from callbacks: an election holds the node lock while jraft - // reconnects to peers, so the probe and the leader gauges must not read the node + // reconnects to peers, so the probe and the gauges must answer without the node. + // getAlivePeerCount() included: it reads what the refresher thread published stateMachine.onStartFollowing(ctx()); stateMachine.onStopFollowing(ctx()); RaftEngine engine = RaftEngine.getInstance(); @@ -184,7 +193,21 @@ public void testProbeNeverTouchesTheRaftNode() { public void testAlivePeerCountSurvivesLeadershipLossRace() { stateMachine.onLeaderStart(5); when(mockNode.listAlivePeers()).thenThrow(new IllegalStateException("Not leader")); + RaftEngine.getInstance().refreshAlivePeerCount(); Assert.assertEquals(-1, RaftEngine.getInstance().getAlivePeerCount()); } + + @Test + public void testAlivePeerCountOnALeaderAlsoSkipsTheNode() { + // Even on a leader, where the count means something, the gauge reads the published + // value: listAlivePeers takes the node read lock before it checks for leadership, so + // a scrape that called it would wait out an election that holds the write lock + stateMachine.onLeaderStart(5); + RaftEngine engine = RaftEngine.getInstance(); + + engine.getAlivePeerCount(); + + verifyNoInteractions(mockNode); + } } From cf95f22ee8d60b0ce3a761dbf5a201e595f118aa Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Tue, 8 Sep 2026 00:18:19 +0530 Subject: [PATCH 16/23] test(pd): run the probe checks through junit assertions Java assert is a no-op unless the jvm runs with -ea, so the health, ready and gauge checks added on this branch could pass without ever running. Convert them, keeping the messages they already carried. The gauge patterns now accept a sample line with no tag block, which micrometer emits for a meter without tags, and anchor on the start of a line so a HELP or TYPE line cannot satisfy them. --- .../apache/hugegraph/pd/rest/RestApiTest.java | 43 +++++++++++-------- 1 file changed, 25 insertions(+), 18 deletions(-) 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 a145b788a2..da90f6f0f9 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 @@ -25,6 +25,7 @@ import org.json.JSONException; import org.json.JSONObject; +import org.junit.Assert; import org.junit.Test; public class RestApiTest extends BaseServerTest { @@ -68,11 +69,12 @@ public void testHealthNeedsNoAuth() throws URISyntaxException, IOException, 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; + Assert.assertEquals(200, response.statusCode()); // 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(); + Assert.assertTrue("expected an empty body, got " + response.body(), + response.body().isEmpty()); } @Test @@ -82,14 +84,14 @@ public void testReadyNeedsNoAuthAndReflectsRaft() throws URISyntaxException, IOE 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(); + Assert.assertEquals("expected 200, body=" + response.body(), 200, response.statusCode()); JSONObject obj = new JSONObject(response.body()); - assert obj.getBoolean("ready"); - assert obj.getBoolean("isLeader"); - assert "STATE_LEADER".equals(obj.getString("state")); + Assert.assertTrue(obj.getBoolean("ready")); + Assert.assertTrue(obj.getBoolean("isLeader")); + Assert.assertEquals("STATE_LEADER", obj.getString("state")); // Unauthenticated, so it must not disclose cluster addresses - assert !obj.has("leader") : "the anonymous body must not carry the leader address"; + Assert.assertFalse("the anonymous body must not carry the leader address", + obj.has("leader")); } @Test @@ -98,18 +100,23 @@ public void testRaftGaugesExported() throws URISyntaxException, IOException, 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; + Assert.assertEquals(200, response.statusCode()); 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"; + // Micrometer only writes a {...} block when the meter carries tags, and the sample + // line is the one that starts with the metric name, unlike its HELP and TYPE lines + Assert.assertTrue("missing hg_raft_leader gauge", + body.matches("(?sm).*^hg_raft_leader(\\{[^}]*\\})? .*")); + Assert.assertTrue("missing hg_raft_has_leader gauge", + body.matches("(?sm).*^hg_raft_has_leader(\\{[^}]*\\})? .*")); + Assert.assertTrue("missing hg_raft_alive_peers gauge", + body.matches("(?sm).*^hg_raft_alive_peers(\\{[^}]*\\})? .*")); // 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"; + Assert.assertTrue("hg_raft_leader should be 1 on a single-node leader", + body.matches("(?sm).*^hg_raft_leader(\\{[^}]*\\})? 1\\.0.*")); + Assert.assertTrue("hg_raft_has_leader should be 1 on a single-node leader", + body.matches("(?sm).*^hg_raft_has_leader(\\{[^}]*\\})? 1\\.0.*")); + Assert.assertTrue("hg_raft_alive_peers should be 1 on a single-node leader", + body.matches("(?sm).*^hg_raft_alive_peers(\\{[^}]*\\})? 1\\.0.*")); } @Test From 41318057b7930ba3d36b6e4799679e1b02c334ac Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Tue, 8 Sep 2026 00:18:19 +0530 Subject: [PATCH 17/23] test(pd): cover the 503 answer of the readiness endpoint The rest suite only reaches the ready path, because the PD it talks to is a single node group that is always its own leader. Nothing pinned the 503 half of the mapping, so an always-200 regression would have shipped. Cover checkReady() over a node that has not started raft and over a follower whose leader went away, plus the leader case so the mapping cannot be inverted either. The test sits in hg-pd-service: the other CI jobs compile hg-pd-test against the repackaged service jar, where the class is not visible. --- hugegraph-pd/hg-pd-service/pom.xml | 6 + .../hugegraph/pd/rest/StoreAPIReadyTest.java | 144 ++++++++++++++++++ 2 files changed, 150 insertions(+) create mode 100644 hugegraph-pd/hg-pd-service/src/test/java/org/apache/hugegraph/pd/rest/StoreAPIReadyTest.java diff --git a/hugegraph-pd/hg-pd-service/pom.xml b/hugegraph-pd/hg-pd-service/pom.xml index ee78863f35..eef38b4073 100644 --- a/hugegraph-pd/hg-pd-service/pom.xml +++ b/hugegraph-pd/hg-pd-service/pom.xml @@ -162,6 +162,12 @@ log4j-jul 2.17.2 + + junit + junit + ${junit.version} + test + diff --git a/hugegraph-pd/hg-pd-service/src/test/java/org/apache/hugegraph/pd/rest/StoreAPIReadyTest.java b/hugegraph-pd/hg-pd-service/src/test/java/org/apache/hugegraph/pd/rest/StoreAPIReadyTest.java new file mode 100644 index 0000000000..75e9581cb1 --- /dev/null +++ b/hugegraph-pd/hg-pd-service/src/test/java/org/apache/hugegraph/pd/rest/StoreAPIReadyTest.java @@ -0,0 +1,144 @@ +/* + * 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; + +import java.lang.reflect.Proxy; +import java.util.Map; + +import org.apache.hugegraph.pd.raft.RaftEngine; +import org.apache.hugegraph.pd.raft.RaftStateMachine; +import org.apache.hugegraph.testutil.Whitebox; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; + +import com.alipay.sofa.jraft.Node; +import com.alipay.sofa.jraft.Status; +import com.alipay.sofa.jraft.core.State; +import com.alipay.sofa.jraft.entity.LeaderChangeContext; +import com.alipay.sofa.jraft.entity.PeerId; + +/** + * Pins the status a readiness probe actually receives from {@code GET /v1/ready}: 503 while + * this PD is outside a raft quorum, 200 once it is inside one, with the same body either way. + * The live REST suite can only reach the 200 path, because the PD it talks to is a single node + * group that is always its own leader. + */ +public class StoreAPIReadyTest { + + private static final PeerId LEADER = new PeerId("10.0.0.1", 8610); + + private final StoreAPI api = new StoreAPI(); + + private Node originalRaftNode; + private RaftStateMachine originalStateMachine; + private RaftStateMachine stateMachine; + + @Before + public void setUp() { + RaftEngine engine = RaftEngine.getInstance(); + this.originalRaftNode = engine.getRaftNode(); + this.originalStateMachine = Whitebox.getInternalState(engine, "stateMachine"); + + // A fresh machine so callbacks fired here reach no listeners of the real one + this.stateMachine = new RaftStateMachine(); + Whitebox.setInternalState(engine, "stateMachine", this.stateMachine); + } + + @After + public void tearDown() { + RaftEngine engine = RaftEngine.getInstance(); + Whitebox.setInternalState(engine, "raftNode", this.originalRaftNode); + Whitebox.setInternalState(engine, "stateMachine", this.originalStateMachine); + } + + @Test + public void testNotReadyBeforeRaftStartsAnswers503() { + Whitebox.setInternalState(RaftEngine.getInstance(), "raftNode", null); + + ResponseEntity> response = this.api.checkReady(); + + Assert.assertEquals(HttpStatus.SERVICE_UNAVAILABLE, response.getStatusCode()); + Map body = response.getBody(); + Assert.assertNotNull(body); + Assert.assertEquals(Boolean.FALSE, body.get("ready")); + Assert.assertEquals(Boolean.FALSE, body.get("isLeader")); + Assert.assertEquals(State.STATE_UNINITIALIZED.name(), body.get("state")); + } + + @Test + public void testLosingTheLeaderAnswers503() { + Whitebox.setInternalState(RaftEngine.getInstance(), "raftNode", untouchableNode()); + // The quorum-loss shape: heartbeats arrive, then they stop + this.stateMachine.onStartFollowing(context()); + this.stateMachine.onStopFollowing(context()); + + ResponseEntity> response = this.api.checkReady(); + + Assert.assertEquals(HttpStatus.SERVICE_UNAVAILABLE, response.getStatusCode()); + Map body = response.getBody(); + Assert.assertNotNull(body); + Assert.assertEquals(Boolean.FALSE, body.get("ready")); + Assert.assertEquals(Boolean.FALSE, body.get("isLeader")); + Assert.assertEquals(State.STATE_FOLLOWER.name(), body.get("state")); + } + + @Test + public void testLeaderAnswers200() { + Whitebox.setInternalState(RaftEngine.getInstance(), "raftNode", untouchableNode()); + this.stateMachine.onLeaderStart(5); + + ResponseEntity> response = this.api.checkReady(); + + Assert.assertEquals(HttpStatus.OK, response.getStatusCode()); + Map body = response.getBody(); + Assert.assertNotNull(body); + Assert.assertEquals(Boolean.TRUE, body.get("ready")); + Assert.assertEquals(Boolean.TRUE, body.get("isLeader")); + Assert.assertEquals(State.STATE_LEADER.name(), body.get("state")); + } + + private static LeaderChangeContext context() { + return new LeaderChangeContext(LEADER, 5, Status.OK()); + } + + /** + * A started raft node that fails the test if the endpoint calls it. Readiness is served + * from the state machine callbacks precisely so that it never waits on the node lock. + */ + private static Node untouchableNode() { + return (Node) Proxy.newProxyInstance( + Node.class.getClassLoader(), new Class[]{Node.class}, + (proxy, method, args) -> { + switch (method.getName()) { + case "toString": + return "untouchable raft node"; + case "hashCode": + return System.identityHashCode(proxy); + case "equals": + return proxy == args[0]; + default: + throw new AssertionError("the ready path must not touch the raft " + + "node, it called " + method.getName()); + } + }); + } +} From 8c6f0a3639a210f88c566ec27758e635eb227323 Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Tue, 8 Sep 2026 00:30:51 +0530 Subject: [PATCH 18/23] test(pd): match the readiness probe body regardless of spacing The probe grepped for the literal "ready":true, so it would stop matching if the serializer ever emitted a space after the colon or pretty-printed the body. Match the key, optional whitespace and the value instead. --- .../src/assembly/travis/test-start-hugegraph-pd.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 7a3f70b602..967b32a69b 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 @@ -121,7 +121,7 @@ wait_for_pd_ready() { while (( elapsed < STARTUP_WAIT )); do local body body=$(curl -fsS "$PD_URL/v1/ready" 2>/dev/null || true) - grep -q '"ready":true' <<<"$body" && return 0 + grep -Eq '"ready"[[:space:]]*:[[:space:]]*true' <<<"$body" && return 0 sleep 2 elapsed=$((elapsed + 2)) done From a5477f5eddb607385b0b808cf55ca955d683e80d Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Tue, 8 Sep 2026 00:47:23 +0530 Subject: [PATCH 19/23] fix(pd): keep the alive peer refresher alive and quiet on shutdown scheduleWithFixedDelay cancels every later run if the task throws, and the catch only covered Exception, so an Error would have stopped the refresher for good while the gauge kept serving its last value with no NaN and no log line. Widen it to Throwable, which is what the comment already claimed. shutDown also reset the count before an in-flight refresh could finish, so a refresh already inside listAlivePeers could publish a positive count afterwards. Wait briefly for the executor to quiesce first. --- .../org/apache/hugegraph/pd/raft/RaftEngine.java | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 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 4e4b954585..de9aab128e 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 @@ -198,6 +198,14 @@ public List backChannelHandlers() { public void shutDown() { if (this.alivePeersRefresher != null) { this.alivePeersRefresher.shutdownNow(); + try { + // shutdownNow only interrupts; a refresh already inside + // listAlivePeers could otherwise publish a positive count + // after the reset below. + this.alivePeersRefresher.awaitTermination(1, TimeUnit.SECONDS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } this.alivePeersRefresher = null; } this.alivePeerCount = -1; @@ -339,8 +347,10 @@ void refreshAlivePeerCount() { } catch (IllegalStateException e) { // Lost leadership between the check and the call this.alivePeerCount = -1; - } catch (Exception e) { - // Never let the refresh schedule die on an unexpected failure + } catch (Throwable e) { + // scheduleWithFixedDelay cancels every later run if the task throws, + // and an Error escaping here would leave the gauge serving its last + // value forever, so this catch has to be wider than Exception. log.warn("Failed to refresh the raft alive peer count", e); this.alivePeerCount = -1; } From 667b6fbc946a0e820b4a3648907f05ee12c00446 Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Tue, 8 Sep 2026 16:24:47 +0530 Subject: [PATCH 20/23] fix(pd): tie the raft gauges to the readiness predicate hasLeader() omitted the active-state guard getRaftStatus() applies, so hg_raft_has_leader and GET /v1/ready agreed only by accident of which callback writes what. Derive the gauge from getRaftStatus().isReady() so the two cannot drift. getRaftStatus() forced STATE_UNINITIALIZED whenever the raft node was null, which masked the shutdown and error states the callbacks announce and contradicted its own doc. The node check is unnecessary: the probe view starts uninitialized, and shutDown() drops the node only after the shutdown callback has run, so neither case reads as ready. onError() and onShutdown() left leaderTerm positive when the node had been leader, so the alive-peer refresher kept calling listAlivePeers() on a broken node and hg_raft_alive_peers reported a live count instead of NaN. Clear the term in both. The wait in shutDown() cannot prevent a late publish: a refresh parked in listAlivePeers() sits on a lock acquire the interrupt does not break, for up to the raft rpc connect timeout per unreachable peer. Keep the wait, say what it actually does, and log when the refresher outlives it. --- .../apache/hugegraph/pd/raft/RaftEngine.java | 40 +++++++++++------ .../hugegraph/pd/raft/RaftStateMachine.java | 4 ++ .../pd/raft/RaftEngineReadinessTest.java | 44 +++++++++++++++++++ 3 files changed, 75 insertions(+), 13 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 de9aab128e..d39384909d 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 @@ -199,10 +199,15 @@ public void shutDown() { if (this.alivePeersRefresher != null) { this.alivePeersRefresher.shutdownNow(); try { - // shutdownNow only interrupts; a refresh already inside - // listAlivePeers could otherwise publish a positive count - // after the reset below. - this.alivePeersRefresher.awaitTermination(1, TimeUnit.SECONDS); + // Best effort: shutdownNow only interrupts, and a refresh parked in + // listAlivePeers waits on a lock acquire the interrupt does not break, + // for up to the raft rpc connect timeout per unreachable peer. A refresh + // that outlives this wait may publish one stale count over the reset + // below; acceptable while shutDown has no production caller. + if (!this.alivePeersRefresher.awaitTermination(1, TimeUnit.SECONDS)) { + log.warn("Raft alive-peers refresher still running after shutdown; " + + "hg_raft_alive_peers may briefly report a stale value"); + } } catch (InterruptedException e) { Thread.currentThread().interrupt(); } @@ -235,16 +240,20 @@ public boolean isLeader() { } /** - * Whether this node currently sees a raft leader. + * Whether this PD is ready in the sense {@code GET /v1/ready} answers: the raft node + * is active and sees a leader. *

* A follower only keeps its leader while heartbeats keep arriving inside the election * timeout, and a leader only keeps its role while it can reach a quorum. Seeing a leader * therefore means this node is part of a quorum from its own point of view, which is the * signal a readiness probe needs. Served from the state machine callbacks, not from the * raft node, so it never waits on the node lock. + *

+ * Derived from {@link #getRaftStatus()} so {@code hg_raft_has_leader} cannot drift from + * the endpoint it is documented to mirror. */ public boolean hasLeader() { - return this.raftNode != null && this.stateMachine.getProbeView().seesLeader; + return getRaftStatus().isReady(); } /** @@ -256,15 +265,20 @@ public boolean hasLeader() { * each other. *

* The state reported is the last one a callback announced: leader, follower, error or - * shutdown. jraft emits no callback for candidacy or leadership transfer, so a candidate - * reads as a follower that sees no leader, which yields the same not-ready answer. The - * view trails the node by whatever sits in the FSM queue ahead of the announcement, - * which is the price of never waiting on the node lock. + * shutdown, and {@code STATE_UNINITIALIZED} until the first callback runs. jraft emits + * no callback for candidacy or leadership transfer, so a candidate reads as a follower + * that sees no leader, which yields the same not-ready answer. The view trails the node + * by whatever sits in the FSM queue ahead of the announcement, which is the price of + * never waiting on the node lock. + *

+ * A missing raft node needs no separate branch: before {@link #init} the view still + * holds its initial {@code STATE_UNINITIALIZED}, and {@code shutDown()} drops the node + * only after {@code join()} has let the shutdown callback announce + * {@code STATE_SHUTDOWN}. Neither state is active, so neither reads as ready, and a + * branch on the node would report an uninitialized PD where the callback said error or + * shutdown. */ public RaftStatus getRaftStatus() { - if (this.raftNode == null) { - return new RaftStatus(false, State.STATE_UNINITIALIZED.name(), false); - } RaftStateMachine.ProbeView view = this.stateMachine.getProbeView(); return new RaftStatus(view.state.isActive() && view.seesLeader, view.state.name(), State.STATE_LEADER == view.state); diff --git a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftStateMachine.java b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftStateMachine.java index cbc6c81c5c..aab90e5331 100644 --- a/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftStateMachine.java +++ b/hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftStateMachine.java @@ -136,12 +136,16 @@ public void onApply(Iterator iter) { @Override public void onError(final RaftException e) { + // A node that errors after it was leader keeps a positive leaderTerm otherwise, and + // the alive-peer refresher would go on reading listAlivePeers off a broken node + this.leaderTerm.set(-1); this.probeView = new ProbeView(State.STATE_ERROR, false); log.error("Raft StateMachine on error {}", e); } @Override public void onShutdown() { + this.leaderTerm.set(-1); this.probeView = new ProbeView(State.STATE_SHUTDOWN, false); super.onShutdown(); } 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 77b83ba8fb..08c35287d8 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 @@ -33,6 +33,8 @@ import com.alipay.sofa.jraft.error.RaftException; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; @@ -198,6 +200,48 @@ public void testAlivePeerCountSurvivesLeadershipLossRace() { Assert.assertEquals(-1, RaftEngine.getInstance().getAlivePeerCount()); } + @Test + public void testShutdownStateSurvivesTheNodeBeingDropped() { + // shutDown() drops the raft node after the shutdown callback ran, so the probe has + // to keep reporting what the callback announced rather than falling back to + // uninitialized, which would read as a PD that has not started yet + stateMachine.onLeaderStart(5); + stateMachine.onShutdown(); + Whitebox.setInternalState(RaftEngine.getInstance(), "raftNode", null); + RaftEngine engine = RaftEngine.getInstance(); + + RaftEngine.RaftStatus status = engine.getRaftStatus(); + Assert.assertFalse(status.isReady()); + Assert.assertFalse(status.isLocalLeader()); + Assert.assertEquals(State.STATE_SHUTDOWN.name(), status.getState()); + Assert.assertFalse(engine.hasLeader()); + } + + @Test + public void testAlivePeerCountStopsAfterALeaderErrorsOrShutsDown() { + // The terminal callbacks clear the leader term, so the refresher stops reading the + // count off a node that errored or shut down and the gauge falls back to NaN + stateMachine.onLeaderStart(5); + when(mockNode.listAlivePeers()).thenReturn(Arrays.asList(LEADER, new PeerId("b", 1))); + RaftEngine engine = RaftEngine.getInstance(); + engine.refreshAlivePeerCount(); + Assert.assertEquals(2, engine.getAlivePeerCount()); + + stateMachine.onError(mock(RaftException.class)); + engine.refreshAlivePeerCount(); + Assert.assertEquals(-1, engine.getAlivePeerCount()); + + stateMachine.onLeaderStart(6); + engine.refreshAlivePeerCount(); + Assert.assertEquals(2, engine.getAlivePeerCount()); + + stateMachine.onShutdown(); + engine.refreshAlivePeerCount(); + Assert.assertEquals(-1, engine.getAlivePeerCount()); + // Twice, once per leader phase: the terminal states never reached the node + verify(mockNode, times(2)).listAlivePeers(); + } + @Test public void testAlivePeerCountOnALeaderAlsoSkipsTheNode() { // Even on a leader, where the count means something, the gauge reads the published From eb6de8f74e2ce490cdb367eb5d39239245b59572 Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Tue, 8 Sep 2026 16:24:55 +0530 Subject: [PATCH 21/23] docs(pd): correct the readiness notes and bound the CI probe State the per-node result of /v1/ready in docker/README.md: in a three-PD group a partitioned node keeps answering 503 while the two that can reach each other elect a leader, rather than all three turning ready together. Note in the store deployment guide that /v1/ready ships after 1.7.0, the version its Docker examples pin, so the readiness check there needs a newer tag or images built from source. Shorten the hg_raft_alive_peers row in the PD API reference and move the leader lease timeout detail into a paragraph after the table. Give the readiness curl in test-start-hugegraph-pd.sh a connect and a total timeout. Without them a PD that accepts the connection and then stops answering parks the loop, elapsed never advances, and STARTUP_WAIT stops bounding the CI wait. --- docker/README.md | 6 ++++-- hugegraph-pd/docs/api-reference.md | 5 ++++- .../src/assembly/travis/test-start-hugegraph-pd.sh | 7 +++++-- hugegraph-store/docs/deployment-guide.md | 8 ++++++++ 4 files changed, 21 insertions(+), 5 deletions(-) diff --git a/docker/README.md b/docker/README.md index f28b6a5524..6fcfdee425 100644 --- a/docker/README.md +++ b/docker/README.md @@ -205,8 +205,10 @@ 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. +and `503` otherwise. Each PD answers for itself: a single PD elects itself, and +in a three-PD group the two that can reach each other elect a leader and turn +ready, while a partitioned third keeps answering `503` until it sees that +leader. The healthchecks in these files still gate on `/v1/health`, because `/v1/ready` ships from the next release onwards while the files run published diff --git a/hugegraph-pd/docs/api-reference.md b/hugegraph-pd/docs/api-reference.md index 79756afeb5..45ffecc21b 100644 --- a/hugegraph-pd/docs/api-reference.md +++ b/hugegraph-pd/docs/api-reference.md @@ -854,7 +854,10 @@ 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 leader lease timeout (90% of the election timeout by default); `NaN` on other nodes | +| `hg_raft_alive_peers` | On the leader, the number of peers it has heard from recently, itself included; `NaN` on other nodes | + +`hg_raft_alive_peers` counts the peers heard from within the leader lease +timeout, which jraft derives as 90% of the election timeout by default. 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-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 967b32a69b..80c657eccb 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 @@ -115,12 +115,15 @@ wait_for_pd() { # -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. # Captured rather than piped: under pipefail a grep -q SIGPIPE could misread a -# ready PD, and wait_for_pd() above uses the same shape. +# ready PD, and wait_for_pd() above uses the same shape. The curl timeouts keep a +# PD that accepts the connection and then stops answering from parking this loop +# past STARTUP_WAIT, since elapsed only moves when the request returns. wait_for_pd_ready() { local elapsed=0 while (( elapsed < STARTUP_WAIT )); do local body - body=$(curl -fsS "$PD_URL/v1/ready" 2>/dev/null || true) + body=$(curl --connect-timeout 2 --max-time 5 -fsS \ + "$PD_URL/v1/ready" 2>/dev/null || true) grep -Eq '"ready"[[:space:]]*:[[:space:]]*true' <<<"$body" && return 0 sleep 2 elapsed=$((elapsed + 2)) diff --git a/hugegraph-store/docs/deployment-guide.md b/hugegraph-store/docs/deployment-guide.md index f002046927..40c70b0e08 100644 --- a/hugegraph-store/docs/deployment-guide.md +++ b/hugegraph-store/docs/deployment-guide.md @@ -870,6 +870,14 @@ curl -i http://192.168.1.10:8620/v1/ready curl http://192.168.1.20:8520/v1/health ``` +> **Note**: `/v1/ready` ships from the release after `1.7.0`, so the Docker +> examples above, which pin `HUGEGRAPH_VERSION=1.7.0`, need a newer tag or +> images built from source before this check means anything. On `1.7.0` the PD +> answers `200` with `{"status":-1,"error":"Unauthorized!"}` on any path its +> auth interceptor does not exclude, `/v1/ready` included, so match on the body +> rather than the status code. See +> [docker/README.md](../../docker/README.md) for the details. + ### Cluster Status ```bash From 4cca88a86702f62408f7899ecaccebd082763da5 Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Tue, 8 Sep 2026 16:28:18 +0530 Subject: [PATCH 22/23] docs(pd): trim the alive peers row to one clause The table cell still carried the full lease-timeout explanation. Move the rest of it into the paragraph under the table so the row reads at a glance. --- hugegraph-pd/docs/api-reference.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/hugegraph-pd/docs/api-reference.md b/hugegraph-pd/docs/api-reference.md index 45ffecc21b..58b9698862 100644 --- a/hugegraph-pd/docs/api-reference.md +++ b/hugegraph-pd/docs/api-reference.md @@ -854,10 +854,11 @@ 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 it has heard from recently, itself included; `NaN` on other nodes | +| `hg_raft_alive_peers` | Number of alive peers on the leader, itself included; `NaN` elsewhere | -`hg_raft_alive_peers` counts the peers heard from within the leader lease -timeout, which jraft derives as 90% of the election timeout by default. +`hg_raft_alive_peers` counts the peers the leader has heard from within the +leader lease timeout, which jraft derives as 90% of the election timeout by +default. 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 From 0daffc65ec32ed2373cecd175df0f350afc1c9bf Mon Sep 17 00:00:00 2001 From: Himanshu Verma Date: Tue, 8 Sep 2026 16:44:40 +0530 Subject: [PATCH 23/23] fix(pd): bound the readiness wait on the wall clock The per-round counter added 2s per iteration while an attempt could cost up to --max-time plus the sleep, so a PD that accepted the connection and then stopped answering ran the loop well past STARTUP_WAIT. Measured against a socket that accepts and never replies: 70s elapsed for a STARTUP_WAIT of 20, which is ~210s at the 60 CI uses. A deadline taken from SECONDS bounds it at 21s. --- .../src/assembly/travis/test-start-hugegraph-pd.sh | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) 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 80c657eccb..754e2baae1 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 @@ -115,18 +115,19 @@ wait_for_pd() { # -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. # Captured rather than piped: under pipefail a grep -q SIGPIPE could misread a -# ready PD, and wait_for_pd() above uses the same shape. The curl timeouts keep a -# PD that accepts the connection and then stops answering from parking this loop -# past STARTUP_WAIT, since elapsed only moves when the request returns. +# ready PD, and wait_for_pd() above uses the same shape. The curl timeouts bound +# each attempt and the loop bounds itself on the wall clock, so a PD that accepts +# the connection and then stops answering cannot park this loop past STARTUP_WAIT. +# A per-round counter could not: an attempt costs up to --max-time plus the sleep, +# which is more than the counter would add. wait_for_pd_ready() { - local elapsed=0 - while (( elapsed < STARTUP_WAIT )); do + local deadline=$((SECONDS + STARTUP_WAIT)) + while (( SECONDS < deadline )); do local body body=$(curl --connect-timeout 2 --max-time 5 -fsS \ "$PD_URL/v1/ready" 2>/dev/null || true) grep -Eq '"ready"[[:space:]]*:[[:space:]]*true' <<<"$body" && return 0 sleep 2 - elapsed=$((elapsed + 2)) done return 1 }