feat(server): add a storage-aware GET /readiness endpoint

[SebastianGruza]

Purpose of the PR

Closes #3212. A Kubernetes readiness probe on /versions keeps a Server in the Service while every graph request fails, because /versions answers 200 as long as the REST layer is up, even with no Store in the cluster (measured under #3132: ready=true with 0 Stores for 150+ s while every GET /graph/vertices/<id> ended in a 500 after the 30 s request bound). Design discussed in the issue.

Main Changes

GET /readiness answers 200 while this Server can serve graph traffic and 503 otherwise, unauthenticated like /versions (whitelisted in AuthenticationFilter and PathFilter, so an httpGet probe needs neither a credential nor a graphspace prefix), with a JSON body that carries no addresses:

{"ready":true,"storage":"hstore","reason":"ok","active_stores":3,"answered_store":6386607161404282741,
 "pd_reachable":true,"pd_checked_age_ms":312,"stores_age_ms":312,"store_millis":3,"cached":false}

Ready means, from this Server's own view: at least one Store from the last Store list PD answered with answers a direct, local, read-only gRPC call (HgStoreState.getScanState, a read of the node's own scan-pool stats that never touches raft). The Store list is refreshed from PD in the background on every probe and never waited for once a list is known, so PD being down, slow or restarting only flips the reported pd_reachable, never the readiness, as long as a Store answers. The pings run in parallel and the first answer wins, so a Store whose pod just left never eats the budget of the healthy ones.

• hugegraph-hstore: HstoreStorageProbe (pure logic over a Store lister and a Store pinger, one shared time budget through an executor so a hung PD or Store yields "did not answer within N ms" instead of a hung probe; KnownStores holds the last PD answer) and a storage_readiness meta handler in HstoreStore.
• hugegraph-api: ReadinessAPI and StorageReadiness (first hstore graph is probed as the internal admin through HugeGraph.metadata(null, "storage_readiness", timeout), result cached for readiness.cache_ttl; Servers with no hstore graph answer 200 with storage=embedded); the api module gains no dependency.
• ServerOptions: readiness.timeout (default 1000 ms) and readiness.cache_ttl (default 2000 ms).

Verifying these changes

• Add new unit test(s): HstoreStorageProbeTest (15: first answer wins, a hung Store does not hide an answering one, known Stores keep the Server ready while PD is down, a hung PD does not delay a probe with known Stores, the PD answer updates the known list, budget bounding, no addresses in the body) and StorageReadinessTest (6, including the two filter whitelists).
• E2E on a three-node k3s with the chart from feat(helm): add HStore deployment chart #3218 / feat(helm): add HStore deployment chart hugegraph/hugegraph#221, 1 Hz sampling of /readiness, /versions, the pod Ready condition and the Service endpoints on every Server pod, load through the Service: 7 PASS (baseline; Stores→0 pulls every Server out of the Service in ~30 s and back in ~20 s; one Store deleted, no flap; rolling restart of the Stores under load, zero readiness transitions; PD→0 stays 200 with pd_reachable=false; rolling restart of PD, zero transitions; SIGSTOP the busiest Store, zero transitions). Scripts, per-scenario JSON and the two superseded probe designs (an existsTable ping flapped on a roll; probing PD on every request tracked PD instead of storage): https://github.com/SebastianGruza/hugegraph-validation/tree/master/results/issue-3212

Does this PR potentially affect the following parts?

• Introduce new configurations: readiness.timeout, readiness.cache_ttl in ServerOptions
• New unauthenticated endpoint GET /readiness; no data-format change.

Chart side

A companion change adds server.readinessPath (default /versions) to the #3132 chart, mirroring pd.readinessPath; set it to /readiness on an image that serves it. Diff and measurements are in the validation repo above and will go to hugegraph#221.

🤖 Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 62.27273% with 83 lines in your changes missing coverage. Please review.
✅ Project coverage is 41.41%. Comparing base (4f5202b) to head (4a4aa1c).

