[Bug] wait-storage.sh binds to the first PD answering /v1/health and never fails over to other peers

[bitflicker64]

Bug Type (问题类型)

server status (启动/运行异常)

Before submit

• I have confirmed and searched that there are no similar problems in the historical issues and documents

Environment (环境信息)

• Server Version: master (verified at b9710a7), also present in 1.7.0
• Backend: hstore, 3 PD x 3 Store x 3 Server (docker/docker-compose-3pd-3store-3server.yml)
• OS: Ubuntu 22.04 containers on Docker 29.4.0
• Data Size: n/a, fails before init

Expected & Actual behavior (期望与实际表现)

In a multi-PD deployment, bin/wait-storage.sh picks one PD and stays on it for the whole wait, even when that PD is the only one that cannot answer correctly. The other PDs are never contacted again.

The relevant part of hugegraph-server/hugegraph-dist/src/assembly/static/bin/wait-storage.sh:

check_any_pd() {
  for peer in $(echo "$PD_REST_LIST" | tr ',' ' '); do
    if curl ${PD_AUTH_ARGS} -f -s http://${peer}/v1/health >/dev/null 2>&1; then
      echo "$peer"
      return 0
    fi
  done
  return 1
}

until PD_REST=$(check_any_pd); do
  log 'No PD peer ready yet, retrying in 5s'
  sleep 5
done
log "PD health check PASSED via $PD_REST"

until curl ${PD_AUTH_ARGS} -f -s \
      http://${PD_REST}/v1/stores 2>/dev/null | \
      grep -qi '"state"[[:space:]]*:[[:space:]]*"Up"'; do
  log 'No Up store yet, retrying in 5s'
  sleep 5
done

PD_REST is assigned once. The second loop then polls that single peer until the 300s timeout expires.

The catch is that /v1/health doesn't tell you whether the PD is a working member of the raft group. In hugegraph-pd/hg-pd-service/.../rest/StoreAPI.java:

@GetMapping(value = "/health", produces = MediaType.TEXT_PLAIN_VALUE)
public Serializable checkHealthy() {
    return "";
}

It returns 200 as soon as the Spring listener is up. Meanwhile /v1/stores reads the node's own raft-backed metadata (PDRestService.getStores to StoreNodeService.getStores to StoreInfoMeta.getStores, which does a scanPrefix over local KV). There is no leader redirect in the PD REST layer. So a PD that is listening but has no raft state returns HTTP 200 with an empty store list. curl -f succeeds, the grep never matches, and the loop spins until timeout.

I confirmed this against a real PD started with a peer list whose other two members do not exist:

/v1/health   -> HTTP 200          (passes the curl -f gate in the script)
/v1/stores   -> {"stores":[],"numOfService":0,"numOfNormalService":0,...}   HTTP 200
/v1/members  -> HTTP 500 Internal Server Error

Docker reported that container as Up (healthy) the whole time.

Since docker-entrypoint.sh runs with set -euo pipefail and calls the script unguarded, the timeout kills the entrypoint and the container restarts.

Reproduction

I couldn't get this to happen from a plain cold docker compose up, since the depends_on: service_healthy ordering usually gives the raft group enough time to form. It shows up when a PD is slow to join, or when the selected PD goes away partway through the wait.

To get a deterministic case I ran the unmodified script against three PD REST endpoints on a docker network, with the first peer serving /v1/health normally but reporting an empty store list, and the other two reporting a store in state Up:

peer list	result
pd0,pd1,pd2 (pd0 storeless)	exit 1 after 301s
pd1,pd0,pd2 (same cluster, order swapped)	exit 0 in 1s
pd0,pd1,pd2, pd0 killed 30s into the wait	exit 1 after 300s

Request counts on the PD side during the failing run:

pd0: 61 GET /v1/stores
pd1:  0 GET /v1/stores
pd2:  0 GET /v1/stores

Peer ordering is the only difference between the passing and failing runs.

The restart does not help as much as it looks like it should. Running the same setup under restart: unless-stopped, with the first PD joining the group at t+330s:

### wait-storage attempt starting 18:23:27
[wait-storage] PD health check PASSED via pd0:8620
[wait-storage] ERROR: Timeout waiting for storage backend
### wait-storage attempt starting 18:28:28
[wait-storage] PD health check PASSED via pd0:8620
[wait-storage] Store registration check PASSED
[wait-storage] Storage backend is VIABLE

RestartCount=1  Running=true  ExitCode=0

The second attempt re-runs selection but picks the same peer, because the list order is fixed. It only passed because the PD had caught up by then. If that PD stays up and storeless, every cycle fails the same way and the server never starts. The error text points at the storage backend, which is the one component that was fine, so this looks like slow storage rather than a peer selection problem. That's probably why it hasn't been noticed.

Suggested fix

Probe /v1/stores across every peer on each attempt instead of binding to one. /v1/stores succeeding is strictly stronger than /v1/health, so the separate health loop can go:

-              check_any_pd() {
+              check_any_pd_stores() {
                 for peer in \$(echo \"\$PD_REST_LIST\" | tr ',' ' '); do
-                  if curl ${PD_AUTH_ARGS} -f -s http://\${peer}/v1/health >/dev/null 2>&1; then
+                  if curl ${PD_AUTH_ARGS} -f -s http://\${peer}/v1/stores 2>/dev/null | \
+                     grep -qi '\"state\"[[:space:]]*:[[:space:]]*\"Up\"'; then
                     echo \"\$peer\"
                     return 0
                   fi
                 done
                 return 1
               }
 
-              until PD_REST=\$(check_any_pd); do
+              until PD_REST=\$(check_any_pd_stores); do

with the second until loop and its log line removed. This gives both all-peer probing and re-selection on every retry. With this change the two failing cases above exit 0 in under a second.

The java client already does the right thing here: PDClient.newLeaderStub() iterates every host, uses getMembers to find the actual raft leader, and moves on when a host throws. Only the shell layer lacks that.

Related, lower priority

checkHealthy() returning an unconditional 200 also means the depends_on: service_healthy gates in all three compose files do not check anything beyond the port being open. The Store health endpoint has the same body. Splitting liveness from readiness would help, with readiness backed by something raft aware such as RaftEngine.getLeaderGrpcAddress(). I would not make a PD exit when it cannot reach quorum, since a node waiting for its peers to come back is often the correct one and exiting turns a recoverable partition into a crash loop.

bin/wait-partition.sh has a similar single endpoint binding, and STORE_REST is pinned to store0:8520 for all three servers in the compose file. Lower impact, since the entrypoint calls it with || log "WARN: partitions not assigned yet".

Two smaller things in the same file:

• Line 78 reads PD_PEERS="${hugegraph_pd_peers:-}", but the entrypoint exports hugegraph.pd.peers with dots, which bash cannot expand, and nothing sets the underscore name. It works only because the env to config loop rewrites pd.peers into the properties file and the line 80 fallback picks it up.
• Line 32 sets WAIT_STORAGE_TIMEOUT_S=300 unconditionally, so the [ -n ... ] guard at line 76 is always true and the value cannot be overridden from the environment. WAIT_STORAGE_TIMEOUT_S="${WAIT_STORAGE_TIMEOUT_S:-300}" would match how wait-partition.sh handles its timeout.

Happy to send a PR for the wait-storage.sh change if that's useful.