Skip to content

Commit a2906f9

Browse files
committed
fix(server): retain full service names
- preserve complete names when reloading service metadata - keep the service suffix when clearing a graph space - cover metadata write, reload, removal and space clearing - document the case of existing Gremlin graph bindings
1 parent dd10fda commit a2906f9

7 files changed

Lines changed: 179 additions & 7 deletions

File tree

‎docker/README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -283,6 +283,8 @@ spaces and backslashes in service IDs round-trip unchanged. Non-ASCII and contro
283283
characters are rejected before the entrypoint writes configuration, because the
284284
Server reads properties using ISO-8859-1. This is the container input's
285285
serialization limit; it does not change the core service-name contract.
286+
The full service ID, including `/` and `-`, is retained when service metadata is
287+
reloaded or its graph space is cleared.
286288
`HG_SERVER_GRAPHSPACE` and a migrated `GRAPH_SPACE` must also match the Server
287289
API graph-space rule: `DEFAULT`, or up to 48 lowercase letters, digits and
288290
underscores, starting with a letter. Invalid environment values are rejected

‎helm/hugegraph/README.md‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,9 @@ requires an authenticated, graph-bound Gremlin answer from every Server Pod
205205
(at least the replica floor). `server.testGraphAlias` selects that binding and
206206
defaults to `DEFAULT-hugegraph`. Set it to an existing target graph's full alias,
207207
such as `team_a-demo`, to check that graph instead. The hook does not create a
208-
graph or infer an alias from `server.graphSpace`. A Server that started during a PD roll
208+
graph or infer an alias from `server.graphSpace`. Match the binding's case: a
209+
local `Demo.properties` can bind `DEFAULT-Demo`, while REST-created graph names
210+
are lowercase. A Server that started during a PD roll
209211
can pass readiness and serve REST while every Gremlin call on it fails (see
210212
Troubleshooting, "Could not rebind"). Transient failures are retried for
211213
150 seconds before the test fails and names the failing Pod IPs. Large HPA
@@ -639,7 +641,10 @@ and backslashes. Dollar signs are escaped in the manifest so Kubernetes passes
639641
the service ID literally, including `$(POD_IP)` and `$$`. The chart and entrypoint
640642
reject non-ASCII and control characters
641643
to prevent the Server's ISO-8859-1 properties reader from changing the value;
642-
this is a deployment input serialization limit, not a change to core service names:
644+
this is a deployment input serialization limit, not a change to core service names.
645+
646+
The full service ID, including `/` and `-`, is retained when service metadata is
647+
reloaded or its graph space is cleared.
643648

