SebastianGruza commented on code in PR #3220:
URL: https://github.com/apache/hugegraph/pull/3220#discussion_r4052207779
##########
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/StandardHugeGraph.java:
##########
@@ -263,9 +263,13 @@ public StandardHugeGraph(HugeConfig config) {
throw new HugeException(message);
}
- if (isHstore()) {
+ if (isHstore() && !MetaManager.instance().isReady()) {
Review Comment:
Done in 68b616a2. Added the `else` branch: when the MetaManager is already
bound and the graph sets `pd.cluster` explicitly (`config.containsKey(...)`) to
another value, a WARN names both clusters and the prefix the keys live under,
and the graph's value is ignored. I kept a WARN rather than a throw: with
`usePD=true` the server already enforced its own `cluster` through
`ensureCluster()`, and with `usePD=false` failing the whole process on the
second file in `conf/graphs` seemed worse than a clear log line. The
`PD_CLUSTER` description now says the value is process-wide: the server's
`cluster` or the first hstore graph opened wins, a later different value is
ignored with a warning. Checked on the lab with a second graph carrying
`pd.cluster=other`:
```
[WARN] o.a.h.StandardHugeGraph - Graph 'hugegraph2' sets pd.cluster='other'
but the meta cluster is already bound to 'hg-test' (keys under
HUGEGRAPH/hg-test/); the graph's value is ignored
```
##########
hugegraph-server/hugegraph-dist/src/main/java/org/apache/hugegraph/dist/HugeGraphServer.java:
##########
@@ -68,6 +69,15 @@ public HugeGraphServer(String gremlinServerConf, String
restServerConf)
ServiceConstant.SERVICE_NAME,
ServiceConstant.AUTHORITY);
+ // Bind the meta cluster name ('cluster' in rest-server.properties)
+ // before any graph is opened: prepare() below opens every graph
+ // in conf/graphs, and an hstore graph would otherwise connect the
+ // MetaManager first under its own 'pd.cluster' (default 'hg'),
+ // hiding the meta written under the configured cluster
+ if (restServerConfig.get(ServerOptions.USE_PD)) {
+ GraphManager.connectMetaManager(restServerConfig);
Review Comment:
Done in 68b616a2. After `connectMetaManager()` the server logs `Meta cluster
bound to 'hg-test' (keys under HUGEGRAPH/hg-test/)` at INFO, in the first lines
of the startup (line 5 of the log on the lab). I added a paragraph to
`hugegraph-store/docs/operations-guide.md`, step "Upgrade Server Nodes": 1.7.0
bound `cluster` (`hg-test`), master builds between #3008 and this PR bound the
literal `hg`, from this PR on `cluster` again; whoever sees an empty schema
after an upgrade compares the log line with the prefix their data lives under
and sets `cluster` in `rest-server.properties`. I did not find a dedicated
upgrade-notes file in the repo, so if there is a better place I'll move it. The
prefix migration tool stays a follow-up, as agreed.
##########
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/MetaManagerClusterTest.java:
##########
@@ -0,0 +1,144 @@
+/*
+ * 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.lang.reflect.Field;
+
+import org.apache.commons.configuration2.PropertiesConfiguration;
+import org.apache.hugegraph.config.CoreOptions;
+import org.apache.hugegraph.config.HugeConfig;
+import org.apache.hugegraph.config.ServerOptions;
+import org.apache.hugegraph.core.GraphManager;
+import org.apache.hugegraph.meta.MetaDriver;
+import org.apache.hugegraph.meta.MetaManager;
+import org.apache.hugegraph.testutil.Assert;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+/**
+ * The meta keys in PD are prefixed with the cluster name, and
+ * MetaManager.connect() binds that name once per process. A server that
+ * connects under the configured 'cluster' must not be preceded by a graph
+ * connecting under its own 'pd.cluster', otherwise every later lookup reads
+ * an empty tree (1.7.0 wrote the schema under 'hg-test', master looked
+ * under 'hg').
+ */
+public class MetaManagerClusterTest {
+
+ private Object originalDriver;
+ private Object originalCluster;
+
+ @Before
+ public void setup() throws Exception {
+ this.originalDriver = swapField("metaDriver", null);
+ this.originalCluster = swapField("cluster", null);
+ }
+
+ @After
+ public void teardown() throws Exception {
+ swapField("metaDriver", this.originalDriver);
Review Comment:
Done in 68b616a2, thanks, I had missed that. `setup()` now snapshots every
non-static instance field of `MetaManager` through reflection and `teardown()`
restores them by reference, so the ten sub-managers return to what they were
before the class, not just `metaDriver` and `cluster`. A new
`testTeardownRestoresTheSubManagers` checks it on `authMetaManager`: a
different reference after `connect()`, the original one after `teardown()`. 7/7.
--
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]
