feat(pd): add quorum-aware /v1/ready endpoint and raft gauges - #3185
Conversation
/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 apache#3183
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3185 +/- ##
=========================================
Coverage 37.78% 37.78%
- Complexity 6560 6563 +3
=========================================
Files 800 800
Lines 68960 68960
Branches 9166 9166
=========================================
+ Hits 26054 26058 +4
+ Misses 39839 39836 -3
+ Partials 3067 3066 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
bitflicker64
left a comment
bitflicker64
left a comment
There was a problem hiding this comment.
Blocking: no. Summary: The liveness and readiness split is the right fix for #3183, the jraft assumptions behind it hold, and the new unit tests pass locally. Four minor notes: one on field visibility, two on comment and doc accuracy, one on image-version compatibility for the compose healthcheck change. Evidence: ran mvn -o -pl hugegraph-pd/hg-pd-test -am -Dtest=RaftEngineReadinessTest test on JDK 11 (9/9 pass) after building hg-pd-core and hg-pd-service; checked jraft 1.3.13 directly, where State.isActive() is ordinal() < STATE_ERROR, NodeImpl.listAlivePeers() throws IllegalStateException off-leader under a read lock, and getLeaderId() already maps an empty peer to null; confirmed MetricsConfig.metricsCommonTags adds hg="pd", so the hg_raft_*{ assertions in RestApiTest will match the Prometheus rendering. CI at 4dd7e71 was still running, with the pd, store and hstore integration jobs incomplete, so the live-PD assertions are unverified here.
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.
…ment health vs ready PD's /v1/health answers 200 as soon as the REST listener is up and never consults raft, so every PD and Store probe and the Store init container's PD wait count listeners, not quorum members (apache#3183). The fix, apache#3185, adds /v1/ready from 1.8.0. - pd.readinessPath and store.waitPath, both defaulting to /v1/health, so the switch to /v1/ready is a values change made with the 1.8.0 pin; the schema rejects paths without a leading slash - README: Limitations entries for the liveness-only health endpoint and for the 45 second discovery lease (measured 30 to 35 seconds); the Store wait is described as a PD wait rather than a quorum wait; the Server now registers its Pod IP, not the Service URL - NOTES and the init container messages no longer claim a quorum - tests: pd_readiness_path_test.yaml, five cases
Points at apache#3185 and says the defaults flip with the 1.8.0 image pin, so the change is not lost once that PR merges.
bitflicker64
left a comment
bitflicker64
left a comment
There was a problem hiding this comment.
Blocking: no. Summary: The endpoint, the raft accessors and the gauges are correct and well covered, and the jraft assumptions behind them hold. Two things to fix before merge: the paragraph added at docker/README.md:210-212 states a failure mode that this PR's own CI disproves, and the compose healthchecks it describes do not actually gate on readiness, because PD answers an unauthenticated request with HTTP 200 and an error body. The pre-PR /v1/health probe had the same property, so this is a missed improvement rather than a regression. Evidence: RestAuthentication.preHandle:61-66 writes the error body and returns false without response.setStatus(...); in CI run 33642694186, job build-server (rocksdb, 11), the hstore smoke pulled Docker Hub hugegraph/pd:latest (git grep '/v1/ready' origin/master -- hugegraph-pd is empty) and logged Container ...-pd-1 Healthy 11 seconds after start; PDCoreSuiteTest (101 run, 2 skipped) and PDRestSuiteTest (16 run) pass at 5bd1b96.
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.
imbajin
left a comment
imbajin
left a comment
There was a problem hiding this comment.
Blocking: no. Summary: The readiness implementation is correct, but the deployment guide overstates which compose probe is active. Evidence: JDK 11 RaftEngineReadinessTest passed 10/10; the current compose files still use /v1/health, and the Codecov patch failure is non-blocking.
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.
bitflicker64
left a comment
bitflicker64
left a comment
There was a problem hiding this comment.
Blocking: no. Summary: The liveness and readiness split is the right fix for #3183, and at this head the endpoint, the raft accessors and the three gauges all check out; three minor notes remain, all about wording rather than behaviour. Evidence: jraft-core 1.3.13 State.isActive() is ordinal() < STATE_ERROR.ordinal() over LEADER, TRANSFERRING, CANDIDATE, FOLLOWER, ERROR, ..., so a candidate counts as active; NodeImpl.getAliveNodes compares against leaderLeaseTimeoutMs and calls no checkReplicator, so hg_raft_alive_peers is side-effect free on every scrape; hugegraph-pd/hg-pd-dist/src/assembly/static/conf/application.yml:61 has a single-peer peers-list, so the new wait_for_pd_ready gate in test-start-hugegraph-pd.sh is reachable on a one-node dist; PDService redirects non-leader gRPC calls to the leader (putLicense at PDService.java:1376 is the one exception), so a follower that sees a leader really can serve. CI on ffa13f9 is green except codecov/patch.
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.
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.
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.
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.
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.
bitflicker64
left a comment
bitflicker64
left a comment
There was a problem hiding this comment.
Blocking: no. Summary: The readiness split is well-reasoned and the jraft usage checks out against the pinned jraft-core 1.3.13 (getNodeState()/State.isActive()/listAlivePeers() all behave as the javadoc claims, and getAliveNodes() really does include the node itself); three minor polish items only, none of which should hold up merge. Evidence: git diff 98477f0f5 refs/remotes/pr/3185 for the exact-head diff; javap on com.alipay.sofa.jraft.Node and com.alipay.sofa.jraft.core.State plus NodeImpl.java from the 1.3.13 sources jar (listAlivePeers() throws IllegalStateException off-leader at L2977, getLeaderId() already maps an empty peer to null at L2487, getAliveNodes() adds serverId at L2266); git show 98477f0f5:.../RestAuthentication.java confirms preHandle writes the error envelope without setStatus, so the docs' "match the body, not the status" guidance is correct; MetricsConfig.metricsCommonTags() registers commonTags("hg", "pd"), so the new gauges render with the {...} block the RestApiTest assertions expect; callers of isLeader()/getLeader() audited repo-wide for the new null-safety and none regress. Not verified: no build or test run against this head, and the effective Spring version comes from a parent BOM, so trailing-slash interceptor matching on /v1/ready/ was left out of these comments.
imbajin
left a comment
imbajin
left a comment
There was a problem hiding this comment.
Blocking: yes. Summary: The new interceptor test does not compile at this head, so the PD test and server build jobs cannot pass. Evidence: GitHub Actions run 33839755062 (pd and hstore) and run 33839755249 (both macOS server jobs) report cannot find symbol for AuthenticationConfigurer and RestAuthentication at AuthenticationConfigurerTest.java:53-54; both classes are defined in hg-pd-service.
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.
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.
PD gains the quorum-aware /v1/ready endpoint, which answers 503 without a raft leader, and the hg_raft_* gauges. The endpoint sits outside the auth interceptor. The pull request is open against master; this branch carries it so the helm-dev images can be tested with pd.readinessPath and store.waitPath set to /v1/ready (apache#3183).
PD REST now checks the password against auth.secret-key and answers 401 on refusal (apache#3188). The PD image requires HG_PD_AUTH_SECRET_KEY, wait-storage.sh sends PD_AUTH_PASSWORD, and Hubble reads operations.pd.password. Two conflicts with the pull requests merged before it, both resolved as the union: the interceptor exclusion list keeps /v1/ready from apache#3185 alongside the /actuator/** widening from apache#3189, and test-compose.sh runs the startup timeout asserts from apache#3187 followed by the Hubble helper check from apache#3189. render_with_timeout from apache#3187 additionally passes HG_PD_AUTH_SECRET_KEY, which the Compose files require since apache#3189; without it the render step would fail on the two HStore topologies. The chart does not yet supply the PD secret, so PD Pods from an image built at this revision will not start under the chart until that wiring lands.
PD images from 1.8.0 (apache#3189) check the Basic-auth password of every management call against auth.secret-key and refuse to start without one. The chart now keeps that value in a kept release-pd-auth Secret, or in pd.auth.existingSecret, and hands it to the three readers: PD as HG_PD_AUTH_SECRET_KEY, the Server storage wait as PD_AUTH_PASSWORD, and Hubble as operations.pd.password written into its properties file by the existing wrapper. A checksum/pd-auth annotation on the three Pod templates rolls them when the Secret changes; the Server annotations block is now rendered unconditionally for it. Priority and lookup semantics mirror server.auth.token. Older images ignore the password, so the wiring is harmless on the images the draft currently tracks. The values schema requires one of existingSecret, value or autoGenerate and refuses newlines, carriage returns and backslashes in an inline value, since it lands in a Java properties file; the template guard repeats the first rule for values that bypass the schema. The three chart-managed variables join the reserved extraEnv lists. README: Chart Details bullet, four parameter rows, Disaster Recovery calls carry the secret, and the Limitations bullet separates the 1.7.0 behaviour from 1.8.0. NOTES prints how to read the secret. New suite pd_auth_secret_test.yaml, 9 tests; 58 in total. Lint on three presets; renders 16 objects by default and 19 with Hubble. Measured on a kind cluster with images built from master plus apache#3185, apache#3187 and apache#3189: the Secret is created, PD starts with the variable, the Server storage wait passes with the credential, and Hubble lists all nine nodes.
|
Tested end to end on Kubernetes on 2026-09-05, with this branch merged into the hugegraph/hugegraph testing tree. Build under test. Tag What held. Deleting two of three PDs with
One thing worth a look before merge: the handler stalls during the election. With The sample logs and scripts are kept with the campaign notes; I can attach them here if useful. |
|
Logs and scripts from the two runs above, hosted on my fork (branch
The stall, from run 2 ( T0 was 1788593447, the third line's timestamp. The fourth line's request went out at about T0+2.3 s and got its 503 at about T0+12.1 s, 9.79 s later; the gauge on that same line was read after the stall, once the replacements were back, which is why it already shows |
bitflicker64
left a comment
bitflicker64
left a comment
There was a problem hiding this comment.
Blocking: no. Summary: The callback-served readiness path is sound and its jraft assumptions hold up, but getAlivePeerCount() still takes the raft node's read lock, so a metrics scrape can block on the very lock /v1/ready was restructured to avoid. Evidence: jraft-core 1.3.13 sources resolved for hg-pd-core (NodeImpl.listAlivePeers, NodeImpl.stepDown, NodeImpl.preVote, AbstractClientService.connect) read against the head diff.
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Adds a quorum-aware readiness signal for PD and corresponding raft membership gauges, clarifying liveness vs readiness for operators and automation.
Changes:
- Introduces unauthenticated
GET /v1/readythat returns200only when PD sees a raft leader (otherwise503) - Exports raft quorum-related gauges (
hg_raft_leader,hg_raft_has_leader,hg_raft_alive_peers) for alerting - Updates tests, scripts, and docs to use/readiness semantics and avoid false-ready probes
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| hugegraph-store/docs/deployment-guide.md | Documents PD liveness (/v1/health) vs readiness (/v1/ready) for deployment gating |
| hugegraph-server/hugegraph-dist/src/assembly/travis/test-start-hugegraph-pd.sh | Adds readiness wait loop that checks /v1/ready content before proceeding |
| hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/RestApiTest.java | Adds REST tests for anonymous health/ready endpoints and new metrics gauges |
| hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/raft/RaftEngineReadinessTest.java | Adds unit tests for raft-derived readiness behavior and edge cases |
| hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/core/PDCoreSuiteTest.java | Includes new readiness test in the PD test suite |
| hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/interceptor/AuthenticationConfigurer.java | Excludes /v1/ready from auth interceptor like /v1/health |
| hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/rest/StoreAPI.java | Implements GET /ready endpoint response and status selection |
| hugegraph-pd/hg-pd-service/src/main/java/org/apache/hugegraph/pd/metrics/PDMetrics.java | Registers new raft membership gauges |
| hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftStateMachine.java | Adds lock-free probe view updated by raft callbacks |
| hugegraph-pd/hg-pd-core/src/main/java/org/apache/hugegraph/pd/raft/RaftEngine.java | Adds quorum-aware status snapshot, leader visibility helpers, and null-safety |
| hugegraph-pd/docs/api-reference.md | Documents health vs ready endpoints and new metrics |
| hugegraph-pd/README.md | Mentions new liveness/readiness endpoints |
| docker/README.md | Explains why compose healthchecks remain on /v1/health and how to safely switch to readiness |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
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.
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.
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.
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.
Brings in the master merge already made on the hugegraph/hugegraph mirror, so the fork and the mirror share one history again. Master now carries apache#3187; it does not touch anything this branch changes.
bitflicker64
left a comment
bitflicker64
left a comment
There was a problem hiding this comment.
Blocking: no. Summary: The readiness design holds up against the jraft 1.3.13 sources actually on the classpath — State.isActive() excludes STATE_UNINITIALIZED, NodeImpl.handleElectionTimeout resets the leader id while still STATE_FOLLOWER so onStopFollowing really does fire on heartbeat loss, stepDown fires onLeaderStop before resetting, and all six hooked callbacks are delivered on FSMCallerImpl's single disruptor consumer, so onStopFollowing's read-modify-write is single-threaded as documented. Three minor points only: hasLeader() drops the active-state guard getRaftStatus() applies, the awaitTermination guard in shutDown() cannot prevent what its comment claims, and the PR description no longer matches this head. Evidence: git diff bed2e457..63a26900 in the configured checkout (15 files, +799/-6, matching gh api repos/apache/hugegraph/pulls/3185/files); read com/alipay/sofa/jraft/core/State.java and NodeImpl.java:615-644,1195-1206,1268-1290 from jraft-core-1.3.13-sources.jar; PDConfig.java:153 (raft.rpc-timeout default 10000 ms) against RaftEngine.init's setRpcConnectTimeoutMs; hugegraph-pd/Dockerfile:68-69, docker/docker-compose-hstore.yml:47 and docker/docker-compose-3pd-3store-3server.yml:40 to check the docs' probe claims; install-dist/scripts/dependency/regenerate_known_dependencies.sh (-DincludeScope=runtime) for the new test-scope junit; and the CI job log for this head (run 34155085574, job pd), which shows StoreAPIReadyTest 3/0/0 under surefire (default-test) @ hg-pd-service, PDCoreSuiteTest 102/0/0, PDRestSuiteTest 16/0/0 and PASS PD readiness endpoint reports a raft leader. Not independently built or run locally (only JDK 17 available, project targets 11), and no 3-PD fault injection was reproduced.
bitflicker64
left a comment
bitflicker64
left a comment
There was a problem hiding this comment.
Blocking: no. Summary: Fresh pass at 63a2690, the same head the last review covered; no new findings, so this adds applyable suggestions for that review's two code items. The third item, the stale PR description, has no code anchor and still needs a body edit before the squash merge. Evidence: the hasLeader() derivation was traced through all ten RaftEngineReadinessTest cases, including testProbeNeverTouchesTheRaftNode (a field null check is not a mock interaction); micrometer-core 1.7.12 sources confirm Gauge.builder(String, Supplier) ends in .strongReference(true), so the supplier-fed gauges cannot be collected into a permanent NaN. Not verified by a local build.
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.
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.
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.
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.
apache#3185 landed and both branches rewrote the same excludePathPatterns call. Took the union: /actuator/** from this branch, /v1/ready from apache#3185. Dropping either side would re-authenticate the readiness endpoint or reinstate the single-star pattern the docs on this branch now contradict.
Brings in the upstream forms of the three fixes this branch had been carrying as PR-branch merges: apache#3185 (quorum-aware PD /v1/ready and raft gauges), apache#3187 (configurable Server startup timeout) and apache#3189 (PD REST credential validation, 401 on refusal). Every conflicted path, and the two that auto-merged into doubled hunks (hg-pd-service/pom.xml, travis/start-pd.sh), is resolved to master's content. Outside helm/ the tree is now identical to master apart from the chart overlay: the helm-chart-ci workflow, the .tgz excludes in pom.xml and .dockerignore, and the chart section in README.md.
apache#3185, apache#3187 and apache#3189 are merged upstream and due in 1.8.0, so the chart no longer has to wait for them behind a TODO. - pd.readinessPath and store.waitPath default to /v1/ready, which answers 503 until a raft leader exists. PD readiness now means quorum membership and the Store's PD wait counts quorum members rather than live listeners. Startup and liveness stay on /v1/health so a PD that merely lost its leader is not restarted. On a PD image that predates apache#3185 both values have to be set back to /v1/health. - The Server gets HG_SERVER_STARTUP_TIMEOUT_S from the startup probe budget (effective failureThreshold * periodSeconds, 450s by default, capped at the entrypoint's 86400s). The image default of 120s is shorter than the storage wait alone, so the start command used to kill a Server that was still coming up. The variable joins the reserved list, because a server.extraEnv duplicate would silently decouple the process timeout from the probe. - Drops the two TODO comments the paths carried, and rewrites the README Limitations bullets on PD REST auth and /v1/health vs /v1/ready so the current behaviour leads and the older behaviour is the caveat. Chart 0.1.6, repackaged. lint x3 presets clean, 63 unit tests pass (5 new), renders 19/17/20 with one /v1/ready path and HG_SERVER_STARTUP_TIMEOUT_S=450.
Picks up apache#3182 (index result filtering) and the three fixes this chart depends on: apache#3185 (quorum-aware PD /v1/ready and raft gauges), apache#3187 (configurable Server startup timeout) and apache#3189 (PD REST credential validation, 401 on refusal). No conflicts; the branch touches only helm/, the chart CI workflow and the README chart section.
apache#3185, apache#3187 and apache#3189 are merged upstream and due in 1.8.0, so the chart no longer has to wait for them behind a TODO. - pd.readinessPath and store.waitPath default to /v1/ready, which answers 503 until a raft leader exists. PD readiness now means quorum membership and the Store's PD wait counts quorum members rather than live listeners. Startup and liveness stay on /v1/health so a PD that merely lost its leader is not restarted. On a PD image that predates apache#3185 both values have to be set back to /v1/health. - The Server gets HG_SERVER_STARTUP_TIMEOUT_S from the startup probe budget (effective failureThreshold * periodSeconds, 450s by default, capped at the entrypoint's 86400s). The image default of 120s is shorter than the storage wait alone, so the start command used to kill a Server that was still coming up. The variable joins the reserved list, because a server.extraEnv duplicate would silently decouple the process timeout from the probe. - Drops the two TODO comments the paths carried, and rewrites the README Limitations bullets on PD REST auth and /v1/health vs /v1/ready so the current behaviour leads and the older behaviour is the caveat. lint x3 presets clean, 63 unit tests pass (5 new), renders 16/14/16 with one /v1/ready path and HG_SERVER_STARTUP_TIMEOUT_S=450. Templates, values.schema.json and tests are byte-identical with the helm-dev testing branch.
Chart 0.1.6 moved pd.readinessPath and store.waitPath to /v1/ready and updated most of the surrounding prose, but three places were left describing the old default: templates/NOTES.txt, which is printed to the operator on every install and every upgrade, and the Configuration rows for both values, where the Default column still read /v1/health. The two rows also gave the wrong reason for setting /v1/health on an older image. They said /v1/ready "exists only on PD images from 1.8.0", which reads as though an older image would simply reject it. It does not. On a PD image predating apache#3189 the auth interceptor writes its error body without setting a status, so every unmapped path under /v1/ answers 200, and /v1/ready is such a path. The readiness probe then passes unconditionally and the signal means nothing. Measured on a PD image built from bed2e45, the parent of the apache#3185 merge, so it carries apache#3187 but neither apache#3185 nor apache#3189: all three PD pods reported Ready, /v1/ready returned 200 with an Unauthorized body, and an arbitrary nonexistent path returned the same 200. On merged master both return 401. No template behaviour changes. Renders stay 16/14/16 and the 63 unit tests pass.
Chart 0.1.6 moved pd.readinessPath and store.waitPath to /v1/ready and updated most of the surrounding prose, but three places were left describing the old default: templates/NOTES.txt, which is printed to the operator on every install and every upgrade, and the Configuration rows for both values, where the Default column still read /v1/health. The two rows also gave the wrong reason for setting /v1/health on an older image. They said /v1/ready "exists only on PD images from 1.8.0", which reads as though an older image would simply reject it. It does not. On a PD image predating apache#3189 the auth interceptor writes its error body without setting a status, so every unmapped path under /v1/ answers 200, and /v1/ready is such a path. The readiness probe then passes unconditionally and the signal means nothing. Measured on a PD image built from bed2e45, the parent of the apache#3185 merge, so it carries apache#3187 but neither apache#3185 nor apache#3189: all three PD pods reported Ready, /v1/ready returned 200 with an Unauthorized body, and an arbitrary nonexistent path returned the same 200. On merged master both return 401. Version bumped to 0.1.7 and the packaged archive rebuilt, because 0.1.6 is already tagged and published and its content must not change. The upgrade note says the step rolls nothing. Renders stay 19/17/20 and the 63 unit tests pass. templates/NOTES.txt stays byte-identical with the same fix on the apache#3132 branch.
Purpose of the PR
/v1/healthon PD reports healthy without a raft quorum. This adds a quorum-aware readiness signal and leaves/v1/healthas pure liveness.Main Changes
RaftEngine: newhasLeader(),getRaftStatus()andgetAlivePeerCount(), plus the nestedRaftStatusview (isReady(),getState(),isLocalLeader()) the endpoint and the gauges both read.hasLeader()isgetRaftStatus().isReady(), so the gauge cannot drift from the endpoint.isLeader()andgetLeader()are now null-safe before the raft node starts.StoreAPI: new unauthenticatedGET /v1/ready. Returns200with{"ready":true,"state":"STATE_LEADER","isLeader":true}while the raft node is active and sees a leader,503with"ready":falseotherwise. The body comes from oneRaftEngine.getRaftStatus()snapshot and carries no cluster addresses. Added to the auth interceptor exclusion list next to/v1/health.PDMetrics: three gauges for alerting on quorum loss:hg_raft_leader,hg_raft_has_leader,hg_raft_alive_peers(leader only,NaNelsewhere)./v1/health: these files run published images, and PD's auth interceptor answers200on any path it does not exclude, so a status-only probe reads a PD without the endpoint as ready. The docker README records what switching them over needs, a body match on"ready":trueand an image that carries the endpoint.Why "sees a leader" is the right local signal: jraft resets a follower's leader id once heartbeats stop arriving inside the election timeout, and a leader steps down when it cannot reach a quorum. So a non-null leader id means this node is inside a quorum from its own point of view, which is what a readiness probe needs. This matches the behaviour measured in the issue, where the survivor logged
Raft lost leaderwithin a second of the fault.Verify the Changes
RaftEngineReadinessTest(added toPDCoreSuiteTest), twelve cases: not ready before the raft node starts, a started node with no callback yet, leader ready, follower with a leader ready, a follower losing its leader, a leader stepping down, error and shutdown after leadership, the shutdown state surviving the raft node being dropped, the probe never touching the raft node, the leadership-loss race ingetAlivePeerCount(), the count on a leader still skipping the node, and the count falling back to-1once a leader errors or shuts down.StoreAPIReadyTestinhg-pd-servicedrives the endpoint itself:503before the raft node starts,503once the leader is lost, and200withSTATE_LEADERon a leader, all without touching the raft node.RestApiTest(runs against the live CI PD) now checks that/v1/healthanswers200with an empty body, that/v1/readyreportsready=trueandSTATE_LEADERon the single-node PD without disclosing an address, and that the three gauges are exported with the expected values.test-start-hugegraph-pd.shwaits for/v1/readyto return200withready=trueafter the health endpoint responds.4cca88a:mvn test -pl hugegraph-pd/hg-pd-test -am -P pd-core-testpasses 104/104 (2 skipped), andtest-start-hugegraph-pd.shpasses 13/13 against a PD built from this tree.pd-rest-testandpd-client-testrun against a live PD in CI. Earlier on the branch, against a live PD,/v1/readyanswered200{"ready":true,...}as leader and503{"ready":false,...}with two unreachable peers, while/v1/healthstayed200throughout and the gauges moved1/1/1to0/0/NaN.Does this PR potentially affect the following parts?
Notes for reviewers:
/v1/healthand point readiness probes at/v1/ready. Using/v1/readyas a liveness probe would restart a PD that merely lost its leader./v1/healthis unchanged; this PR only covers PD./v1/readymust match the body, not just the status code.RestAuthentication.preHandlerejects by writing an error envelope without callingsetStatus, so any non-excluded path answers200with{"status":-1,"error":"Unauthorized!"}. Fixing that root cause is out of scope here.Documentation Status
Doc - Updated