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


##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java:
##########
@@ -1267,7 +1269,9 @@ public void loadGraphsFromLocal(Map<String, String> 
graphConfs) {
             HugeFactory.checkGraphName(name, "rest-server.properties");
             try {
                 this.loadGraph(name, graphConfPath);
+                this.failedGraphs.remove(name);

Review Comment:
   Right. `createGraphLocal()` now removes the name from `failedGraphs` once 
the create succeeded, so a graph created through the API after a failed startup 
load clears the gate; the README says that otherwise the gate holds until the 
Server restarts.
   



##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java:
##########
@@ -0,0 +1,392 @@
+/*
+ * 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

Review Comment:
   Done: the class Javadoc of `StorageReadiness`, the `remoteGraphs` Javadoc 
and the `ReadinessAPI` Javadoc now describe what the code does (every table of 
every store on hbase, one scope per hbase graph, the failed-load gate, no names 
in the body) and point to the README section.
   



##########
hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/StorageReadiness.java:
##########
@@ -0,0 +1,392 @@
+/*
+ * 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);
+            String storage = remotes.isEmpty() ? "embedded" : 
remotes.stream().map(r -> r.backend).distinct()

Review Comment:
   Right. `GraphManager` records the backend named in the failed graph's config 
(`unknown` when the file cannot be read), and the failed-load body lists those 
backends in `failed_backends` and merges them into `storage`, so two failed 
hstore graphs read as `storage: "hstore"` and a mixed case as `"hbase,hstore"`; 
`embedded` is used only when nothing is remote and nothing failed. 
`testFailedGraphLoadIsNotReady` covers the all-remote, mixed and 
unreadable-config cases.
   



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