diff --git a/helm/hugegraph/docs/operations.md b/helm/hugegraph/docs/operations.md index cb7f2e8048..3e9ecad7ad 100644 --- a/helm/hugegraph/docs/operations.md +++ b/helm/hugegraph/docs/operations.md @@ -293,7 +293,7 @@ Updating chart values alone does not change an existing image's behavior. | `HG_PD_AUTH_SECRET_KEY` missing | PD Pod Secret reference and key; chart injects the shared PD REST password automatically | | Server storage wait timeout | Ready PD majority, Store registration, network policies and `server.storageWaitTimeoutSeconds` | | NodePort/Ingress unreachable | Assigned port, reachable node/controller address, firewall and NetworkPolicy rules | -| REST works but Gremlin reports `Could not rebind` | Run `helm test`; after PD stabilizes, replace the failing Server Pod and retry | +| REST works but Gremlin reports `Could not rebind` | Run `helm test`. Server images built from source that includes apache/hugegraph#3351 (the fix for #3228) rebind Gremlin when REST loads the graph; if it persists, check the Server log for `could not be bound in Gremlin Server` or `can't be rebound in Gremlin Server`, then replace the failing Server Pod after PD stabilizes and retry | ```bash kubectl -n hugegraph get pods,pvc,svc 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 800ba6ebed..f24bf6ff1d 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 @@ -1811,13 +1811,6 @@ private String defaultSpaceGraphName(String graphName) { } private void loadGraph(String name, String graphConfPath) { - // TODO: offer a loaded graph to Gremlin Server (notify GRAPH_CREATE, as the create paths - // do) or fail startup when its static Gremlin instantiation failed. Today a Server whose - // first open failed against a PD member mid-restart serves REST and passes readiness - // while every Gremlin call on it fails for the life of the process. The Helm chart - // (helm/hugegraph) detects this with a per-Pod Gremlin query in `helm test` and documents - // the manual Pod deletion; retire both once this is fixed. - // https://github.com/apache/hugegraph/issues/3228 HugeConfig config = new HugeConfig(graphConfPath); // Transfer `raft.group_peers` from server config to graph config @@ -1831,11 +1824,15 @@ private void loadGraph(String name, String graphConfPath) { this.graphs.put(defaultSpaceGraphName(name), graph); HugeConfig graphConfig = (HugeConfig) graph.configuration(); - assert graphConfPath.equals(Objects.requireNonNull(graphConfig.file()).getPath()); + // A graph Gremlin Server opened first from this path is cached without a file + assert graphConfig.file() == null || + graphConfPath.equals(graphConfig.file().getPath()); LOG.info("Graph '{}' was successfully configured via '{}'", name, graphConfPath); + this.bindLocalGraph(name, (HugeGraph) graph); + if (this.requireAuthentication() && !(graph instanceof HugeGraphAuthProxy)) { LOG.warn("You may need to support access control for '{}' with {}", @@ -1843,6 +1840,31 @@ private void loadGraph(String name, String graphConfPath) { } } + /* + * Gremlin Server opens the graphs in conf/graphs once, while it is being + * prepared, and drops any graph whose open fails there (for example when + * PD is briefly unreachable). Offer each locally loaded graph to it again, + * as the create paths do, so such a graph still gets its Gremlin binding. + * Rebinding the graph Gremlin already holds is harmless: both opens return + * the same cached instance. + */ + private void bindLocalGraph(String name, HugeGraph graph) { + String graphName = defaultSpaceGraphName(name); + if (!graphName.equals(graph.spaceGraphName())) { + LOG.warn("Graph '{}' is named '{}' by its config, so it can't be " + + "rebound in Gremlin Server if Gremlin failed to open it", + graphName, graph.spaceGraphName()); + return; + } + try { + this.notifyEvent(Events.GRAPH_CREATE, graph); + } catch (Throwable e) { + LOG.error("Graph '{}' is served by REST but could not be bound " + + "in Gremlin Server, restart the Server to retry", + graphName, e); + } + } + private void transferPdPeersConfig(HugeConfig config) { if (config.containsKey(CoreOptions.PD_PEERS.name())) { return; 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 3bf95f910a..f400cb53bd 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 @@ -78,6 +78,7 @@ import org.apache.hugegraph.unit.core.ExceptionTest; import org.apache.hugegraph.unit.core.GraphManagerAdminInitTest; import org.apache.hugegraph.unit.core.GraphManagerConfigTest; +import org.apache.hugegraph.unit.core.GraphManagerLocalGraphBindTest; import org.apache.hugegraph.unit.core.GraphManagerServiceNamesTest; import org.apache.hugegraph.unit.core.GroovyScriptEngineCompatibilityTest; import org.apache.hugegraph.unit.core.HugeFeaturesTest; @@ -225,6 +226,7 @@ ExceptionTest.class, GraphManagerAdminInitTest.class, GraphManagerConfigTest.class, + GraphManagerLocalGraphBindTest.class, GraphManagerServiceNamesTest.class, HstoreSessionsTest.class, HstoreTableRoutingTest.class, diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerLocalGraphBindTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerLocalGraphBindTest.java new file mode 100644 index 0000000000..91a5ea2bed --- /dev/null +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerLocalGraphBindTest.java @@ -0,0 +1,207 @@ +/* + * 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.io.File; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.TimeUnit; + +import org.apache.commons.configuration2.PropertiesConfiguration; +import org.apache.commons.io.FileUtils; +import org.apache.hugegraph.HugeFactory; +import org.apache.hugegraph.HugeGraph; +import org.apache.hugegraph.auth.ContextGremlinServer; +import org.apache.hugegraph.config.HugeConfig; +import org.apache.hugegraph.core.GraphManager; +import org.apache.hugegraph.event.EventHub; +import org.apache.hugegraph.event.EventListener; +import org.apache.hugegraph.testutil.Assert; +import org.apache.hugegraph.util.Events; +import org.apache.tinkerpop.gremlin.server.Settings; +import org.apache.tinkerpop.gremlin.structure.Graph; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; + +/** + * Gremlin Server opens conf/graphs on its own, once, and drops a graph whose + * open fails there. The REST load of the same graph has to offer it to + * Gremlin again, or that Server answers REST while every Gremlin call on the + * graph fails until it restarts (#3228). + */ +public class GraphManagerLocalGraphBindTest { + + private File graphsDir; + private EventHub hub; + private GraphManager manager; + private ContextGremlinServer gremlin; + private List created; + private EventListener listener; + + @Before + public void setup() throws IOException { + this.graphsDir = Files.createTempDirectory("local-graph-bind").toFile(); + this.hub = new EventHub("local-graph-bind-test"); + this.manager = new GraphManager( + new HugeConfig(new PropertiesConfiguration()), this.hub); + this.created = new CopyOnWriteArrayList<>(); + this.listener = event -> { + event.checkArgs(HugeGraph.class); + this.created.add(((HugeGraph) event.args()[0]).spaceGraphName()); + return null; + }; + this.hub.listen(Events.GRAPH_CREATE, this.listener); + } + + @After + public void teardown() throws Exception { + List graphs = new ArrayList<>(); + for (String name : this.manager.graphs()) { + graphs.add(this.manager.graph(name)); + } + try { + if (this.gremlin != null) { + this.gremlin.stop().get(30, TimeUnit.SECONDS); + } + this.hub.unlisten(Events.GRAPH_CREATE, this.listener); + this.manager.close(); + } finally { + graphs.forEach(HugeFactory::remove); + FileUtils.deleteQuietly(this.graphsDir); + } + } + + @Test + public void testLocalLoadOffersGraphToGremlin() throws IOException { + String path = this.writeGraphConf("bind_local", "bind_local"); + + this.manager.loadGraphsFromLocal(ImmutableMap.of("bind_local", path)); + + Assert.assertEquals(1, this.created.size()); + Assert.assertEquals("DEFAULT-bind_local", this.created.get(0)); + Assert.assertNotNull(this.manager.graph("DEFAULT", "bind_local")); + } + + @Test + public void testLoadingTwiceRebindsTheSameGraph() throws IOException { + String path = this.writeGraphConf("bind_twice", "bind_twice"); + Map confs = ImmutableMap.of("bind_twice", path); + + this.manager.loadGraphsFromLocal(confs); + HugeGraph first = this.manager.graph("DEFAULT", "bind_twice"); + this.manager.loadGraphsFromLocal(confs); + + Assert.assertEquals(2, this.created.size()); + Assert.assertSame(first, this.manager.graph("DEFAULT", "bind_twice")); + } + + @Test + public void testGraphNamedDifferentlyByConfigIsNotRebound() throws IOException { + String path = this.writeGraphConf("bind_file", "bind_store"); + + this.manager.loadGraphsFromLocal(ImmutableMap.of("bind_file", path)); + + Assert.assertTrue(this.created.isEmpty()); + Assert.assertNotNull(this.manager.graph("DEFAULT", "bind_file")); + } + + @Test + public void testFailedBindKeepsGraphLoaded() throws IOException { + String path = this.writeGraphConf("bind_failed", "bind_failed"); + EventListener failing = event -> { + throw new IllegalStateException("gremlin binding failed"); + }; + this.hub.listen(Events.GRAPH_CREATE, failing); + + try { + this.manager.loadGraphsFromLocal(ImmutableMap.of("bind_failed", path)); + } finally { + this.hub.unlisten(Events.GRAPH_CREATE, failing); + } + + Assert.assertEquals(1, this.created.size()); + Assert.assertNotNull(this.manager.graph("DEFAULT", "bind_failed")); + } + + @Test + public void testGraphMissingFromGremlinGetsBound() throws IOException { + String path = this.writeGraphConf("bind_missing", "bind_missing"); + // No settings.graphs entry stands in for a failed static open + this.gremlin = new ContextGremlinServer(newSettings(), this.hub); + org.apache.tinkerpop.gremlin.server.GraphManager bindings = + this.gremlin.getServerGremlinExecutor().getGraphManager(); + Assert.assertNull(bindings.getGraph("DEFAULT-bind_missing")); + + this.manager.loadGraphsFromLocal(ImmutableMap.of("bind_missing", path)); + + Assert.assertSame(this.manager.graph("DEFAULT", "bind_missing"), + bindings.getGraph("DEFAULT-bind_missing")); + Assert.assertNotNull(bindings.getTraversalSource("__g_DEFAULT-bind_missing")); + } + + @Test + public void testGraphOpenedByGremlinKeepsItsInstance() throws IOException { + String path = this.writeGraphConf("bind_static", "bind_static"); + Settings settings = newSettings(); + settings.graphs.put("DEFAULT-bind_static", path); + this.gremlin = new ContextGremlinServer(settings, this.hub); + org.apache.tinkerpop.gremlin.server.GraphManager bindings = + this.gremlin.getServerGremlinExecutor().getGraphManager(); + Graph opened = bindings.getGraph("DEFAULT-bind_static"); + Assert.assertNotNull(opened); + + this.manager.loadGraphsFromLocal(ImmutableMap.of("bind_static", path)); + + // The rebind ran and handed Gremlin the instance it already held + Assert.assertEquals(ImmutableList.of("DEFAULT-bind_static"), this.created); + Assert.assertSame(opened, bindings.getGraph("DEFAULT-bind_static")); + Assert.assertSame(opened, this.manager.graph("DEFAULT", "bind_static")); + Assert.assertSame(opened, bindings.getTraversalSource("__g_DEFAULT-bind_static") + .getGraph()); + } + + private String writeGraphConf(String fileName, String store) + throws IOException { + File conf = new File(this.graphsDir, fileName + ".properties"); + String content = "gremlin.graph=org.apache.hugegraph.HugeFactory\n" + + "backend=memory\n" + + "serializer=text\n" + + "store=" + store + "\n"; + FileUtils.writeStringToFile(conf, content, StandardCharsets.UTF_8); + return conf.getPath(); + } + + private static Settings newSettings() { + Settings settings = new Settings(); + settings.host = "127.0.0.1"; + settings.port = 0; + settings.gremlinPool = 1; + settings.threadPoolBoss = 1; + settings.threadPoolWorker = 1; + return settings; + } +}