From 716426dc43792e1b0ce24363950ffe887095441b Mon Sep 17 00:00:00 2001 From: Sebastian Gruza Date: Fri, 18 Sep 2026 18:31:56 +0000 Subject: [PATCH 01/10] feat(server): add a storage-aware GET /readiness endpoint 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 after the 30 s request bound). GET /readiness answers 200 while this server can serve graph traffic and 503 otherwise, unauthenticated like /versions (whitelisted in both AuthenticationFilter and PathFilter, so an httpGet probe needs neither a credential nor a graphspace prefix), with a JSON body that carries no addresses. Ready means, from this server's own view: 1. its PD client answers (getActiveStores()), and 2. at least one Store that PD reports as active answers one cheap direct call (a node session and Table/EXISTS on the vertex table). The first Store that answers ends the probe: a rolling restart of the Stores never pulls the servers out of the Service, while zero Stores does. - hugegraph-hstore: HstoreStorageProbe (pure logic over an active-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) and a "storage_readiness" meta handler in HstoreStore - hugegraph-api: ReadinessAPI and StorageReadiness (first hstore graph is probed through HugeGraph.metadata(null, "storage_readiness", timeout), result cached for readiness.cache_ttl; servers without an 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) Tests: HstoreStorageProbeTest (9) and StorageReadinessTest (6, including the filter whitelists). Co-Authored-By: Claude Fable 5.1 --- .../api/filter/AuthenticationFilter.java | 1 + .../hugegraph/api/filter/PathFilter.java | 1 + .../hugegraph/api/profile/ReadinessAPI.java | 66 ++++ .../api/profile/StorageReadiness.java | 129 +++++++ .../hugegraph/config/ServerOptions.java | 22 ++ .../store/hstore/HstoreStorageProbe.java | 348 ++++++++++++++++++ .../backend/store/hstore/HstoreStore.java | 6 + .../store/hstore/HstoreStorageProbeTest.java | 264 +++++++++++++ .../apache/hugegraph/unit/UnitTestSuite.java | 2 + .../unit/core/StorageReadinessTest.java | 126 +++++++ 10 files changed, 965 insertions(+) create mode 100644 hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/ReadinessAPI.java create mode 100644 hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java create mode 100644 hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbe.java create mode 100644 hugegraph-server/hugegraph-hstore/src/test/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbeTest.java create mode 100644 hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/StorageReadinessTest.java diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AuthenticationFilter.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AuthenticationFilter.java index 13bb05d8d9..49931fd9e5 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AuthenticationFilter.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/AuthenticationFilter.java @@ -81,6 +81,7 @@ public class AuthenticationFilter implements ContainerRequestFilter, ContainerRe // https://github.com/apache/hugegraph/issues/3212 private static final Set FIXED_WHITE_API_SET = ImmutableSet.of( "versions", + "readiness", "openapi.json" ); /** Remove auth/login API from whitelist */ diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/PathFilter.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/PathFilter.java index 5e4dd5081c..016740d181 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/PathFilter.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/PathFilter.java @@ -57,6 +57,7 @@ public class PathFilter implements ContainerRequestFilter { "apis", "metrics", "versions", + "readiness", "health", "gremlin", "graphs/auth", diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/ReadinessAPI.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/ReadinessAPI.java new file mode 100644 index 0000000000..af9bf9c8ce --- /dev/null +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/ReadinessAPI.java @@ -0,0 +1,66 @@ +/* + * 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.api.profile; + +import java.util.Map; + +import org.apache.hugegraph.api.API; +import org.apache.hugegraph.config.HugeConfig; +import org.apache.hugegraph.config.ServerOptions; +import org.apache.hugegraph.core.GraphManager; +import org.apache.hugegraph.util.JsonUtil; + +import com.codahale.metrics.annotation.Timed; + +import io.swagger.v3.oas.annotations.tags.Tag; +import jakarta.annotation.security.PermitAll; +import jakarta.inject.Singleton; +import jakarta.ws.rs.GET; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.Produces; +import jakarta.ws.rs.core.Context; +import jakarta.ws.rs.core.Response; + +/** + * Storage-aware readiness for Kubernetes and load balancers: 200 while this + * server can serve graph traffic, 503 while PD or every Store is unreachable + * from it. Unauthenticated, like /versions, so that an httpGet probe needs + * no credential; the body carries no addresses. + */ +@Path("readiness") +@Singleton +@Tag(name = "ReadinessAPI") +public class ReadinessAPI extends API { + + @GET + @Timed + @Produces(APPLICATION_JSON_WITH_CHARSET) + @PermitAll + public Response get(@Context GraphManager manager, @Context HugeConfig conf) { + Map body = StorageReadiness.check( + manager, conf.get(ServerOptions.READINESS_TIMEOUT), + conf.get(ServerOptions.READINESS_CACHE_TTL)); + Response.Status status = StorageReadiness.isReady(body) ? + Response.Status.OK : + Response.Status.SERVICE_UNAVAILABLE; + return Response.status(status) + .type(APPLICATION_JSON_WITH_CHARSET) + .entity(JsonUtil.toJson(body)) + .build(); + } +} diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java new file mode 100644 index 0000000000..415202d7b8 --- /dev/null +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java @@ -0,0 +1,129 @@ +/* + * 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.api.profile; + +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +import org.apache.hugegraph.HugeGraph; +import org.apache.hugegraph.auth.HugeGraphAuthProxy; +import org.apache.hugegraph.core.GraphManager; +import org.apache.hugegraph.util.Log; +import org.slf4j.Logger; + +/** + * Whether this server can serve graph traffic, for a readiness probe. + * Graphs on an embedded backend are ready as soon as the REST layer answers. + * Graphs on hstore are probed through the backend's "storage_readiness" + * metadata (PD answers and one active Store answers). The storage is shared + * by every hstore graph of the process, so one graph is probed and the + * result is reused for a short TTL to keep repeated probes cheap. + */ +public final class StorageReadiness { + + public static final String STORAGE_READINESS_META = "storage_readiness"; + public static final String BACKEND_HSTORE = "hstore"; + + private static final Logger LOG = Log.logger(StorageReadiness.class); + + private static volatile Map lastResult; + private static volatile long lastCheckedAt; + + private StorageReadiness() { + } + + /** One storage probe with a time budget in ms. */ + public interface Probe { + + Map probe(long timeoutMs) throws Exception; + } + + public static Map check(GraphManager manager, + long timeoutMs, long cacheTtlMs) { + // The graphs are auth proxies and the probe request carries no user, + // so look the graph up and probe it as the internal admin, the way + // other internal paths do; the result carries no data or addresses + List> holder = new ArrayList<>(1); + HugeGraphAuthProxy.runAsAdmin(() -> { + HugeGraph graph = firstHstoreGraph(manager); + if (graph == null) { + Map body = new LinkedHashMap<>(); + body.put("ready", true); + body.put("storage", "embedded"); + body.put("reason", "no graph on a remote storage"); + holder.add(body); + return; + } + holder.add(check(t -> graph.metadata(null, STORAGE_READINESS_META, t), + timeoutMs, cacheTtlMs)); + }); + return holder.get(0); + } + + public static synchronized Map check(Probe probe, long timeoutMs, + long cacheTtlMs) { + long now = System.currentTimeMillis(); + Map cached = lastResult; + if (cached != null && now - lastCheckedAt < cacheTtlMs) { + Map body = new LinkedHashMap<>(cached); + body.put("cached", true); + return body; + } + Map body = new LinkedHashMap<>(); + body.put("ready", false); + body.put("storage", BACKEND_HSTORE); + try { + Map result = probe.probe(timeoutMs); + body.putAll(result); + } catch (Throwable e) { + LOG.debug("Storage readiness probe failed", e); + body.put("ready", false); + body.put("reason", "probe failed: " + e.getClass().getSimpleName() + + (e.getMessage() == null ? "" : ": " + e.getMessage())); + } + body.put("cached", false); + lastResult = body; + lastCheckedAt = System.currentTimeMillis(); + return new LinkedHashMap<>(body); + } + + public static boolean isReady(Map body) { + return Boolean.TRUE.equals(body.get("ready")); + } + + public static synchronized void resetCache() { + lastResult = null; + lastCheckedAt = 0L; + } + + private static HugeGraph firstHstoreGraph(GraphManager manager) { + for (String name : manager.graphs()) { + try { + HugeGraph graph = manager.graph(name); + if (graph != null && BACKEND_HSTORE.equals(graph.backend())) { + return graph; + } + } catch (Throwable e) { + LOG.debug("Skip graph {} while looking for a remote storage", name, e); + } + } + return null; + } +} diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java index 285c4b23a5..628f584f05 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java @@ -202,6 +202,28 @@ public class ServerOptions extends OptionHolder { 300 ); + public static final ConfigOption READINESS_TIMEOUT = + new ConfigOption<>( + "readiness.timeout", + "The whole time budget in ms of one GET /readiness probe " + + "(PD call plus one cheap call to an active Store); a PD " + + "or Store that does not answer within it makes the " + + "server report not ready.", + rangeInt(100, 60000), + 1000 + ); + + public static final ConfigOption READINESS_CACHE_TTL = + new ConfigOption<>( + "readiness.cache_ttl", + "How many ms a GET /readiness result is reused before " + + "the storage is probed again, so that several probes " + + "(Kubernetes, load balancers) cost one PD and one Store " + + "call per interval; 0 probes on every request.", + rangeInt(0, 60000), + 2000 + ); + public static final ConfigOption SERVER_USE_K8S = new ConfigOption<>( "server.use_k8s", diff --git a/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbe.java b/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbe.java new file mode 100644 index 0000000000..f1d77ee349 --- /dev/null +++ b/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbe.java @@ -0,0 +1,348 @@ +/* + * 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.backend.store.hstore; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionService; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorCompletionService; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicInteger; + +import org.apache.hugegraph.pd.client.PDClient; +import org.apache.hugegraph.pd.grpc.Metapb; +import org.apache.hugegraph.store.grpc.state.HgStoreStateGrpc; +import org.apache.hugegraph.store.grpc.state.SubStateReq; +import org.apache.hugegraph.util.E; + +import io.grpc.ManagedChannel; +import io.grpc.ManagedChannelBuilder; + +/** + * Storage-aware readiness of this server, from this server's point of view: + * at least one Store answers a direct, local, read-only gRPC call + * (HgStoreState.getScanState, which reads the node's own scan-pool stats and + * never touches raft). The Store list comes from PD, but PD is refreshed in + * the background and the last known list is used right away, so a PD that is + * slow, restarting or down does not change the readiness of a server whose + * Stores still answer. Every wait is bounded by one shared time budget, so a + * hung PD or Store turns into "not ready" instead of a hung probe. The result + * carries no addresses, since it is served without authentication. + */ +public final class HstoreStorageProbe { + + public static final String META_STORAGE_READINESS = "storage_readiness"; + + private static final ExecutorService EXECUTOR = Executors.newCachedThreadPool( + new ThreadFactory() { + private final AtomicInteger seq = new AtomicInteger(); + + @Override + public Thread newThread(Runnable r) { + Thread t = new Thread(r, "storage-readiness-" + this.seq.incrementAndGet()); + t.setDaemon(true); + return t; + } + }); + + private static final KnownStores KNOWN = new KnownStores(); + private static final Map CHANNELS = new ConcurrentHashMap<>(); + + private HstoreStorageProbe() { + } + + /** The active stores as PD sees them. */ + public interface StoreLister { + + List activeStores() throws Exception; + } + + /** One cheap call to one store; returning (any value) means it answered. */ + public interface StorePinger { + + void ping(Metapb.Store store, long timeoutMs) throws Exception; + } + + /** The last store list PD answered with, shared by consecutive probes. */ + public static final class KnownStores { + + private volatile List stores = Collections.emptyList(); + private volatile long at; + private volatile Boolean pdOk; + private volatile long pdAt; + + public List stores() { + return this.stores; + } + + public long ageMs() { + return this.at == 0L ? -1L : System.currentTimeMillis() - this.at; + } + + /** Outcome of the last finished PD refresh, null before the first one. */ + public Boolean pdOk() { + return this.pdOk; + } + + public long pdAgeMs() { + return this.pdAt == 0L ? -1L : System.currentTimeMillis() - this.pdAt; + } + + public void update(List stores) { + this.pdOk = true; + this.pdAt = System.currentTimeMillis(); + if (stores != null && !stores.isEmpty()) { + this.stores = Collections.unmodifiableList(new ArrayList<>(stores)); + this.at = this.pdAt; + } + } + + public void pdFailed() { + this.pdOk = false; + this.pdAt = System.currentTimeMillis(); + } + } + + public static final class Result { + + private final boolean ready; + private final String reason; + private final int activeStores; + private final Long answeredStore; + private final Boolean pdReachable; + private final long pdAgeMs; + private final long storesAgeMs; + private final long storeMillis; + + Result(boolean ready, String reason, int activeStores, Long answeredStore, + Boolean pdReachable, long pdAgeMs, long storesAgeMs, long storeMillis) { + this.ready = ready; + this.reason = reason; + this.activeStores = activeStores; + this.answeredStore = answeredStore; + this.pdReachable = pdReachable; + this.pdAgeMs = pdAgeMs; + this.storesAgeMs = storesAgeMs; + this.storeMillis = storeMillis; + } + + public boolean ready() { + return this.ready; + } + + public String reason() { + return this.reason; + } + + public int activeStores() { + return this.activeStores; + } + + public Long answeredStore() { + return this.answeredStore; + } + + public Boolean pdReachable() { + return this.pdReachable; + } + + public Map toMap() { + Map map = new LinkedHashMap<>(); + map.put("ready", this.ready); + map.put("reason", this.reason); + map.put("active_stores", this.activeStores); + map.put("answered_store", this.answeredStore); + map.put("pd_reachable", this.pdReachable); + map.put("pd_checked_age_ms", this.pdAgeMs); + map.put("stores_age_ms", this.storesAgeMs); + map.put("store_millis", this.storeMillis); + return map; + } + } + + /** + * Probe through the process-wide PD client and this probe's own plaintext + * channels to the stores (the store gRPC server takes no credentials). + * + * @param graphName the store-side graph name, kept for the meta handler + * @param timeoutMs the whole budget for PD plus stores + */ + public static Map probe(String graphName, long timeoutMs) { + PDClient pd = HstoreSessionsImpl.getDefaultPdClient(); + if (pd == null) { + return new Result(false, "pd client not initialised", 0, null, + false, -1L, -1L, 0L).toMap(); + } + return probe(KNOWN, pd::getActiveStores, HstoreStorageProbe::pingScanState, + timeoutMs, EXECUTOR).toMap(); + } + + private static void pingScanState(Metapb.Store store, long timeoutMs) { + ManagedChannel channel = CHANNELS.computeIfAbsent(store.getAddress(), address -> { + return ManagedChannelBuilder.forTarget(address).usePlaintext().build(); + }); + HgStoreStateGrpc.newBlockingStub(channel) + .withDeadlineAfter(timeoutMs, TimeUnit.MILLISECONDS) + .getScanState(SubStateReq.getDefaultInstance()); + } + + public static Result probe(KnownStores known, StoreLister lister, StorePinger pinger, + long timeoutMs, ExecutorService executor) { + E.checkArgument(timeoutMs > 0, "The probe timeout must be > 0, but got %s", timeoutMs); + long deadline = System.currentTimeMillis() + timeoutMs; + + // Refresh the store list from PD in the background; whatever PD + // answers lands in `known` for this or the next probe + CompletableFuture> refresh = new CompletableFuture<>(); + executor.execute(() -> { + try { + List stores = lister.activeStores(); + known.update(stores); + refresh.complete(stores == null ? Collections.emptyList() : stores); + } catch (Throwable e) { + known.pdFailed(); + refresh.completeExceptionally(e); + } + }); + + List stores = known.stores(); + Boolean pdReachable = null; + if (stores.isEmpty()) { + // Nothing known yet (first probe after start): PD is the only source + try { + stores = await(refresh, deadline); + pdReachable = true; + } catch (TimeoutException e) { + return new Result(false, "no store list known and pd did not answer within " + + timeoutMs + " ms", 0, null, false, -1L, -1L, 0L); + } catch (Exception e) { + return new Result(false, "no store list known and pd failed: " + message(e), + 0, null, false, known.pdAgeMs(), -1L, 0L); + } + if (stores.isEmpty()) { + return new Result(false, "no active store registered in pd", + 0, null, true, known.pdAgeMs(), known.ageMs(), 0L); + } + } + + long storeStart = System.currentTimeMillis(); + // Ping every known store at once and take the first answer: a store + // whose connection hangs (a pod that just went away) must not eat the + // budget of the stores that are fine, or a rolling restart would + // flap the readiness of every server + CompletionService pings = new ExecutorCompletionService<>(executor); + List> futures = new ArrayList<>(stores.size()); + for (Metapb.Store store : stores) { + futures.add(pings.submit(() -> { + pinger.ping(store, Math.max(1L, deadline - System.currentTimeMillis())); + return store; + })); + } + List failures = new ArrayList<>(); + Result result = null; + try { + for (int done = 0; done < stores.size() && result == null; done++) { + long remaining = deadline - System.currentTimeMillis(); + Future first; + try { + first = remaining > 0 ? + pings.poll(remaining, TimeUnit.MILLISECONDS) : + pings.poll(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + failures.add("interrupted"); + break; + } + if (first == null) { + failures.add((stores.size() - done) + " store(s) did not answer within " + + timeoutMs + " ms"); + break; + } + try { + Metapb.Store store = first.get(); + result = new Result(true, "ok", stores.size(), store.getId(), + pdState(refresh, known, pdReachable), known.pdAgeMs(), + known.ageMs(), elapsed(storeStart)); + } catch (ExecutionException e) { + Throwable cause = e.getCause() != null ? e.getCause() : e; + failures.add("a store failed: " + message(cause)); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + failures.add("interrupted"); + break; + } + } + } finally { + for (Future f : futures) { + f.cancel(true); + } + } + if (result != null) { + return result; + } + return new Result(false, "none of " + stores.size() + " known store(s) answered: " + + String.join("; ", failures), + stores.size(), null, pdState(refresh, known, pdReachable), + known.pdAgeMs(), known.ageMs(), elapsed(storeStart)); + } + + /** + * The outcome of this probe's PD refresh when it already finished, else + * the outcome of the last finished one (null before any finished). + */ + private static Boolean pdState(CompletableFuture refresh, KnownStores known, + Boolean awaited) { + if (awaited != null) { + return awaited; + } + if (refresh.isDone()) { + return !refresh.isCompletedExceptionally(); + } + return known.pdOk(); + } + + private static T await(Future future, long deadline) throws Exception { + long remaining = Math.max(1L, deadline - System.currentTimeMillis()); + try { + return future.get(remaining, TimeUnit.MILLISECONDS); + } catch (ExecutionException e) { + Throwable cause = e.getCause() != null ? e.getCause() : e; + throw cause instanceof Exception ? (Exception) cause : new RuntimeException(cause); + } + } + + private static long elapsed(long since) { + return System.currentTimeMillis() - since; + } + + private static String message(Throwable e) { + String msg = e.getMessage(); + return e.getClass().getSimpleName() + (msg == null ? "" : ": " + msg); + } +} diff --git a/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStore.java b/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStore.java index 22cc02676b..7977dfc248 100644 --- a/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStore.java +++ b/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStore.java @@ -113,6 +113,12 @@ private void registerMetaHandlers() { HstoreMetrics metrics = new HstoreMetrics(dbsGet.get(), session); return metrics.metrics(); }); + this.registerMetaHandler(HstoreStorageProbe.META_STORAGE_READINESS, (session, meta, args) -> { + E.checkArgument(args.length == 1 && args[0] instanceof Number, + "Expect the timeout in ms as the only argument"); + return HstoreStorageProbe.probe(this.namespace + "/" + this.store, + ((Number) args[0]).longValue()); + }); this.registerMetaHandler("mode", (session, meta, args) -> { E.checkArgument(args.length == 1, "The args count of %s must be 1", meta); diff --git a/hugegraph-server/hugegraph-hstore/src/test/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbeTest.java b/hugegraph-server/hugegraph-hstore/src/test/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbeTest.java new file mode 100644 index 0000000000..473f253fe5 --- /dev/null +++ b/hugegraph-server/hugegraph-hstore/src/test/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbeTest.java @@ -0,0 +1,264 @@ +/* + * 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.backend.store.hstore; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.Collectors; + +import org.apache.hugegraph.backend.store.hstore.HstoreStorageProbe.KnownStores; +import org.apache.hugegraph.backend.store.hstore.HstoreStorageProbe.Result; +import org.apache.hugegraph.pd.grpc.Metapb; +import org.junit.AfterClass; +import org.junit.Assert; +import org.junit.Test; + +public class HstoreStorageProbeTest { + + private static final ExecutorService EXECUTOR = Executors.newCachedThreadPool(); + private static final long BUDGET = 500L; + + @AfterClass + public static void shutdown() { + EXECUTOR.shutdownNow(); + } + + private static Metapb.Store store(long id) { + return Metapb.Store.newBuilder().setId(id).setAddress("10.0.0." + id + ":8500") + .setState(Metapb.StoreState.Up).build(); + } + + private static List stores(long... ids) { + return Arrays.stream(ids).mapToObj(HstoreStorageProbeTest::store) + .collect(Collectors.toList()); + } + + private static KnownStores knowing(long... ids) { + KnownStores known = new KnownStores(); + known.update(stores(ids)); + return known; + } + + private static final HstoreStorageProbe.StorePinger ANSWERS = (store, timeout) -> { + }; + + @Test + public void testReadyWhenPdAndOneStoreAnswer() { + Result r = HstoreStorageProbe.probe(new KnownStores(), () -> stores(1L, 2L, 3L), + ANSWERS, BUDGET, EXECUTOR); + Assert.assertTrue(r.reason(), r.ready()); + Assert.assertEquals(3, r.activeStores()); + Assert.assertNotNull(r.answeredStore()); + Assert.assertEquals(Boolean.TRUE, r.pdReachable()); + Assert.assertEquals("ok", r.reason()); + } + + @Test + public void testFirstAnsweringStoreWinsAfterFailures() { + AtomicInteger pings = new AtomicInteger(); + HstoreStorageProbe.StorePinger onlyThird = (store, timeout) -> { + pings.incrementAndGet(); + if (store.getId() != 3L) { + throw new IllegalStateException("UNAVAILABLE"); + } + }; + Result r = HstoreStorageProbe.probe(knowing(1L, 2L, 3L), () -> stores(1L, 2L, 3L), + onlyThird, BUDGET, EXECUTOR); + Assert.assertTrue(r.reason(), r.ready()); + Assert.assertEquals(Long.valueOf(3L), r.answeredStore()); + // the pings run in parallel; the failing ones may or may not have run + Assert.assertTrue(pings.get() >= 1 && pings.get() <= 3); + } + + /** + * The store whose pod just went away hangs until its deadline; it must + * not eat the budget of a store that answers (otherwise a rolling restart + * would flap every server's readiness). + */ + @Test + public void testHungStoreDoesNotHideAnAnsweringOne() { + HstoreStorageProbe.StorePinger onlySecondAnswers = (store, timeout) -> { + if (store.getId() != 2L) { + Thread.sleep(10_000L); + } + }; + long start = System.currentTimeMillis(); + Result r = HstoreStorageProbe.probe(knowing(1L, 2L, 3L), () -> stores(1L, 2L, 3L), + onlySecondAnswers, BUDGET, EXECUTOR); + long took = System.currentTimeMillis() - start; + Assert.assertTrue(r.reason(), r.ready()); + Assert.assertEquals(Long.valueOf(2L), r.answeredStore()); + Assert.assertTrue("took " + took, took < BUDGET); + } + + @Test + public void testFirstProbeWithoutKnownStoresNeedsPd() { + Result r = HstoreStorageProbe.probe(new KnownStores(), () -> { + throw new IllegalStateException("UNAVAILABLE: io exception"); + }, ANSWERS, BUDGET, EXECUTOR); + Assert.assertFalse(r.ready()); + Assert.assertTrue(r.reason(), r.reason().startsWith( + "no store list known and pd failed: IllegalStateException")); + Assert.assertEquals(Boolean.FALSE, r.pdReachable()); + } + + @Test + public void testFirstProbeWithHungPdStaysWithinBudget() { + long start = System.currentTimeMillis(); + Result r = HstoreStorageProbe.probe(new KnownStores(), () -> { + Thread.sleep(10_000L); + return stores(1L); + }, ANSWERS, BUDGET, EXECUTOR); + long took = System.currentTimeMillis() - start; + Assert.assertFalse(r.ready()); + Assert.assertTrue(r.reason(), r.reason().contains("pd did not answer within")); + Assert.assertTrue("took " + took, took < BUDGET * 4); + } + + /** + * PD down, restarting or slow must not change the readiness of a server + * whose stores still answer: the last known list is used right away. + */ + @Test + public void testKnownStoresKeepTheServerReadyWhilePdIsDown() { + Result r = HstoreStorageProbe.probe(knowing(1L, 2L), () -> { + throw new IllegalStateException("PD unreachable"); + }, ANSWERS, BUDGET, EXECUTOR); + Assert.assertTrue(r.reason(), r.ready()); + Assert.assertEquals(2, r.activeStores()); + } + + @Test + public void testHungPdDoesNotDelayAProbeWithKnownStores() { + long start = System.currentTimeMillis(); + Result r = HstoreStorageProbe.probe(knowing(1L, 2L), () -> { + Thread.sleep(10_000L); + return stores(1L, 2L); + }, ANSWERS, BUDGET, EXECUTOR); + long took = System.currentTimeMillis() - start; + Assert.assertTrue(r.reason(), r.ready()); + // the refresh is still pending, so the outcome of the last finished + // one (the seed) is reported + Assert.assertEquals(Boolean.TRUE, r.pdReachable()); + Assert.assertTrue("took " + took, took < BUDGET); + } + + @Test + public void testLastPdOutcomeIsReportedWhileTheRefreshIsPending() throws Exception { + KnownStores known = knowing(1L); + HstoreStorageProbe.probe(known, () -> { + throw new IllegalStateException("PD unreachable"); + }, ANSWERS, BUDGET, EXECUTOR); + for (int i = 0; i < 50 && known.pdOk() == null; i++) { + Thread.sleep(20L); + } + Assert.assertEquals(Boolean.FALSE, known.pdOk()); + Result r = HstoreStorageProbe.probe(known, () -> { + Thread.sleep(10_000L); + return stores(1L); + }, ANSWERS, BUDGET, EXECUTOR); + Assert.assertTrue(r.ready()); + Assert.assertEquals(Boolean.FALSE, r.pdReachable()); + Assert.assertTrue(r.toMap().containsKey("pd_checked_age_ms")); + } + + @Test + public void testPdAnswerUpdatesTheKnownStoresForTheNextProbe() throws Exception { + KnownStores known = knowing(1L); + HstoreStorageProbe.probe(known, () -> stores(1L, 2L, 3L), ANSWERS, BUDGET, EXECUTOR); + for (int i = 0; i < 50 && known.stores().size() != 3; i++) { + Thread.sleep(20L); + } + Assert.assertEquals(3, known.stores().size()); + Assert.assertTrue(known.ageMs() >= 0L); + } + + @Test + public void testEmptyPdAnswerIsNotReadyAndKeepsTheOldList() { + KnownStores fresh = new KnownStores(); + Result r = HstoreStorageProbe.probe(fresh, Collections::emptyList, ANSWERS, + BUDGET, EXECUTOR); + Assert.assertFalse(r.ready()); + Assert.assertEquals("no active store registered in pd", r.reason()); + KnownStores known = knowing(1L); + HstoreStorageProbe.probe(known, Collections::emptyList, ANSWERS, BUDGET, EXECUTOR); + Assert.assertEquals(1, known.stores().size()); + } + + @Test + public void testNotReadyWhenEveryStoreFails() { + HstoreStorageProbe.StorePinger refused = (store, timeout) -> { + throw new IllegalStateException("connection refused"); + }; + Result r = HstoreStorageProbe.probe(knowing(7L, 8L), () -> stores(7L, 8L), + refused, BUDGET, EXECUTOR); + Assert.assertFalse(r.ready()); + Assert.assertTrue(r.reason(), r.reason().startsWith("none of 2 known store(s) answered")); + Assert.assertTrue(r.reason(), r.reason().contains("connection refused")); + Assert.assertNull(r.answeredStore()); + } + + @Test + public void testHungStoresStayWithinTheBudget() { + HstoreStorageProbe.StorePinger hung = (store, timeout) -> { + Thread.sleep(10_000L); + }; + long start = System.currentTimeMillis(); + Result r = HstoreStorageProbe.probe(knowing(1L, 2L, 3L), () -> stores(1L, 2L, 3L), + hung, BUDGET, EXECUTOR); + long took = System.currentTimeMillis() - start; + Assert.assertFalse(r.ready()); + Assert.assertTrue(r.reason(), r.reason().contains("did not answer within")); + Assert.assertTrue("took " + took, took < BUDGET * 4); + } + + @Test + public void testPingGetsTheRemainingBudget() { + List budgets = Collections.synchronizedList(new java.util.ArrayList<>()); + HstoreStorageProbe.probe(knowing(1L), () -> stores(1L), (store, timeout) -> { + budgets.add(timeout); + }, BUDGET, EXECUTOR); + Assert.assertEquals(1, budgets.size()); + Assert.assertTrue(budgets.get(0) > 0L && budgets.get(0) <= BUDGET); + } + + @Test + public void testMapCarriesNoAddresses() { + Map map = HstoreStorageProbe.probe(knowing(1L), () -> stores(1L), + ANSWERS, BUDGET, EXECUTOR).toMap(); + Assert.assertEquals(true, map.get("ready")); + Assert.assertEquals(1, map.get("active_stores")); + Assert.assertEquals(1L, map.get("answered_store")); + Assert.assertTrue(map.containsKey("pd_reachable")); + Assert.assertTrue(map.containsKey("stores_age_ms")); + Assert.assertFalse(map.toString().contains("10.0.0.")); + } + + @Test + public void testRejectsNonPositiveBudget() { + Assert.assertThrows(IllegalArgumentException.class, () -> { + HstoreStorageProbe.probe(new KnownStores(), Collections::emptyList, ANSWERS, + 0L, EXECUTOR); + }); + } +} diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java index 3346f8ad6d..293dac67ce 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java @@ -63,6 +63,7 @@ import org.apache.hugegraph.unit.core.GraphSpaceInfoLocaleTest; import org.apache.hugegraph.unit.core.GraphManagerStoresWaitTest; import org.apache.hugegraph.unit.core.MetaManagerClusterTest; +import org.apache.hugegraph.unit.core.StorageReadinessTest; import org.apache.hugegraph.unit.core.DirectionsTest; import org.apache.hugegraph.unit.core.ExceptionTest; import org.apache.hugegraph.unit.core.GraphManagerAdminInitTest; @@ -165,6 +166,7 @@ GraphSpaceInfoLocaleTest.class, GraphManagerStoresWaitTest.class, MetaManagerClusterTest.class, + StorageReadinessTest.class, DirectionsTest.class, SerialEnumTest.class, diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/StorageReadinessTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/StorageReadinessTest.java new file mode 100644 index 0000000000..d4bda1dd50 --- /dev/null +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/StorageReadinessTest.java @@ -0,0 +1,126 @@ +/* + * 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.unit.core; + +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; + +import org.apache.hugegraph.api.filter.AuthenticationFilter; +import org.apache.hugegraph.api.filter.PathFilter; +import org.apache.hugegraph.api.profile.StorageReadiness; +import org.apache.hugegraph.testutil.Assert; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mockito; + +import jakarta.ws.rs.container.ContainerRequestContext; +import jakarta.ws.rs.core.UriInfo; + +public class StorageReadinessTest { + + @Before + @After + public void reset() { + StorageReadiness.resetCache(); + } + + private static Map result(boolean ready, String reason) { + Map map = new LinkedHashMap<>(); + map.put("ready", ready); + map.put("reason", reason); + map.put("active_stores", 3); + return map; + } + + @Test + public void testReadyBodyAndCacheReuse() { + AtomicInteger probes = new AtomicInteger(); + StorageReadiness.Probe probe = t -> { + probes.incrementAndGet(); + return result(true, "ok"); + }; + Map first = StorageReadiness.check(probe, 1000L, 60_000L); + Map second = StorageReadiness.check(probe, 1000L, 60_000L); + Assert.assertTrue(StorageReadiness.isReady(first)); + Assert.assertEquals("hstore", first.get("storage")); + Assert.assertEquals(false, first.get("cached")); + Assert.assertEquals(true, second.get("cached")); + Assert.assertEquals(1, probes.get()); + } + + @Test + public void testZeroTtlProbesEveryTime() { + AtomicInteger probes = new AtomicInteger(); + StorageReadiness.Probe probe = t -> { + probes.incrementAndGet(); + return result(false, "no active store registered in pd"); + }; + StorageReadiness.check(probe, 1000L, 0L); + Map body = StorageReadiness.check(probe, 1000L, 0L); + Assert.assertFalse(StorageReadiness.isReady(body)); + Assert.assertEquals(2, probes.get()); + Assert.assertEquals("no active store registered in pd", body.get("reason")); + } + + @Test + public void testProbeFailureIsNotReadyWithReason() { + StorageReadiness.Probe probe = t -> { + throw new IllegalStateException("The 'hugegraph' store of hstore has not been opened"); + }; + Map body = StorageReadiness.check(probe, 1000L, 0L); + Assert.assertFalse(StorageReadiness.isReady(body)); + Assert.assertContains("probe failed: IllegalStateException", (String) body.get("reason")); + Assert.assertContains("has not been opened", (String) body.get("reason")); + } + + @Test + public void testTimeoutIsPassedToTheProbe() { + StorageReadiness.Probe probe = t -> result(true, "budget " + t); + Map body = StorageReadiness.check(probe, 750L, 0L); + Assert.assertEquals("budget 750", body.get("reason")); + } + + @Test + public void testCachedCopyIsIsolated() { + StorageReadiness.Probe probe = t -> result(true, "ok"); + Map first = StorageReadiness.check(probe, 1000L, 60_000L); + first.put("ready", false); + Map second = StorageReadiness.check(probe, 1000L, 60_000L); + Assert.assertTrue(StorageReadiness.isReady(second)); + } + + /** + * A Kubernetes httpGet probe carries no credential and no graphspace, so + * the endpoint must pass both the graphspace path rewrite and the auth + * filter, exactly like /versions. + */ + @Test + public void testReadinessBypassesPathAndAuthFilters() { + Assert.assertTrue(PathFilter.isWhiteAPI("readiness")); + Assert.assertTrue(PathFilter.isWhiteAPI("versions")); + UriInfo uri = Mockito.mock(UriInfo.class); + Mockito.when(uri.getPath()).thenReturn("readiness"); + ContainerRequestContext ctx = Mockito.mock(ContainerRequestContext.class); + Mockito.when(ctx.getUriInfo()).thenReturn(uri); + Assert.assertTrue(AuthenticationFilter.isWhiteAPI(ctx)); + Mockito.when(uri.getPath()).thenReturn("readiness/"); + Assert.assertFalse(AuthenticationFilter.isWhiteAPI(ctx)); + } +} From d4ebed5756a766d1546ee0c81249ce4f476b172d Mon Sep 17 00:00:00 2001 From: Sebastian Gruza Date: Sat, 19 Sep 2026 04:38:17 +0000 Subject: [PATCH 02/10] feat(server): keep addresses out of the readiness body, single-flight PD refresh, prune dead Store channels Review round 1 of #3221: - the reason field carries a fixed category (gRPC status code, 'pd unreachable' or the exception class) instead of raw exception text, which could name PD peers and Store hosts on an unauthenticated endpoint; the full messages go to the log - the background PD refresh is single-flight (KnownStores keeps the future in flight), so a hung PD parks one thread, not one per cache-missed probe - channels of addresses PD no longer lists are shut down after a successful listing - Javadoc and option descriptions say what the probe does: PD only until the first Store list is known, one call per known Store in parallel Co-Authored-By: Claude Fable 5.1 --- .../hugegraph/api/profile/ReadinessAPI.java | 9 +- .../api/profile/StorageReadiness.java | 13 +- .../hugegraph/config/ServerOptions.java | 14 ++- .../store/hstore/HstoreStorageProbe.java | 117 +++++++++++++---- .../store/hstore/HstoreStorageProbeTest.java | 119 +++++++++++++++++- .../unit/core/StorageReadinessTest.java | 5 +- 6 files changed, 232 insertions(+), 45 deletions(-) diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/ReadinessAPI.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/ReadinessAPI.java index af9bf9c8ce..3f5a985aff 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/ReadinessAPI.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/ReadinessAPI.java @@ -37,10 +37,11 @@ import jakarta.ws.rs.core.Response; /** - * Storage-aware readiness for Kubernetes and load balancers: 200 while this - * server can serve graph traffic, 503 while PD or every Store is unreachable - * from it. Unauthenticated, like /versions, so that an httpGet probe needs - * no credential; the body carries no addresses. + * Storage-aware readiness for Kubernetes and load balancers: 200 while at + * least one known Store answers this server, 503 while none does (or, before + * any Store list is known, while PD does not answer). Unauthenticated, like + * /versions, so that an httpGet probe needs no credential; the body carries + * no addresses and no raw exception text. */ @Path("readiness") @Singleton diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java index 415202d7b8..b455be1d61 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java @@ -32,9 +32,11 @@ * Whether this server can serve graph traffic, for a readiness probe. * Graphs on an embedded backend are ready as soon as the REST layer answers. * Graphs on hstore are probed through the backend's "storage_readiness" - * metadata (PD answers and one active Store answers). The storage is shared - * by every hstore graph of the process, so one graph is probed and the - * result is reused for a short TTL to keep repeated probes cheap. + * metadata: at least one known Store answers a cheap direct call; PD is only + * needed until the first Store list is known. The storage is shared by every + * hstore graph of the process, so one graph is probed and the result is + * reused for a short TTL to keep repeated probes cheap. The body never + * carries raw exception text, since the endpoint is unauthenticated. */ public final class StorageReadiness { @@ -93,10 +95,9 @@ public static synchronized Map check(Probe probe, long timeoutMs Map result = probe.probe(timeoutMs); body.putAll(result); } catch (Throwable e) { - LOG.debug("Storage readiness probe failed", e); + LOG.warn("Storage readiness probe failed", e); body.put("ready", false); - body.put("reason", "probe failed: " + e.getClass().getSimpleName() + - (e.getMessage() == null ? "" : ": " + e.getMessage())); + body.put("reason", "probe failed: " + e.getClass().getSimpleName()); } body.put("cached", false); lastResult = body; diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java index 628f584f05..f73bdac2b5 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java @@ -205,10 +205,12 @@ public class ServerOptions extends OptionHolder { public static final ConfigOption READINESS_TIMEOUT = new ConfigOption<>( "readiness.timeout", - "The whole time budget in ms of one GET /readiness probe " + - "(PD call plus one cheap call to an active Store); a PD " + - "or Store that does not answer within it makes the " + - "server report not ready.", + "The whole time budget in ms of one GET /readiness probe: " + + "one cheap call to every known Store in parallel, the " + + "first answer wins; the PD call that refreshes the Store " + + "list runs in the background and is only waited for " + + "before the first list is known. No Store answering " + + "within the budget makes the server report not ready.", rangeInt(100, 60000), 1000 ); @@ -218,8 +220,8 @@ public class ServerOptions extends OptionHolder { "readiness.cache_ttl", "How many ms a GET /readiness result is reused before " + "the storage is probed again, so that several probes " + - "(Kubernetes, load balancers) cost one PD and one Store " + - "call per interval; 0 probes on every request.", + "(Kubernetes, load balancers) cost one round of Store " + + "calls per interval; 0 probes on every request.", rangeInt(0, 60000), 2000 ); diff --git a/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbe.java b/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbe.java index f1d77ee349..390bbacc1e 100644 --- a/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbe.java +++ b/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbe.java @@ -19,6 +19,8 @@ import java.util.ArrayList; import java.util.Collections; +import java.util.HashSet; +import java.util.Set; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -34,31 +36,40 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; import org.apache.hugegraph.pd.client.PDClient; +import org.apache.hugegraph.pd.common.PDException; import org.apache.hugegraph.pd.grpc.Metapb; import org.apache.hugegraph.store.grpc.state.HgStoreStateGrpc; import org.apache.hugegraph.store.grpc.state.SubStateReq; import org.apache.hugegraph.util.E; +import org.apache.hugegraph.util.Log; +import org.slf4j.Logger; import io.grpc.ManagedChannel; import io.grpc.ManagedChannelBuilder; +import io.grpc.StatusRuntimeException; /** * Storage-aware readiness of this server, from this server's point of view: * at least one Store answers a direct, local, read-only gRPC call * (HgStoreState.getScanState, which reads the node's own scan-pool stats and - * never touches raft). The Store list comes from PD, but PD is refreshed in - * the background and the last known list is used right away, so a PD that is - * slow, restarting or down does not change the readiness of a server whose - * Stores still answer. Every wait is bounded by one shared time budget, so a - * hung PD or Store turns into "not ready" instead of a hung probe. The result - * carries no addresses, since it is served without authentication. + * never touches raft). The Store list comes from PD, refreshed in the + * background (single-flight) while the last known list is used right away, so + * PD only matters until the first list is known: a PD that is slow, restarting + * or down afterwards does not change the readiness of a server whose Stores + * still answer. Every known Store is pinged in parallel and the first answer + * wins; every wait is bounded by one shared time budget. The result carries + * no addresses and no raw exception text, since it is served without + * authentication; the full messages go to the log. */ public final class HstoreStorageProbe { public static final String META_STORAGE_READINESS = "storage_readiness"; + private static final Logger LOG = Log.logger(HstoreStorageProbe.class); + private static final ExecutorService EXECUTOR = Executors.newCachedThreadPool( new ThreadFactory() { private final AtomicInteger seq = new AtomicInteger(); @@ -96,6 +107,8 @@ public static final class KnownStores { private volatile long at; private volatile Boolean pdOk; private volatile long pdAt; + private final AtomicReference>> inFlight = + new AtomicReference<>(); public List stores() { return this.stores; @@ -127,6 +140,34 @@ public void pdFailed() { this.pdOk = false; this.pdAt = System.currentTimeMillis(); } + + /** + * The refresh in flight, or a new one started on `executor`: only one + * PD call runs at a time no matter how many probes miss the cache, + * so a hung PD parks one thread, not one per probe. + */ + CompletableFuture> refresh(StoreLister lister, + ExecutorService executor) { + CompletableFuture> running = this.inFlight.get(); + if (running != null && !running.isDone()) { + return running; + } + CompletableFuture> mine = new CompletableFuture<>(); + if (!this.inFlight.compareAndSet(running, mine)) { + return this.inFlight.get(); + } + executor.execute(() -> { + try { + List stores = lister.activeStores(); + this.update(stores); + mine.complete(stores == null ? Collections.emptyList() : stores); + } catch (Throwable e) { + this.pdFailed(); + mine.completeExceptionally(e); + } + }); + return mine; + } } public static final class Result { @@ -199,8 +240,30 @@ public static Map probe(String graphName, long timeoutMs) { return new Result(false, "pd client not initialised", 0, null, false, -1L, -1L, 0L).toMap(); } - return probe(KNOWN, pd::getActiveStores, HstoreStorageProbe::pingScanState, - timeoutMs, EXECUTOR).toMap(); + return probe(KNOWN, () -> { + List stores = pd.getActiveStores(); + pruneChannels(CHANNELS, stores); + return stores; + }, HstoreStorageProbe::pingScanState, timeoutMs, EXECUTOR).toMap(); + } + + /** Shut down the channels of addresses PD no longer lists (replaced Stores). */ + static void pruneChannels(Map channels, + List stores) { + if (stores == null) { + return; + } + Set live = new HashSet<>(); + for (Metapb.Store store : stores) { + live.add(store.getAddress()); + } + channels.entrySet().removeIf(e -> { + if (live.contains(e.getKey())) { + return false; + } + e.getValue().shutdownNow(); + return true; + }); } private static void pingScanState(Metapb.Store store, long timeoutMs) { @@ -217,19 +280,9 @@ public static Result probe(KnownStores known, StoreLister lister, StorePinger pi E.checkArgument(timeoutMs > 0, "The probe timeout must be > 0, but got %s", timeoutMs); long deadline = System.currentTimeMillis() + timeoutMs; - // Refresh the store list from PD in the background; whatever PD - // answers lands in `known` for this or the next probe - CompletableFuture> refresh = new CompletableFuture<>(); - executor.execute(() -> { - try { - List stores = lister.activeStores(); - known.update(stores); - refresh.complete(stores == null ? Collections.emptyList() : stores); - } catch (Throwable e) { - known.pdFailed(); - refresh.completeExceptionally(e); - } - }); + // Refresh the store list from PD in the background (single-flight); + // whatever PD answers lands in `known` for this or the next probe + CompletableFuture> refresh = known.refresh(lister, executor); List stores = known.stores(); Boolean pdReachable = null; @@ -242,7 +295,8 @@ public static Result probe(KnownStores known, StoreLister lister, StorePinger pi return new Result(false, "no store list known and pd did not answer within " + timeoutMs + " ms", 0, null, false, -1L, -1L, 0L); } catch (Exception e) { - return new Result(false, "no store list known and pd failed: " + message(e), + LOG.warn("Storage readiness: no store list known and pd failed", e); + return new Result(false, "no store list known and pd failed: " + category(e), 0, null, false, known.pdAgeMs(), -1L, 0L); } if (stores.isEmpty()) { @@ -291,7 +345,8 @@ public static Result probe(KnownStores known, StoreLister lister, StorePinger pi known.ageMs(), elapsed(storeStart)); } catch (ExecutionException e) { Throwable cause = e.getCause() != null ? e.getCause() : e; - failures.add("a store failed: " + message(cause)); + LOG.debug("Storage readiness: a store ping failed", cause); + failures.add("a store failed: " + category(cause)); } catch (InterruptedException e) { Thread.currentThread().interrupt(); failures.add("interrupted"); @@ -341,8 +396,18 @@ private static long elapsed(long since) { return System.currentTimeMillis() - since; } - private static String message(Throwable e) { - String msg = e.getMessage(); - return e.getClass().getSimpleName() + (msg == null ? "" : ": " + msg); + /** + * A fixed category for the unauthenticated body: the gRPC status code, "pd + * unreachable" or the exception class, never the message (it can carry PD + * peers and Store host names). + */ + static String category(Throwable e) { + if (e instanceof StatusRuntimeException) { + return ((StatusRuntimeException) e).getStatus().getCode().name(); + } + if (e instanceof PDException) { + return "pd unreachable"; + } + return e.getClass().getSimpleName(); } } diff --git a/hugegraph-server/hugegraph-hstore/src/test/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbeTest.java b/hugegraph-server/hugegraph-hstore/src/test/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbeTest.java index 473f253fe5..2447944247 100644 --- a/hugegraph-server/hugegraph-hstore/src/test/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbeTest.java +++ b/hugegraph-server/hugegraph-hstore/src/test/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbeTest.java @@ -23,16 +23,24 @@ import java.util.Map; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; import org.apache.hugegraph.backend.store.hstore.HstoreStorageProbe.KnownStores; import org.apache.hugegraph.backend.store.hstore.HstoreStorageProbe.Result; +import org.apache.hugegraph.pd.common.PDException; import org.apache.hugegraph.pd.grpc.Metapb; import org.junit.AfterClass; import org.junit.Assert; import org.junit.Test; +import io.grpc.CallOptions; +import io.grpc.ClientCall; +import io.grpc.ManagedChannel; +import io.grpc.MethodDescriptor; +import io.grpc.Status; + public class HstoreStorageProbeTest { private static final ExecutorService EXECUTOR = Executors.newCachedThreadPool(); @@ -214,7 +222,8 @@ public void testNotReadyWhenEveryStoreFails() { refused, BUDGET, EXECUTOR); Assert.assertFalse(r.ready()); Assert.assertTrue(r.reason(), r.reason().startsWith("none of 2 known store(s) answered")); - Assert.assertTrue(r.reason(), r.reason().contains("connection refused")); + Assert.assertTrue(r.reason(), r.reason().contains("a store failed: IllegalStateException")); + Assert.assertFalse(r.reason(), r.reason().contains("connection refused")); Assert.assertNull(r.answeredStore()); } @@ -261,4 +270,112 @@ public void testRejectsNonPositiveBudget() { 0L, EXECUTOR); }); } + + /** + * The body is served without authentication: PD peers from the PD client's + * "PD unreachable, pd.peers=..." and Store host names from gRPC's "Unable + * to resolve host ..." must not reach it, only a category. + */ + @Test + public void testReasonCarriesNoPdPeersNorStoreHosts() { + Result pd = HstoreStorageProbe.probe(new KnownStores(), () -> { + throw new PDException(1, "PD unreachable, pd.peers=pd-0.internal:8686,pd-1.internal:8686"); + }, ANSWERS, BUDGET, EXECUTOR); + Assert.assertFalse(pd.ready()); + Assert.assertEquals("no store list known and pd failed: pd unreachable", pd.reason()); + + HstoreStorageProbe.StorePinger unresolved = (store, timeout) -> { + throw Status.UNAVAILABLE.withDescription( + "Unable to resolve host store-0.hugegraph-store.svc").asRuntimeException(); + }; + Result st = HstoreStorageProbe.probe(knowing(1L), () -> stores(1L), unresolved, + BUDGET, EXECUTOR); + Assert.assertFalse(st.ready()); + Assert.assertTrue(st.reason(), st.reason().contains("a store failed: UNAVAILABLE")); + String all = pd.toMap().toString() + st.toMap().toString(); + Assert.assertFalse(all, all.contains("internal") || all.contains("svc") || + all.contains("8686")); + } + + /** A hung PD parks one refresh, not one per probe. */ + @Test + public void testRefreshIsSingleFlight() throws Exception { + AtomicInteger calls = new AtomicInteger(); + KnownStores known = knowing(1L); + HstoreStorageProbe.StoreLister hung = () -> { + calls.incrementAndGet(); + Thread.sleep(3_000L); + return stores(1L, 2L); + }; + for (int i = 0; i < 5; i++) { + Assert.assertTrue(HstoreStorageProbe.probe(known, hung, ANSWERS, BUDGET, + EXECUTOR).ready()); + } + Thread.sleep(200L); + Assert.assertEquals(1, calls.get()); + for (int i = 0; i < 40 && known.stores().size() != 2; i++) { + Thread.sleep(100L); + } + Assert.assertEquals(2, known.stores().size()); + HstoreStorageProbe.probe(known, hung, ANSWERS, BUDGET, EXECUTOR); + Thread.sleep(100L); + Assert.assertEquals("a finished refresh allows a new one", 2, calls.get()); + } + + private static final class FakeChannel extends ManagedChannel { + + boolean shut; + + @Override + public ManagedChannel shutdown() { + this.shut = true; + return this; + } + + @Override + public boolean isShutdown() { + return this.shut; + } + + @Override + public boolean isTerminated() { + return this.shut; + } + + @Override + public ManagedChannel shutdownNow() { + return this.shutdown(); + } + + @Override + public boolean awaitTermination(long timeout, TimeUnit unit) { + return true; + } + + @Override + public ClientCall newCall(MethodDescriptor method, + CallOptions options) { + throw new UnsupportedOperationException(); + } + + @Override + public String authority() { + return "fake"; + } + } + + @Test + public void testChannelsOfReplacedStoresAreShutDown() { + Map channels = new java.util.concurrent.ConcurrentHashMap<>(); + FakeChannel kept = new FakeChannel(); + FakeChannel gone = new FakeChannel(); + channels.put("10.0.0.1:8500", kept); + channels.put("10.0.0.9:8500", gone); + HstoreStorageProbe.pruneChannels(channels, stores(1L, 2L)); + Assert.assertEquals(1, channels.size()); + Assert.assertFalse(kept.shut); + Assert.assertTrue(gone.shut); + HstoreStorageProbe.pruneChannels(channels, null); + Assert.assertEquals("a failed listing prunes nothing", 1, channels.size()); + } } diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/StorageReadinessTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/StorageReadinessTest.java index d4bda1dd50..64e6a0c126 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/StorageReadinessTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/StorageReadinessTest.java @@ -86,8 +86,9 @@ public void testProbeFailureIsNotReadyWithReason() { }; Map body = StorageReadiness.check(probe, 1000L, 0L); Assert.assertFalse(StorageReadiness.isReady(body)); - Assert.assertContains("probe failed: IllegalStateException", (String) body.get("reason")); - Assert.assertContains("has not been opened", (String) body.get("reason")); + Assert.assertEquals("probe failed: IllegalStateException", body.get("reason")); + // the endpoint is unauthenticated: no raw message in the body + Assert.assertFalse(body.toString().contains("has not been opened")); } @Test From dcdd1792795341be1437389feaef426a7dcb6aed Mon Sep 17 00:00:00 2001 From: Sebastian Gruza Date: Sat, 19 Sep 2026 08:07:26 +0000 Subject: [PATCH 03/10] feat(server): simplify the readiness probe (BasicThreadFactory, no unused graphName, map result) Review round 2 of #3221: the executor uses commons-lang3's BasicThreadFactory like ExecutorUtil does, the unread graphName parameter and its concat at the HstoreStore call site are gone, and probe() builds the body map through one private helper instead of a Result holder whose only consumer was toMap(); the tests assert on map entries. Co-Authored-By: Claude Fable 5.1 --- .../store/hstore/HstoreStorageProbe.java | 109 +++++------------- .../backend/store/hstore/HstoreStore.java | 3 +- .../store/hstore/HstoreStorageProbeTest.java | 108 ++++++++--------- 3 files changed, 86 insertions(+), 134 deletions(-) diff --git a/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbe.java b/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbe.java index 390bbacc1e..35d0773058 100644 --- a/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbe.java +++ b/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbe.java @@ -32,12 +32,11 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; -import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; -import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; +import org.apache.commons.lang3.concurrent.BasicThreadFactory; import org.apache.hugegraph.pd.client.PDClient; import org.apache.hugegraph.pd.common.PDException; import org.apache.hugegraph.pd.grpc.Metapb; @@ -71,16 +70,8 @@ public final class HstoreStorageProbe { private static final Logger LOG = Log.logger(HstoreStorageProbe.class); private static final ExecutorService EXECUTOR = Executors.newCachedThreadPool( - new ThreadFactory() { - private final AtomicInteger seq = new AtomicInteger(); - - @Override - public Thread newThread(Runnable r) { - Thread t = new Thread(r, "storage-readiness-" + this.seq.incrementAndGet()); - t.setDaemon(true); - return t; - } - }); + new BasicThreadFactory.Builder().namingPattern("storage-readiness-%d") + .daemon(true).build()); private static final KnownStores KNOWN = new KnownStores(); private static final Map CHANNELS = new ConcurrentHashMap<>(); @@ -170,81 +161,38 @@ CompletableFuture> refresh(StoreLister lister, } } - public static final class Result { - - private final boolean ready; - private final String reason; - private final int activeStores; - private final Long answeredStore; - private final Boolean pdReachable; - private final long pdAgeMs; - private final long storesAgeMs; - private final long storeMillis; - - Result(boolean ready, String reason, int activeStores, Long answeredStore, - Boolean pdReachable, long pdAgeMs, long storesAgeMs, long storeMillis) { - this.ready = ready; - this.reason = reason; - this.activeStores = activeStores; - this.answeredStore = answeredStore; - this.pdReachable = pdReachable; - this.pdAgeMs = pdAgeMs; - this.storesAgeMs = storesAgeMs; - this.storeMillis = storeMillis; - } - - public boolean ready() { - return this.ready; - } - - public String reason() { - return this.reason; - } - - public int activeStores() { - return this.activeStores; - } - - public Long answeredStore() { - return this.answeredStore; - } - - public Boolean pdReachable() { - return this.pdReachable; - } - - public Map toMap() { - Map map = new LinkedHashMap<>(); - map.put("ready", this.ready); - map.put("reason", this.reason); - map.put("active_stores", this.activeStores); - map.put("answered_store", this.answeredStore); - map.put("pd_reachable", this.pdReachable); - map.put("pd_checked_age_ms", this.pdAgeMs); - map.put("stores_age_ms", this.storesAgeMs); - map.put("store_millis", this.storeMillis); - return map; - } + /** The unauthenticated body: no addresses, no raw exception text. */ + private static Map result(boolean ready, String reason, int activeStores, + Long answeredStore, Boolean pdReachable, + long pdAgeMs, long storesAgeMs, long storeMillis) { + Map map = new LinkedHashMap<>(); + map.put("ready", ready); + map.put("reason", reason); + map.put("active_stores", activeStores); + map.put("answered_store", answeredStore); + map.put("pd_reachable", pdReachable); + map.put("pd_checked_age_ms", pdAgeMs); + map.put("stores_age_ms", storesAgeMs); + map.put("store_millis", storeMillis); + return map; } /** * Probe through the process-wide PD client and this probe's own plaintext * channels to the stores (the store gRPC server takes no credentials). * - * @param graphName the store-side graph name, kept for the meta handler * @param timeoutMs the whole budget for PD plus stores */ - public static Map probe(String graphName, long timeoutMs) { + public static Map probe(long timeoutMs) { PDClient pd = HstoreSessionsImpl.getDefaultPdClient(); if (pd == null) { - return new Result(false, "pd client not initialised", 0, null, - false, -1L, -1L, 0L).toMap(); + return result(false, "pd client not initialised", 0, null, false, -1L, -1L, 0L); } return probe(KNOWN, () -> { List stores = pd.getActiveStores(); pruneChannels(CHANNELS, stores); return stores; - }, HstoreStorageProbe::pingScanState, timeoutMs, EXECUTOR).toMap(); + }, HstoreStorageProbe::pingScanState, timeoutMs, EXECUTOR); } /** Shut down the channels of addresses PD no longer lists (replaced Stores). */ @@ -275,8 +223,9 @@ private static void pingScanState(Metapb.Store store, long timeoutMs) { .getScanState(SubStateReq.getDefaultInstance()); } - public static Result probe(KnownStores known, StoreLister lister, StorePinger pinger, - long timeoutMs, ExecutorService executor) { + public static Map probe(KnownStores known, StoreLister lister, + StorePinger pinger, long timeoutMs, + ExecutorService executor) { E.checkArgument(timeoutMs > 0, "The probe timeout must be > 0, but got %s", timeoutMs); long deadline = System.currentTimeMillis() + timeoutMs; @@ -292,15 +241,15 @@ public static Result probe(KnownStores known, StoreLister lister, StorePinger pi stores = await(refresh, deadline); pdReachable = true; } catch (TimeoutException e) { - return new Result(false, "no store list known and pd did not answer within " + + return result(false, "no store list known and pd did not answer within " + timeoutMs + " ms", 0, null, false, -1L, -1L, 0L); } catch (Exception e) { LOG.warn("Storage readiness: no store list known and pd failed", e); - return new Result(false, "no store list known and pd failed: " + category(e), + return result(false, "no store list known and pd failed: " + category(e), 0, null, false, known.pdAgeMs(), -1L, 0L); } if (stores.isEmpty()) { - return new Result(false, "no active store registered in pd", + return result(false, "no active store registered in pd", 0, null, true, known.pdAgeMs(), known.ageMs(), 0L); } } @@ -319,7 +268,7 @@ public static Result probe(KnownStores known, StoreLister lister, StorePinger pi })); } List failures = new ArrayList<>(); - Result result = null; + Map result = null; try { for (int done = 0; done < stores.size() && result == null; done++) { long remaining = deadline - System.currentTimeMillis(); @@ -340,7 +289,7 @@ public static Result probe(KnownStores known, StoreLister lister, StorePinger pi } try { Metapb.Store store = first.get(); - result = new Result(true, "ok", stores.size(), store.getId(), + result = result(true, "ok", stores.size(), store.getId(), pdState(refresh, known, pdReachable), known.pdAgeMs(), known.ageMs(), elapsed(storeStart)); } catch (ExecutionException e) { @@ -361,7 +310,7 @@ public static Result probe(KnownStores known, StoreLister lister, StorePinger pi if (result != null) { return result; } - return new Result(false, "none of " + stores.size() + " known store(s) answered: " + + return result(false, "none of " + stores.size() + " known store(s) answered: " + String.join("; ", failures), stores.size(), null, pdState(refresh, known, pdReachable), known.pdAgeMs(), known.ageMs(), elapsed(storeStart)); diff --git a/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStore.java b/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStore.java index 7977dfc248..869c25e915 100644 --- a/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStore.java +++ b/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStore.java @@ -116,8 +116,7 @@ private void registerMetaHandlers() { this.registerMetaHandler(HstoreStorageProbe.META_STORAGE_READINESS, (session, meta, args) -> { E.checkArgument(args.length == 1 && args[0] instanceof Number, "Expect the timeout in ms as the only argument"); - return HstoreStorageProbe.probe(this.namespace + "/" + this.store, - ((Number) args[0]).longValue()); + return HstoreStorageProbe.probe(((Number) args[0]).longValue()); }); this.registerMetaHandler("mode", (session, meta, args) -> { E.checkArgument(args.length == 1, diff --git a/hugegraph-server/hugegraph-hstore/src/test/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbeTest.java b/hugegraph-server/hugegraph-hstore/src/test/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbeTest.java index 2447944247..33fe18b59d 100644 --- a/hugegraph-server/hugegraph-hstore/src/test/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbeTest.java +++ b/hugegraph-server/hugegraph-hstore/src/test/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbeTest.java @@ -28,7 +28,6 @@ import java.util.stream.Collectors; import org.apache.hugegraph.backend.store.hstore.HstoreStorageProbe.KnownStores; -import org.apache.hugegraph.backend.store.hstore.HstoreStorageProbe.Result; import org.apache.hugegraph.pd.common.PDException; import org.apache.hugegraph.pd.grpc.Metapb; import org.junit.AfterClass; @@ -70,15 +69,19 @@ private static KnownStores knowing(long... ids) { private static final HstoreStorageProbe.StorePinger ANSWERS = (store, timeout) -> { }; + private static String reason(Map body) { + return (String) body.get("reason"); + } + @Test public void testReadyWhenPdAndOneStoreAnswer() { - Result r = HstoreStorageProbe.probe(new KnownStores(), () -> stores(1L, 2L, 3L), + Map r = HstoreStorageProbe.probe(new KnownStores(), () -> stores(1L, 2L, 3L), ANSWERS, BUDGET, EXECUTOR); - Assert.assertTrue(r.reason(), r.ready()); - Assert.assertEquals(3, r.activeStores()); - Assert.assertNotNull(r.answeredStore()); - Assert.assertEquals(Boolean.TRUE, r.pdReachable()); - Assert.assertEquals("ok", r.reason()); + Assert.assertTrue(reason(r), Boolean.TRUE.equals(r.get("ready"))); + Assert.assertEquals(3, r.get("active_stores")); + Assert.assertNotNull(r.get("answered_store")); + Assert.assertEquals(Boolean.TRUE, r.get("pd_reachable")); + Assert.assertEquals("ok", reason(r)); } @Test @@ -90,10 +93,10 @@ public void testFirstAnsweringStoreWinsAfterFailures() { throw new IllegalStateException("UNAVAILABLE"); } }; - Result r = HstoreStorageProbe.probe(knowing(1L, 2L, 3L), () -> stores(1L, 2L, 3L), + Map r = HstoreStorageProbe.probe(knowing(1L, 2L, 3L), () -> stores(1L, 2L, 3L), onlyThird, BUDGET, EXECUTOR); - Assert.assertTrue(r.reason(), r.ready()); - Assert.assertEquals(Long.valueOf(3L), r.answeredStore()); + Assert.assertTrue(reason(r), Boolean.TRUE.equals(r.get("ready"))); + Assert.assertEquals(3L, r.get("answered_store")); // the pings run in parallel; the failing ones may or may not have run Assert.assertTrue(pings.get() >= 1 && pings.get() <= 3); } @@ -111,35 +114,35 @@ public void testHungStoreDoesNotHideAnAnsweringOne() { } }; long start = System.currentTimeMillis(); - Result r = HstoreStorageProbe.probe(knowing(1L, 2L, 3L), () -> stores(1L, 2L, 3L), + Map r = HstoreStorageProbe.probe(knowing(1L, 2L, 3L), () -> stores(1L, 2L, 3L), onlySecondAnswers, BUDGET, EXECUTOR); long took = System.currentTimeMillis() - start; - Assert.assertTrue(r.reason(), r.ready()); - Assert.assertEquals(Long.valueOf(2L), r.answeredStore()); + Assert.assertTrue(reason(r), Boolean.TRUE.equals(r.get("ready"))); + Assert.assertEquals(2L, r.get("answered_store")); Assert.assertTrue("took " + took, took < BUDGET); } @Test public void testFirstProbeWithoutKnownStoresNeedsPd() { - Result r = HstoreStorageProbe.probe(new KnownStores(), () -> { + Map r = HstoreStorageProbe.probe(new KnownStores(), () -> { throw new IllegalStateException("UNAVAILABLE: io exception"); }, ANSWERS, BUDGET, EXECUTOR); - Assert.assertFalse(r.ready()); - Assert.assertTrue(r.reason(), r.reason().startsWith( + Assert.assertFalse(Boolean.TRUE.equals(r.get("ready"))); + Assert.assertTrue(reason(r), reason(r).startsWith( "no store list known and pd failed: IllegalStateException")); - Assert.assertEquals(Boolean.FALSE, r.pdReachable()); + Assert.assertEquals(Boolean.FALSE, r.get("pd_reachable")); } @Test public void testFirstProbeWithHungPdStaysWithinBudget() { long start = System.currentTimeMillis(); - Result r = HstoreStorageProbe.probe(new KnownStores(), () -> { + Map r = HstoreStorageProbe.probe(new KnownStores(), () -> { Thread.sleep(10_000L); return stores(1L); }, ANSWERS, BUDGET, EXECUTOR); long took = System.currentTimeMillis() - start; - Assert.assertFalse(r.ready()); - Assert.assertTrue(r.reason(), r.reason().contains("pd did not answer within")); + Assert.assertFalse(Boolean.TRUE.equals(r.get("ready"))); + Assert.assertTrue(reason(r), reason(r).contains("pd did not answer within")); Assert.assertTrue("took " + took, took < BUDGET * 4); } @@ -149,25 +152,25 @@ public void testFirstProbeWithHungPdStaysWithinBudget() { */ @Test public void testKnownStoresKeepTheServerReadyWhilePdIsDown() { - Result r = HstoreStorageProbe.probe(knowing(1L, 2L), () -> { + Map r = HstoreStorageProbe.probe(knowing(1L, 2L), () -> { throw new IllegalStateException("PD unreachable"); }, ANSWERS, BUDGET, EXECUTOR); - Assert.assertTrue(r.reason(), r.ready()); - Assert.assertEquals(2, r.activeStores()); + Assert.assertTrue(reason(r), Boolean.TRUE.equals(r.get("ready"))); + Assert.assertEquals(2, r.get("active_stores")); } @Test public void testHungPdDoesNotDelayAProbeWithKnownStores() { long start = System.currentTimeMillis(); - Result r = HstoreStorageProbe.probe(knowing(1L, 2L), () -> { + Map r = HstoreStorageProbe.probe(knowing(1L, 2L), () -> { Thread.sleep(10_000L); return stores(1L, 2L); }, ANSWERS, BUDGET, EXECUTOR); long took = System.currentTimeMillis() - start; - Assert.assertTrue(r.reason(), r.ready()); + Assert.assertTrue(reason(r), Boolean.TRUE.equals(r.get("ready"))); // the refresh is still pending, so the outcome of the last finished // one (the seed) is reported - Assert.assertEquals(Boolean.TRUE, r.pdReachable()); + Assert.assertEquals(Boolean.TRUE, r.get("pd_reachable")); Assert.assertTrue("took " + took, took < BUDGET); } @@ -181,13 +184,13 @@ public void testLastPdOutcomeIsReportedWhileTheRefreshIsPending() throws Excepti Thread.sleep(20L); } Assert.assertEquals(Boolean.FALSE, known.pdOk()); - Result r = HstoreStorageProbe.probe(known, () -> { + Map r = HstoreStorageProbe.probe(known, () -> { Thread.sleep(10_000L); return stores(1L); }, ANSWERS, BUDGET, EXECUTOR); - Assert.assertTrue(r.ready()); - Assert.assertEquals(Boolean.FALSE, r.pdReachable()); - Assert.assertTrue(r.toMap().containsKey("pd_checked_age_ms")); + Assert.assertTrue(Boolean.TRUE.equals(r.get("ready"))); + Assert.assertEquals(Boolean.FALSE, r.get("pd_reachable")); + Assert.assertTrue(r.containsKey("pd_checked_age_ms")); } @Test @@ -204,10 +207,10 @@ public void testPdAnswerUpdatesTheKnownStoresForTheNextProbe() throws Exception @Test public void testEmptyPdAnswerIsNotReadyAndKeepsTheOldList() { KnownStores fresh = new KnownStores(); - Result r = HstoreStorageProbe.probe(fresh, Collections::emptyList, ANSWERS, + Map r = HstoreStorageProbe.probe(fresh, Collections::emptyList, ANSWERS, BUDGET, EXECUTOR); - Assert.assertFalse(r.ready()); - Assert.assertEquals("no active store registered in pd", r.reason()); + Assert.assertFalse(Boolean.TRUE.equals(r.get("ready"))); + Assert.assertEquals("no active store registered in pd", reason(r)); KnownStores known = knowing(1L); HstoreStorageProbe.probe(known, Collections::emptyList, ANSWERS, BUDGET, EXECUTOR); Assert.assertEquals(1, known.stores().size()); @@ -218,13 +221,13 @@ public void testNotReadyWhenEveryStoreFails() { HstoreStorageProbe.StorePinger refused = (store, timeout) -> { throw new IllegalStateException("connection refused"); }; - Result r = HstoreStorageProbe.probe(knowing(7L, 8L), () -> stores(7L, 8L), + Map r = HstoreStorageProbe.probe(knowing(7L, 8L), () -> stores(7L, 8L), refused, BUDGET, EXECUTOR); - Assert.assertFalse(r.ready()); - Assert.assertTrue(r.reason(), r.reason().startsWith("none of 2 known store(s) answered")); - Assert.assertTrue(r.reason(), r.reason().contains("a store failed: IllegalStateException")); - Assert.assertFalse(r.reason(), r.reason().contains("connection refused")); - Assert.assertNull(r.answeredStore()); + Assert.assertFalse(Boolean.TRUE.equals(r.get("ready"))); + Assert.assertTrue(reason(r), reason(r).startsWith("none of 2 known store(s) answered")); + Assert.assertTrue(reason(r), reason(r).contains("a store failed: IllegalStateException")); + Assert.assertFalse(reason(r), reason(r).contains("connection refused")); + Assert.assertNull(r.get("answered_store")); } @Test @@ -233,11 +236,11 @@ public void testHungStoresStayWithinTheBudget() { Thread.sleep(10_000L); }; long start = System.currentTimeMillis(); - Result r = HstoreStorageProbe.probe(knowing(1L, 2L, 3L), () -> stores(1L, 2L, 3L), + Map r = HstoreStorageProbe.probe(knowing(1L, 2L, 3L), () -> stores(1L, 2L, 3L), hung, BUDGET, EXECUTOR); long took = System.currentTimeMillis() - start; - Assert.assertFalse(r.ready()); - Assert.assertTrue(r.reason(), r.reason().contains("did not answer within")); + Assert.assertFalse(Boolean.TRUE.equals(r.get("ready"))); + Assert.assertTrue(reason(r), reason(r).contains("did not answer within")); Assert.assertTrue("took " + took, took < BUDGET * 4); } @@ -254,7 +257,7 @@ public void testPingGetsTheRemainingBudget() { @Test public void testMapCarriesNoAddresses() { Map map = HstoreStorageProbe.probe(knowing(1L), () -> stores(1L), - ANSWERS, BUDGET, EXECUTOR).toMap(); + ANSWERS, BUDGET, EXECUTOR); Assert.assertEquals(true, map.get("ready")); Assert.assertEquals(1, map.get("active_stores")); Assert.assertEquals(1L, map.get("answered_store")); @@ -278,21 +281,21 @@ public void testRejectsNonPositiveBudget() { */ @Test public void testReasonCarriesNoPdPeersNorStoreHosts() { - Result pd = HstoreStorageProbe.probe(new KnownStores(), () -> { + Map pd = HstoreStorageProbe.probe(new KnownStores(), () -> { throw new PDException(1, "PD unreachable, pd.peers=pd-0.internal:8686,pd-1.internal:8686"); }, ANSWERS, BUDGET, EXECUTOR); - Assert.assertFalse(pd.ready()); - Assert.assertEquals("no store list known and pd failed: pd unreachable", pd.reason()); + Assert.assertFalse(Boolean.TRUE.equals(pd.get("ready"))); + Assert.assertEquals("no store list known and pd failed: pd unreachable", reason(pd)); HstoreStorageProbe.StorePinger unresolved = (store, timeout) -> { throw Status.UNAVAILABLE.withDescription( "Unable to resolve host store-0.hugegraph-store.svc").asRuntimeException(); }; - Result st = HstoreStorageProbe.probe(knowing(1L), () -> stores(1L), unresolved, + Map st = HstoreStorageProbe.probe(knowing(1L), () -> stores(1L), unresolved, BUDGET, EXECUTOR); - Assert.assertFalse(st.ready()); - Assert.assertTrue(st.reason(), st.reason().contains("a store failed: UNAVAILABLE")); - String all = pd.toMap().toString() + st.toMap().toString(); + Assert.assertFalse(Boolean.TRUE.equals(st.get("ready"))); + Assert.assertTrue(reason(st), reason(st).contains("a store failed: UNAVAILABLE")); + String all = pd.toString() + st.toString(); Assert.assertFalse(all, all.contains("internal") || all.contains("svc") || all.contains("8686")); } @@ -308,8 +311,9 @@ public void testRefreshIsSingleFlight() throws Exception { return stores(1L, 2L); }; for (int i = 0; i < 5; i++) { - Assert.assertTrue(HstoreStorageProbe.probe(known, hung, ANSWERS, BUDGET, - EXECUTOR).ready()); + Assert.assertEquals(Boolean.TRUE, HstoreStorageProbe.probe(known, hung, ANSWERS, + BUDGET, EXECUTOR) + .get("ready")); } Thread.sleep(200L); Assert.assertEquals(1, calls.get()); From 8483f6c58cbfc65db5cf3675812a14824749e623 Mon Sep 17 00:00:00 2001 From: Sebastian Gruza Date: Sat, 19 Sep 2026 15:31:05 +0000 Subject: [PATCH 04/10] feat(server): exempt /readiness from load shedding, keep channels on an empty PD answer, add ReadinessApiTest Review round 3 of #3221: - readiness joins LoadDetectFilter.WHITE_API_LIST next to versions, so a busy server still answers its readiness probe from the storage state instead of shedding it (LoadReleaseFilter reads the same list); test case in LoadDetectFilterTest - pruneChannels() ignores an empty PD answer, the same rule KnownStores.update() applies, so the channels the pings still use stay open while PD reports no active Store; the channel test covers it - ReadinessApiTest drives GET /readiness through the API suite, with and without credentials, on every backend of the suite Co-Authored-By: Claude Fable 5.1 --- .../api/filter/LoadDetectFilter.java | 3 +- .../store/hstore/HstoreStorageProbe.java | 9 ++- .../store/hstore/HstoreStorageProbeTest.java | 4 ++ .../apache/hugegraph/api/ApiTestSuite.java | 1 + .../hugegraph/api/ReadinessApiTest.java | 71 +++++++++++++++++++ .../unit/api/filter/LoadDetectFilterTest.java | 17 +++++ 6 files changed, 102 insertions(+), 3 deletions(-) create mode 100644 hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/ReadinessApiTest.java diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/LoadDetectFilter.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/LoadDetectFilter.java index 1df19f5e5c..390c2c9e80 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/LoadDetectFilter.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/filter/LoadDetectFilter.java @@ -53,7 +53,8 @@ public class LoadDetectFilter implements ContainerRequestFilter { "", "apis", "metrics", - "versions" + "versions", + "readiness" ); // Call gc every 30+ seconds if memory is low and request frequently diff --git a/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbe.java b/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbe.java index 35d0773058..f2fe3c3f58 100644 --- a/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbe.java +++ b/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbe.java @@ -195,10 +195,15 @@ public static Map probe(long timeoutMs) { }, HstoreStorageProbe::pingScanState, timeoutMs, EXECUTOR); } - /** Shut down the channels of addresses PD no longer lists (replaced Stores). */ + /** + * Shut down the channels of addresses PD no longer lists (replaced + * Stores). An empty answer is ignored, the same rule KnownStores.update + * applies: the pings keep using the last known Stores, so their channels + * must stay open. + */ static void pruneChannels(Map channels, List stores) { - if (stores == null) { + if (stores == null || stores.isEmpty()) { return; } Set live = new HashSet<>(); diff --git a/hugegraph-server/hugegraph-hstore/src/test/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbeTest.java b/hugegraph-server/hugegraph-hstore/src/test/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbeTest.java index 33fe18b59d..939433f49f 100644 --- a/hugegraph-server/hugegraph-hstore/src/test/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbeTest.java +++ b/hugegraph-server/hugegraph-hstore/src/test/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbeTest.java @@ -381,5 +381,9 @@ public void testChannelsOfReplacedStoresAreShutDown() { Assert.assertTrue(gone.shut); HstoreStorageProbe.pruneChannels(channels, null); Assert.assertEquals("a failed listing prunes nothing", 1, channels.size()); + HstoreStorageProbe.pruneChannels(channels, Collections.emptyList()); + Assert.assertEquals("an empty listing keeps the channels the pings still use", + 1, channels.size()); + Assert.assertFalse(kept.shut); } } diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/ApiTestSuite.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/ApiTestSuite.java index f4d74e49b3..a2ebf17926 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/ApiTestSuite.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/ApiTestSuite.java @@ -35,6 +35,7 @@ TaskApiTest.class, GremlinApiTest.class, MetricsApiTest.class, + ReadinessApiTest.class, UserApiTest.class, LoginApiTest.class, ProjectApiTest.class, diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/ReadinessApiTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/ReadinessApiTest.java new file mode 100644 index 0000000000..636c1f7fea --- /dev/null +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/ReadinessApiTest.java @@ -0,0 +1,71 @@ +/* + * 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.api; + +import java.util.Map; + +import org.apache.hugegraph.testutil.Assert; +import org.apache.hugegraph.util.JsonUtil; +import org.junit.Test; + +import jakarta.ws.rs.client.ClientBuilder; +import jakarta.ws.rs.core.Response; + +/** + * The readiness endpoint answers 200 on a healthy server whatever the + * backend: "embedded" for the in-process backends of the API suite, "hstore" + * (with the storage fields) on the hstore job. + */ +public class ReadinessApiTest extends BaseApiTest { + + private static final String PATH = "/readiness"; + + @Test + public void testReadyOnAHealthyServer() { + Response r = client().get(PATH); + String result = assertResponseStatus(200, r); + Map body = JsonUtil.fromJson(result, Map.class); + Assert.assertEquals(true, body.get("ready")); + Assert.assertTrue(String.valueOf(body.get("storage")), + "embedded".equals(body.get("storage")) || + "hstore".equals(body.get("storage"))); + Assert.assertNotNull(body.get("reason")); + if ("hstore".equals(body.get("storage"))) { + Assert.assertEquals("ok", body.get("reason")); + Assert.assertTrue(((Number) body.get("active_stores")).intValue() >= 1); + Assert.assertNotNull(body.get("answered_store")); + Assert.assertTrue(body.containsKey("cached")); + } + } + + /** + * A Kubernetes httpGet probe carries no credential and no graphspace + * prefix, so the endpoint must answer without either. + */ + @Test + public void testReadyWithoutCredentials() { + Response r = ClientBuilder.newClient().target(BASE_URL + PATH) + .request().get(); + try { + Assert.assertEquals(200, r.getStatus()); + Assert.assertContains("\"ready\":true", r.readEntity(String.class)); + } finally { + r.close(); + } + } +} diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/filter/LoadDetectFilterTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/filter/LoadDetectFilterTest.java index 5be5b64a92..1cfd22e757 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/filter/LoadDetectFilterTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/filter/LoadDetectFilterTest.java @@ -112,6 +112,23 @@ public void testFilter_WhiteListPathIgnored() { Assert.assertTrue(this.testAppender.events().isEmpty()); } + /** + * A readiness probe must answer from the storage state, not from the + * worker load: a Server that is merely busy is still ready, and shedding + * probes would pull every busy Server out of the Service during a spike. + */ + @Test + public void testFilter_ReadinessIgnoredLikeVersions() { + setupPath("readiness", List.of("readiness")); + this.setConfigProvider(createConfig(2, 0)); + this.workLoad.incrementAndGet(); + + this.loadDetectFilter.filter(this.requestContext); + + Assert.assertEquals(1, this.workLoad.get().get()); + Assert.assertTrue(this.testAppender.events().isEmpty()); + } + @Test public void testFilter_RejectsWhenWorkerLoadIsTooHigh() { setupPath("graphs/hugegraph/vertices", From acb01d5d890bfd374c4c38a81545c93993c1eceb Mon Sep 17 00:00:00 2001 From: Sebastian Gruza Date: Wed, 30 Sep 2026 07:58:11 +0000 Subject: [PATCH 05/10] feat(server): share one in-flight readiness probe instead of serialising callers on a monitor StorageReadiness.check no longer holds the class monitor while the Store probe does network I/O: the first caller runs the probe on its own thread, concurrent callers wait for that result bounded by their own timeout ("a probe is still running after N ms"). With readiness.cache_ttl=0 a slow Store can no longer queue REST workers behind it. StorageReadinessTest 8: concurrent callers share one probe, the follower wait is bounded. Co-Authored-By: Claude Opus 4.8 --- .../api/profile/StorageReadiness.java | 71 +++++++++++++++-- .../store/hstore/HstoreStorageProbe.java | 9 +++ .../unit/core/StorageReadinessTest.java | 79 +++++++++++++++++++ 3 files changed, 153 insertions(+), 6 deletions(-) diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java index b455be1d61..f04a139c8c 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java @@ -21,6 +21,11 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicReference; import org.apache.hugegraph.HugeGraph; import org.apache.hugegraph.auth.HugeGraphAuthProxy; @@ -47,6 +52,8 @@ public final class StorageReadiness { private static volatile Map lastResult; private static volatile long lastCheckedAt; + private static final AtomicReference>> IN_FLIGHT = + new AtomicReference<>(); private StorageReadiness() { } @@ -79,8 +86,15 @@ public static Map check(GraphManager manager, return holder.get(0); } - public static synchronized Map check(Probe probe, long timeoutMs, - long cacheTtlMs) { + /** + * At most one probe runs at a time and every caller gets its answer: + * the first caller runs the probe on its own thread, concurrent callers + * wait for that result, each bounded by its own timeout. No monitor is + * held during the probe, so a slow store cannot queue REST workers + * behind it (relevant with readiness.cache_ttl=0, where every request + * probes). + */ + public static Map check(Probe probe, long timeoutMs, long cacheTtlMs) { long now = System.currentTimeMillis(); Map cached = lastResult; if (cached != null && now - lastCheckedAt < cacheTtlMs) { @@ -88,6 +102,43 @@ public static synchronized Map check(Probe probe, long timeoutMs body.put("cached", true); return body; } + CompletableFuture> mine = new CompletableFuture<>(); + CompletableFuture> running = IN_FLIGHT.get(); + if (running == null && IN_FLIGHT.compareAndSet(null, mine)) { + Map body; + try { + body = probeOnce(probe, timeoutMs); + lastResult = body; + lastCheckedAt = System.currentTimeMillis(); + } finally { + IN_FLIGHT.set(null); + } + mine.complete(body); + return new LinkedHashMap<>(body); + } + if (running == null) { + running = IN_FLIGHT.get(); + } + if (running == null) { + // The owner finished between our two reads: its result is cached + return check(probe, timeoutMs, cacheTtlMs); + } + try { + Map body = new LinkedHashMap<>( + running.get(timeoutMs, TimeUnit.MILLISECONDS)); + body.put("shared", true); + return body; + } catch (TimeoutException e) { + return notReady("a probe is still running after " + timeoutMs + " ms"); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + return notReady("interrupted"); + } catch (ExecutionException e) { + return notReady("probe failed: " + e.getCause().getClass().getSimpleName()); + } + } + + private static Map probeOnce(Probe probe, long timeoutMs) { Map body = new LinkedHashMap<>(); body.put("ready", false); body.put("storage", BACKEND_HSTORE); @@ -100,18 +151,26 @@ public static synchronized Map check(Probe probe, long timeoutMs body.put("reason", "probe failed: " + e.getClass().getSimpleName()); } body.put("cached", false); - lastResult = body; - lastCheckedAt = System.currentTimeMillis(); - return new LinkedHashMap<>(body); + return body; + } + + private static Map notReady(String reason) { + Map body = new LinkedHashMap<>(); + body.put("ready", false); + body.put("storage", BACKEND_HSTORE); + body.put("reason", reason); + body.put("cached", false); + return body; } public static boolean isReady(Map body) { return Boolean.TRUE.equals(body.get("ready")); } - public static synchronized void resetCache() { + public static void resetCache() { lastResult = null; lastCheckedAt = 0L; + IN_FLIGHT.set(null); } private static HugeGraph firstHstoreGraph(GraphManager manager) { diff --git a/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbe.java b/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbe.java index f2fe3c3f58..87defb43a5 100644 --- a/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbe.java +++ b/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbe.java @@ -62,6 +62,15 @@ * wins; every wait is bounded by one shared time budget. The result carries * no addresses and no raw exception text, since it is served without * authentication; the full messages go to the log. + *

+ * Scope: ready means this server knows a Store list and reaches at least one + * Store over gRPC. A Store whose status RPC answers while its raft or + * partition path is broken is not detected. A read through the graph path + * was measured as the gate and rejected: a failed read invalidates the + * partition cache the queries share (with PD down the data plane then fails + * too), and one moving partition stalls every read through that client past + * the budget, so a rolling Store restart took every server out of the + * Service while the traffic was fine. */ public final class HstoreStorageProbe { diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/StorageReadinessTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/StorageReadinessTest.java index 64e6a0c126..1f108fc349 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/StorageReadinessTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/StorageReadinessTest.java @@ -19,6 +19,13 @@ import java.util.LinkedHashMap; import java.util.Map; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import org.apache.hugegraph.api.filter.AuthenticationFilter; @@ -107,6 +114,78 @@ public void testCachedCopyIsIsolated() { Assert.assertTrue(StorageReadiness.isReady(second)); } + /** + * Concurrent callers with no cache share one probe: the first runs it on + * its own thread, the others wait for that result. Nothing holds a + * monitor while the probe does its I/O. + */ + @Test + public void testConcurrentCallersShareOneProbe() throws Exception { + AtomicInteger probes = new AtomicInteger(); + CountDownLatch started = new CountDownLatch(1); + CountDownLatch release = new CountDownLatch(1); + StorageReadiness.Probe probe = t -> { + probes.incrementAndGet(); + started.countDown(); + release.await(5, TimeUnit.SECONDS); + return result(true, "ok"); + }; + ExecutorService pool = Executors.newFixedThreadPool(4); + try { + Future> owner = pool.submit(() -> { + return StorageReadiness.check(probe, 2000L, 0L); + }); + Assert.assertTrue(started.await(2, TimeUnit.SECONDS)); + List>> followers = new ArrayList<>(); + for (int i = 0; i < 3; i++) { + followers.add(pool.submit(() -> StorageReadiness.check(probe, 2000L, 0L))); + } + Thread.sleep(100L); + release.countDown(); + Assert.assertTrue(StorageReadiness.isReady(owner.get(2, TimeUnit.SECONDS))); + Assert.assertEquals(false, owner.get().get("cached")); + for (Future> f : followers) { + Map body = f.get(2, TimeUnit.SECONDS); + Assert.assertTrue(StorageReadiness.isReady(body)); + Assert.assertEquals(true, body.get("shared")); + } + Assert.assertEquals(1, probes.get()); + } finally { + release.countDown(); + pool.shutdownNow(); + } + } + + /** A follower waits at most its own timeout for the running probe. */ + @Test + public void testFollowerWaitIsBounded() throws Exception { + CountDownLatch started = new CountDownLatch(1); + CountDownLatch release = new CountDownLatch(1); + StorageReadiness.Probe probe = t -> { + started.countDown(); + release.await(5, TimeUnit.SECONDS); + return result(true, "ok"); + }; + ExecutorService pool = Executors.newFixedThreadPool(2); + try { + Future> owner = pool.submit(() -> { + return StorageReadiness.check(probe, 5000L, 0L); + }); + Assert.assertTrue(started.await(2, TimeUnit.SECONDS)); + long start = System.currentTimeMillis(); + Map follower = StorageReadiness.check(probe, 200L, 0L); + long took = System.currentTimeMillis() - start; + Assert.assertFalse(StorageReadiness.isReady(follower)); + Assert.assertEquals("a probe is still running after 200 ms", follower.get("reason")); + Assert.assertTrue("took " + took, took >= 200L && took < 2000L); + release.countDown(); + Assert.assertTrue(StorageReadiness.isReady(owner.get(2, TimeUnit.SECONDS))); + } finally { + release.countDown(); + pool.shutdownNow(); + } + } + /** * A Kubernetes httpGet probe carries no credential and no graphspace, so * the endpoint must pass both the graphspace path rewrite and the auth From 62ddb19d5309e26c5f2cdbb16d80b425b6a80701 Mon Sep 17 00:00:00 2001 From: Sebastian Gruza Date: Thu, 1 Oct 2026 09:03:42 +0000 Subject: [PATCH 06/10] feat(server): readiness result and timestamp in one holder, a cap on waiting callers, an HBase probe - StorageReadiness keeps the last result with its timestamp in one immutable holder behind a single AtomicReference, so the TTL check and the returned body always come from the same probe. - readiness.max_waiters (default 16): callers beyond it get an immediate 503 instead of waiting on the in-flight probe, so a burst of probes during slow storage cannot hold the REST worker pool (the endpoint is unauthenticated and outside the load-shedding filter). - Graphs on hbase are probed too: HbaseStore answers storage_readiness with one admin round trip (does the graph's first table exist) within the budget, on its own executor; the body's storage field names the probed backend. - StorageReadinessTest 10: excess waiters rejected at once, storage name follows the backend. Co-Authored-By: Claude Opus 4.8 --- .../hugegraph/api/profile/ReadinessAPI.java | 10 +- .../api/profile/StorageReadiness.java | 102 ++++++++++++------ .../hugegraph/config/ServerOptions.java | 11 ++ .../backend/store/hbase/HbaseStore.java | 55 ++++++++++ .../unit/core/StorageReadinessTest.java | 55 ++++++++++ 5 files changed, 195 insertions(+), 38 deletions(-) diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/ReadinessAPI.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/ReadinessAPI.java index 3f5a985aff..44186db48e 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/ReadinessAPI.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/ReadinessAPI.java @@ -37,9 +37,10 @@ import jakarta.ws.rs.core.Response; /** - * Storage-aware readiness for Kubernetes and load balancers: 200 while at - * least one known Store answers this server, 503 while none does (or, before - * any Store list is known, while PD does not answer). Unauthenticated, like + * Storage-aware readiness for Kubernetes and load balancers: on hstore 200 + * while at least one known Store answers this server, 503 while none does + * (or, before any Store list is known, while PD does not answer); on hbase + * 200 while the cluster answers an admin call within the budget. Unauthenticated, like * /versions, so that an httpGet probe needs no credential; the body carries * no addresses and no raw exception text. */ @@ -55,7 +56,8 @@ public class ReadinessAPI extends API { public Response get(@Context GraphManager manager, @Context HugeConfig conf) { Map body = StorageReadiness.check( manager, conf.get(ServerOptions.READINESS_TIMEOUT), - conf.get(ServerOptions.READINESS_CACHE_TTL)); + conf.get(ServerOptions.READINESS_CACHE_TTL), + conf.get(ServerOptions.READINESS_MAX_WAITERS)); Response.Status status = StorageReadiness.isReady(body) ? Response.Status.OK : Response.Status.SERVICE_UNAVAILABLE; diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java index f04a139c8c..3e51563936 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java @@ -21,10 +21,12 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import org.apache.hugegraph.HugeGraph; @@ -33,12 +35,15 @@ import org.apache.hugegraph.util.Log; import org.slf4j.Logger; +import com.google.common.collect.ImmutableSet; + /** * Whether this server can serve graph traffic, for a readiness probe. * Graphs on an embedded backend are ready as soon as the REST layer answers. - * Graphs on hstore are probed through the backend's "storage_readiness" - * metadata: at least one known Store answers a cheap direct call; PD is only - * needed until the first Store list is known. The storage is shared by every + * Graphs on a remote backend are probed through the backend's + * "storage_readiness" metadata: on hstore at least one known Store answers a + * cheap direct call (PD is only needed until the first Store list is known), + * on hbase the cluster answers an admin call about one of the graph's tables. The storage is shared by every * hstore graph of the process, so one graph is probed and the result is * reused for a short TTL to keep repeated probes cheap. The body never * carries raw exception text, since the endpoint is unauthenticated. @@ -47,13 +52,29 @@ public final class StorageReadiness { public static final String STORAGE_READINESS_META = "storage_readiness"; public static final String BACKEND_HSTORE = "hstore"; + public static final String BACKEND_HBASE = "hbase"; + /** Backends on a remote cluster, whose availability the probe checks. */ + public static final Set REMOTE_BACKENDS = ImmutableSet.of(BACKEND_HSTORE, + BACKEND_HBASE); private static final Logger LOG = Log.logger(StorageReadiness.class); - private static volatile Map lastResult; - private static volatile long lastCheckedAt; + /** A probe result with the time it was taken: the TTL check and the body come from one probe. */ + private static final class Cached { + + private final Map result; + private final long at; + + private Cached(Map result, long at) { + this.result = result; + this.at = at; + } + } + + private static final AtomicReference LAST = new AtomicReference<>(); private static final AtomicReference>> IN_FLIGHT = new AtomicReference<>(); + private static final AtomicInteger WAITERS = new AtomicInteger(); private StorageReadiness() { } @@ -64,14 +85,14 @@ public interface Probe { Map probe(long timeoutMs) throws Exception; } - public static Map check(GraphManager manager, - long timeoutMs, long cacheTtlMs) { + public static Map check(GraphManager manager, long timeoutMs, + long cacheTtlMs, int maxWaiters) { // The graphs are auth proxies and the probe request carries no user, // so look the graph up and probe it as the internal admin, the way // other internal paths do; the result carries no data or addresses List> holder = new ArrayList<>(1); HugeGraphAuthProxy.runAsAdmin(() -> { - HugeGraph graph = firstHstoreGraph(manager); + HugeGraph graph = firstRemoteGraph(manager); if (graph == null) { Map body = new LinkedHashMap<>(); body.put("ready", true); @@ -80,25 +101,31 @@ public static Map check(GraphManager manager, holder.add(body); return; } - holder.add(check(t -> graph.metadata(null, STORAGE_READINESS_META, t), - timeoutMs, cacheTtlMs)); + holder.add(check(graph.backend(), + t -> graph.metadata(null, STORAGE_READINESS_META, t), + timeoutMs, cacheTtlMs, maxWaiters)); }); return holder.get(0); } + public static Map check(Probe probe, long timeoutMs, long cacheTtlMs) { + return check(BACKEND_HSTORE, probe, timeoutMs, cacheTtlMs, Integer.MAX_VALUE); + } + /** * At most one probe runs at a time and every caller gets its answer: * the first caller runs the probe on its own thread, concurrent callers - * wait for that result, each bounded by its own timeout. No monitor is - * held during the probe, so a slow store cannot queue REST workers - * behind it (relevant with readiness.cache_ttl=0, where every request - * probes). + * wait for that result, each bounded by its own timeout, and at most + * {@code maxWaiters} of them wait at once: the rest get an immediate + * not-ready, so a burst of probes during slow storage cannot hold the + * REST worker pool (the endpoint is unauthenticated and outside the + * load-shedding filter). No monitor is held during the probe. */ - public static Map check(Probe probe, long timeoutMs, long cacheTtlMs) { - long now = System.currentTimeMillis(); - Map cached = lastResult; - if (cached != null && now - lastCheckedAt < cacheTtlMs) { - Map body = new LinkedHashMap<>(cached); + public static Map check(String storage, Probe probe, long timeoutMs, + long cacheTtlMs, int maxWaiters) { + Cached last = LAST.get(); + if (last != null && System.currentTimeMillis() - last.at < cacheTtlMs) { + Map body = new LinkedHashMap<>(last.result); body.put("cached", true); return body; } @@ -107,9 +134,8 @@ public static Map check(Probe probe, long timeoutMs, long cacheT if (running == null && IN_FLIGHT.compareAndSet(null, mine)) { Map body; try { - body = probeOnce(probe, timeoutMs); - lastResult = body; - lastCheckedAt = System.currentTimeMillis(); + body = probeOnce(storage, probe, timeoutMs); + LAST.set(new Cached(body, System.currentTimeMillis())); } finally { IN_FLIGHT.set(null); } @@ -121,7 +147,12 @@ public static Map check(Probe probe, long timeoutMs, long cacheT } if (running == null) { // The owner finished between our two reads: its result is cached - return check(probe, timeoutMs, cacheTtlMs); + return check(storage, probe, timeoutMs, cacheTtlMs, maxWaiters); + } + if (WAITERS.incrementAndGet() > maxWaiters) { + WAITERS.decrementAndGet(); + return notReady(storage, "too many readiness callers waiting for the probe (" + + maxWaiters + ")"); } try { Map body = new LinkedHashMap<>( @@ -129,19 +160,22 @@ public static Map check(Probe probe, long timeoutMs, long cacheT body.put("shared", true); return body; } catch (TimeoutException e) { - return notReady("a probe is still running after " + timeoutMs + " ms"); + return notReady(storage, "a probe is still running after " + timeoutMs + " ms"); } catch (InterruptedException e) { Thread.currentThread().interrupt(); - return notReady("interrupted"); + return notReady(storage, "interrupted"); } catch (ExecutionException e) { - return notReady("probe failed: " + e.getCause().getClass().getSimpleName()); + return notReady(storage, "probe failed: " + + e.getCause().getClass().getSimpleName()); + } finally { + WAITERS.decrementAndGet(); } } - private static Map probeOnce(Probe probe, long timeoutMs) { + private static Map probeOnce(String storage, Probe probe, long timeoutMs) { Map body = new LinkedHashMap<>(); body.put("ready", false); - body.put("storage", BACKEND_HSTORE); + body.put("storage", storage); try { Map result = probe.probe(timeoutMs); body.putAll(result); @@ -154,10 +188,10 @@ private static Map probeOnce(Probe probe, long timeoutMs) { return body; } - private static Map notReady(String reason) { + private static Map notReady(String storage, String reason) { Map body = new LinkedHashMap<>(); body.put("ready", false); - body.put("storage", BACKEND_HSTORE); + body.put("storage", storage); body.put("reason", reason); body.put("cached", false); return body; @@ -168,16 +202,16 @@ public static boolean isReady(Map body) { } public static void resetCache() { - lastResult = null; - lastCheckedAt = 0L; + LAST.set(null); IN_FLIGHT.set(null); + WAITERS.set(0); } - private static HugeGraph firstHstoreGraph(GraphManager manager) { + private static HugeGraph firstRemoteGraph(GraphManager manager) { for (String name : manager.graphs()) { try { HugeGraph graph = manager.graph(name); - if (graph != null && BACKEND_HSTORE.equals(graph.backend())) { + if (graph != null && REMOTE_BACKENDS.contains(graph.backend())) { return graph; } } catch (Throwable e) { diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java index f73bdac2b5..a74463267d 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java @@ -226,6 +226,17 @@ public class ServerOptions extends OptionHolder { 2000 ); + public static final ConfigOption READINESS_MAX_WAITERS = + new ConfigOption<>( + "readiness.max_waiters", + "How many GET /readiness callers may wait at once for " + + "the probe already in flight; the rest get an immediate " + + "503, so a burst of probes during slow storage cannot " + + "hold the REST worker pool.", + rangeInt(1, 10000), + 16 + ); + public static final ConfigOption SERVER_USE_K8S = new ConfigOption<>( "server.use_k8s", diff --git a/hugegraph-server/hugegraph-hbase/src/main/java/org/apache/hugegraph/backend/store/hbase/HbaseStore.java b/hugegraph-server/hugegraph-hbase/src/main/java/org/apache/hugegraph/backend/store/hbase/HbaseStore.java index b539fc5427..42c2811d88 100644 --- a/hugegraph-server/hugegraph-hbase/src/main/java/org/apache/hugegraph/backend/store/hbase/HbaseStore.java +++ b/hugegraph-server/hugegraph-hbase/src/main/java/org/apache/hugegraph/backend/store/hbase/HbaseStore.java @@ -20,10 +20,15 @@ import java.io.IOException; import java.util.HashMap; import java.util.Iterator; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import java.util.function.BiFunction; import java.util.stream.Collectors; @@ -87,6 +92,56 @@ private void registerMetaHandlers() { HbaseMetrics metrics = new HbaseMetrics(this.sessions); return metrics.compact(this.tableNames()); }); + + this.registerMetaHandler(META_STORAGE_READINESS, (session, meta, args) -> { + E.checkArgument(args.length == 1 && args[0] instanceof Number, + "Expect the timeout in ms as the only argument"); + return this.storageReadiness(((Number) args[0]).longValue()); + }); + } + + public static final String META_STORAGE_READINESS = "storage_readiness"; + + private static final ExecutorService READINESS_EXECUTOR = Executors.newCachedThreadPool(r -> { + Thread thread = new Thread(r, "hbase-readiness"); + thread.setDaemon(true); + return thread; + }); + + /** + * Whether the HBase cluster answers this server: one admin round trip + * (does the graph's first table exist) within the budget. The body + * carries no addresses and no raw exception text, since the readiness + * endpoint is unauthenticated. + */ + private Map storageReadiness(long timeoutMs) { + E.checkArgument(timeoutMs > 0, "The probe timeout must be > 0, but got %s", timeoutMs); + Map body = new LinkedHashMap<>(); + body.put("ready", false); + List tables = this.tableNames(); + String table = tables.isEmpty() ? null : tables.get(0); + long start = System.currentTimeMillis(); + Future exists = READINESS_EXECUTOR.submit(() -> { + return table != null && this.sessions.existsTable(table); + }); + try { + boolean ok = exists.get(timeoutMs, TimeUnit.MILLISECONDS); + body.put("ready", ok); + body.put("reason", ok ? "ok" : "the graph's first table does not exist"); + } catch (TimeoutException e) { + exists.cancel(true); + body.put("reason", "hbase did not answer within " + timeoutMs + " ms"); + } catch (ExecutionException e) { + Throwable cause = e.getCause() != null ? e.getCause() : e; + LOG.warn("Storage readiness: the hbase admin call failed", cause); + body.put("reason", "hbase failed: " + cause.getClass().getSimpleName()); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + exists.cancel(true); + body.put("reason", "interrupted"); + } + body.put("hbase_millis", System.currentTimeMillis() - start); + return body; } protected void registerTableManager(HugeType type, HbaseTable table) { diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/StorageReadinessTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/StorageReadinessTest.java index 1f108fc349..bf411c7d86 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/StorageReadinessTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/StorageReadinessTest.java @@ -156,6 +156,61 @@ public void testConcurrentCallersShareOneProbe() throws Exception { } } + /** Beyond maxWaiters, callers get an immediate 503 instead of a worker-pool slot. */ + @Test + public void testExcessWaitersAreRejectedAtOnce() throws Exception { + CountDownLatch started = new CountDownLatch(1); + CountDownLatch release = new CountDownLatch(1); + StorageReadiness.Probe probe = t -> { + started.countDown(); + release.await(5, TimeUnit.SECONDS); + return result(true, "ok"); + }; + ExecutorService pool = Executors.newFixedThreadPool(3); + try { + Future> owner = pool.submit(() -> { + return StorageReadiness.check("hstore", probe, 5000L, 0L, 1); + }); + Assert.assertTrue(started.await(2, TimeUnit.SECONDS)); + Future> waiter = pool.submit(() -> { + return StorageReadiness.check("hstore", probe, 5000L, 0L, 1); + }); + Thread.sleep(100L); + long start = System.currentTimeMillis(); + Map rejected = StorageReadiness.check("hstore", probe, 5000L, 0L, 1); + long took = System.currentTimeMillis() - start; + Assert.assertFalse(StorageReadiness.isReady(rejected)); + Assert.assertEquals("too many readiness callers waiting for the probe (1)", + rejected.get("reason")); + Assert.assertTrue("took " + took, took < 500L); + release.countDown(); + Assert.assertTrue(StorageReadiness.isReady(owner.get(2, TimeUnit.SECONDS))); + Map shared = waiter.get(2, TimeUnit.SECONDS); + Assert.assertTrue(StorageReadiness.isReady(shared)); + Assert.assertEquals(true, shared.get("shared")); + // the slot is free again + Assert.assertTrue(StorageReadiness.isReady( + StorageReadiness.check("hstore", t -> result(true, "ok"), 1000L, 0L, 1))); + } finally { + release.countDown(); + pool.shutdownNow(); + } + } + + /** The storage name in the body is the probed backend's. */ + @Test + public void testStorageNameFollowsTheBackend() { + Map body = StorageReadiness.check("hbase", t -> result(true, "ok"), + 1000L, 0L, 4); + Assert.assertEquals("hbase", body.get("storage")); + StorageReadiness.resetCache(); + body = StorageReadiness.check("hbase", t -> { + throw new IllegalStateException("x"); + }, 1000L, 0L, 4); + Assert.assertEquals("hbase", body.get("storage")); + Assert.assertFalse(StorageReadiness.isReady(body)); + } + /** A follower waits at most its own timeout for the running probe. */ @Test public void testFollowerWaitIsBounded() throws Exception { From 4925e766ec628d6be3908e6259c8d807fc5ee725 Mon Sep 17 00:00:00 2001 From: Sebastian Gruza Date: Fri, 2 Oct 2026 11:02:50 +0000 Subject: [PATCH 07/10] feat(server): readiness probes every remote backend configuration - probe one graph per distinct hstore (pd.peers) / hbase (hosts+namespace) configuration in parallel within the budget; ready = all, body lists probes - hbase: table enabled and available (not only existing), logic testable - hstore: channels of replaced stores are shut down only after two listings - ReadinessApiTest accepts the hbase CI job --- .../api/profile/StorageReadiness.java | 157 +++++++++++++++--- .../backend/store/hbase/HbaseSessions.java | 12 ++ .../backend/store/hbase/HbaseStore.java | 32 ++-- .../store/hstore/HstoreStorageProbe.java | 22 ++- .../store/hstore/HstoreStorageProbeTest.java | 17 +- .../hugegraph/api/ReadinessApiTest.java | 19 ++- .../apache/hugegraph/unit/UnitTestSuite.java | 2 + .../unit/core/HbaseReadinessTest.java | 72 ++++++++ .../unit/core/StorageReadinessTest.java | 44 ++++- 9 files changed, 328 insertions(+), 49 deletions(-) create mode 100644 hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/HbaseReadinessTest.java diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java index 3e51563936..b6db002f93 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java @@ -18,16 +18,20 @@ package org.apache.hugegraph.api.profile; import java.util.ArrayList; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; +import java.util.stream.Collectors; import org.apache.hugegraph.HugeGraph; import org.apache.hugegraph.auth.HugeGraphAuthProxy; @@ -75,6 +79,13 @@ private Cached(Map result, long at) { private static final AtomicReference>> IN_FLIGHT = new AtomicReference<>(); private static final AtomicInteger WAITERS = new AtomicInteger(); + /** Probes of independent backend configurations run side by side here. */ + private static final ExecutorService PROBES = + Executors.newCachedThreadPool(r -> { + Thread t = new Thread(r, "storage-readiness-probe"); + t.setDaemon(true); + return t; + }); private StorageReadiness() { } @@ -88,12 +99,12 @@ public interface Probe { public static Map check(GraphManager manager, long timeoutMs, long cacheTtlMs, int maxWaiters) { // The graphs are auth proxies and the probe request carries no user, - // so look the graph up and probe it as the internal admin, the way + // so look the graphs up and probe them as the internal admin, the way // other internal paths do; the result carries no data or addresses List> holder = new ArrayList<>(1); HugeGraphAuthProxy.runAsAdmin(() -> { - HugeGraph graph = firstRemoteGraph(manager); - if (graph == null) { + List remotes = remoteGraphs(manager); + if (remotes.isEmpty()) { Map body = new LinkedHashMap<>(); body.put("ready", true); body.put("storage", "embedded"); @@ -101,13 +112,134 @@ public static Map check(GraphManager manager, long timeoutMs, holder.add(body); return; } - holder.add(check(graph.backend(), - t -> graph.metadata(null, STORAGE_READINESS_META, t), - timeoutMs, cacheTtlMs, maxWaiters)); + String storage = remotes.stream().map(r -> r.backend).distinct() + .collect(Collectors.joining(",")); + holder.add(check(storage, t -> probeAll(remotes, t), timeoutMs, cacheTtlMs, maxWaiters)); }); return holder.get(0); } + /** One graph per distinct remote backend configuration, in graph order; public for the unit test. */ + public static final class RemoteGraph { + + final String name; + final String backend; + final Probe probe; + + public RemoteGraph(String name, String backend, Probe probe) { + this.name = name; + this.backend = backend; + this.probe = probe; + } + } + + /** + * Readiness covers every remote backend configuration this server + * uses: graphs that share a configuration (the same PD peers, the same + * HBase hosts and namespace) share one probe; independent clusters are + * probed side by side within the common budget, and ready means all of + * them are ready. + */ + static List remoteGraphs(GraphManager manager) { + List out = new ArrayList<>(); + Set seen = new HashSet<>(); + for (String name : manager.graphs()) { + try { + HugeGraph graph = manager.graph(name); + if (graph == null || !REMOTE_BACKENDS.contains(graph.backend())) { + continue; + } + String key = graph.backend() + "|" + configKey(graph); + if (!seen.add(key)) { + continue; + } + out.add(new RemoteGraph(name, graph.backend(), + t -> graph.metadata(null, STORAGE_READINESS_META, t))); + } catch (Throwable e) { + LOG.debug("Skip graph {} while looking for remote storages", name, e); + } + } + return out; + } + + private static String configKey(HugeGraph graph) { + try { + if (BACKEND_HSTORE.equals(graph.backend())) { + return String.valueOf(graph.configuration().getString("pd.peers")); + } + return graph.configuration().getString("hbase.hosts") + "/" + + graph.configuration().getString("hbase.namespace"); + } catch (Throwable e) { + return graph.name(); + } + } + + /** Every configuration's probe in parallel on the shared budget; the body merges them. */ + public static Map probeAll(List remotes, long timeoutMs) + throws Exception { + if (remotes.size() == 1) { + Map body = new LinkedHashMap<>(remotes.get(0).probe.probe(timeoutMs)); + body.put("probes", List.of(probeEntry(remotes.get(0), body))); + return body; + } + long deadline = System.currentTimeMillis() + timeoutMs; + List>> futures = new ArrayList<>(); + for (RemoteGraph r : remotes) { + futures.add(CompletableFuture.supplyAsync(() -> { + try { + return r.probe.probe(timeoutMs); + } catch (Exception e) { + Map failed = new LinkedHashMap<>(); + failed.put("ready", false); + failed.put("reason", "probe failed: " + e.getClass().getSimpleName()); + return failed; + } + }, PROBES)); + } + List> entries = new ArrayList<>(); + Map first = null; + boolean ready = true; + String reason = "ok"; + for (int i = 0; i < remotes.size(); i++) { + Map result; + try { + long remaining = Math.max(1L, deadline - System.currentTimeMillis()); + result = futures.get(i).get(remaining, TimeUnit.MILLISECONDS); + } catch (TimeoutException e) { + futures.get(i).cancel(true); + result = new LinkedHashMap<>(); + result.put("ready", false); + result.put("reason", "did not answer within " + timeoutMs + " ms"); + } + if (first == null) { + first = result; + } + Map entry = probeEntry(remotes.get(i), result); + entries.add(entry); + if (!isReady(result)) { + if (ready) { + reason = remotes.get(i).backend + " of graph " + remotes.get(i).name + ": " + + result.get("reason"); + } + ready = false; + } + } + Map body = new LinkedHashMap<>(first); + body.put("ready", ready); + body.put("reason", reason); + body.put("probes", entries); + return body; + } + + private static Map probeEntry(RemoteGraph r, Map result) { + Map entry = new LinkedHashMap<>(); + entry.put("graph", r.name); + entry.put("storage", r.backend); + entry.put("ready", isReady(result)); + entry.put("reason", result.get("reason")); + return entry; + } + public static Map check(Probe probe, long timeoutMs, long cacheTtlMs) { return check(BACKEND_HSTORE, probe, timeoutMs, cacheTtlMs, Integer.MAX_VALUE); } @@ -207,17 +339,4 @@ public static void resetCache() { WAITERS.set(0); } - private static HugeGraph firstRemoteGraph(GraphManager manager) { - for (String name : manager.graphs()) { - try { - HugeGraph graph = manager.graph(name); - if (graph != null && REMOTE_BACKENDS.contains(graph.backend())) { - return graph; - } - } catch (Throwable e) { - LOG.debug("Skip graph {} while looking for a remote storage", name, e); - } - } - return null; - } } diff --git a/hugegraph-server/hugegraph-hbase/src/main/java/org/apache/hugegraph/backend/store/hbase/HbaseSessions.java b/hugegraph-server/hugegraph-hbase/src/main/java/org/apache/hugegraph/backend/store/hbase/HbaseSessions.java index d27ce99591..d8cca194d9 100644 --- a/hugegraph-server/hugegraph-hbase/src/main/java/org/apache/hugegraph/backend/store/hbase/HbaseSessions.java +++ b/hugegraph-server/hugegraph-hbase/src/main/java/org/apache/hugegraph/backend/store/hbase/HbaseSessions.java @@ -209,6 +209,18 @@ public void dropNamespace() throws IOException { } } + /** + * Whether the table can serve requests: it exists, is enabled and all its + * regions are assigned (an existing but disabled table is not available). + */ + public boolean tableAvailable(String table) throws IOException { + TableName tableName = TableName.valueOf(this.namespace, table); + try (Admin admin = this.hbase.getAdmin()) { + return admin.tableExists(tableName) && admin.isTableEnabled(tableName) && + admin.isTableAvailable(tableName); + } + } + public boolean existsTable(String table) throws IOException { TableName tableName = TableName.valueOf(this.namespace, table); try (Admin admin = this.hbase.getAdmin()) { diff --git a/hugegraph-server/hugegraph-hbase/src/main/java/org/apache/hugegraph/backend/store/hbase/HbaseStore.java b/hugegraph-server/hugegraph-hbase/src/main/java/org/apache/hugegraph/backend/store/hbase/HbaseStore.java index 42c2811d88..de282e3278 100644 --- a/hugegraph-server/hugegraph-hbase/src/main/java/org/apache/hugegraph/backend/store/hbase/HbaseStore.java +++ b/hugegraph-server/hugegraph-hbase/src/main/java/org/apache/hugegraph/backend/store/hbase/HbaseStore.java @@ -23,6 +23,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -109,27 +110,32 @@ private void registerMetaHandlers() { }); /** - * Whether the HBase cluster answers this server: one admin round trip - * (does the graph's first table exist) within the budget. The body - * carries no addresses and no raw exception text, since the readiness - * endpoint is unauthenticated. + * Whether the HBase cluster can serve this graph: one admin round trip + * (is the graph's first table enabled and available) within the budget. + * The body carries no addresses and no raw exception text, since the + * readiness endpoint is unauthenticated. */ private Map storageReadiness(long timeoutMs) { + List tables = this.tableNames(); + String table = tables.isEmpty() ? null : tables.get(0); + return readinessOf(() -> table != null && this.sessions.tableAvailable(table), + timeoutMs, READINESS_EXECUTOR); + } + + /** The probe outcome of one bounded availability check; public for the unit test. */ + public static Map readinessOf(Callable available, long timeoutMs, + ExecutorService executor) { E.checkArgument(timeoutMs > 0, "The probe timeout must be > 0, but got %s", timeoutMs); Map body = new LinkedHashMap<>(); body.put("ready", false); - List tables = this.tableNames(); - String table = tables.isEmpty() ? null : tables.get(0); long start = System.currentTimeMillis(); - Future exists = READINESS_EXECUTOR.submit(() -> { - return table != null && this.sessions.existsTable(table); - }); + Future check = executor.submit(available); try { - boolean ok = exists.get(timeoutMs, TimeUnit.MILLISECONDS); + boolean ok = check.get(timeoutMs, TimeUnit.MILLISECONDS); body.put("ready", ok); - body.put("reason", ok ? "ok" : "the graph's first table does not exist"); + body.put("reason", ok ? "ok" : "the graph's first table is not available"); } catch (TimeoutException e) { - exists.cancel(true); + check.cancel(true); body.put("reason", "hbase did not answer within " + timeoutMs + " ms"); } catch (ExecutionException e) { Throwable cause = e.getCause() != null ? e.getCause() : e; @@ -137,7 +143,7 @@ private Map storageReadiness(long timeoutMs) { body.put("reason", "hbase failed: " + cause.getClass().getSimpleName()); } catch (InterruptedException e) { Thread.currentThread().interrupt(); - exists.cancel(true); + check.cancel(true); body.put("reason", "interrupted"); } body.put("hbase_millis", System.currentTimeMillis() - start); diff --git a/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbe.java b/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbe.java index 87defb43a5..5706c5ee4a 100644 --- a/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbe.java +++ b/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbe.java @@ -20,10 +20,10 @@ import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; -import java.util.Set; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionService; import java.util.concurrent.ConcurrentHashMap; @@ -84,6 +84,7 @@ public final class HstoreStorageProbe { private static final KnownStores KNOWN = new KnownStores(); private static final Map CHANNELS = new ConcurrentHashMap<>(); + private static final Set STALE_CHANNELS = ConcurrentHashMap.newKeySet(); private HstoreStorageProbe() { } @@ -199,19 +200,20 @@ public static Map probe(long timeoutMs) { } return probe(KNOWN, () -> { List stores = pd.getActiveStores(); - pruneChannels(CHANNELS, stores); + pruneChannels(CHANNELS, stores, STALE_CHANNELS); return stores; }, HstoreStorageProbe::pingScanState, timeoutMs, EXECUTOR); } /** - * Shut down the channels of addresses PD no longer lists (replaced - * Stores). An empty answer is ignored, the same rule KnownStores.update - * applies: the pings keep using the last known Stores, so their channels - * must stay open. + * Channels of stores that left the PD list are shut down only when they + * are absent from two consecutive listings: a probe that snapshotted the + * previous store list may still be pinging them (and a stale-list ping + * through a closed channel would be a false not-ready, cached for the + * TTL). `stale` remembers the addresses missing from the last listing. */ static void pruneChannels(Map channels, - List stores) { + List stores, Set stale) { if (stores == null || stores.isEmpty()) { return; } @@ -219,11 +221,17 @@ static void pruneChannels(Map channels, for (Metapb.Store store : stores) { live.add(store.getAddress()); } + stale.retainAll(channels.keySet()); channels.entrySet().removeIf(e -> { if (live.contains(e.getKey())) { + stale.remove(e.getKey()); return false; } + if (stale.add(e.getKey())) { + return false; // first listing without it: keep for the probes in flight + } e.getValue().shutdownNow(); + stale.remove(e.getKey()); return true; }); } diff --git a/hugegraph-server/hugegraph-hstore/src/test/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbeTest.java b/hugegraph-server/hugegraph-hstore/src/test/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbeTest.java index 939433f49f..b6d6343364 100644 --- a/hugegraph-server/hugegraph-hstore/src/test/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbeTest.java +++ b/hugegraph-server/hugegraph-hstore/src/test/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbeTest.java @@ -375,13 +375,24 @@ public void testChannelsOfReplacedStoresAreShutDown() { FakeChannel gone = new FakeChannel(); channels.put("10.0.0.1:8500", kept); channels.put("10.0.0.9:8500", gone); - HstoreStorageProbe.pruneChannels(channels, stores(1L, 2L)); + java.util.Set stale = new java.util.HashSet<>(); + // first listing without the store: kept, a probe may still use the old list + HstoreStorageProbe.pruneChannels(channels, stores(1L, 2L), stale); + Assert.assertEquals(2, channels.size()); + Assert.assertFalse(gone.shut); + Assert.assertTrue(stale.contains("10.0.0.9:8500")); + // the store comes back: forgotten + HstoreStorageProbe.pruneChannels(channels, stores(1L, 2L, 9L), stale); + Assert.assertFalse(stale.contains("10.0.0.9:8500")); + // gone twice in a row: shut down + HstoreStorageProbe.pruneChannels(channels, stores(1L, 2L), stale); + HstoreStorageProbe.pruneChannels(channels, stores(1L, 2L), stale); Assert.assertEquals(1, channels.size()); Assert.assertFalse(kept.shut); Assert.assertTrue(gone.shut); - HstoreStorageProbe.pruneChannels(channels, null); + HstoreStorageProbe.pruneChannels(channels, null, stale); Assert.assertEquals("a failed listing prunes nothing", 1, channels.size()); - HstoreStorageProbe.pruneChannels(channels, Collections.emptyList()); + HstoreStorageProbe.pruneChannels(channels, Collections.emptyList(), stale); Assert.assertEquals("an empty listing keeps the channels the pings still use", 1, channels.size()); Assert.assertFalse(kept.shut); diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/ReadinessApiTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/ReadinessApiTest.java index 636c1f7fea..d3937f4804 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/ReadinessApiTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/ReadinessApiTest.java @@ -29,7 +29,8 @@ /** * The readiness endpoint answers 200 on a healthy server whatever the * backend: "embedded" for the in-process backends of the API suite, "hstore" - * (with the storage fields) on the hstore job. + * (with the Store fields) on the hstore job, "hbase" (with hbase_millis) on + * the hbase job. */ public class ReadinessApiTest extends BaseApiTest { @@ -41,16 +42,24 @@ public void testReadyOnAHealthyServer() { String result = assertResponseStatus(200, r); Map body = JsonUtil.fromJson(result, Map.class); Assert.assertEquals(true, body.get("ready")); - Assert.assertTrue(String.valueOf(body.get("storage")), - "embedded".equals(body.get("storage")) || - "hstore".equals(body.get("storage"))); + String storage = String.valueOf(body.get("storage")); + Assert.assertTrue(storage, "embedded".equals(storage) || "hstore".equals(storage) || + "hbase".equals(storage)); Assert.assertNotNull(body.get("reason")); - if ("hstore".equals(body.get("storage"))) { + if ("hstore".equals(storage)) { Assert.assertEquals("ok", body.get("reason")); Assert.assertTrue(((Number) body.get("active_stores")).intValue() >= 1); Assert.assertNotNull(body.get("answered_store")); Assert.assertTrue(body.containsKey("cached")); } + if ("hbase".equals(storage)) { + Assert.assertEquals("ok", body.get("reason")); + Assert.assertTrue(((Number) body.get("hbase_millis")).longValue() >= 0L); + Assert.assertTrue(body.containsKey("cached")); + } + if (!"embedded".equals(storage)) { + Assert.assertNotNull("one entry per probed backend configuration", body.get("probes")); + } } /** diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java index 293dac67ce..f3f3eca673 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java @@ -63,6 +63,7 @@ import org.apache.hugegraph.unit.core.GraphSpaceInfoLocaleTest; import org.apache.hugegraph.unit.core.GraphManagerStoresWaitTest; import org.apache.hugegraph.unit.core.MetaManagerClusterTest; +import org.apache.hugegraph.unit.core.HbaseReadinessTest; import org.apache.hugegraph.unit.core.StorageReadinessTest; import org.apache.hugegraph.unit.core.DirectionsTest; import org.apache.hugegraph.unit.core.ExceptionTest; @@ -167,6 +168,7 @@ GraphManagerStoresWaitTest.class, MetaManagerClusterTest.class, StorageReadinessTest.class, + HbaseReadinessTest.class, DirectionsTest.class, SerialEnumTest.class, diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/HbaseReadinessTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/HbaseReadinessTest.java new file mode 100644 index 0000000000..205f5a7147 --- /dev/null +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/HbaseReadinessTest.java @@ -0,0 +1,72 @@ +/* + * 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.unit.core; + +import java.util.Map; +import java.util.concurrent.Callable; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; + +import org.apache.hugegraph.backend.store.hbase.HbaseStore; +import org.apache.hugegraph.testutil.Assert; +import org.junit.AfterClass; +import org.junit.Test; + +/** The HBase probe outcome for an available, a disabled, a failing and a hung table check. */ +public class HbaseReadinessTest { + + private static final ExecutorService EXECUTOR = Executors.newCachedThreadPool(); + + @AfterClass + public static void shutdown() { + EXECUTOR.shutdownNow(); + } + + private static Map readiness(Callable check, long timeout) { + return HbaseStore.readinessOf(check, timeout, EXECUTOR); + } + + @Test + public void testOutcomes() { + Map ok = readiness(() -> true, 500L); + Assert.assertEquals(true, ok.get("ready")); + Assert.assertEquals("ok", ok.get("reason")); + Assert.assertTrue(((Number) ok.get("hbase_millis")).longValue() >= 0L); + + // an existing but disabled table is not available + Map disabled = readiness(() -> false, 500L); + Assert.assertEquals(false, disabled.get("ready")); + Assert.assertEquals("the graph's first table is not available", disabled.get("reason")); + + Map failed = readiness(() -> { + throw new java.io.IOException("Unable to resolve host hbase-master.svc"); + }, 500L); + Assert.assertEquals(false, failed.get("ready")); + Assert.assertEquals("hbase failed: IOException", failed.get("reason")); + Assert.assertFalse(failed.toString().contains("svc")); + + long start = System.currentTimeMillis(); + Map hung = readiness(() -> { + Thread.sleep(5_000L); + return true; + }, 300L); + Assert.assertEquals(false, hung.get("ready")); + Assert.assertEquals("hbase did not answer within 300 ms", hung.get("reason")); + Assert.assertTrue(System.currentTimeMillis() - start < 2_000L); + } +} diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/StorageReadinessTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/StorageReadinessTest.java index bf411c7d86..c094ab13a5 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/StorageReadinessTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/StorageReadinessTest.java @@ -17,10 +17,10 @@ package org.apache.hugegraph.unit.core; -import java.util.LinkedHashMap; -import java.util.Map; import java.util.ArrayList; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -197,6 +197,46 @@ public void testExcessWaitersAreRejectedAtOnce() throws Exception { } } + /** Independent backend configurations are probed side by side; one failing makes the server not ready. */ + @Test + public void testEveryRemoteConfigurationIsProbed() throws Exception { + AtomicInteger healthy = new AtomicInteger(); + AtomicInteger failing = new AtomicInteger(); + List remotes = new ArrayList<>(); + remotes.add(new StorageReadiness.RemoteGraph("g1", "hbase", t -> { + healthy.incrementAndGet(); + return result(true, "ok"); + })); + remotes.add(new StorageReadiness.RemoteGraph("g2", "hbase", t -> { + failing.incrementAndGet(); + return result(false, "hbase failed: IOException"); + })); + Map body = StorageReadiness.probeAll(remotes, 1000L); + Assert.assertEquals(1, healthy.get()); + Assert.assertEquals(1, failing.get()); + Assert.assertFalse(StorageReadiness.isReady(body)); + Assert.assertEquals("hbase of graph g2: hbase failed: IOException", body.get("reason")); + List probes = (List) body.get("probes"); + Assert.assertEquals(2, probes.size()); + Assert.assertEquals(false, ((Map) probes.get(1)).get("ready")); + + // a hung configuration is bounded by the shared budget + remotes.add(new StorageReadiness.RemoteGraph("g3", "hstore", t -> { + Thread.sleep(5_000L); + return result(true, "ok"); + })); + long start = System.currentTimeMillis(); + body = StorageReadiness.probeAll(remotes, 300L); + Assert.assertTrue(System.currentTimeMillis() - start < 2_000L); + Assert.assertFalse(StorageReadiness.isReady(body)); + Assert.assertEquals(3, ((List) body.get("probes")).size()); + + // a single configuration keeps the plain body plus its one probe entry + body = StorageReadiness.probeAll(remotes.subList(0, 1), 1000L); + Assert.assertTrue(StorageReadiness.isReady(body)); + Assert.assertEquals(1, ((List) body.get("probes")).size()); + } + /** The storage name in the body is the probed backend's. */ @Test public void testStorageNameFollowsTheBackend() { From a7e1548a51d4f710c0f6eae56cf11ede3a91f43b Mon Sep 17 00:00:00 2001 From: Sebastian Gruza Date: Mon, 5 Oct 2026 10:09:25 +0000 Subject: [PATCH 08/10] feat(server): readiness probes carry the admin context, close the probe transaction, check every hbase table, hide graph names, restore the interrupt flag --- .../api/profile/StorageReadiness.java | 50 ++++++++++++++----- .../backend/store/hbase/HbaseStore.java | 38 +++++++++++--- .../store/hstore/HstoreStorageProbe.java | 4 ++ .../store/hstore/HstoreStorageProbeTest.java | 3 +- .../hugegraph/api/ReadinessApiTest.java | 1 + .../unit/core/HbaseReadinessTest.java | 29 ++++++++--- .../unit/core/StorageReadinessTest.java | 26 +++++++++- 7 files changed, 120 insertions(+), 31 deletions(-) diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java index b6db002f93..47ad8c6355 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java @@ -153,8 +153,7 @@ static List remoteGraphs(GraphManager manager) { if (!seen.add(key)) { continue; } - out.add(new RemoteGraph(name, graph.backend(), - t -> graph.metadata(null, STORAGE_READINESS_META, t))); + out.add(new RemoteGraph(name, graph.backend(), probeOf(graph))); } catch (Throwable e) { LOG.debug("Skip graph {} while looking for remote storages", name, e); } @@ -162,6 +161,24 @@ static List remoteGraphs(GraphManager manager) { return out; } + /** + * The probe of one graph. `metadata()` auto-opens the thread-local graph + * transaction (it goes through graphTransaction()), and a transaction + * left open on a request or probe thread makes the graph's close() fail + * its all-threads-closed check, so the probe closes it on the way out. + */ + public static Probe probeOf(HugeGraph graph) { + return t -> { + try { + return graph.metadata(null, STORAGE_READINESS_META, t); + } finally { + if (graph.tx().isOpen()) { + graph.tx().close(); + } + } + }; + } + private static String configKey(HugeGraph graph) { try { if (BACKEND_HSTORE.equals(graph.backend())) { @@ -186,14 +203,20 @@ public static Map probeAll(List remotes, long timeo List>> futures = new ArrayList<>(); for (RemoteGraph r : remotes) { futures.add(CompletableFuture.supplyAsync(() -> { - try { - return r.probe.probe(timeoutMs); - } catch (Exception e) { - Map failed = new LinkedHashMap<>(); - failed.put("ready", false); - failed.put("reason", "probe failed: " + e.getClass().getSimpleName()); - return failed; - } + // the auth context is a thread local of the request thread: the + // probe runs as the internal admin on its own thread as well + List> holder = new ArrayList<>(1); + HugeGraphAuthProxy.runAsAdmin(() -> { + try { + holder.add(r.probe.probe(timeoutMs)); + } catch (Exception e) { + Map failed = new LinkedHashMap<>(); + failed.put("ready", false); + failed.put("reason", "probe failed: " + e.getClass().getSimpleName()); + holder.add(failed); + } + }); + return holder.get(0); }, PROBES)); } List> entries = new ArrayList<>(); @@ -218,8 +241,9 @@ public static Map probeAll(List remotes, long timeo entries.add(entry); if (!isReady(result)) { if (ready) { - reason = remotes.get(i).backend + " of graph " + remotes.get(i).name + ": " + - result.get("reason"); + // no graph name: the endpoint is unauthenticated + reason = remotes.get(i).backend + " configuration " + (i + 1) + " of " + + remotes.size() + ": " + result.get("reason"); } ready = false; } @@ -231,9 +255,9 @@ public static Map probeAll(List remotes, long timeo return body; } + /** One probe in the public body: backend and outcome, no graph name (unauthenticated endpoint). */ private static Map probeEntry(RemoteGraph r, Map result) { Map entry = new LinkedHashMap<>(); - entry.put("graph", r.name); entry.put("storage", r.backend); entry.put("ready", isReady(result)); entry.put("reason", result.get("reason")); diff --git a/hugegraph-server/hugegraph-hbase/src/main/java/org/apache/hugegraph/backend/store/hbase/HbaseStore.java b/hugegraph-server/hugegraph-hbase/src/main/java/org/apache/hugegraph/backend/store/hbase/HbaseStore.java index de282e3278..02095d8b6f 100644 --- a/hugegraph-server/hugegraph-hbase/src/main/java/org/apache/hugegraph/backend/store/hbase/HbaseStore.java +++ b/hugegraph-server/hugegraph-hbase/src/main/java/org/apache/hugegraph/backend/store/hbase/HbaseStore.java @@ -110,30 +110,52 @@ private void registerMetaHandlers() { }); /** - * Whether the HBase cluster can serve this graph: one admin round trip - * (is the graph's first table enabled and available) within the budget. + * Whether the HBase cluster can serve this graph: admin round trips + * (is every table of the graph enabled and available) within the budget. * The body carries no addresses and no raw exception text, since the * readiness endpoint is unauthenticated. */ private Map storageReadiness(long timeoutMs) { List tables = this.tableNames(); - String table = tables.isEmpty() ? null : tables.get(0); - return readinessOf(() -> table != null && this.sessions.tableAvailable(table), + return readinessOf(() -> firstUnavailable(tables, this.sessions::tableAvailable), timeoutMs, READINESS_EXECUTOR); } + /** + * Every table of the graph (vertices, edges, indexes, counters) must be + * enabled and available: the number of the first one that is not, or 0. + * Checked one after another inside the probe's time budget. + */ + public static int firstUnavailable(List tables, TableCheck check) throws Exception { + if (tables.isEmpty()) { + return 1; + } + for (int i = 0; i < tables.size(); i++) { + if (!check.available(tables.get(i))) { + return i + 1; + } + } + return 0; + } + + @FunctionalInterface + public interface TableCheck { + boolean available(String table) throws Exception; + } + /** The probe outcome of one bounded availability check; public for the unit test. */ - public static Map readinessOf(Callable available, long timeoutMs, + public static Map readinessOf(Callable unavailable, long timeoutMs, ExecutorService executor) { E.checkArgument(timeoutMs > 0, "The probe timeout must be > 0, but got %s", timeoutMs); Map body = new LinkedHashMap<>(); body.put("ready", false); long start = System.currentTimeMillis(); - Future check = executor.submit(available); + Future check = executor.submit(unavailable); try { - boolean ok = check.get(timeoutMs, TimeUnit.MILLISECONDS); + int missing = check.get(timeoutMs, TimeUnit.MILLISECONDS); + boolean ok = missing == 0; body.put("ready", ok); - body.put("reason", ok ? "ok" : "the graph's first table is not available"); + body.put("reason", ok ? "ok" : "table " + missing + " of the graph is not available"); } catch (TimeoutException e) { check.cancel(true); body.put("reason", "hbase did not answer within " + timeoutMs + " ms"); diff --git a/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbe.java b/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbe.java index 5706c5ee4a..c32537913d 100644 --- a/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbe.java +++ b/hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbe.java @@ -265,6 +265,10 @@ public static Map probe(KnownStores known, StoreLister lister, } catch (TimeoutException e) { return result(false, "no store list known and pd did not answer within " + timeoutMs + " ms", 0, null, false, -1L, -1L, 0L); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + return result(false, "interrupted while waiting for the store list", + 0, null, false, -1L, -1L, 0L); } catch (Exception e) { LOG.warn("Storage readiness: no store list known and pd failed", e); return result(false, "no store list known and pd failed: " + category(e), diff --git a/hugegraph-server/hugegraph-hstore/src/test/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbeTest.java b/hugegraph-server/hugegraph-hstore/src/test/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbeTest.java index b6d6343364..46e3bf848c 100644 --- a/hugegraph-server/hugegraph-hstore/src/test/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbeTest.java +++ b/hugegraph-server/hugegraph-hstore/src/test/java/org/apache/hugegraph/backend/store/hstore/HstoreStorageProbeTest.java @@ -180,7 +180,8 @@ public void testLastPdOutcomeIsReportedWhileTheRefreshIsPending() throws Excepti HstoreStorageProbe.probe(known, () -> { throw new IllegalStateException("PD unreachable"); }, ANSWERS, BUDGET, EXECUTOR); - for (int i = 0; i < 50 && known.pdOk() == null; i++) { + // the seed reports PD ok; wait for the failed refresh to land + for (int i = 0; i < 100 && !Boolean.FALSE.equals(known.pdOk()); i++) { Thread.sleep(20L); } Assert.assertEquals(Boolean.FALSE, known.pdOk()); diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/ReadinessApiTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/ReadinessApiTest.java index d3937f4804..ea08c7451b 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/ReadinessApiTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/ReadinessApiTest.java @@ -59,6 +59,7 @@ public void testReadyOnAHealthyServer() { } if (!"embedded".equals(storage)) { Assert.assertNotNull("one entry per probed backend configuration", body.get("probes")); + Assert.assertFalse("no graph name on the unauthenticated endpoint", result.contains("\"graph\"")); } } diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/HbaseReadinessTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/HbaseReadinessTest.java index 205f5a7147..fc59de36ec 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/HbaseReadinessTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/HbaseReadinessTest.java @@ -17,6 +17,7 @@ package org.apache.hugegraph.unit.core; +import java.util.List; import java.util.Map; import java.util.concurrent.Callable; import java.util.concurrent.ExecutorService; @@ -27,7 +28,9 @@ import org.junit.AfterClass; import org.junit.Test; -/** The HBase probe outcome for an available, a disabled, a failing and a hung table check. */ +import com.google.common.collect.ImmutableList; + +/** The HBase probe outcome for available tables, one disabled table, a failing and a hung check. */ public class HbaseReadinessTest { private static final ExecutorService EXECUTOR = Executors.newCachedThreadPool(); @@ -37,21 +40,31 @@ public static void shutdown() { EXECUTOR.shutdownNow(); } - private static Map readiness(Callable check, long timeout) { + private static Map readiness(Callable check, long timeout) { return HbaseStore.readinessOf(check, timeout, EXECUTOR); } @Test - public void testOutcomes() { - Map ok = readiness(() -> true, 500L); + public void testOutcomes() throws Exception { + Map ok = readiness(() -> 0, 500L); Assert.assertEquals(true, ok.get("ready")); Assert.assertEquals("ok", ok.get("reason")); Assert.assertTrue(((Number) ok.get("hbase_millis")).longValue() >= 0L); - // an existing but disabled table is not available - Map disabled = readiness(() -> false, 500L); + // an existing but disabled table is not available, whichever table it is + List tables = ImmutableList.of("g_v", "g_oe", "g_ie", "g_si"); + Map disabled = readiness(() -> HbaseStore.firstUnavailable(tables, t -> !t.equals("g_ie")), + 500L); Assert.assertEquals(false, disabled.get("ready")); - Assert.assertEquals("the graph's first table is not available", disabled.get("reason")); + Assert.assertEquals("table 3 of the graph is not available", disabled.get("reason")); + Assert.assertEquals(0, HbaseStore.firstUnavailable(tables, t -> true)); + Assert.assertEquals(1, HbaseStore.firstUnavailable(ImmutableList.of(), t -> true)); + // a throwing check surfaces as a failure, not as ready + Assert.assertThrows(java.io.IOException.class, () -> { + HbaseStore.firstUnavailable(tables, t -> { + throw new java.io.IOException("x"); + }); + }); Map failed = readiness(() -> { throw new java.io.IOException("Unable to resolve host hbase-master.svc"); @@ -63,7 +76,7 @@ public void testOutcomes() { long start = System.currentTimeMillis(); Map hung = readiness(() -> { Thread.sleep(5_000L); - return true; + return 0; }, 300L); Assert.assertEquals(false, hung.get("ready")); Assert.assertEquals("hbase did not answer within 300 ms", hung.get("reason")); diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/StorageReadinessTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/StorageReadinessTest.java index c094ab13a5..7d6d5e9398 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/StorageReadinessTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/StorageReadinessTest.java @@ -215,10 +215,13 @@ public void testEveryRemoteConfigurationIsProbed() throws Exception { Assert.assertEquals(1, healthy.get()); Assert.assertEquals(1, failing.get()); Assert.assertFalse(StorageReadiness.isReady(body)); - Assert.assertEquals("hbase of graph g2: hbase failed: IOException", body.get("reason")); + Assert.assertEquals("hbase configuration 2 of 2: hbase failed: IOException", body.get("reason")); List probes = (List) body.get("probes"); Assert.assertEquals(2, probes.size()); Assert.assertEquals(false, ((Map) probes.get(1)).get("ready")); + // the unauthenticated body carries no graph names + Assert.assertFalse(body.toString().contains("g2")); + Assert.assertNull(((Map) probes.get(0)).get("graph")); // a hung configuration is bounded by the shared budget remotes.add(new StorageReadiness.RemoteGraph("g3", "hstore", t -> { @@ -237,6 +240,27 @@ public void testEveryRemoteConfigurationIsProbed() throws Exception { Assert.assertEquals(1, ((List) body.get("probes")).size()); } + /** Every async probe runs as the internal admin: the auth context is a thread local. */ + @Test + public void testAsyncProbesCarryTheAdminContext() throws Exception { + List remotes = new ArrayList<>(); + List users = java.util.Collections.synchronizedList(new ArrayList<>()); + for (int i = 0; i < 3; i++) { + remotes.add(new StorageReadiness.RemoteGraph("g" + i, "hstore", t -> { + org.apache.hugegraph.auth.HugeGraphAuthProxy.Context ctx = + org.apache.hugegraph.auth.HugeGraphAuthProxy.getContext(); + users.add(ctx == null ? "none" : ctx.user().username()); + return result(true, "ok"); + })); + } + Map body = StorageReadiness.probeAll(remotes, 1000L); + Assert.assertTrue(StorageReadiness.isReady(body)); + Assert.assertEquals(3, users.size()); + for (String u : users) { + Assert.assertEquals("admin", u); + } + } + /** The storage name in the body is the probed backend's. */ @Test public void testStorageNameFollowsTheBackend() { From 3932be60b6c459eaaf88524b81d7bc1c5e6c375d Mon Sep 17 00:00:00 2001 From: Sebastian Gruza Date: Sat, 10 Oct 2026 05:50:23 +0000 Subject: [PATCH 09/10] feat(server): readiness covers every hbase store of the graph, probes each hbase graph scope with its full connection settings, fails on graphs that did not load, documents the endpoint and its options --- hugegraph-server/README.md | 32 ++++++++++++ .../api/profile/StorageReadiness.java | 44 ++++++++++++---- .../apache/hugegraph/core/GraphManager.java | 9 ++++ .../store/AbstractBackendStoreProvider.java | 11 ++++ .../static/conf/rest-server.properties | 7 +++ .../backend/store/hbase/HbaseStore.java | 51 ++++++++++++++++--- .../unit/core/HbaseReadinessTest.java | 6 +++ .../unit/core/StorageReadinessTest.java | 39 ++++++++++++++ 8 files changed, 183 insertions(+), 16 deletions(-) diff --git a/hugegraph-server/README.md b/hugegraph-server/README.md index c83e4a305d..b9a0fecc80 100644 --- a/hugegraph-server/README.md +++ b/hugegraph-server/README.md @@ -33,6 +33,38 @@ Memory remains a test-only backend throughout. Historical backend users must ope maintain a compatible release; these implementations are not restored to the current source tree or distribution packages. +## Readiness endpoint + +`GET /readiness` answers `200` while this Server can serve graph traffic and `503` otherwise. It is +unauthenticated and needs no graphspace prefix, so a Kubernetes `httpGet` probe can call it as is. The +JSON body carries `ready`, `storage` (`embedded`, `hstore`, `hbase`, or a comma-separated list), a +`reason`, whether the answer was `cached`, and one `probes` entry per probed backend configuration +(backend, `ready`, `reason`); it carries no addresses and no graph names. + +What ready means: + +- `embedded`: no graph on a remote storage; ready once the REST layer is up. +- `hstore`: this Server knows a Store list and at least one Store answers a direct status ping + (`HgStoreState.getScanState`). A Store whose status answers while its raft or partition path is + broken is not detected. +- `hbase`: every table of the graph (schema store, graph store, system store) exists, is enabled and + is available. +- A configured graph that failed to load at startup makes the Server not ready whatever its backend + (`reason` says how many, `failed_graphs` carries the count). + +Graphs that share a backend configuration (the same `pd.peers`; for hbase every graph is its own +scope, since the table namespace is derived from the graph name) share one probe; independent +configurations are probed side by side within one time budget and the Server is ready only when all +of them are. + +Options in `rest-server.properties`: + +| option | default | meaning | +|---|---|---| +| `readiness.timeout` | `1000` | time budget of one probe in ms; a backend that does not answer within it reads as not ready | +| `readiness.cache_ttl` | `2000` | a probe result is reused for this many ms | +| `readiness.max_waiters` | `16` | callers that may wait for the probe in flight; beyond it a caller gets `503` at once | + ## Docker ### Standalone Mode diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java index 47ad8c6355..a3e4fbf4c8 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java @@ -104,6 +104,16 @@ public static Map check(GraphManager manager, long timeoutMs, List> holder = new ArrayList<>(1); HugeGraphAuthProxy.runAsAdmin(() -> { List remotes = remoteGraphs(manager); + String storage = remotes.isEmpty() ? "embedded" : remotes.stream().map(r -> r.backend).distinct() + .collect(Collectors.joining(",")); + // a configured graph that did not load is not served, whatever its + // backend: GraphManager logs and skips it, so it is absent from the + // graphs looked at above and would otherwise read as "embedded" + int failed = manager.failedGraphs().size(); + if (failed > 0) { + holder.add(failedBody(failed, storage)); + return; + } if (remotes.isEmpty()) { Map body = new LinkedHashMap<>(); body.put("ready", true); @@ -112,13 +122,21 @@ public static Map check(GraphManager manager, long timeoutMs, holder.add(body); return; } - String storage = remotes.stream().map(r -> r.backend).distinct() - .collect(Collectors.joining(",")); holder.add(check(storage, t -> probeAll(remotes, t), timeoutMs, cacheTtlMs, maxWaiters)); }); return holder.get(0); } + /** Not ready: `failed` configured graphs could not be loaded (no names, the endpoint is public). */ + public static Map failedBody(int failed, String storage) { + Map body = new LinkedHashMap<>(); + body.put("ready", false); + body.put("storage", storage); + body.put("reason", failed + " configured graph(s) failed to load"); + body.put("failed_graphs", failed); + return body; + } + /** One graph per distinct remote backend configuration, in graph order; public for the unit test. */ public static final class RemoteGraph { @@ -149,7 +167,7 @@ static List remoteGraphs(GraphManager manager) { if (graph == null || !REMOTE_BACKENDS.contains(graph.backend())) { continue; } - String key = graph.backend() + "|" + configKey(graph); + String key = configKey(graph.backend(), graph.configuration(), graph.name()); if (!seen.add(key)) { continue; } @@ -179,15 +197,23 @@ public static Probe probeOf(HugeGraph graph) { }; } - private static String configKey(HugeGraph graph) { + /** + * What makes two graphs share one probe. hstore: the PD cluster, since + * the probe pings the Stores PD lists. hbase: nothing, every graph is its + * own scope, because the HBase provider derives the table namespace from + * the graph name; the key still carries the full connection settings + * (hosts, port, znode parent) so the body's grouping is honest. + */ + public static String configKey(String backend, org.apache.commons.configuration2.Configuration conf, + String graphName) { try { - if (BACKEND_HSTORE.equals(graph.backend())) { - return String.valueOf(graph.configuration().getString("pd.peers")); + if (BACKEND_HSTORE.equals(backend)) { + return backend + "|" + conf.getString("pd.peers"); } - return graph.configuration().getString("hbase.hosts") + "/" + - graph.configuration().getString("hbase.namespace"); + return backend + "|" + conf.getString("hbase.hosts") + ":" + conf.getString("hbase.port") + + conf.getString("hbase.znode_parent") + "|" + graphName; } catch (Throwable e) { - return graph.name(); + return backend + "|" + graphName; } } diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java index 1a8c1b5d1e..acfd7aa062 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java @@ -166,6 +166,8 @@ public final class GraphManager { private final Map services; //FIXME: add one class like graphKey as key,which contains graphSpace and graphName private final Map graphs; + /** configured graphs whose load failed (name -> exception class), for the readiness gate */ + private final Map failedGraphs = new java.util.concurrent.ConcurrentHashMap<>(); private final Set localGraphs; private final Set removingGraphs; private final Set creatingGraphs; @@ -1267,7 +1269,9 @@ public void loadGraphsFromLocal(Map graphConfs) { HugeFactory.checkGraphName(name, "rest-server.properties"); try { this.loadGraph(name, graphConfPath); + this.failedGraphs.remove(name); } catch (Throwable e) { + this.failedGraphs.put(name, e.getClass().getSimpleName()); LOG.error("Graph '{}' can't be loaded: '{}'", name, graphConfPath, e); } @@ -1652,6 +1656,11 @@ public Set graphs() { return Collections.unmodifiableSet(this.graphs.keySet()); } + /** The configured graphs whose last load failed: name -> exception class. */ + public Map failedGraphs() { + return Collections.unmodifiableMap(this.failedGraphs); + } + public HugeGraph graph(String spaceGraphName) { Graph graph = this.graphs.get(spaceGraphName); if (graph == null) { diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/store/AbstractBackendStoreProvider.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/store/AbstractBackendStoreProvider.java index 2ec8662375..897b67c011 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/store/AbstractBackendStoreProvider.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/backend/store/AbstractBackendStoreProvider.java @@ -86,6 +86,17 @@ public String graph() { return this.graph; } + /** + * The stores opened so far (schema, graph, system), for a probe that has + * to cover every table of the graph, whichever store owns it. + */ + public java.util.Collection openedStores() { + if (this.stores == null) { + return java.util.Collections.emptyList(); + } + return new java.util.ArrayList<>(this.stores.values()); + } + @Override public void open(String graph) { LOG.debug("Graph '{}' open StoreProvider", this.graph); diff --git a/hugegraph-server/hugegraph-dist/src/assembly/static/conf/rest-server.properties b/hugegraph-server/hugegraph-dist/src/assembly/static/conf/rest-server.properties index daeb092b77..2446fda036 100644 --- a/hugegraph-server/hugegraph-dist/src/assembly/static/conf/rest-server.properties +++ b/hugegraph-server/hugegraph-dist/src/assembly/static/conf/rest-server.properties @@ -26,6 +26,13 @@ arthas.disabled_commands=jad # use pd # usePD=true +# storage-aware readiness: GET /readiness (unauthenticated) answers 200 while this server can +# serve graph traffic and 503 otherwise; the probe result is cached for readiness.cache_ttl ms, +# one probe runs at a time and at most readiness.max_waiters callers wait for it (the rest get 503) +#readiness.timeout=1000 +#readiness.cache_ttl=2000 +#readiness.max_waiters=16 + # slow query log log.slow_query_threshold=1000 # bytes of request body recorded as-is (may contain sensitive literals), 0 to disable diff --git a/hugegraph-server/hugegraph-hbase/src/main/java/org/apache/hugegraph/backend/store/hbase/HbaseStore.java b/hugegraph-server/hugegraph-hbase/src/main/java/org/apache/hugegraph/backend/store/hbase/HbaseStore.java index 02095d8b6f..f41000faca 100644 --- a/hugegraph-server/hugegraph-hbase/src/main/java/org/apache/hugegraph/backend/store/hbase/HbaseStore.java +++ b/hugegraph-server/hugegraph-hbase/src/main/java/org/apache/hugegraph/backend/store/hbase/HbaseStore.java @@ -21,6 +21,7 @@ import java.util.HashMap; import java.util.Iterator; import java.util.LinkedHashMap; +import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.concurrent.Callable; @@ -44,6 +45,8 @@ import org.apache.hugegraph.backend.store.BackendEntry; import org.apache.hugegraph.backend.store.BackendFeatures; import org.apache.hugegraph.backend.store.BackendMutation; +import org.apache.hugegraph.backend.store.AbstractBackendStoreProvider; +import org.apache.hugegraph.backend.store.BackendStore; import org.apache.hugegraph.backend.store.BackendStoreProvider; import org.apache.hugegraph.backend.store.BackendTable; import org.apache.hugegraph.config.HugeConfig; @@ -111,20 +114,54 @@ private void registerMetaHandlers() { /** * Whether the HBase cluster can serve this graph: admin round trips - * (is every table of the graph enabled and available) within the budget. - * The body carries no addresses and no raw exception text, since the - * readiness endpoint is unauthenticated. + * (is every table of every store of the graph enabled and available) + * within the budget. The probe is dispatched to the graph store, so it + * collects the tables of the schema store (labels, property keys, + * counters) and of the system store from the provider as well; every + * store shares the graph's namespace, so this store's sessions can check + * them all. The body carries no addresses and no raw exception text, + * since the readiness endpoint is unauthenticated. */ private Map storageReadiness(long timeoutMs) { - List tables = this.tableNames(); + List tables = graphTables(this); return readinessOf(() -> firstUnavailable(tables, this.sessions::tableAvailable), timeoutMs, READINESS_EXECUTOR); } /** - * Every table of the graph (vertices, edges, indexes, counters) must be - * enabled and available: the number of the first one that is not, or 0. - * Checked one after another inside the probe's time budget. + * The tables of every opened store of the graph, schema store first (so + * a disabled schema table is reported with the lowest number), without + * duplicates; the store's own tables when the provider has none opened. + */ + static List graphTables(HbaseStore self) { + List stores = new ArrayList<>(); + if (self.provider instanceof AbstractBackendStoreProvider) { + stores.addAll(((AbstractBackendStoreProvider) self.provider).openedStores()); + } + List ordered = new ArrayList<>(); + for (BackendStore store : stores) { + if (store instanceof HbaseSchemaStore) { + ordered.add(0, (HbaseStore) store); + } else if (store instanceof HbaseStore) { + ordered.add((HbaseStore) store); + } + } + if (ordered.isEmpty()) { + ordered.add(self); + } + java.util.LinkedHashSet tables = new java.util.LinkedHashSet<>(); + for (HbaseStore store : ordered) { + tables.addAll(store.tableNames()); + } + return new ArrayList<>(tables); + } + + /** + * Every table of the graph (labels, property keys and counters of the + * schema store; vertices, edges and indexes of the graph store; the + * system store's meta) must be enabled and available: the number of the + * first one that is not, or 0. Checked one after another inside the + * probe's time budget. */ public static int firstUnavailable(List tables, TableCheck check) throws Exception { if (tables.isEmpty()) { diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/HbaseReadinessTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/HbaseReadinessTest.java index fc59de36ec..04b118b57d 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/HbaseReadinessTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/HbaseReadinessTest.java @@ -58,6 +58,12 @@ public void testOutcomes() throws Exception { Assert.assertEquals(false, disabled.get("ready")); Assert.assertEquals("table 3 of the graph is not available", disabled.get("reason")); Assert.assertEquals(0, HbaseStore.firstUnavailable(tables, t -> true)); + // the schema store's tables and its counters come first: a disabled counters table alone + // is not ready, with its own number + List all = ImmutableList.of("s_pk", "s_vl", "s_el", "s_il", "c", "g_v", "g_oe", "g_ie", "g_si", "m"); + Assert.assertEquals(5, HbaseStore.firstUnavailable(all, t -> !t.equals("c"))); + Assert.assertEquals(1, HbaseStore.firstUnavailable(all, t -> !t.equals("s_pk"))); + Assert.assertEquals(0, HbaseStore.firstUnavailable(all, t -> true)); Assert.assertEquals(1, HbaseStore.firstUnavailable(ImmutableList.of(), t -> true)); // a throwing check surfaces as a failure, not as ready Assert.assertThrows(java.io.IOException.class, () -> { diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/StorageReadinessTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/StorageReadinessTest.java index 7d6d5e9398..ae1066a1a4 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/StorageReadinessTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/StorageReadinessTest.java @@ -28,6 +28,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; +import org.apache.commons.configuration2.PropertiesConfiguration; import org.apache.hugegraph.api.filter.AuthenticationFilter; import org.apache.hugegraph.api.filter.PathFilter; import org.apache.hugegraph.api.profile.StorageReadiness; @@ -240,6 +241,44 @@ public void testEveryRemoteConfigurationIsProbed() throws Exception { Assert.assertEquals(1, ((List) body.get("probes")).size()); } + private static PropertiesConfiguration conf(Object... kv) { + PropertiesConfiguration c = new PropertiesConfiguration(); + for (int i = 0; i + 1 < kv.length; i += 2) { + c.setProperty((String) kv[i], kv[i + 1]); + } + return c; + } + + /** hbase graphs never share a probe (the namespace is per graph); hstore graphs share the PD cluster. */ + @Test + public void testConfigKeys() { + PropertiesConfiguration a = conf("hbase.hosts", "zk1,zk2", "hbase.port", 2181, "hbase.znode_parent", "/hbase"); + PropertiesConfiguration b = conf("hbase.hosts", "zk1,zk2", "hbase.port", 2181, + "hbase.znode_parent", "/hbase-2"); + Assert.assertNotEquals(StorageReadiness.configKey("hbase", a, "g1"), + StorageReadiness.configKey("hbase", a, "g2")); + Assert.assertNotEquals(StorageReadiness.configKey("hbase", a, "g1"), + StorageReadiness.configKey("hbase", b, "g1")); + Assert.assertEquals(StorageReadiness.configKey("hbase", a, "g1"), + StorageReadiness.configKey("hbase", a, "g1")); + PropertiesConfiguration p1 = conf("pd.peers", "pd:8686"); + PropertiesConfiguration p2 = conf("pd.peers", "pd2:8686"); + Assert.assertEquals(StorageReadiness.configKey("hstore", p1, "g1"), + StorageReadiness.configKey("hstore", p1, "g2")); + Assert.assertNotEquals(StorageReadiness.configKey("hstore", p1, "g1"), + StorageReadiness.configKey("hstore", p2, "g1")); + } + + /** A configured graph that failed to load makes the server not ready, remote or not. */ + @Test + public void testFailedGraphLoadIsNotReady() { + Map body = StorageReadiness.failedBody(2, "embedded"); + Assert.assertFalse(StorageReadiness.isReady(body)); + Assert.assertEquals("2 configured graph(s) failed to load", body.get("reason")); + Assert.assertEquals(2, body.get("failed_graphs")); + Assert.assertFalse(body.toString().contains("hugegraph")); + } + /** Every async probe runs as the internal admin: the auth context is a thread local. */ @Test public void testAsyncProbesCarryTheAdminContext() throws Exception { From 7f15a583ef2dbcfa6789cbe7cb1ac88b0149b813 Mon Sep 17 00:00:00 2001 From: Sebastian Gruza Date: Sun, 11 Oct 2026 20:13:26 +0000 Subject: [PATCH 10/10] feat(server): a graph created through the API clears its failed startup load, the failed-load body names the failed backends, Javadocs match the README --- hugegraph-server/README.md | 4 +- .../hugegraph/api/profile/ReadinessAPI.java | 14 ++--- .../api/profile/StorageReadiness.java | 54 +++++++++++++------ .../apache/hugegraph/core/GraphManager.java | 17 ++++-- .../unit/core/StorageReadinessTest.java | 13 ++++- 5 files changed, 75 insertions(+), 27 deletions(-) diff --git a/hugegraph-server/README.md b/hugegraph-server/README.md index b9a0fecc80..90371a779b 100644 --- a/hugegraph-server/README.md +++ b/hugegraph-server/README.md @@ -50,7 +50,9 @@ What ready means: - `hbase`: every table of the graph (schema store, graph store, system store) exists, is enabled and is available. - A configured graph that failed to load at startup makes the Server not ready whatever its backend - (`reason` says how many, `failed_graphs` carries the count). + (`reason` says how many, `failed_graphs` carries the count, `failed_backends` the backends of the + failed configs, and `storage` includes them). The gate holds until the Server restarts or a graph of + that name is created through the graphs API (which replaces the failed load). Graphs that share a backend configuration (the same `pd.peers`; for hbase every graph is its own scope, since the table namespace is derived from the graph name) share one probe; independent diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/ReadinessAPI.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/ReadinessAPI.java index 44186db48e..ad0ac6d5c7 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/ReadinessAPI.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/ReadinessAPI.java @@ -37,12 +37,14 @@ import jakarta.ws.rs.core.Response; /** - * Storage-aware readiness for Kubernetes and load balancers: on hstore 200 - * while at least one known Store answers this server, 503 while none does - * (or, before any Store list is known, while PD does not answer); on hbase - * 200 while the cluster answers an admin call within the budget. Unauthenticated, like - * /versions, so that an httpGet probe needs no credential; the body carries - * no addresses and no raw exception text. + * Storage-aware readiness for Kubernetes and load balancers: 503 while a + * configured graph failed to load; on hstore 200 while at least one known + * Store answers this server, 503 while none does (or, before any Store list + * is known, while PD does not answer); on hbase 200 while every table of + * every store of the graph is enabled and available within the budget. + * Unauthenticated, like /versions, so that an httpGet probe needs no + * credential; the body carries no addresses, graph names or raw exception + * text. The contract and the options are described in the server README. */ @Path("readiness") @Singleton diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java index a3e4fbf4c8..5812849871 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java @@ -43,14 +43,19 @@ /** * Whether this server can serve graph traffic, for a readiness probe. - * Graphs on an embedded backend are ready as soon as the REST layer answers. - * Graphs on a remote backend are probed through the backend's - * "storage_readiness" metadata: on hstore at least one known Store answers a - * cheap direct call (PD is only needed until the first Store list is known), - * on hbase the cluster answers an admin call about one of the graph's tables. The storage is shared by every - * hstore graph of the process, so one graph is probed and the result is - * reused for a short TTL to keep repeated probes cheap. The body never - * carries raw exception text, since the endpoint is unauthenticated. + * A configured graph that failed to load makes the server not ready + * whatever its backend. Graphs on an embedded backend are ready as soon as + * the REST layer answers. Graphs on a remote backend are probed through the + * backend's "storage_readiness" metadata: on hstore at least one known Store + * answers a cheap direct call (PD is only needed until the first Store list + * is known), on hbase every table of every store of the graph (schema, + * graph, system) is enabled and available. Graphs that share a backend + * configuration (hstore: the same PD peers) share one probe; every hbase + * graph is its own scope, since its table namespace is derived from the + * graph name. Independent configurations are probed side by side within one + * budget, and the result is reused for a short TTL to keep repeated probes + * cheap. The body never carries raw exception text, addresses or graph + * names, since the endpoint is unauthenticated. */ public final class StorageReadiness { @@ -109,9 +114,9 @@ public static Map check(GraphManager manager, long timeoutMs, // a configured graph that did not load is not served, whatever its // backend: GraphManager logs and skips it, so it is absent from the // graphs looked at above and would otherwise read as "embedded" - int failed = manager.failedGraphs().size(); - if (failed > 0) { - holder.add(failedBody(failed, storage)); + Map failedGraphs = manager.failedGraphs(); + if (!failedGraphs.isEmpty()) { + holder.add(failedBody(failedGraphs.size(), storage, failedGraphs.values())); return; } if (remotes.isEmpty()) { @@ -127,13 +132,29 @@ public static Map check(GraphManager manager, long timeoutMs, return holder.get(0); } - /** Not ready: `failed` configured graphs could not be loaded (no names, the endpoint is public). */ - public static Map failedBody(int failed, String storage) { + /** + * Not ready: `failed` configured graphs could not be loaded (no names, + * the endpoint is public). `storage` lists the loaded graphs' backends + * plus the failed configs' backends ("unknown" when a config could not + * be read), so an all-remote failure does not read as "embedded". + */ + public static Map failedBody(int failed, String storage, + java.util.Collection failedBackends) { + java.util.LinkedHashSet backends = new java.util.LinkedHashSet<>(); + if (!"embedded".equals(storage)) { + backends.addAll(java.util.Arrays.asList(storage.split(","))); + } + for (String b : failedBackends) { + backends.add(b == null || b.isEmpty() ? "unknown" : b); + } Map body = new LinkedHashMap<>(); body.put("ready", false); - body.put("storage", storage); + body.put("storage", backends.isEmpty() ? "unknown" : String.join(",", backends)); body.put("reason", failed + " configured graph(s) failed to load"); body.put("failed_graphs", failed); + body.put("failed_backends", failedBackends.stream() + .map(b -> b == null || b.isEmpty() ? "unknown" : b) + .distinct().sorted().collect(Collectors.toList())); return body; } @@ -153,8 +174,9 @@ public RemoteGraph(String name, String backend, Probe probe) { /** * Readiness covers every remote backend configuration this server - * uses: graphs that share a configuration (the same PD peers, the same - * HBase hosts and namespace) share one probe; independent clusters are + * uses: hstore graphs with the same PD peers share one probe, every + * hbase graph is probed on its own (its namespace is derived from the + * graph name, see {@link #configKey}); independent configurations are * probed side by side within the common budget, and ready means all of * them are ready. */ diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java index acfd7aa062..dce367d982 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java @@ -166,7 +166,7 @@ public final class GraphManager { private final Map services; //FIXME: add one class like graphKey as key,which contains graphSpace and graphName private final Map graphs; - /** configured graphs whose load failed (name -> exception class), for the readiness gate */ + /** configured graphs whose load failed (name -> backend of the config), for the readiness gate */ private final Map failedGraphs = new java.util.concurrent.ConcurrentHashMap<>(); private final Set localGraphs; private final Set removingGraphs; @@ -1271,7 +1271,7 @@ public void loadGraphsFromLocal(Map graphConfs) { this.loadGraph(name, graphConfPath); this.failedGraphs.remove(name); } catch (Throwable e) { - this.failedGraphs.put(name, e.getClass().getSimpleName()); + this.failedGraphs.put(name, backendOf(graphConfPath)); LOG.error("Graph '{}' can't be loaded: '{}'", name, graphConfPath, e); } @@ -1457,6 +1457,8 @@ private HugeGraph createGraphLocal(HugeConfig config, String name) { // Let gremlin server and rest server add graph to context this.notifyEvent(Events.GRAPH_CREATE, graph); + // a graph created through the API replaces a failed startup load of that name + this.failedGraphs.remove(name); } catch (Throwable e) { LOG.error("Failed to create graph '{}' due to: {}", name, e.getMessage(), e); @@ -1656,11 +1658,20 @@ public Set graphs() { return Collections.unmodifiableSet(this.graphs.keySet()); } - /** The configured graphs whose last load failed: name -> exception class. */ + /** The configured graphs whose last load failed: name -> the backend of the config. */ public Map failedGraphs() { return Collections.unmodifiableMap(this.failedGraphs); } + /** The backend named in a graph config file, "unknown" when the file cannot be read. */ + private static String backendOf(String graphConfPath) { + try { + return new HugeConfig(graphConfPath).get(CoreOptions.BACKEND); + } catch (Throwable e) { + return "unknown"; + } + } + public HugeGraph graph(String spaceGraphName) { Graph graph = this.graphs.get(spaceGraphName); if (graph == null) { diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/StorageReadinessTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/StorageReadinessTest.java index ae1066a1a4..f518c94626 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/StorageReadinessTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/StorageReadinessTest.java @@ -36,6 +36,8 @@ import org.junit.After; import org.junit.Before; import org.junit.Test; + +import com.google.common.collect.ImmutableList; import org.mockito.Mockito; import jakarta.ws.rs.container.ContainerRequestContext; @@ -272,11 +274,20 @@ public void testConfigKeys() { /** A configured graph that failed to load makes the server not ready, remote or not. */ @Test public void testFailedGraphLoadIsNotReady() { - Map body = StorageReadiness.failedBody(2, "embedded"); + // every remote graph failed: the body names their backend, not "embedded" + Map body = StorageReadiness.failedBody(2, "embedded", ImmutableList.of("hstore", "hstore")); Assert.assertFalse(StorageReadiness.isReady(body)); Assert.assertEquals("2 configured graph(s) failed to load", body.get("reason")); Assert.assertEquals(2, body.get("failed_graphs")); + Assert.assertEquals("hstore", body.get("storage")); + Assert.assertEquals(ImmutableList.of("hstore"), body.get("failed_backends")); Assert.assertFalse(body.toString().contains("hugegraph")); + // a mixed failure keeps the loaded graphs' backend and adds the failed one + body = StorageReadiness.failedBody(1, "hbase", ImmutableList.of("hstore")); + Assert.assertEquals("hbase,hstore", body.get("storage")); + // a config that could not be read + body = StorageReadiness.failedBody(1, "embedded", ImmutableList.of("unknown")); + Assert.assertEquals("unknown", body.get("storage")); } /** Every async probe runs as the internal admin: the auth context is a thread local. */