644649
```bash
645650
helm upgrade --install hg ./helm/hugegraph \

‎helm/hugegraph/tests/test_hook_gremlin_test.yaml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,15 @@ tests:
8282
path: spec.containers[0].command[2]
8383
pattern: '"graph":"DEFAULT-hugegraph"'
8484

85+
- it: preserves a static graph binding with an uppercase local file name
86+
template: tests/test-connection.yaml
87+
set:
88+
server.testGraphAlias: DEFAULT-Demo
89+
asserts:
90+
- matchRegex:
91+
path: spec.containers[0].command[2]
92+
pattern: '"graph":"DEFAULT-Demo"'
93+
8594
- it: floors the per-Pod count at the HPA minimum when HPA is enabled
8695
template: tests/test-connection.yaml
8796
set:

‎hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/core/GraphManager.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1124,7 +1124,7 @@ public void clearGraphSpace(String name) {
11241124
// Clear all services
11251125
for (String key : this.services.keySet()) {
11261126
if (key.startsWith(name + DELIMITER)) {
1127-
String[] parts = key.split(DELIMITER);
1127+
String[] parts = key.split(DELIMITER, 2);
11281128
this.dropService(parts[0], parts[1]);
11291129
}
11301130
}

‎hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/meta/managers/ServiceMetaManager.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,11 @@ private static String serviceName(String graphSpace, String name) {
5050

5151
public Map<String, Service> serviceConfigs(String graphSpace) {
5252
Map<String, Service> serviceMap = new HashMap<>();
53-
Map<String, String> keyValues = this.metaDriver.scanWithPrefix(
54-
this.serviceConfPrefix(graphSpace));
53+
String prefix = this.serviceConfPrefix(graphSpace);
54+
Map<String, String> keyValues = this.metaDriver.scanWithPrefix(prefix);
5555
for (Map.Entry<String, String> entry : keyValues.entrySet()) {
5656
String key = entry.getKey();
57-
String[] parts = key.split(META_PATH_DELIMITER);
58-
serviceMap.put(parts[parts.length - 1],
57+
serviceMap.put(key.substring(prefix.length()),
5958
JsonUtil.fromJson(entry.getValue(), Service.class));
6059
}
6160
return serviceMap;

‎hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@
7474
import org.apache.hugegraph.unit.core.ExceptionTest;
7575
import org.apache.hugegraph.unit.core.GraphManagerAdminInitTest;
7676
import org.apache.hugegraph.unit.core.GraphManagerConfigTest;
77+
import org.apache.hugegraph.unit.core.GraphManagerServiceNamesTest;
7778
import org.apache.hugegraph.unit.core.GroovyScriptEngineCompatibilityTest;
7879
import org.apache.hugegraph.unit.core.HugeFeaturesTest;
7980
import org.apache.hugegraph.unit.core.HstoreSessionsTest;
@@ -212,6 +213,7 @@
212213
ExceptionTest.class,
213214
GraphManagerAdminInitTest.class,
214215
GraphManagerConfigTest.class,
216+
GraphManagerServiceNamesTest.class,
215217
HstoreSessionsTest.class,
216218
BackendStoreInfoTest.class,
217219
TraversalUtilTest.class,
Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with this
4+
* work for additional information regarding copyright ownership. The ASF
5+
* licenses this file to You under the Apache License, Version 2.0 (the
6+
* "License"); you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
13+
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
14+
* License for the specific language governing permissions and limitations
15+
* under the License.
16+
*/
17+
18+
package org.apache.hugegraph.unit.core;
19+
20+
import java.lang.reflect.Constructor;
21+
import java.lang.reflect.Field;
22+
import java.util.LinkedHashMap;
23+
import java.util.Map;
24+
import java.util.stream.Collectors;
25+
26+
import org.apache.commons.configuration2.PropertiesConfiguration;
27+
import org.apache.hugegraph.config.HugeConfig;
28+
import org.apache.hugegraph.core.GraphManager;
29+
import org.apache.hugegraph.event.EventHub;
30+
import org.apache.hugegraph.meta.MetaDriver;
31+
import org.apache.hugegraph.meta.MetaManager;
32+
import org.apache.hugegraph.meta.lock.LockResult;
33+
import org.apache.hugegraph.meta.managers.ServiceMetaManager;
34+
import org.apache.hugegraph.space.GraphSpace;
35+
import org.apache.hugegraph.space.Service;
36+
import org.apache.hugegraph.testutil.Assert;
37+
import org.apache.hugegraph.testutil.Whitebox;
38+
import org.junit.Test;
39+
import org.mockito.Mockito;
40+
41+
public class GraphManagerServiceNamesTest {
42+
43+
@Test
44+
public void testReloadPreservesCompleteServiceNames() {
45+
MetaDriver driver = storageDriver();
46+
ServiceMetaManager first = new ServiceMetaManager(driver, "cluster");
47+
for (String name : new String[]{"analytics", "team/analytics",
48+
"analytics-prod", "team/analytics-prod"}) {
49+
first.addServiceConfig("team_a", service(name));
50+
}
51+
first.addServiceConfig("other", service("analytics"));
52+
53+
ServiceMetaManager reloaded = new ServiceMetaManager(driver, "cluster");
54+
Map<String, Service> services = reloaded.serviceConfigs("team_a");
55+
Assert.assertEquals(4, services.size());
56+
for (String name : new String[]{"analytics", "team/analytics",
57+
"analytics-prod", "team/analytics-prod"}) {
58+
Assert.assertEquals(name, services.get(name).name());
59+
Assert.assertEquals(name, reloaded.service("team_a", name).name());
60+
}
61+
Assert.assertEquals(1, reloaded.serviceConfigs("other").size());
62+
63+
reloaded.removeServiceConfig("team_a", "team/analytics");
64+
Assert.assertNull(reloaded.service("team_a", "team/analytics"));
65+
Assert.assertNotNull(reloaded.service("team_a", "analytics"));
66+
Assert.assertEquals(3, reloaded.serviceConfigs("team_a").size());
67+
}
68+
69+
@Test
70+
public void testClearSpaceDropsFullServiceNames() throws Exception {
71+
MetaDriver driver = storageDriver();
72+
Constructor<MetaManager> constructor = MetaManager.class.getDeclaredConstructor();
73+
constructor.setAccessible(true);
74+
MetaManager metadata = constructor.newInstance();
75+
setField(metadata, "metaDriver", driver);
76+
setField(metadata, "cluster", "cluster");
77+
Whitebox.invoke(MetaManager.class, new Class<?>[]{String.class},
78+
"initManagers", metadata, "cluster");
79+
GraphManager manager = new GraphManager(
80+
new HugeConfig(new PropertiesConfiguration()),
81+
new EventHub("service-name-test"));
82+
try {
83+
setField(manager, "metaManager", metadata);
84+
// Exercise the metadata-backed clear path without bootstrapping
85+
// a PD/HStore cluster; the real managers use a storage-only seam.
86+
setField(manager, "PDExist", true);
87+
Map<String, GraphSpace> spaces = Whitebox.getInternalState(manager, "graphSpaces");
88+
spaces.put("team_a", new GraphSpace("team_a"));
89+
spaces.put("other", new GraphSpace("other"));
90+
Map<String, Service> cache = Whitebox.getInternalState(manager, "services");
91+
for (String name : new String[]{"analytics", "analytics-prod",
92+
"team/analytics-prod"}) {
93+
Service service = service(name);
94+
metadata.addServiceConfig("team_a", service);
95+
String key = Whitebox.invoke(GraphManager.class,
96+
new Class<?>[]{String.class, String.class}, "serviceName",
97+
null, "team_a", name);
98+
cache.put(key, service);
99+
}
100+
metadata.addServiceConfig("other", service("analytics"));
101+
cache.put("other-analytics", service("analytics"));
102+
103+
manager.clearGraphSpace("team_a");
104+
105+
Assert.assertTrue(metadata.serviceConfigs("team_a").isEmpty());
106+
Assert.assertEquals(1, metadata.serviceConfigs("other").size());
107+
Assert.assertEquals(1, cache.size());
108+
Assert.assertTrue(cache.containsKey("other-analytics"));
109+
} finally {
110+
manager.close();
111+
}
112+
}
113+
114+
private static Service service(String name) {
115+
return new Service(name, "admin", Service.ServiceType.OLTP,
116+
Service.DeploymentType.MANUAL);
117+
}
118+
119+
private static void setField(Object target, String name, Object value)
120+
throws Exception {
121+
Field field = target.getClass().getDeclaredField(name);
122+
field.setAccessible(true);
123+
field.set(target, value);
124+
}
125+
126+
private static MetaDriver storageDriver() {
127+
Map<String, String> storage = new LinkedHashMap<>();
128+
MetaDriver driver = Mockito.mock(MetaDriver.class);
129+
Mockito.doAnswer(call -> {
130+
storage.put(call.getArgument(0), call.getArgument(1));
131+
return null;
132+
}).when(driver).put(Mockito.anyString(), Mockito.anyString());
133+
Mockito.when(driver.get(Mockito.anyString()))
134+
.thenAnswer(call -> storage.get(call.<String>getArgument(0)));
135+
Mockito.doAnswer(call -> {
136+
storage.remove(call.<String>getArgument(0));
137+
return null;
138+
}).when(driver).delete(Mockito.anyString());
139+
Mockito.doAnswer(call -> {
140+
storage.keySet().removeIf(key -> key.startsWith(call.getArgument(0)));
141+
return null;
142+
}).when(driver).deleteWithPrefix(Mockito.anyString());
143+
Mockito.when(driver.scanWithPrefix(Mockito.anyString())).thenAnswer(call ->
144+
storage.entrySet().stream()
145+
.filter(entry -> entry.getKey().startsWith(call.getArgument(0)))
146+
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)));
147+
Mockito.when(driver.tryLock(Mockito.anyString(), Mockito.anyLong(), Mockito.anyLong()))
148+
.thenAnswer(call -> {
149+
LockResult lock = new LockResult();
150+
lock.lockSuccess(true);
151+
return lock;
152+
});
153+
return driver;
154+
}
155+
}

0 commit comments

Comments
 (0)