Files with missing lines	Patch %	Lines
...graph/backend/store/hstore/HstoreStorageProbe.java	54.90%	53 Missing and 16 partials ⚠️
...apache/hugegraph/api/profile/StorageReadiness.java	74.46%	9 Missing and 3 partials ⚠️
...org/apache/hugegraph/api/profile/ReadinessAPI.java	90.90%	0 Missing and 1 partial ⚠️
...he/hugegraph/backend/store/hstore/HstoreStore.java	66.66%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #3221      +/-   ##
============================================
+ Coverage     41.33%   41.41%   +0.07%     
- Complexity     7289     7322      +33     
============================================
  Files           802      805       +3     
  Lines         69659    69879     +220     
  Branches       9285     9311      +26     
============================================
+ Hits          28795    28941     +146     
- Misses        37574    37632      +58     
- Partials       3290     3306      +16     

☔ 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.

[bitflicker64]

Blocking: no. Summary: the probe design (last known Store list, parallel pings, one shared budget) holds up and is well tested, but the unauthenticated endpoint's body can leak PD peer and Store host names through exception text, and a hung PD makes every cache-missed probe park another thread on an unbounded pool. Two doc and resource nits besides. Evidence: static read of the d0f4a25 diff against PDClient (getStub/newBlockingStub, PDConfig.grpcTimeOut=60000), HstoreSessionsImpl.initStoreNode and grpc-core 1.47 DnsNameResolver; CI green on this head apart from codecov, and HstoreStorageProbeTest (15) passes in the hstore job log.

[bitflicker64]

⚠️ /readiness is unauthenticated and the PR says the body carries no addresses, but reason embeds raw exception messages. Two paths:

• No Store list known yet and the PD client has no live stub (never connected, or reset by closeStub after a watch error) with peers refusing fast: await() unwraps the PDException from newBlockingStub(), whose message is "PD unreachable, pd.peers=" + config.getServerHost() (PDClient.java:142-143), so every PD peer is listed.
• Every known Store fails and one failed name resolution: UNAVAILABLE: Unable to resolve host <host> (grpc-core 1.47 DnsNameResolver), if Stores register by DNS name.

StorageReadiness.check also appends e.getMessage(); testMapCarriesNoAddresses covers only the ready path.

Could reason use a fixed category (pd unreachable, the gRPC status code or exception class) and log the full message instead?

[SebastianGruza]

Done in c9e70b1, thanks, both paths were real: with no Store list yet the PDException from newBlockingStub() carried the whole pd.peers, and with every Store down gRPC carried Unable to resolve host … (it was in my own Stores→0 samples, I had not connected the dots). reason now carries a fixed category only: the gRPC status code for StatusRuntimeException, pd unreachable for PDException, the class name otherwise (HstoreStorageProbe.category()); the full messages go to the log (WARN for PD, DEBUG for Store pings). StorageReadiness.check() likewise: class name only, message to the log. testReasonCarriesNoPdPeersNorStoreHosts feeds a PDException with pd.peers=pd-0.internal:8686,… and an UNAVAILABLE: Unable to resolve host store-0…svc and asserts neither map contains internal, svc or 8686; StorageReadinessTest checks the same for a probe exception. On k3s with this build the reasons read none of 3 known store(s) answered: a store failed: UNAVAILABLE; a store failed: UNAVAILABLE; a store failed: UNAVAILABLE, and a grep of every sampled body from stores-zero and pd-zero for .svc, 8686 and hugegraph-store- finds 0 occurrences.

[bitflicker64]

⚠️ Every cache-missed probe submits a new PD refresh here, even while the previous one is still running, and EXECUTOR is an unbounded cached pool. getActiveStores() is bounded only by the PD client deadline, 60 s by default (PDConfig.grpcTimeOut; HstoreSessionsImpl.initStoreNode does not override it).

