Skip to content

fix(server): retry all PD peers while waiting for storage - #3129

Merged
imbajin merged 2 commits into
apache:masterfrom
bitflicker64:fix/wait-storage-pd-failover-3123
Jul 31, 2026
Merged

fix(server): retry all PD peers while waiting for storage#3129
imbajin merged 2 commits into
apache:masterfrom
bitflicker64:fix/wait-storage-pd-failover-3123

Conversation

@bitflicker64

@bitflicker64 bitflicker64 commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Kept in sync with hugegraph#175

Purpose of the PR

wait-storage.sh selected the first PD answering /v1/health, then polled
/v1/stores only on that peer. Since the health endpoint can succeed before
the peer has raft-backed store state, a storeless first peer could block Server
startup even when another configured PD already reported an Up store.

Main Changes

  • Probe /v1/stores across every configured PD peer on every retry.
  • Succeed as soon as any peer reports a store in state Up, without a separate
    /v1/health selection stage.
  • Preserve authentication, configured peer order, custom REST endpoints, retry
    timing, and the existing outer timeout failure.
  • Add a deterministic shell regression suite and an isolated Server CI job.

Verifying these changes

  • Need tests and can be verified as follows:
    • test-wait-storage.sh: 4 scenarios passed, 0 failed.
    • The same suite fails against the unmodified script on the pinned first peer.
    • bash -n passed for the production and test scripts.
    • Server CI workflow YAML parsing and git diff --check passed.
    • Apache RAT and EditorConfig checks passed for the 20-module dependency set.

Does this PR potentially affect the following parts?

  • Dependencies
  • Modify configurations
  • The public API
  • Other affects (HStore Server startup readiness)
  • Nope

Documentation Status

  • Doc - TODO
  • Doc - Done
  • Doc - No Need

Out of scope

  • PD and Store health endpoint semantics
  • wait-partition.sh
  • Environment-variable and timeout configurability cleanup
  • Docker Compose health checks and documentation

@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. ci-cd Build or deploy labels Jul 29, 2026
@imbajin
imbajin requested a review from Copilot July 30, 2026 07:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request fixes wait-storage.sh behavior in multi-PD HStore deployments by avoiding “pinning” to the first PD that answers /v1/health, and instead probing store readiness (/v1/stores) across all configured PD REST peers until any peer reports an Up store. This improves Server startup robustness when some PD peers are listening but not yet raft-ready.

Changes:

  • Update wait-storage.sh to retry /v1/stores across every configured PD peer on each retry cycle and succeed as soon as any peer reports "state":"Up".
  • Add a deterministic shell regression test suite that mocks curl, sleep, and timeout to validate failover / retry behavior.
  • Add a dedicated GitHub Actions job to run the new shell regression suite in CI.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.

File Description
hugegraph-server/hugegraph-dist/src/assembly/static/bin/wait-storage.sh Removes /v1/health gating and probes /v1/stores across all PD peers per retry, succeeding on the first peer that reports an Up store.
hugegraph-server/hugegraph-dist/src/assembly/travis/test-wait-storage.sh Adds deterministic, mocked shell tests covering peer order, retry/failover, and timeout behavior.
.github/workflows/server-ci.yml Adds a CI job that runs the new test-wait-storage.sh regression suite.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- cap each PD connection attempt at 2 seconds
- cap each PD request at 3 seconds
- cover failover after a hanging first peer
@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 38.20%. Comparing base (b9710a7) to head (6140721).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3129      +/-   ##
============================================
- Coverage     39.19%   38.20%   -0.99%     
- Complexity      264      424     +160     
============================================
  Files           770      770              
  Lines         65779    65779              
  Branches       8726     8726              
============================================
- Hits          25779    25131     -648     
- Misses        37247    37927     +680     
+ Partials       2753     2721      -32     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@imbajin imbajin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: ensure the image version → ubuntu22?

@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Jul 31, 2026
@imbajin
imbajin merged commit 8b2932c into apache:master Jul 31, 2026
18 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-cd Build or deploy lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

3 participants