SebastianGruza commented on code in PR #3221:
URL: https://github.com/apache/hugegraph/pull/3221#discussion_r4236613451


##########
hugegraph-server/hugegraph-hbase/src/main/java/org/apache/hugegraph/backend/store/hbase/HbaseStore.java:
##########
@@ -87,6 +93,83 @@ 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 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<String, Object> storageReadiness(long timeoutMs) {
+        List<String> tables = this.tableNames();

Review Comment:
   Right, thanks for tracing the dispatch; I had read `tableNames()` as the 
graph's tables. The probe still runs on the graph store (that is where 
`metadata()` lands), but it now collects the tables of every opened store of 
the graph from the provider (`AbstractBackendStoreProvider.openedStores()`): 
the schema store first (labels, property keys, counters), then the graph store 
and the system store (meta). Every store shares the graph's namespace, so the 
graph store's sessions check them all. The `firstUnavailable` Javadoc now lists 
the actual tables, and the unit test has the case where only the counters 
table, or only a schema table, is unavailable (the number reported is theirs). 
The wiring itself (the stores a provider holds) has no unit test without an 
HBase cluster; it runs on the CI hbase job through `ReadinessApiTest`.
   



##########
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:
   Right. The HBase provider derives the namespace from the graph name, so two 
HBase graphs never share a scope: the key is now the backend, the full 
connection settings (`hbase.hosts`, `hbase.port`, `hbase.znode_parent`) and the 
graph name, which means every HBase graph is probed on its own, and the body's 
`probes` list has one entry per graph. hstore keeps `pd.peers` as the key, 
since the probe pings the Stores that PD lists. `configKey` is static and 
`testConfigKeys` covers: two hbase graphs on one cluster get different keys, 
the same graph on two ZooKeeper parents gets different keys, two hstore graphs 
on one PD share one.
   



##########
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:
   Right. `GraphManager` now records the configured graphs whose load failed 
(`failedGraphs()`, name to exception class, cleared when a later load of that 
graph succeeds), and `StorageReadiness.check()` answers not ready with "N 
configured graph(s) failed to load" and `failed_graphs: N` before looking at 
the remote backends, whatever the failed graph's backend was; no graph names in 
the body. `testFailedGraphLoadIsNotReady` covers the body; the 
empty-configuration case stays "embedded".
   



##########
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:
   Done: a "Readiness endpoint" section in `hugegraph-server/README.md` (status 
contract, body fields, what ready means per backend, the shared-configuration 
rule, the three options with defaults) and the three options as commented 
entries in `conf/rest-server.properties`.
   



-- 
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]

Reply via email to