If PD hangs instead of refusing connections, the probe still returns in milliseconds from the known Stores, so each probe parks one more thread on PD: up to about 30 at the default 2 s TTL, and one per request with readiness.cache_ttl=0, which the option allows, on an endpoint anyone can call. Without a live stub they also queue on the synchronized PDClient.newBlockingStub() alongside graph traffic.

Could the refresh be single-flight, e.g. keep the in-flight CompletableFuture and skip submitting until it completes?

[SebastianGruza]

Done in c9e70b1. The refresh is single-flight: KnownStores keeps an AtomicReference to the in-flight CompletableFuture, refresh() returns the running one while it is not isDone() and only starts a new one after it finished (CAS on the reference, so two racing probes also share one). A hung PD therefore parks one thread per process regardless of readiness.cache_ttl, including 0. testRefreshIsSingleFlight: a lister sleeping 3 s, five probes in a row → one call, all five ready from the known list; after it completes the next probe starts the second. The queueing on the synchronized newBlockingStub() next to graph traffic is thereby bounded to that one thread as well.

[bitflicker64]

🧹 This Javadoc says 503 while PD is unreachable. So do the StorageReadiness Javadoc ("PD answers and one active Store answers"), the HstoreStorageProbe class Javadoc ("a hung PD or Store turns into not ready") and the readiness.timeout description. The probe deliberately stays ready with PD down once a Store list is known (testKnownStoresKeepTheServerReadyWhilePdIsDown). Both option descriptions also say one Store call per probe, while it pings every known Store in parallel. Please reword these to match: PD only matters until the first Store list is known, and the Store cost is one call per known Store.

[SebastianGruza]

Done in c9e70b1. The Javadoc of ReadinessAPI, StorageReadiness and HstoreStorageProbe and the descriptions of readiness.timeout and readiness.cache_ttl now say what the code does: PD only matters until the first Store list is known, afterwards it is refreshed in the background; the cost of a probe is one cheap call to every known Store in parallel, first answer wins; 503 only when none answers within the budget (or, before the first list, when PD does not answer).

[bitflicker64]

🧹 CHANNELS keeps one plaintext channel per Store address and nothing ever removes or shuts one down. When a Store is replaced or comes back under a different address, the old channel stays allocated for the life of the process. Could channels whose address is no longer in the latest PD list be shut down after a successful refresh in probe?

[SebastianGruza]

Done in c9e70b1. After every successful listing from PD, pruneChannels() shuts down (shutdownNow) and removes the channels of addresses no longer listed; a failed listing touches nothing. Wired into the lister lambda of probe(graphName, timeoutMs), so the pure probe(...) stays stateless. testChannelsOfReplacedStoresAreShutDown uses a small ManagedChannel subclass (the hstore module has no Mockito): the channel of a listed address stays open, the replaced one is shut down and removed, a null listing removes nothing.

[bitflicker64]

Blocking: no. Summary: the design is measured, justified and tested — the simplifications left are local: the probe hand-rolls a ThreadFactory that commons-lang3's BasicThreadFactory (the repo's own ExecutorUtil pattern) does in one expression, carries a graphName parameter nothing reads, and wraps its result in a 55-line Result holder whose only production consumer is toMap(). Evidence: reviewed the full diff at head d0f4a25 (10 files, +965/-0) with the head checked out; grep -n graphName HstoreStorageProbe.java shows the parameter declared at line 196 and never read; grep -rn HstoreStorageProbe.Result outside the file matches only the test; grep -rn BasicThreadFactory shows four uses in hugegraph-common's ExecutorUtil and commons-lang3 already imported in the hstore module. The endpoint itself, KnownStores, the background PD refresh, the first-answer-wins loop and the api/hstore split via the meta handler are all justified in the PR description (two superseded designs measured, seven E2E scenarios) and are not raised.

[bitflicker64]

⚠️ This ten-line anonymous ThreadFactory with its own AtomicInteger is the thing BasicThreadFactory.Builder exists for — and this repo already uses it exactly this way, four times, in hugegraph-common's ExecutorUtil. commons-lang3 is already imported elsewhere in this module (HstoreSessionsImpl, HstoreTable), so no new dependency.

