imbajin commented on code in PR #3221: URL: https://github.com/apache/hugegraph/pull/3221#discussion_r4189074486
########## hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java: ########## @@ -0,0 +1,366 @@ +/* + * 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.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; +import org.apache.hugegraph.core.GraphManager; +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 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. + */ +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<String> REMOTE_BACKENDS = ImmutableSet.of(BACKEND_HSTORE, + BACKEND_HBASE); + + private static final Logger LOG = Log.logger(StorageReadiness.class); + + /** 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<String, Object> result; + private final long at; + + private Cached(Map<String, Object> result, long at) { + this.result = result; + this.at = at; + } + } + + private static final AtomicReference<Cached> LAST = new AtomicReference<>(); + private static final AtomicReference<CompletableFuture<Map<String, Object>>> 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() { + } + + /** One storage probe with a time budget in ms. */ + public interface Probe { + + Map<String, Object> probe(long timeoutMs) throws Exception; + } + + public static Map<String, Object> check(GraphManager manager, long timeoutMs, + long cacheTtlMs, int maxWaiters) { + // The graphs are auth proxies and the probe request carries no user, + // 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<Map<String, Object>> holder = new ArrayList<>(1); + HugeGraphAuthProxy.runAsAdmin(() -> { + List<RemoteGraph> remotes = remoteGraphs(manager); + if (remotes.isEmpty()) { + Map<String, Object> body = new LinkedHashMap<>(); + body.put("ready", true); Review Comment: ‼️ Blocking: yes. Summary: This returns ready/embedded whenever no loaded remote graph is found, but `GraphManager.loadGraphsFromLocal()` logs and skips failed local graph loads; a configured HBase/HStore graph can disappear from `manager.graphs()` and still yield 200 even though that graph cannot serve traffic. Please distinguish a genuinely empty/no-remote configuration from graph-load failures before marking ready. Evidence: `GraphManager.graphs()` exposes loaded keys only and `loadGraphsFromLocal()` catches `Throwable`. ########## hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/config/ServerOptions.java: ########## @@ -202,6 +202,41 @@ public class ServerOptions extends OptionHolder { 300 ); + public static final ConfigOption<Integer> READINESS_TIMEOUT = Review Comment: 🧹 Blocking: no. Summary: This adds the unauthenticated `/readiness` endpoint and three readiness options, but no operator documentation describes the endpoint contract or settings. Please document its status meaning and the timeout, cache TTL, and waiter options. Evidence: server `AGENTS.md` requires matching docs for user-visible features and configuration behavior; this PR changes source and tests only. ########## hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java: ########## @@ -0,0 +1,366 @@ +/* + * 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.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; +import org.apache.hugegraph.core.GraphManager; +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 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. + */ +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<String> REMOTE_BACKENDS = ImmutableSet.of(BACKEND_HSTORE, + BACKEND_HBASE); + + private static final Logger LOG = Log.logger(StorageReadiness.class); + + /** 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<String, Object> result; + private final long at; + + private Cached(Map<String, Object> result, long at) { + this.result = result; + this.at = at; + } + } + + private static final AtomicReference<Cached> LAST = new AtomicReference<>(); + private static final AtomicReference<CompletableFuture<Map<String, Object>>> 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() { + } + + /** One storage probe with a time budget in ms. */ + public interface Probe { + + Map<String, Object> probe(long timeoutMs) throws Exception; + } + + public static Map<String, Object> check(GraphManager manager, long timeoutMs, + long cacheTtlMs, int maxWaiters) { + // The graphs are auth proxies and the probe request carries no user, + // 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<Map<String, Object>> holder = new ArrayList<>(1); + HugeGraphAuthProxy.runAsAdmin(() -> { + List<RemoteGraph> remotes = remoteGraphs(manager); + if (remotes.isEmpty()) { + Map<String, Object> body = new LinkedHashMap<>(); + body.put("ready", true); + body.put("storage", "embedded"); + body.put("reason", "no graph on a remote storage"); + 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); + } + + /** 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<RemoteGraph> remoteGraphs(GraphManager manager) { + List<RemoteGraph> out = new ArrayList<>(); + Set<String> 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(), probeOf(graph))); + } catch (Throwable e) { + LOG.debug("Skip graph {} while looking for remote storages", name, e); + } + } + 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())) { + return String.valueOf(graph.configuration().getString("pd.peers")); + } + return graph.configuration().getString("hbase.hosts") + "/" + Review Comment: ‼️ Blocking: yes. Summary: The HBase key uses `hbase.namespace`, which the provider does not use because it derives table namespaces from graph names, and omits `hbase.port`/`hbase.znode_parent`; graphs on the same hosts or distinct ZooKeeper clusters can collapse to the first probe, leaving `/readiness` green when a skipped graph is down. Please key by the effective table namespace and full connection settings or probe every graph scope, with multi-graph coverage. Evidence: `HbaseStoreProvider.namespace()` derives the namespace from the graph name; `HbaseSessions.open()` uses hosts, port, and znode parent. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