Requested change:

private static final ExecutorService EXECUTOR = Executors.newCachedThreadPool(
        new BasicThreadFactory.Builder().namingPattern("storage-readiness-%d")
                                        .daemon(true).build());

Same names, same daemon flag, eight lines and one import (java.util.concurrent.ThreadFactory + AtomicInteger) gone.

[SebastianGruza]

Done in 213fd9c, exactly as proposed: new BasicThreadFactory.Builder().namingPattern("storage-readiness-%d").daemon(true).build(), like ExecutorUtil. The anonymous factory, the AtomicInteger and two imports are gone; same thread names, same daemon flag.

[bitflicker64]

🧹 graphName is never read in this method — the javadoc even says "kept for the meta handler", which is scaffolding for a use that doesn't exist yet. The call site in HstoreStore builds this.namespace + "/" + this.store just to feed it.

Requested change: drop the parameter here and the concat at the call site. Re-add it the day something reads it — pd.getActiveStores(graphName) exists if a per-graph store list is ever wanted.

[SebastianGruza]

Done in 213fd9c. probe(long timeoutMs) without the parameter, and the namespace + "/" + store concat in HstoreStore is gone with it. If a per-graph store list is ever wanted, pd.getActiveStores(graphName) comes back together with the parameter.

[bitflicker64]

🧹 The only production consumer of Result is probe(String, long) calling .toMap() on it immediately; outside this file the class appears only in HstoreStorageProbeTest. So these ~55 lines — eight final fields, an eight-positional-arg constructor (new Result(false, "...", 0, null, false, -1L, -1L, 0L) is hard to read at the four construction sites), five getters — exist to make test assertions prettier.

Requested change: have probe(...) build the LinkedHashMap directly (one small private result(boolean ready, String reason, ...) helper covers the four return points) and let the tests assert on map entries, the way testMapCarriesNoAddresses already does. Deletes the class and the .toMap() hops.

[SebastianGruza]

Done in 213fd9c. Result is gone: probe(...) returns the Map<String, Object> built by one private result(ready, reason, activeStores, answeredStore, pdReachable, pdAgeMs, storesAgeMs, storeMillis) helper at the four return points, and StorageReadiness.check() gets the map directly (no more toMap() hops). The tests assert on map entries the way testMapCarriesNoAddresses already did; 18/18.

[SebastianGruza]

Round 1 in c9e70b1: reason carries a category only (gRPC status code, pd unreachable, class name) with the full messages in the log; the PD refresh is single-flight; channels of replaced Stores are shut down after a successful listing; Javadoc and option descriptions match the probe. Tests 18 + 6. On k3s with this build stores-zero and pd-zero PASS, and the sampled bodies contain 0 occurrences of host names or the PD port; results in results/issue-3212/round1-3212g of the validation repo.

[SebastianGruza]

Round 2 in 213fd9c: BasicThreadFactory instead of the hand-rolled factory, the graphName parameter and its concat removed, the body built directly as a map through one helper instead of the Result holder; tests on map entries, 18/18; net -48 lines. Smoke on k3s with this build: the three Server pods answer 200 with the same body as before the refactor, no readiness errors in the log.

[bitflicker64]

Blocking: no. Summary: the earlier rounds are addressed at 213fd9c and the probe logic holds up. The main gap is that /readiness is not exempt from LoadDetectFilter the way /versions is, so a busy Server fails the probe; separately, an empty PD answer closes the Store channels the probe keeps pinging. Evidence: static read of the full diff at 213fd9c against LoadDetectFilter.WHITE_API_LIST, ServerOptions.MAX_WORKER_THREADS (default 2 * CPUS), KnownStores.update and pruneChannels; gh pr checks green on this head apart from the two codecov statuses.

[bitflicker64]

⚠️ readiness is added to the auth and path whitelists but not to LoadDetectFilter.WHITE_API_LIST ("", apis, metrics, versions). So /readiness goes through the worker-load and free-memory checks, before the result cache is reached, and gets a 503 once max_worker_threads - 1 other requests are in flight. restserver.max_worker_threads defaults to 2 * CPUS, so on a 2-CPU pod the probe fails while 3 other requests run.

Under sustained load Kubernetes then drops busy Servers from the Service and shifts their traffic to the rest, so a deployment that moves its readiness probe from /versions to /readiness can lose every endpoint during a spike while storage is fine.

Could readiness go into WHITE_API_LIST next to versions (LoadReleaseFilter reads the same list, so the counter stays balanced), with a case like testFilter_WhiteListPathIgnored? If shedding load through readiness is intended, please say so in the ReadinessAPI Javadoc.

[SebastianGruza]

Done in 4a4aa1c, thanks, that would have been a nasty interaction: a readiness probe shed under load would pull exactly the busiest Servers out of the Service while the storage is healthy. readiness is in LoadDetectFilter.WHITE_API_LIST next to versions; LoadReleaseFilter reads the same list, so the workLoad counter stays balanced. testFilter_ReadinessIgnoredLikeVersions in LoadDetectFilterTest: with a 2-thread limit and one request in flight the filter lets /readiness through without touching the counter and without a log entry, in the same shape as testFilter_WhiteListPathIgnored. Readiness is not meant to shed load; the ReadinessAPI Javadoc says it answers from the storage state.

[bitflicker64]

🧹 Follow-up to the channel pruning from round 1. An empty PD answer shuts down every channel here, while KnownStores.update ignores an empty answer and keeps the old list (testEmptyPdAnswerIsNotReadyAndKeepsTheOldList). As long as PD answers with no active Store, each refresh closes the channels to the Stores that the pings still use, and the next probe opens new ones. probe() starts the refresh and pings right away, so a ping in flight when shutdownNow() runs fails with UNAVAILABLE, and the old-list rule that keeps the Server ready can still turn a probe into a 503.

Could this return early on an empty list as well (if (stores == null || stores.isEmpty())), the same rule update applies, and could testChannelsOfReplacedStoresAreShutDown check that an empty listing keeps the channels?

[SebastianGruza]

Done in 4a4aa1c. pruneChannels() returns on an empty list as well (stores == null || stores.isEmpty()), the same rule KnownStores.update() applies: as long as the pings use the last known list, their channels stay open. testChannelsOfReplacedStoresAreShutDown now also checks that an empty listing neither shuts down nor removes the channel of a known Store. 18/18.

[SebastianGruza]

Round 3 in 4a4aa1c: /readiness on the LoadDetectFilter whitelist (a busy Server still answers its probe from the storage state), an empty PD answer no longer closes the channels, plus ReadinessApiTest in the API suite (with and without credentials, on every backend of the suite) so the endpoint is measured by jacoco in the api-test job. Tests: LoadDetectFilterTest 11/11, HstoreStorageProbeTest 18/18, StorageReadinessTest 6/6, ReadinessApiTest 2/2 against a server built from this head on rocksdb.

[bitflicker64]

Blocking: no. Summary: both findings from the last round are fixed at 4a4aa1c. readiness is in LoadDetectFilter.WHITE_API_LIST, and LoadReleaseFilter skips it through the same isWhiteAPI, so the work-load counter stays balanced. pruneChannels now returns on an empty PD answer, the same rule KnownStores.update applies. The fixes from the first two rounds are still in place, and I found nothing new in the rest of the diff. Evidence: read the full diff at 4a4aa1c (14 files, +1204/-1) and the 213fd9c..4a4aa1c delta, plus the call sites in LoadReleaseFilter, GraphManager.graphs(), HugeGraphAuthProxy.runAsAdmin and HstoreStore.registerMetaHandlers. gh pr checks is green on this head, codecov included. The hstore job log shows HstoreStorageProbeTest 18/18, and its API suite grew from 155 to 157 tests (the two in ReadinessApiTest) with no failures.