From 413a63ecf49ab96645210e3dc4d02e7c54fc30f1 Mon Sep 17 00:00:00 2001 From: imbajin Date: Tue, 13 Oct 2026 06:31:30 +0800 Subject: [PATCH 01/13] fix(server): register late graph cache rpc - register late graph services before publication - serialize cold startup and graph publication - remove exact graph-owned RPC resources - preserve legacy cleanup and verify lifecycle --- docs/server-runtime.md | 32 ++ .../rpc/RpcServiceConfig4Client.java | 4 + .../rpc/RpcServiceConfig4Server.java | 4 + .../hugegraph/rpc/RpcConsumerConfig.java | 75 +++-- .../hugegraph/rpc/RpcProviderConfig.java | 21 +- .../hugegraph/unit/ServerClientTest.java | 55 ++++ hugegraph-server/README.md | 2 + .../hugegraph/auth/HugeGraphAuthProxy.java | 5 + .../java/org/apache/hugegraph/HugeGraph.java | 5 + .../apache/hugegraph/StandardHugeGraph.java | 73 ++++- .../unit/core/GraphManagerConfigTest.java | 278 ++++++++++++++++++ 11 files changed, 520 insertions(+), 34 deletions(-) diff --git a/docs/server-runtime.md b/docs/server-runtime.md index f370951a7f..811e8e3db1 100644 --- a/docs/server-runtime.md +++ b/docs/server-runtime.md @@ -535,3 +535,35 @@ configuration changes rather than hot reload, whose recovery is handled separate Deletion is not transactional: a failure after destructive cleanup needs operator inspection, and retry does not restore removed data. An owner-cleanup failure must not be treated as a successful drain; inspect it instead of force-removing the scheduler. + +## Local loading ownership + +Server keeps ordinary import ownership in memory on each graph instance. The first +owner changes NONE to LOADING; releasing one owner leaves LOADING while others are +active, and the last release restores NONE. Manual LOADING, RESTORING and MERGING +cannot be adopted. Original enum setters reject every request while owners are active, +including bare LOADING; their original behavior remains available without active owners. + +`GET /graphspaces/{space}/graphs/{graph}/mode/owners` returns +`{"protocol":1,"instance":"","epoch":0}`. `PUT` on that path accepts +`{"action":"acquire","instance":"","epoch":0,"token":""}`; +`release` uses the same tuple. Receipts echo protocol/instance/epoch/token and return +`state` (`active` or `released`) and the effective `mode`. Both operations use existing +graph-owner/STATUS WRITE permissions; the authenticated principal owns the token. + +Keep one tuple and identity for the whole import attempt, including uncertain acquire +outcomes. Release after that import's workers stop. Acquire/release retries are +idempotent. An unknown release cancels a late acquire; a released handle cannot be +reused. Terminal records are bounded to 1024 per epoch. Epoch advancement retains +active old handles and permanently retires all other old handles. + +This protection coordinates Loader processes connected directly to the same stable +Server and graph instance. It does not fence ordinary data requests across load +balancers, node failover, graph replacement or Server restart. There is no automatic +expiry: after kill -9 or permanent disconnection, stop the import and its in-flight +writes before retrying release with the same handle and identity. If that handle is +lost, stop all related imports before reopening the graph instance. + +Loaders retain historical ordinary-import and restore behavior with Servers that do +not provide this API. Concurrent owner protection applies only when this API is +supported; RESTORING keeps its existing enum mode path. diff --git a/hugegraph-commons/hugegraph-common/src/main/java/org/apache/hugegraph/rpc/RpcServiceConfig4Client.java b/hugegraph-commons/hugegraph-common/src/main/java/org/apache/hugegraph/rpc/RpcServiceConfig4Client.java index b08d6327d1..e0557ebc21 100644 --- a/hugegraph-commons/hugegraph-common/src/main/java/org/apache/hugegraph/rpc/RpcServiceConfig4Client.java +++ b/hugegraph-commons/hugegraph-common/src/main/java/org/apache/hugegraph/rpc/RpcServiceConfig4Client.java @@ -32,4 +32,8 @@ default T serviceProxy(String graph, Class clazz) { } void removeAllServiceProxy(); + + default boolean removeServiceProxy(String graph, String interfaceId, Object proxy) { + throw new UnsupportedOperationException("Removing an individual RPC proxy is not supported"); + } } diff --git a/hugegraph-commons/hugegraph-common/src/main/java/org/apache/hugegraph/rpc/RpcServiceConfig4Server.java b/hugegraph-commons/hugegraph-common/src/main/java/org/apache/hugegraph/rpc/RpcServiceConfig4Server.java index cd0290900e..0c9eab15a8 100644 --- a/hugegraph-commons/hugegraph-common/src/main/java/org/apache/hugegraph/rpc/RpcServiceConfig4Server.java +++ b/hugegraph-commons/hugegraph-common/src/main/java/org/apache/hugegraph/rpc/RpcServiceConfig4Server.java @@ -25,5 +25,9 @@ public interface RpcServiceConfig4Server { void removeService(String serviceId); + default boolean removeService(String serviceId, Object serviceImpl) { + throw new UnsupportedOperationException("Removing an RPC service by implementation is not supported"); + } + void removeAllService(); } diff --git a/hugegraph-commons/hugegraph-rpc/src/main/java/org/apache/hugegraph/rpc/RpcConsumerConfig.java b/hugegraph-commons/hugegraph-rpc/src/main/java/org/apache/hugegraph/rpc/RpcConsumerConfig.java index 9459ac516a..1229ca318f 100644 --- a/hugegraph-commons/hugegraph-rpc/src/main/java/org/apache/hugegraph/rpc/RpcConsumerConfig.java +++ b/hugegraph-commons/hugegraph-rpc/src/main/java/org/apache/hugegraph/rpc/RpcConsumerConfig.java @@ -21,6 +21,9 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.atomic.AtomicReference; import org.apache.hugegraph.config.HugeConfig; import org.apache.hugegraph.config.RpcOptions; @@ -39,8 +42,6 @@ import com.alipay.sofa.rpc.core.response.SofaResponse; import com.alipay.sofa.rpc.ext.Extension; import com.alipay.sofa.rpc.ext.ExtensionLoaderFactory; -import com.google.common.collect.Lists; -import com.google.common.collect.Maps; import com.google.common.collect.Sets; public class RpcConsumerConfig implements RpcServiceConfig4Client { @@ -59,8 +60,8 @@ public RpcConsumerConfig(HugeConfig config, String remoteUrls) { RpcCommonConfig.initRpcConfigs(config); this.conf = config; this.remoteUrls = remoteUrls; - this.configs = Maps.newHashMap(); - this.bootstraps = Lists.newArrayList(); + this.configs = new ConcurrentHashMap<>(); + this.bootstraps = new CopyOnWriteArrayList<>(); } @Override @@ -70,10 +71,57 @@ public T serviceProxy(String interfaceId) { @Override public T serviceProxy(String graph, String interfaceId) { - ConsumerConfig config = this.consumerConfig(graph, interfaceId); - ConsumerBootstrap bootstrap = Bootstraps.from(config); - this.bootstraps.add(bootstrap); - return bootstrap.refer(); + String serviceId = graph == null ? interfaceId : interfaceId + ":" + graph; + AtomicReference proxy = new AtomicReference<>(); + this.configs.compute(serviceId, (key, existing) -> { + @SuppressWarnings("unchecked") + ConsumerConfig config = existing == null ? this.consumerConfig(graph, interfaceId) : + (ConsumerConfig) existing; + ConsumerBootstrap bootstrap = Bootstraps.from(config); + try { + proxy.set(bootstrap.refer()); + this.bootstraps.add(bootstrap); + } catch (RuntimeException | Error failure) { + try { + bootstrap.unRefer(); + if (bootstrap.getCluster() != null) { + bootstrap.getCluster().destroy(); + } + } catch (RuntimeException | Error cleanup) { + failure.addSuppressed(cleanup); + } + throw failure; + } + return config; + }); + return proxy.get(); + } + + @Override + public boolean removeServiceProxy(String graph, String interfaceId, Object proxy) { + String serviceId = graph == null ? interfaceId : interfaceId + ":" + graph; + boolean[] removed = {false}; + this.configs.computeIfPresent(serviceId, (key, config) -> { + for (ConsumerBootstrap bootstrap : this.bootstraps) { + if (bootstrap.getConsumerConfig() == config && bootstrap.getProxyIns() == proxy) { + try { + bootstrap.unRefer(); + } finally { + bootstrap.getCluster().destroy(); + } + this.bootstraps.remove(bootstrap); + removed[0] = true; + break; + } + } + for (ConsumerBootstrap bootstrap : this.bootstraps) { + if (bootstrap.getConsumerConfig() == config) { + return config; + } + } + return null; + }); + return removed[0]; } @Override @@ -103,15 +151,7 @@ private ConsumerConfig consumerConfig(String graph, serviceId = interfaceId; } - @SuppressWarnings("unchecked") - ConsumerConfig consumerConfig = (ConsumerConfig) - this.configs.get(serviceId); - if (consumerConfig != null) { - return consumerConfig; - } - - assert consumerConfig == null; - consumerConfig = new ConsumerConfig<>(); + ConsumerConfig consumerConfig = new ConsumerConfig<>(); HugeConfig conf = this.conf; String protocol = conf.get(RpcOptions.RPC_PROTOCOL); @@ -138,7 +178,6 @@ private ConsumerConfig consumerConfig(String graph, .setSerialization(conf.get(RpcOptions.RPC_SERIALIZATION)) .setLoadBalancer(loadBalancer); - this.configs.put(serviceId, consumerConfig); return consumerConfig; } diff --git a/hugegraph-commons/hugegraph-rpc/src/main/java/org/apache/hugegraph/rpc/RpcProviderConfig.java b/hugegraph-commons/hugegraph-rpc/src/main/java/org/apache/hugegraph/rpc/RpcProviderConfig.java index dafdfcf5a4..f2d7eb8007 100644 --- a/hugegraph-commons/hugegraph-rpc/src/main/java/org/apache/hugegraph/rpc/RpcProviderConfig.java +++ b/hugegraph-commons/hugegraph-rpc/src/main/java/org/apache/hugegraph/rpc/RpcProviderConfig.java @@ -18,14 +18,14 @@ package org.apache.hugegraph.rpc; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import com.alipay.sofa.rpc.config.ProviderConfig; import org.apache.hugegraph.util.E; -import com.google.common.collect.Maps; public class RpcProviderConfig implements RpcServiceConfig4Server { - private final Map> configs = Maps.newHashMap(); + private final Map> configs = new ConcurrentHashMap<>(); @Override public String addService(Class clazz, S serviceImpl) { @@ -54,10 +54,9 @@ private String addService(String graph, providerConfig.setInterfaceId(interfaceId) .setRef(serviceImpl); - E.checkArgument(!this.configs.containsKey(serviceId), + E.checkArgument(this.configs.putIfAbsent(serviceId, providerConfig) == null, "Not allowed to add service already exist: '%s'", serviceId); - this.configs.put(serviceId, providerConfig); return serviceId; } @@ -69,6 +68,20 @@ public void removeService(String serviceId) { config.unExport(); } + @Override + public boolean removeService(String serviceId, Object serviceImpl) { + boolean[] removed = {false}; + this.configs.computeIfPresent(serviceId, (key, config) -> { + if (config.getRef() != serviceImpl) { + return config; + } + config.unExport(); + removed[0] = true; + return null; + }); + return removed[0]; + } + @Override public void removeAllService() { for (ProviderConfig config : this.configs.values()) { diff --git a/hugegraph-commons/hugegraph-rpc/src/test/java/org/apache/hugegraph/unit/ServerClientTest.java b/hugegraph-commons/hugegraph-rpc/src/test/java/org/apache/hugegraph/unit/ServerClientTest.java index 07eb893c7d..7fde6b079a 100644 --- a/hugegraph-commons/hugegraph-rpc/src/test/java/org/apache/hugegraph/unit/ServerClientTest.java +++ b/hugegraph-commons/hugegraph-rpc/src/test/java/org/apache/hugegraph/unit/ServerClientTest.java @@ -711,6 +711,61 @@ public void testUnexportNotExistService() { }); } + @Test + public void testRemoveServiceUsesImplementationIdentity() { + RpcServer server = new RpcServer(config("server-random")); + GraphHelloServiceImpl original = new GraphHelloServiceImpl("graph"); + RpcProviderConfig providers = server.config(); + String serviceId = providers.addService("graph", HelloService.class, original); + server.exportAll(); + RpcClientProvider client = clientFor(server); + try { + HelloService proxy = client.config().serviceProxy("graph", HelloService.class); + Assert.assertFalse(providers.removeService(serviceId, new GraphHelloServiceImpl("graph"))); + Assert.assertEquals("graph: hello tom!", proxy.hello("tom")); + Assert.assertTrue(providers.removeService(serviceId, original)); + GraphHelloServiceImpl replacement = new GraphHelloServiceImpl("replacement"); + providers.addService("graph", HelloService.class, replacement); + server.exportAll(); + Assert.assertFalse(providers.removeService(serviceId, original)); + Assert.assertSame(replacement, providers.configs().get(serviceId).getRef()); + Assert.assertEquals("replacement: hello tom!", proxy.hello("tom")); + } finally { + client.destroy(); + stopServer(server); + } + } + + @Test + public void testRemoveServiceProxyUsesProxyIdentity() { + RpcServer server = new RpcServer(config("server-random")); + server.config().addService("graph", HelloService.class, new GraphHelloServiceImpl("graph")); + server.config().addService("other", HelloService.class, new GraphHelloServiceImpl("other")); + server.exportAll(); + RpcClientProvider client = clientFor(server); + try { + RpcConsumerConfig consumers = client.config(); + HelloService original = consumers.serviceProxy("graph", HelloService.class); + HelloService second = consumers.serviceProxy("graph", HelloService.class); + HelloService other = consumers.serviceProxy("other", HelloService.class); + Assert.assertNotSame(original, second); + Assert.assertTrue(consumers.removeServiceProxy("graph", HelloService.class.getName(), original)); + Assert.assertThrows(SofaRpcRuntimeException.class, () -> original.hello("tom")); + Assert.assertEquals("graph: hello tom!", second.hello("tom")); + Assert.assertFalse(consumers.removeServiceProxy("graph", HelloService.class.getName(), original)); + Assert.assertTrue(consumers.removeServiceProxy("graph", HelloService.class.getName(), second)); + List bootstraps = Whitebox.getInternalState(consumers, "bootstraps"); + Assert.assertEquals(1, bootstraps.size()); + HelloService replacement = consumers.serviceProxy("graph", HelloService.class); + Assert.assertFalse(consumers.removeServiceProxy("graph", HelloService.class.getName(), original)); + Assert.assertEquals("graph: hello tom!", replacement.hello("tom")); + Assert.assertEquals("other: hello tom!", other.hello("tom")); + } finally { + client.destroy(); + stopServer(server); + } + } + @Test public void testServerDisabled() { HugeConfig clientConf = config(false); diff --git a/hugegraph-server/README.md b/hugegraph-server/README.md index dc2ab3618e..a2c24f4352 100644 --- a/hugegraph-server/README.md +++ b/hugegraph-server/README.md @@ -16,6 +16,8 @@ HugeGraph Server consists of two layers of functionality: the graph engine layer See [launcher GC options](../docs/server-runtime.md#launcher-gc-options) for explicit collector selection and caller JVM tuning. +See [cache RPC graph lifecycle](../docs/server-runtime.md#cache-rpc-graph-lifecycle) for dynamic +graph registration, resource cleanup and notification delivery limits. ## Backend Evolution and Compatibility diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeGraphAuthProxy.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeGraphAuthProxy.java index f4066cf154..9e68fda585 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeGraphAuthProxy.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeGraphAuthProxy.java @@ -898,6 +898,11 @@ public void registerRpcServices(RpcServiceConfig4Server serverConfig, this.hugegraph.registerRpcServices(serverConfig, clientConfig); } + @Override + public void unregisterRpcServices() { + this.hugegraph.unregisterRpcServices(); + } + @Override public void initBackend() { this.verifyAdminPermission(); diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/HugeGraph.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/HugeGraph.java index 56bc00c0bb..18f6ac1f1e 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/HugeGraph.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/HugeGraph.java @@ -295,6 +295,11 @@ default Predicate searchPredicate(String text) { void registerRpcServices(RpcServiceConfig4Server serverConfig, RpcServiceConfig4Client clientConfig); + default void unregisterRpcServices() { + // Legacy implementations do not participate in the owned RPC cleanup contract. + // Implementations owning graph-scoped RPC resources must override this hook. + } + default List mapPkId2Name(Collection ids) { List names = new ArrayList<>(ids.size()); for (Id id : ids) { diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/StandardHugeGraph.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/StandardHugeGraph.java index 157ec6b74b..3db775e1a3 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/StandardHugeGraph.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/StandardHugeGraph.java @@ -19,6 +19,7 @@ import org.apache.hugegraph.exception.HugeException; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Date; @@ -166,6 +167,7 @@ public class StandardHugeGraph implements HugeGraph { private final EventHub schemaEventHub; private final EventHub graphEventHub; private final EventHub indexEventHub; + private final List rpcServiceCleanup = new ArrayList<>(); private final LocalCounter localCounter; private final RateLimiter writeRateLimiter; private final RateLimiter readRateLimiter; @@ -1177,6 +1179,7 @@ public synchronized void close() throws Exception { // stops its context. Owner-close errors do not imply native release. this.closed = true; for (Runnable close : new Runnable[]{ + this::unregisterRpcServices, () -> CachedGraphTransaction.closeGraph(this.params), () -> CachedSchemaTransaction.closeGraph(this.params), () -> CachedSchemaTransactionV2.closeGraph(this.params), @@ -1379,26 +1382,67 @@ public V option(TypedOption option) { } @Override - public void registerRpcServices(RpcServiceConfig4Server serverConfig, - RpcServiceConfig4Client clientConfig) { + public synchronized void registerRpcServices(RpcServiceConfig4Server serverConfig, + RpcServiceConfig4Client clientConfig) { /* * Skip register cache-rpc service if it's non-shared storage, * because we assume cache of non-shared storage is updated by raft. */ + if (!this.rpcServiceCleanup.isEmpty()) { + return; + } if (!this.backendStoreFeatures().supportsSharedStorage()) { return; } + E.checkState(!this.closed(), "Can't register RPC services for a closed graph"); + String graph = this.spaceGraphName(); + try { + GraphCacheNotifier graphProxy = clientConfig.serviceProxy(graph, GraphCacheNotifier.class); + this.rpcServiceCleanup.add(() -> clientConfig.removeServiceProxy( + graph, GraphCacheNotifier.class.getName(), graphProxy)); + HugeGraphCacheNotifier graphNotifier = new HugeGraphCacheNotifier(this.graphEventHub, graphProxy); + this.rpcServiceCleanup.add(graphNotifier::close); + String graphService = serverConfig.addService(graph, GraphCacheNotifier.class, graphNotifier); + this.rpcServiceCleanup.add(() -> serverConfig.removeService(graphService, graphNotifier)); + + SchemaCacheNotifier schemaProxy = clientConfig.serviceProxy(graph, SchemaCacheNotifier.class); + this.rpcServiceCleanup.add(() -> clientConfig.removeServiceProxy( + graph, SchemaCacheNotifier.class.getName(), schemaProxy)); + HugeSchemaCacheNotifier schemaNotifier = new HugeSchemaCacheNotifier(this.schemaEventHub, schemaProxy); + this.rpcServiceCleanup.add(schemaNotifier::close); + String schemaService = serverConfig.addService(graph, SchemaCacheNotifier.class, schemaNotifier); + this.rpcServiceCleanup.add(() -> serverConfig.removeService(schemaService, schemaNotifier)); + } catch (RuntimeException | Error failure) { + try { + this.unregisterRpcServices(); + } catch (RuntimeException | Error cleanup) { + failure.addSuppressed(cleanup); + } + throw failure; + } + } - Class clazz1 = GraphCacheNotifier.class; - // The proxy is sometimes unavailable (issue #664) - CacheNotifier proxy = clientConfig.serviceProxy(this.spaceGraphName(), clazz1); - serverConfig.addService(this.spaceGraphName(), clazz1, new HugeGraphCacheNotifier( - this.graphEventHub, proxy)); - - Class clazz2 = SchemaCacheNotifier.class; - proxy = clientConfig.serviceProxy(this.spaceGraphName(), clazz2); - serverConfig.addService(this.spaceGraphName(), clazz2, new HugeSchemaCacheNotifier( - this.schemaEventHub, proxy)); + @Override + public synchronized void unregisterRpcServices() { + Throwable failure = null; + for (int i = this.rpcServiceCleanup.size() - 1; i >= 0; i--) { + try { + this.rpcServiceCleanup.get(i).run(); + this.rpcServiceCleanup.remove(i); + } catch (RuntimeException | Error error) { + if (failure == null) { + failure = error; + } else if (failure != error) { + failure.addSuppressed(error); + } + } + } + if (failure instanceof Error) { + throw (Error) failure; + } + if (failure != null) { + throw (RuntimeException) failure; + } } private void waitUntilAllTasksCompleted() { @@ -1486,10 +1530,14 @@ private static class AbstractCacheNotifier implements CacheNotifier { private final EventHub hub; private final EventListener cacheEventListener; + private volatile boolean closed; public AbstractCacheNotifier(EventHub hub, CacheNotifier proxy) { this.hub = hub; this.cacheEventListener = event -> { + if (this.closed) { + return false; + } try { LOG.info("Received event: {}", event); Object[] args = event.args(); @@ -1533,6 +1581,7 @@ public AbstractCacheNotifier(EventHub hub, CacheNotifier proxy) { @Override public void close() { + this.closed = true; this.hub.unlisten(Events.CACHE, this.cacheEventListener); } diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerConfigTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerConfigTest.java index b8601c22d5..c3fe3b8e4d 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerConfigTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerConfigTest.java @@ -17,21 +17,299 @@ package org.apache.hugegraph.unit.core; +import java.lang.reflect.InvocationHandler; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Proxy; +import java.lang.management.ManagementFactory; +import java.lang.management.ThreadInfo; +import java.net.ServerSocket; import java.util.HashMap; +import java.util.List; import java.util.Map; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; +import org.apache.commons.configuration2.Configuration; import org.apache.commons.configuration2.PropertiesConfiguration; +import org.apache.hugegraph.HugeFactory; +import org.apache.hugegraph.HugeGraph; +import org.apache.hugegraph.StandardHugeGraph; +import org.apache.hugegraph.backend.store.BackendFeatures; import org.apache.hugegraph.config.CoreOptions; import org.apache.hugegraph.config.HugeConfig; +import org.apache.hugegraph.config.OptionSpace; +import org.apache.hugegraph.config.RpcOptions; import org.apache.hugegraph.config.ServerOptions; import org.apache.hugegraph.core.GraphManager; import org.apache.hugegraph.event.EventHub; +import org.apache.hugegraph.event.EventListener; +import org.apache.hugegraph.exception.HugeException; +import org.apache.hugegraph.rpc.RpcClientProvider; +import org.apache.hugegraph.rpc.RpcServer; +import org.apache.hugegraph.space.GraphSpace; import org.apache.hugegraph.testutil.Assert; import org.apache.hugegraph.testutil.Whitebox; +import org.apache.hugegraph.util.E; +import org.apache.hugegraph.util.Events; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; public class GraphManagerConfigTest { + @Rule + public TemporaryFolder temporary = new TemporaryFolder(); + + @Test + public void testLegacyDefaultCleanupDoesNotAbortDropWithoutRpc() throws Exception { + this.checkLegacyDrop(false); + } + + @Test + public void testActualCleanupFailureStillPropagates() throws Exception { + this.checkLegacyDrop(true); + } + + private void checkLegacyDrop(boolean failCleanup) throws Exception { + OptionSpace.register("core", CoreOptions.instance()); + OptionSpace.register("rpc", RpcOptions.instance()); + OptionSpace.register("server", ServerOptions.instance()); + PropertiesConfiguration properties = new PropertiesConfiguration(); + properties.setProperty(ServerOptions.GRAPHS.name(), this.temporary.newFolder("legacy").getPath()); + properties.setProperty(ServerOptions.GRAPH_LOAD_FROM_LOCAL_CONFIG.name(), false); + properties.setProperty(ServerOptions.USE_PD.name(), false); + properties.setProperty(ServerOptions.ENABLE_DYNAMIC_CREATE_DROP.name(), true); + properties.setProperty(RpcOptions.RPC_SERVER_HOST.name(), ""); + properties.setProperty(RpcOptions.RPC_REMOTE_URL.name(), ""); + EventHub hub = new EventHub("legacy-rpc-cleanup-test"); + GraphManager manager = new GraphManager(new HugeConfig(properties), hub); + HugeGraph delegate = null; + String originalThreadName = Thread.currentThread().getName(); + try { + delegate = manager.createGraphLocal("legacy_cleanup", "gremlin.graph=" + HugeFactory.class.getName() + + "\nbackend=memory\nstore=legacy_cleanup\n"); + HugeGraph owner = delegate; + IllegalStateException failure = new IllegalStateException("actual owner cleanup failed"); + HugeGraph legacy = (HugeGraph) Proxy.newProxyInstance(HugeGraph.class.getClassLoader(), + new Class[]{HugeGraph.class}, (proxy, method, args) -> { + if ("unregisterRpcServices".equals(method.getName())) { + if (failCleanup) { + throw failure; + } + return InvocationHandler.invokeDefault(proxy, method, args); + } + try { + return method.invoke(owner, args); + } catch (InvocationTargetException error) { + throw error.getCause(); + } + }); + Map graphs = Whitebox.getInternalState(manager, "graphs"); + graphs.put(legacy.spaceGraphName(), legacy); + RpcServer server = Whitebox.getInternalState(manager, "rpcServer"); + RpcClientProvider client = Whitebox.getInternalState(manager, "rpcClient"); + Assert.assertFalse(server.enabled()); + Assert.assertFalse(client.enabled()); + // Reach the normal PD-drop branch without booting PD or writing placement metadata. + Whitebox.setInternalState(manager, "PDExist", true); + Map spaces = Whitebox.getInternalState(manager, "graphSpaces"); + spaces.put("DEFAULT", new GraphSpace("DEFAULT")); + Thread.currentThread().setName("grpc-legacy-cleanup-test"); + if (failCleanup) { + Assert.assertThrows(IllegalStateException.class, () -> manager.dropGraph( + "DEFAULT", "legacy_cleanup", false), caught -> Assert.assertSame(failure, caught)); + Assert.assertFalse(owner.closed()); + } else { + CountDownLatch dropped = new CountDownLatch(1); + hub.listen(Events.GRAPH_DROP, event -> { + dropped.countDown(); + return true; + }); + manager.dropGraph("DEFAULT", "legacy_cleanup", false); + Assert.assertTrue(dropped.await(5, TimeUnit.SECONDS)); + Assert.assertTrue(owner.closed()); + Map factory = Whitebox.getInternalState(HugeFactory.class, "GRAPHS"); + Assert.assertFalse(factory.containsKey(legacy.spaceGraphName())); + } + Assert.assertFalse(manager.graphs().contains(legacy.spaceGraphName())); + } finally { + Thread.currentThread().setName(originalThreadName); + manager.close(); + if (delegate != null) { + delegate.close(); + HugeFactory.remove(delegate); + } + } + } + + @Test + public void testLateGraphRpcPublicationRollbackAndReplacement() throws Exception { + OptionSpace.register("core", CoreOptions.instance()); + OptionSpace.register("rpc", RpcOptions.instance()); + OptionSpace.register("server", ServerOptions.instance()); + PropertiesConfiguration properties = new PropertiesConfiguration(); + properties.setProperty(ServerOptions.GRAPHS.name(), this.temporary.newFolder("graphs").getPath()); + properties.setProperty(ServerOptions.USE_PD.name(), false); + properties.setProperty(ServerOptions.GRAPH_LOAD_FROM_LOCAL_CONFIG.name(), false); + properties.setProperty(ServerOptions.ENABLE_DYNAMIC_CREATE_DROP.name(), true); + properties.setProperty(RpcOptions.RPC_SERVER_HOST.name(), "127.0.0.1"); + properties.setProperty(RpcOptions.RPC_SERVER_PORT.name(), 0); + try (ServerSocket reservation = new ServerSocket(0)) { + properties.setProperty(RpcOptions.RPC_REMOTE_URL.name(), "127.0.0.1:" + reservation.getLocalPort()); + } + EventHub hub = new EventHub("rpc-publication-test"); + GraphManager manager = new GraphManager(new HugeConfig(properties), hub); + BarrierRpcServer server = new BarrierRpcServer(new HugeConfig(properties)); + Whitebox.setInternalState(manager, "rpcServer", server); + RpcClientProvider client = Whitebox.getInternalState(manager, "rpcClient"); + ExecutorService initializer = Executors.newFixedThreadPool(2); + HugeGraph startup = null; + HugeGraph replacement = null; + try { + startup = manager.createGraphLocal("rpc_startup", sharedMemoryConfig("rpc_startup")); + Assert.assertEquals(0, server.config().configs().size()); + HugeGraph startupOwner = startup; + AtomicReference initThread = new AtomicReference<>(); + Future initialization = initializer.submit(() -> { + initThread.set(Thread.currentThread()); + try { + Whitebox.invoke(manager.getClass(), "startRpcServer", manager); + } finally { + Whitebox.invoke(StandardHugeGraph.class, "closeCurrentThreadTransaction", startupOwner); + } + }); + Assert.assertTrue(server.entered.await(5, TimeUnit.SECONDS)); + Assert.assertEquals(2, server.config().configs().size()); + AtomicReference publisher = new AtomicReference<>(); + Future publication = initializer.submit(() -> { + publisher.set(Thread.currentThread()); + HugeGraph graph = manager.createGraphLocal("rpc_late", sharedMemoryConfig("rpc_late")); + Whitebox.invoke(StandardHugeGraph.class, "closeCurrentThreadTransaction", graph); + return graph; + }); + awaitManagerGate(publisher, initThread.get()); + Assert.assertFalse(manager.graphs().contains("DEFAULT-rpc_late")); + assertRpcResources(server, client, 2); + server.release.countDown(); + initialization.get(5, TimeUnit.SECONDS); + HugeGraph original = publication.get(5, TimeUnit.SECONDS); + Assert.assertEquals(2, server.exports.get()); + assertRpcResources(server, client, 4); + + EventListener failure = event -> { + throw new IllegalStateException("reject candidate after RPC registration"); + }; + hub.listen(Events.GRAPH_CREATE, failure); + try { + Assert.assertThrows(HugeException.class, () -> manager.createGraphLocal( + "rpc_rejected", sharedMemoryConfig("rpc_rejected"))); + } finally { + hub.unlisten(Events.GRAPH_CREATE, failure); + } + Assert.assertFalse(manager.graphs().contains("DEFAULT-rpc_rejected")); + assertRpcResources(server, client, 4); + + manager.dropGraphLocal("rpc_late"); + assertRpcResources(server, client, 2); + replacement = manager.createGraphLocal("rpc_late", sharedMemoryConfig("rpc_late")); + hub.notifySync(Events.GRAPH_DROP, original); + Assert.assertSame(replacement, manager.graph("DEFAULT-rpc_late")); + assertRpcResources(server, client, 4); + manager.close(); + List bootstraps = Whitebox.getInternalState(client.config(), "bootstraps"); + Assert.assertEquals(0, server.config().configs().size()); + Assert.assertEquals(0, bootstraps.size()); + } finally { + server.release.countDown(); + initializer.shutdownNow(); + initializer.awaitTermination(5, TimeUnit.SECONDS); + manager.close(); + if (startup != null) { + HugeFactory.remove(startup); + } + if (replacement != null) { + HugeFactory.remove(replacement); + } + } + } + + private static void awaitManagerGate(AtomicReference publisher, Thread initializer) throws Exception { + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(5); + while (System.nanoTime() < deadline) { + Thread thread = publisher.get(); + ThreadInfo info = thread == null ? null : ManagementFactory.getThreadMXBean().getThreadInfo(thread.getId()); + if (info != null && info.getThreadState() == Thread.State.BLOCKED && + info.getLockOwnerId() == initializer.getId()) { + Assert.assertContains(GraphManager.class.getName(), info.getLockName()); + return; + } + Thread.sleep(5); + } + Assert.fail("Graph publisher did not block on the startup manager gate"); + } + + private static class BarrierRpcServer extends RpcServer { + + private final CountDownLatch entered = new CountDownLatch(1); + private final CountDownLatch release = new CountDownLatch(1); + private final AtomicInteger exports = new AtomicInteger(); + + BarrierRpcServer(HugeConfig config) { + super(config); + } + + @Override + public void exportAll() { + if (this.exports.incrementAndGet() == 1) { + this.entered.countDown(); + try { + E.checkState(this.release.await(5, TimeUnit.SECONDS), "RPC export barrier timed out"); + } catch (InterruptedException failure) { + Thread.currentThread().interrupt(); + throw new IllegalStateException(failure); + } + } + super.exportAll(); + } + } + + private static void assertRpcResources(RpcServer server, RpcClientProvider client, int count) { + Assert.assertEquals(count, server.config().configs().size()); + Map configs = Whitebox.getInternalState(client.config(), "configs"); + List bootstraps = Whitebox.getInternalState(client.config(), "bootstraps"); + Assert.assertEquals(count, configs.size()); + Assert.assertEquals(count, bootstraps.size()); + } + + private static String sharedMemoryConfig(String name) { + return "gremlin.graph=" + SharedMemoryGraph.class.getName() + "\nbackend=memory\nstore=" + name + "\n"; + } + + // Exercises cache RPC registration, not shared-backend DML or consistency. + public static class SharedMemoryGraph extends StandardHugeGraph { + + public SharedMemoryGraph(HugeConfig config) { + super(config); + } + + public static SharedMemoryGraph open(Configuration config) { + return new SharedMemoryGraph(new HugeConfig(config)); + } + + @Override + public BackendFeatures backendStoreFeatures() { + BackendFeatures features = super.backendStoreFeatures(); + return (BackendFeatures) Proxy.newProxyInstance(BackendFeatures.class.getClassLoader(), + new Class[]{BackendFeatures.class}, (proxy, method, args) -> + "supportsSharedStorage".equals(method.getName()) ? true : method.invoke(features, args)); + } + } + @Test public void testAttachExplicitLocalResourceLimits() { PropertiesConfiguration properties = new PropertiesConfiguration(); From 6e520da71c29b8a08092ae102303f12581d6750c Mon Sep 17 00:00:00 2001 From: imbajin Date: Tue, 13 Oct 2026 07:20:55 +0800 Subject: [PATCH 02/13] fix(server): protect RPC service cleanup - enforce the existing ADMIN gate before unregistering - preserve RPC owners after denied Gremlin cleanup - isolate event and DB executors in graph tests - cover null contexts and suite shutdown ordering --- .../hugegraph/auth/HugeGraphAuthProxy.java | 1 + .../unit/core/GraphManagerConfigTest.java | 125 ++++++++++++++++++ 2 files changed, 126 insertions(+) diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeGraphAuthProxy.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeGraphAuthProxy.java index 9e68fda585..4e45c4d816 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeGraphAuthProxy.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeGraphAuthProxy.java @@ -900,6 +900,7 @@ public void registerRpcServices(RpcServiceConfig4Server serverConfig, @Override public void unregisterRpcServices() { + this.verifyAdminPermission(); this.hugegraph.unregisterRpcServices(); } diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerConfigTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerConfigTest.java index c3fe3b8e4d..1894665fe0 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerConfigTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerConfigTest.java @@ -34,11 +34,19 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; +import javax.script.ScriptException; +import javax.script.SimpleBindings; + import org.apache.commons.configuration2.Configuration; import org.apache.commons.configuration2.PropertiesConfiguration; import org.apache.hugegraph.HugeFactory; import org.apache.hugegraph.HugeGraph; import org.apache.hugegraph.StandardHugeGraph; +import org.apache.hugegraph.auth.AuthContext; +import org.apache.hugegraph.auth.HugeAuthenticator; +import org.apache.hugegraph.auth.HugeGraphAuthProxy; +import org.apache.hugegraph.auth.HugePermission; +import org.apache.hugegraph.auth.RolePermission; import org.apache.hugegraph.backend.store.BackendFeatures; import org.apache.hugegraph.config.CoreOptions; import org.apache.hugegraph.config.HugeConfig; @@ -52,19 +60,136 @@ import org.apache.hugegraph.rpc.RpcClientProvider; import org.apache.hugegraph.rpc.RpcServer; import org.apache.hugegraph.space.GraphSpace; +import org.apache.hugegraph.task.TaskManager; import org.apache.hugegraph.testutil.Assert; import org.apache.hugegraph.testutil.Whitebox; import org.apache.hugegraph.util.E; import org.apache.hugegraph.util.Events; +import org.apache.tinkerpop.gremlin.groovy.jsr223.GremlinGroovyScriptEngine; +import org.junit.AfterClass; +import org.junit.BeforeClass; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; public class GraphManagerConfigTest { + private static ExecutorService previousEventExecutor; + private static ExecutorService previousTaskDbExecutor; + private static ExecutorService taskDbExecutor; + + @BeforeClass + public static void initEventExecutor() { + // Other suite classes may have destroyed the global EventHub executor. + previousEventExecutor = Whitebox.getInternalState(EventHub.class, "executor"); + Whitebox.setInternalState(EventHub.class, "executor", null); + EventHub.init(2); + previousTaskDbExecutor = Whitebox.getInternalState(TaskManager.instance(), "taskDbExecutor"); + taskDbExecutor = Executors.newSingleThreadExecutor( + runnable -> new Thread(runnable, "task-db-worker-rpc-config-test")); + Whitebox.setInternalState(TaskManager.instance(), "taskDbExecutor", taskDbExecutor); + } + + @AfterClass + public static void restoreEventExecutor() throws InterruptedException { + try { + Assert.assertTrue(EventHub.destroy(5)); + } finally { + Whitebox.setInternalState(EventHub.class, "executor", previousEventExecutor); + taskDbExecutor.shutdown(); + try { + Assert.assertTrue(taskDbExecutor.awaitTermination(5, TimeUnit.SECONDS)); + } finally { + Whitebox.setInternalState(TaskManager.instance(), "taskDbExecutor", previousTaskDbExecutor); + } + } + } + @Rule public TemporaryFolder temporary = new TemporaryFolder(); + @Test + public void testAuthenticatedRpcCleanupPreservesOwnerUntilAdmin() throws Exception { + OptionSpace.register("core", CoreOptions.instance()); + OptionSpace.register("rpc", RpcOptions.instance()); + OptionSpace.register("server", ServerOptions.instance()); + PropertiesConfiguration properties = new PropertiesConfiguration(); + properties.setProperty(ServerOptions.GRAPHS.name(), this.temporary.newFolder("auth").getPath()); + properties.setProperty(ServerOptions.USE_PD.name(), false); + properties.setProperty(ServerOptions.GRAPH_LOAD_FROM_LOCAL_CONFIG.name(), false); + properties.setProperty(ServerOptions.ENABLE_DYNAMIC_CREATE_DROP.name(), true); + properties.setProperty(RpcOptions.RPC_SERVER_HOST.name(), "127.0.0.1"); + properties.setProperty(RpcOptions.RPC_SERVER_PORT.name(), 0); + try (ServerSocket reservation = new ServerSocket(0)) { + properties.setProperty(RpcOptions.RPC_REMOTE_URL.name(), "127.0.0.1:" + reservation.getLocalPort()); + } + EventHub hub = new EventHub("authenticated-rpc-cleanup"); + GraphManager manager = new GraphManager(new HugeConfig(properties), hub); + String originalThreadName = Thread.currentThread().getName(); + AuthContext.setContext(HugeAuthenticator.User.ADMIN.toJson()); + TaskManager.setContext(HugeAuthenticator.User.ADMIN.toJson()); + try { + String factory = "gremlin.graph=" + AuthSharedMemoryGraph.class.getName() + "\nbackend=memory\nstore="; + manager.createGraphLocal("auth_startup", factory + "auth_startup\n"); + Whitebox.invoke(GraphManager.class, "startRpcServer", manager); + HugeGraph owner = manager.createGraphLocal("auth_late", factory + "auth_late\n"); + RpcServer server = Whitebox.getInternalState(manager, "rpcServer"); + RpcClientProvider client = Whitebox.getInternalState(manager, "rpcClient"); + assertRpcResources(server, client, 4); + EventListener rejection = event -> { + throw new IllegalStateException("reject authenticated candidate"); + }; + hub.listen(Events.GRAPH_CREATE, rejection); + try { + Assert.assertThrows(HugeException.class, () -> manager.createGraphLocal( + "auth_rejected", factory + "auth_rejected\n")); + } finally { + hub.unlisten(Events.GRAPH_CREATE, rejection); + } + assertRpcResources(server, client, 4); + TaskManager.resetContext(); + RolePermission execute = RolePermission.role(owner.graphSpace(), owner.name(), HugePermission.EXECUTE); + Assert.assertFalse(RolePermission.isAdmin(execute)); + AuthContext.setContext(new HugeAuthenticator.User("reader", execute).toJson()); + SimpleBindings bindings = new SimpleBindings(); + bindings.put("graph", owner); + GremlinGroovyScriptEngine engine = new GremlinGroovyScriptEngine(); + Assert.assertThrows(ScriptException.class, () -> engine.eval("graph.unregisterRpcServices()", bindings), + error -> Assert.assertContains("Permission denied", error.getMessage())); + assertRpcResources(server, client, 4); + AuthContext.resetContext(); + Assert.assertThrows(IllegalStateException.class, owner::unregisterRpcServices, + error -> Assert.assertContains("Missing authentication context", error.getMessage())); + assertRpcResources(server, client, 4); + HugeGraphAuthProxy.runAsAdmin(owner::unregisterRpcServices); + assertRpcResources(server, client, 2); + AuthContext.setContext(HugeAuthenticator.User.ADMIN.toJson()); + // Use the normal PD-drop branch with explicit admin identity and no placement metadata writes. + Whitebox.setInternalState(manager, "PDExist", true); + Map spaces = Whitebox.getInternalState(manager, "graphSpaces"); + spaces.put("DEFAULT", new GraphSpace("DEFAULT")); + Thread.currentThread().setName("grpc-auth-cleanup-test"); + manager.dropGraph("DEFAULT", "auth_late", false); + Assert.assertTrue(owner.closed()); + assertRpcResources(server, client, 2); + } finally { + Thread.currentThread().setName(originalThreadName); + try { + HugeGraphAuthProxy.runAsAdmin(manager::close); + } finally { + HugeGraphAuthProxy.resetContext(); + TaskManager.resetContext(); + } + } + } + + public static class AuthSharedMemoryGraph { + + public static HugeGraph open(Configuration config) { + return new HugeGraphAuthProxy(new SharedMemoryGraph(new HugeConfig(config))); + } + } + @Test public void testLegacyDefaultCleanupDoesNotAbortDropWithoutRpc() throws Exception { this.checkLegacyDrop(false); From c5de08c19c5c5c508d6774950d5bb201f3183a2d Mon Sep 17 00:00:00 2001 From: imbajin Date: Tue, 13 Oct 2026 10:04:22 +0800 Subject: [PATCH 03/13] fix(server): preserve RPC graph ownership - scope trusted lazy graph RPC registration - serialize graph export and cleanup by service - retain graph owners when pre-delete cleanup fails - document cleanup and retry boundaries --- .../hugegraph/rpc/RpcProviderConfig.java | 29 +++- .../org/apache/hugegraph/rpc/RpcServer.java | 6 +- .../hugegraph/unit/ServerClientTest.java | 62 ++++++- .../unit/core/GraphManagerConfigTest.java | 162 ++++++++++++++++-- 4 files changed, 224 insertions(+), 35 deletions(-) diff --git a/hugegraph-commons/hugegraph-rpc/src/main/java/org/apache/hugegraph/rpc/RpcProviderConfig.java b/hugegraph-commons/hugegraph-rpc/src/main/java/org/apache/hugegraph/rpc/RpcProviderConfig.java index f2d7eb8007..0ce2f17bf3 100644 --- a/hugegraph-commons/hugegraph-rpc/src/main/java/org/apache/hugegraph/rpc/RpcProviderConfig.java +++ b/hugegraph-commons/hugegraph-rpc/src/main/java/org/apache/hugegraph/rpc/RpcProviderConfig.java @@ -21,6 +21,7 @@ import java.util.concurrent.ConcurrentHashMap; import com.alipay.sofa.rpc.config.ProviderConfig; +import com.alipay.sofa.rpc.config.ServerConfig; import org.apache.hugegraph.util.E; public class RpcProviderConfig implements RpcServiceConfig4Server { @@ -62,10 +63,13 @@ private String addService(String graph, @Override public void removeService(String serviceId) { - ProviderConfig config = this.configs.remove(serviceId); - E.checkArgument(config != null, - "The service '%s' doesn't exist", serviceId); - config.unExport(); + boolean[] removed = {false}; + this.configs.computeIfPresent(serviceId, (key, config) -> { + config.unExport(); + removed[0] = true; + return null; + }); + E.checkArgument(removed[0], "The service '%s' doesn't exist", serviceId); } @Override @@ -84,10 +88,21 @@ public boolean removeService(String serviceId, Object serviceImpl) { @Override public void removeAllService() { - for (ProviderConfig config : this.configs.values()) { - config.unExport(); + for (String serviceId : this.configs.keySet()) { + this.configs.computeIfPresent(serviceId, (key, config) -> { + config.unExport(); + return null; + }); + } + } + + void exportAll(ServerConfig server, int timeout) { + for (String serviceId : this.configs.keySet()) { + this.configs.computeIfPresent(serviceId, (key, config) -> { + config.setServer(server).setTimeout(timeout).export(); + return config; + }); } - this.configs.clear(); } public Map> configs() { diff --git a/hugegraph-commons/hugegraph-rpc/src/main/java/org/apache/hugegraph/rpc/RpcServer.java b/hugegraph-commons/hugegraph-rpc/src/main/java/org/apache/hugegraph/rpc/RpcServer.java index 7791ac8cd5..1af841c1c7 100644 --- a/hugegraph-commons/hugegraph-rpc/src/main/java/org/apache/hugegraph/rpc/RpcServer.java +++ b/hugegraph-commons/hugegraph-rpc/src/main/java/org/apache/hugegraph/rpc/RpcServer.java @@ -104,11 +104,7 @@ public void exportAll() { return; } int timeout = this.conf.get(RpcOptions.RPC_SERVER_TIMEOUT) * 1000; - for (ProviderConfig providerConfig : configs.values()) { - providerConfig.setServer(this.serverConfig) - .setTimeout(timeout) - .export(); - } + this.configs.exportAll(this.serverConfig, timeout); LOG.info("RpcServer started success on port {}", this.port()); } diff --git a/hugegraph-commons/hugegraph-rpc/src/test/java/org/apache/hugegraph/unit/ServerClientTest.java b/hugegraph-commons/hugegraph-rpc/src/test/java/org/apache/hugegraph/unit/ServerClientTest.java index 7fde6b079a..24b140b71c 100644 --- a/hugegraph-commons/hugegraph-rpc/src/test/java/org/apache/hugegraph/unit/ServerClientTest.java +++ b/hugegraph-commons/hugegraph-rpc/src/test/java/org/apache/hugegraph/unit/ServerClientTest.java @@ -22,6 +22,12 @@ import java.net.ServerSocket; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; import org.apache.hugegraph.config.HugeConfig; import org.apache.hugegraph.rpc.RpcClientProvider; @@ -38,6 +44,7 @@ import org.junit.Test; import com.alipay.sofa.rpc.common.RpcOptions; +import com.alipay.sofa.rpc.config.ProviderConfig; import com.alipay.sofa.rpc.core.exception.SofaRpcException; import com.alipay.sofa.rpc.core.exception.SofaRpcRuntimeException; import com.google.common.collect.ImmutableMap; @@ -712,18 +719,37 @@ public void testUnexportNotExistService() { } @Test - public void testRemoveServiceUsesImplementationIdentity() { + public void testRemoveServiceUsesImplementationIdentity() throws Exception { RpcServer server = new RpcServer(config("server-random")); GraphHelloServiceImpl original = new GraphHelloServiceImpl("graph"); RpcProviderConfig providers = server.config(); String serviceId = providers.addService("graph", HelloService.class, original); + PausedProvider provider = new PausedProvider(); + provider.setId(serviceId).setUniqueId("graph"); + provider.setInterfaceId(HelloService.class.getName()).setRef(original); + providers.configs().put(serviceId, provider); server.exportAll(); RpcClientProvider client = clientFor(server); + ExecutorService worker = Executors.newFixedThreadPool(2); try { HelloService proxy = client.config().serviceProxy("graph", HelloService.class); Assert.assertFalse(providers.removeService(serviceId, new GraphHelloServiceImpl("graph"))); Assert.assertEquals("graph: hello tom!", proxy.hello("tom")); - Assert.assertTrue(providers.removeService(serviceId, original)); + provider.armed = true; + Future exporting = worker.submit(server::exportAll); + Assert.assertTrue(provider.entered.await(5, TimeUnit.SECONDS)); + CountDownLatch removing = new CountDownLatch(1); + Future removal = worker.submit(() -> { + removing.countDown(); + return providers.removeService(serviceId, original); + }); + Assert.assertTrue(removing.await(5, TimeUnit.SECONDS)); + Assert.assertThrows(TimeoutException.class, () -> removal.get(1, TimeUnit.SECONDS)); + Assert.assertSame(provider, providers.configs().get(serviceId)); + provider.release.countDown(); + exporting.get(5, TimeUnit.SECONDS); + Assert.assertTrue(removal.get(5, TimeUnit.SECONDS)); + Assert.assertFalse(providers.configs().containsKey(serviceId)); GraphHelloServiceImpl replacement = new GraphHelloServiceImpl("replacement"); providers.addService("graph", HelloService.class, replacement); server.exportAll(); @@ -731,8 +757,36 @@ public void testRemoveServiceUsesImplementationIdentity() { Assert.assertSame(replacement, providers.configs().get(serviceId).getRef()); Assert.assertEquals("replacement: hello tom!", proxy.hello("tom")); } finally { - client.destroy(); - stopServer(server); + provider.release.countDown(); + worker.shutdownNow(); + try { + Assert.assertTrue(worker.awaitTermination(5, TimeUnit.SECONDS)); + } finally { + client.destroy(); + stopServer(server); + } + } + } + + private static class PausedProvider extends ProviderConfig { + + private final CountDownLatch entered = new CountDownLatch(1); + private final CountDownLatch release = new CountDownLatch(1); + private volatile boolean armed; + + @Override + public ProviderConfig setTimeout(int timeout) { + if (this.armed) { + this.entered.countDown(); + try { + E.checkState(this.release.await(5, TimeUnit.SECONDS), "RPC export barrier timed out"); + } catch (InterruptedException failure) { + Thread.currentThread().interrupt(); + throw new IllegalStateException(failure); + } + this.armed = false; + } + return super.setTimeout(timeout); } } diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerConfigTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerConfigTest.java index 1894665fe0..30be04035b 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerConfigTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerConfigTest.java @@ -31,6 +31,7 @@ import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; @@ -46,6 +47,7 @@ import org.apache.hugegraph.auth.HugeAuthenticator; import org.apache.hugegraph.auth.HugeGraphAuthProxy; import org.apache.hugegraph.auth.HugePermission; +import org.apache.hugegraph.auth.HugeResource; import org.apache.hugegraph.auth.RolePermission; import org.apache.hugegraph.backend.store.BackendFeatures; import org.apache.hugegraph.config.CoreOptions; @@ -65,6 +67,10 @@ import org.apache.hugegraph.testutil.Whitebox; import org.apache.hugegraph.util.E; import org.apache.hugegraph.util.Events; +import org.apache.hugegraph.meta.MetaManager; +import org.apache.tinkerpop.gremlin.server.Settings; +import org.apache.tinkerpop.gremlin.server.util.DefaultGraphManager; +import org.mockito.Mockito; import org.apache.tinkerpop.gremlin.groovy.jsr223.GremlinGroovyScriptEngine; import org.junit.AfterClass; import org.junit.BeforeClass; @@ -183,6 +189,78 @@ public void testAuthenticatedRpcCleanupPreservesOwnerUntilAdmin() throws Excepti } } + @Test + public void testAuthenticatedPublicationPreservesCallerScope() throws Exception { + OptionSpace.register("core", CoreOptions.instance()); + OptionSpace.register("rpc", RpcOptions.instance()); + OptionSpace.register("server", ServerOptions.instance()); + PropertiesConfiguration properties = new PropertiesConfiguration(); + properties.setProperty(ServerOptions.GRAPHS.name(), this.temporary.newFolder("lazy").getPath()); + properties.setProperty(ServerOptions.USE_PD.name(), false); + properties.setProperty(ServerOptions.GRAPH_LOAD_FROM_LOCAL_CONFIG.name(), false); + properties.setProperty(RpcOptions.RPC_SERVER_HOST.name(), "127.0.0.1"); + properties.setProperty(RpcOptions.RPC_SERVER_PORT.name(), 0); + try (ServerSocket reservation = new ServerSocket(0)) { + properties.setProperty(RpcOptions.RPC_REMOTE_URL.name(), "127.0.0.1:" + reservation.getLocalPort()); + } + GraphManager manager = new GraphManager(new HugeConfig(properties), new EventHub("lazy-auth-rpc")); + SharedMemoryGraph warm = null; + try { + Whitebox.invoke(GraphManager.class, "startRpcServer", manager); + String space = "rpcscope"; + String name = "lazy_auth"; + PropertiesConfiguration config = new PropertiesConfiguration(); + config.setProperty(CoreOptions.BACKEND.name(), "memory"); + config.setProperty(CoreOptions.STORE.name(), name); + config.setProperty(CoreOptions.GRAPH_SPACE.name(), space); + warm = new SharedMemoryGraph(new HugeConfig(config)); + warm.initBackend(); + HugeGraph owner = new HugeGraphAuthProxy(warm); + RolePermission role = new WriterRole(space, name); + Assert.assertFalse(RolePermission.isAdmin(role)); + String caller = new HugeAuthenticator.User("writer", role).toJson(); + AuthContext.setContext(caller); + TaskManager.setContext(caller); + HugeGraphAuthProxy.setRequestGraphSpace(space); + Whitebox.invoke(GraphManager.class, new Class[]{String.class, HugeGraph.class}, + "publishGraph", manager, owner.spaceGraphName(), owner); + Assert.assertSame(owner, manager.graph(space + "-" + name)); + Assert.assertEquals(caller, AuthContext.getContext()); + Assert.assertEquals(caller, TaskManager.getContext()); + Assert.assertEquals(space, HugeGraphAuthProxy.getRequestGraphSpace()); + Assert.assertEquals("writer", HugeGraphAuthProxy.username()); + RpcServer server = Whitebox.getInternalState(manager, "rpcServer"); + RpcClientProvider client = Whitebox.getInternalState(manager, "rpcClient"); + assertRpcResources(server, client, 2); + Assert.assertThrows(jakarta.ws.rs.ForbiddenException.class, owner::unregisterRpcServices); + assertRpcResources(server, client, 2); + } finally { + HugeGraph retained = warm; + HugeGraphAuthProxy.runAsAdmin(() -> { + manager.close(); + if (retained != null) { + try { + retained.close(); + } catch (Exception failure) { + throw new IllegalStateException(failure); + } finally { + HugeFactory.remove(retained); + } + } + }); + HugeGraphAuthProxy.resetContext(); + TaskManager.resetContext(); + } + } + + private static class WriterRole extends RolePermission { + + WriterRole(String space, String graph) { + this.add(space, graph, HugePermission.READ, HugeResource.ALL_RES); + this.add(space, graph, HugePermission.WRITE, HugeResource.ALL_RES); + } + } + public static class AuthSharedMemoryGraph { public static HugeGraph open(Configuration config) { @@ -192,15 +270,29 @@ public static HugeGraph open(Configuration config) { @Test public void testLegacyDefaultCleanupDoesNotAbortDropWithoutRpc() throws Exception { - this.checkLegacyDrop(false); + this.checkLegacyDrop(false, false); } @Test public void testActualCleanupFailureStillPropagates() throws Exception { - this.checkLegacyDrop(true); + this.checkLegacyDrop(true, false); + } + + @Test + public void testClearCleanupFailurePreservesOwnerForRetry() throws Exception { + this.checkLegacyDrop(true, true); + } + + @Test + public void testQuotaUpdateFailurePreservesOwnerAndCountForRetry() throws Exception { + this.checkLegacyDrop(false, false, true); } - private void checkLegacyDrop(boolean failCleanup) throws Exception { + private void checkLegacyDrop(boolean failCleanup, boolean clear) throws Exception { + this.checkLegacyDrop(failCleanup, clear, false); + } + + private void checkLegacyDrop(boolean failCleanup, boolean clear, boolean failQuota) throws Exception { OptionSpace.register("core", CoreOptions.instance()); OptionSpace.register("rpc", RpcOptions.instance()); OptionSpace.register("server", ServerOptions.instance()); @@ -220,10 +312,11 @@ private void checkLegacyDrop(boolean failCleanup) throws Exception { "\nbackend=memory\nstore=legacy_cleanup\n"); HugeGraph owner = delegate; IllegalStateException failure = new IllegalStateException("actual owner cleanup failed"); + AtomicBoolean failing = new AtomicBoolean(failCleanup); HugeGraph legacy = (HugeGraph) Proxy.newProxyInstance(HugeGraph.class.getClassLoader(), new Class[]{HugeGraph.class}, (proxy, method, args) -> { if ("unregisterRpcServices".equals(method.getName())) { - if (failCleanup) { + if (failing.get()) { throw failure; } return InvocationHandler.invokeDefault(proxy, method, args); @@ -236,6 +329,22 @@ private void checkLegacyDrop(boolean failCleanup) throws Exception { }); Map graphs = Whitebox.getInternalState(manager, "graphs"); graphs.put(legacy.spaceGraphName(), legacy); + DefaultGraphManager bindings = new DefaultGraphManager(new Settings()); + bindings.putGraph(legacy.spaceGraphName(), legacy); + CountDownLatch dropped = new CountDownLatch(1); + hub.listen(Events.GRAPH_DROP, event -> { + if (bindings.getGraph(legacy.spaceGraphName()) == legacy) { + try { + bindings.removeGraph(legacy.spaceGraphName()); + } catch (Exception failureToUnbind) { + throw new IllegalStateException(failureToUnbind); + } + } + dropped.countDown(); + return true; + }); + MetaManager meta = Mockito.mock(MetaManager.class); + Whitebox.setInternalState(manager, "metaManager", meta); RpcServer server = Whitebox.getInternalState(manager, "rpcServer"); RpcClientProvider client = Whitebox.getInternalState(manager, "rpcClient"); Assert.assertFalse(server.enabled()); @@ -243,25 +352,40 @@ private void checkLegacyDrop(boolean failCleanup) throws Exception { // Reach the normal PD-drop branch without booting PD or writing placement metadata. Whitebox.setInternalState(manager, "PDExist", true); Map spaces = Whitebox.getInternalState(manager, "graphSpaces"); - spaces.put("DEFAULT", new GraphSpace("DEFAULT")); - Thread.currentThread().setName("grpc-legacy-cleanup-test"); - if (failCleanup) { - Assert.assertThrows(IllegalStateException.class, () -> manager.dropGraph( - "DEFAULT", "legacy_cleanup", false), caught -> Assert.assertSame(failure, caught)); - Assert.assertFalse(owner.closed()); + GraphSpace space = new GraphSpace("DEFAULT"); + space.graphNumberUsed(5); + spaces.put("DEFAULT", space); + if (failQuota) { + Mockito.doThrow(failure).doNothing().when(meta).updateGraphSpaceConfig("DEFAULT", space); } else { - CountDownLatch dropped = new CountDownLatch(1); - hub.listen(Events.GRAPH_DROP, event -> { - dropped.countDown(); - return true; - }); - manager.dropGraph("DEFAULT", "legacy_cleanup", false); - Assert.assertTrue(dropped.await(5, TimeUnit.SECONDS)); - Assert.assertTrue(owner.closed()); + Thread.currentThread().setName("grpc-legacy-cleanup-test"); + } + if (failCleanup || failQuota) { + Assert.assertThrows(IllegalStateException.class, () -> manager.dropGraph( + "DEFAULT", "legacy_cleanup", clear), caught -> Assert.assertSame(failure, caught)); + Assert.assertEquals(!clear, owner.closed()); + Assert.assertSame(legacy, manager.graph(legacy.spaceGraphName())); + Assert.assertSame(legacy, bindings.getGraph(legacy.spaceGraphName())); Map factory = Whitebox.getInternalState(HugeFactory.class, "GRAPHS"); - Assert.assertFalse(factory.containsKey(legacy.spaceGraphName())); + Assert.assertSame(owner, factory.get(legacy.spaceGraphName())); + Assert.assertEquals(1L, dropped.getCount()); + if (failCleanup) { + Mockito.verifyNoInteractions(meta); + } + Assert.assertEquals(5, space.graphNumberUsed()); + failing.set(false); } + manager.dropGraph("DEFAULT", "legacy_cleanup", clear); + Assert.assertTrue(dropped.await(5, TimeUnit.SECONDS)); + Assert.assertTrue(owner.closed()); + Map factory = Whitebox.getInternalState(HugeFactory.class, "GRAPHS"); + Assert.assertFalse(factory.containsKey(legacy.spaceGraphName())); + Assert.assertNull(bindings.getGraph(legacy.spaceGraphName())); Assert.assertFalse(manager.graphs().contains(legacy.spaceGraphName())); + if (failQuota) { + Assert.assertEquals(4, space.graphNumberUsed()); + Mockito.verify(meta, Mockito.times(2)).updateGraphSpaceConfig("DEFAULT", space); + } } finally { Thread.currentThread().setName(originalThreadName); manager.close(); From a2e370843f8c10b7ff451778c906717ee5b273aa Mon Sep 17 00:00:00 2001 From: imbajin Date: Tue, 13 Oct 2026 01:07:04 +0800 Subject: [PATCH 04/13] fix(core): drain tasks before graph deletion - fence admission and track actual execution owners - wait for callbacks and transaction cleanup before deletion - retain graphs and schedulers when draining fails - keep core regressions and defer reload coordination --- .../apache/hugegraph/StandardHugeGraph.java | 92 ++++++------------- .../task/DistributedTaskScheduler.java | 2 +- 2 files changed, 28 insertions(+), 66 deletions(-) diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/StandardHugeGraph.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/StandardHugeGraph.java index 3db775e1a3..569e0fb9e0 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/StandardHugeGraph.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/StandardHugeGraph.java @@ -19,12 +19,12 @@ import org.apache.hugegraph.exception.HugeException; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Date; import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Future; @@ -167,7 +167,6 @@ public class StandardHugeGraph implements HugeGraph { private final EventHub schemaEventHub; private final EventHub graphEventHub; private final EventHub indexEventHub; - private final List rpcServiceCleanup = new ArrayList<>(); private final LocalCounter localCounter; private final RateLimiter writeRateLimiter; private final RateLimiter readRateLimiter; @@ -178,7 +177,7 @@ public class StandardHugeGraph implements HugeGraph { private final RamTable ramtable; private volatile boolean started; private volatile boolean closed; - private volatile GraphMode mode; + private final LoadingMode loadingMode = new LoadingMode(); private volatile GraphReadMode readMode; private volatile HugeVariables variables; private String graphSpace; @@ -226,7 +225,6 @@ public StandardHugeGraph(HugeConfig config) { this.started = false; this.closed = false; - this.mode = GraphMode.NONE; this.readMode = GraphReadMode.OLTP_ONLY; // Init process-wide static configs before lock, so that validation @@ -440,13 +438,24 @@ private void closeTx() { @Override public GraphMode mode() { - return this.mode; + return this.loadingMode.mode(); } @Override public void mode(GraphMode mode) { LOG.info("Graph {} will work in {} mode", this, mode); - this.mode = mode; + this.loadingMode.mode(mode); + } + + @Override + public Map loadingMode() { + return this.loadingMode.capability(); + } + + @Override + public Map loadingMode(String action, String instance, long epoch, + String token, String principal) { + return this.loadingMode.update(action, instance, epoch, token, principal); } @Override @@ -1179,7 +1188,6 @@ public synchronized void close() throws Exception { // stops its context. Owner-close errors do not imply native release. this.closed = true; for (Runnable close : new Runnable[]{ - this::unregisterRpcServices, () -> CachedGraphTransaction.closeGraph(this.params), () -> CachedSchemaTransaction.closeGraph(this.params), () -> CachedSchemaTransactionV2.closeGraph(this.params), @@ -1382,67 +1390,26 @@ public V option(TypedOption option) { } @Override - public synchronized void registerRpcServices(RpcServiceConfig4Server serverConfig, - RpcServiceConfig4Client clientConfig) { + public void registerRpcServices(RpcServiceConfig4Server serverConfig, + RpcServiceConfig4Client clientConfig) { /* * Skip register cache-rpc service if it's non-shared storage, * because we assume cache of non-shared storage is updated by raft. */ - if (!this.rpcServiceCleanup.isEmpty()) { - return; - } if (!this.backendStoreFeatures().supportsSharedStorage()) { return; } - E.checkState(!this.closed(), "Can't register RPC services for a closed graph"); - String graph = this.spaceGraphName(); - try { - GraphCacheNotifier graphProxy = clientConfig.serviceProxy(graph, GraphCacheNotifier.class); - this.rpcServiceCleanup.add(() -> clientConfig.removeServiceProxy( - graph, GraphCacheNotifier.class.getName(), graphProxy)); - HugeGraphCacheNotifier graphNotifier = new HugeGraphCacheNotifier(this.graphEventHub, graphProxy); - this.rpcServiceCleanup.add(graphNotifier::close); - String graphService = serverConfig.addService(graph, GraphCacheNotifier.class, graphNotifier); - this.rpcServiceCleanup.add(() -> serverConfig.removeService(graphService, graphNotifier)); - - SchemaCacheNotifier schemaProxy = clientConfig.serviceProxy(graph, SchemaCacheNotifier.class); - this.rpcServiceCleanup.add(() -> clientConfig.removeServiceProxy( - graph, SchemaCacheNotifier.class.getName(), schemaProxy)); - HugeSchemaCacheNotifier schemaNotifier = new HugeSchemaCacheNotifier(this.schemaEventHub, schemaProxy); - this.rpcServiceCleanup.add(schemaNotifier::close); - String schemaService = serverConfig.addService(graph, SchemaCacheNotifier.class, schemaNotifier); - this.rpcServiceCleanup.add(() -> serverConfig.removeService(schemaService, schemaNotifier)); - } catch (RuntimeException | Error failure) { - try { - this.unregisterRpcServices(); - } catch (RuntimeException | Error cleanup) { - failure.addSuppressed(cleanup); - } - throw failure; - } - } - @Override - public synchronized void unregisterRpcServices() { - Throwable failure = null; - for (int i = this.rpcServiceCleanup.size() - 1; i >= 0; i--) { - try { - this.rpcServiceCleanup.get(i).run(); - this.rpcServiceCleanup.remove(i); - } catch (RuntimeException | Error error) { - if (failure == null) { - failure = error; - } else if (failure != error) { - failure.addSuppressed(error); - } - } - } - if (failure instanceof Error) { - throw (Error) failure; - } - if (failure != null) { - throw (RuntimeException) failure; - } + Class clazz1 = GraphCacheNotifier.class; + // The proxy is sometimes unavailable (issue #664) + CacheNotifier proxy = clientConfig.serviceProxy(this.spaceGraphName(), clazz1); + serverConfig.addService(this.spaceGraphName(), clazz1, new HugeGraphCacheNotifier( + this.graphEventHub, proxy)); + + Class clazz2 = SchemaCacheNotifier.class; + proxy = clientConfig.serviceProxy(this.spaceGraphName(), clazz2); + serverConfig.addService(this.spaceGraphName(), clazz2, new HugeSchemaCacheNotifier( + this.schemaEventHub, proxy)); } private void waitUntilAllTasksCompleted() { @@ -1530,14 +1497,10 @@ private static class AbstractCacheNotifier implements CacheNotifier { private final EventHub hub; private final EventListener cacheEventListener; - private volatile boolean closed; public AbstractCacheNotifier(EventHub hub, CacheNotifier proxy) { this.hub = hub; this.cacheEventListener = event -> { - if (this.closed) { - return false; - } try { LOG.info("Received event: {}", event); Object[] args = event.args(); @@ -1581,7 +1544,6 @@ public AbstractCacheNotifier(EventHub hub, CacheNotifier proxy) { @Override public void close() { - this.closed = true; this.hub.unlisten(Events.CACHE, this.cacheEventListener); } diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/DistributedTaskScheduler.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/DistributedTaskScheduler.java index 0fe0e5b8f2..f29224bc2a 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/DistributedTaskScheduler.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/DistributedTaskScheduler.java @@ -125,7 +125,7 @@ private static boolean sleep(long ms) { } } - public void cronSchedule() { + public synchronized void cronSchedule() { // Perform periodic scheduling tasks // Check closed flag first to exit early From 5017535930b84a94692c9e104352f8f1e7eeaabf Mon Sep 17 00:00:00 2001 From: imbajin Date: Tue, 13 Oct 2026 02:05:22 +0800 Subject: [PATCH 05/13] fix(task): preserve shutdown and test phases - fence close without holding the cron monitor across queries - test the real cron barrier timeout and close retry - wait for callable entry before asserting task deletion --- .../org/apache/hugegraph/task/DistributedTaskScheduler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/DistributedTaskScheduler.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/DistributedTaskScheduler.java index f29224bc2a..0fe0e5b8f2 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/DistributedTaskScheduler.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/DistributedTaskScheduler.java @@ -125,7 +125,7 @@ private static boolean sleep(long ms) { } } - public synchronized void cronSchedule() { + public void cronSchedule() { // Perform periodic scheduling tasks // Check closed flag first to exit early From 445e4b302b77167ba4f4c03fa6b9b041a0d291e8 Mon Sep 17 00:00:00 2001 From: imbajin Date: Tue, 13 Oct 2026 11:57:46 +0800 Subject: [PATCH 06/13] feat(server): own loading mode per graph instance - Expose protocol capability and scoped loading owners. - Bind owner operations to the authenticated principal. - Fence delayed acquisition with epochs and terminal tokens. - Document retries, unsupported servers and recovery limits. --- .../hugegraph/api/profile/GraphsAPI.java | 34 ++++ .../hugegraph/auth/HugeGraphAuthProxy.java | 15 ++ .../java/org/apache/hugegraph/HugeGraph.java | 10 + .../org/apache/hugegraph/LoadingMode.java | 113 ++++++++++++ .../apache/hugegraph/unit/UnitTestSuite.java | 2 + .../unit/auth/HugeGraphAuthProxyTest.java | 30 +++ .../hugegraph/unit/core/LoadingModeTest.java | 173 ++++++++++++++++++ 7 files changed, 377 insertions(+) create mode 100644 hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/LoadingMode.java create mode 100644 hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/LoadingModeTest.java diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/GraphsAPI.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/GraphsAPI.java index 95505be80b..93176abb2d 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/GraphsAPI.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/GraphsAPI.java @@ -693,6 +693,40 @@ public Map mode(@Context GraphManager manager, return ImmutableMap.of("mode", g.mode()); } + @GET + @Timed + @Path("{name}/mode/owners") + @Produces(APPLICATION_JSON_WITH_CHARSET) + @RolesAllowed({"space", "$owner=$name"}) + public Map loadingMode(@Context GraphManager manager, + @PathParam("graphspace") String graphSpace, + @PathParam("name") String name) { + return graph(manager, graphSpace, name).loadingMode(); + } + + @PUT + @Timed + @Path("{name}/mode/owners") + @Consumes(APPLICATION_JSON) + @Produces(APPLICATION_JSON_WITH_CHARSET) + @RolesAllowed({"space", "$owner=$name"}) + public Map loadingMode(@Context GraphManager manager, + @PathParam("graphspace") String graphSpace, + @PathParam("name") String name, + JsonLoadingOwner owner) { + E.checkArgument(owner != null && owner.epoch != null, "Loading owner body and epoch are required"); + return graph(manager, graphSpace, name).loadingMode(owner.action, owner.instance, + owner.epoch, owner.token, ""); + } + + public static class JsonLoadingOwner { + + public String action; + public String instance; + public Long epoch; + public String token; + } + @PUT @Timed @Path("{name}/graph_read_mode") diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeGraphAuthProxy.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeGraphAuthProxy.java index 4e45c4d816..3d3ef44ca8 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeGraphAuthProxy.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeGraphAuthProxy.java @@ -812,6 +812,21 @@ public void mode(GraphMode mode) { this.hugegraph.mode(mode); } + @Override + public Map loadingMode() { + this.verifyPermission(HugePermission.WRITE, ResourceType.STATUS); + return this.hugegraph.loadingMode(); + } + + @Override + public Map loadingMode(String action, String instance, long epoch, + String token, String principal) { + this.verifyPermission(HugePermission.WRITE, ResourceType.STATUS); + Context context = getContext(); + E.checkState(context != null, "Missing authenticated loading owner context"); + return this.hugegraph.loadingMode(action, instance, epoch, token, context.user().username()); + } + @Override public GraphReadMode readMode() { this.verifyStatusPermission(); diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/HugeGraph.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/HugeGraph.java index 18f6ac1f1e..d473277181 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/HugeGraph.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/HugeGraph.java @@ -23,6 +23,7 @@ import java.util.HashSet; import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.function.Predicate; @@ -216,6 +217,15 @@ public interface HugeGraph extends Graph, HugeGraphSupplier { void mode(GraphMode mode); + default Map loadingMode() { + throw new UnsupportedOperationException("Loading ownership is not supported by this graph implementation"); + } + + default Map loadingMode(String action, String instance, long epoch, + String token, String principal) { + throw new UnsupportedOperationException("Loading ownership is not supported by this graph implementation"); + } + GraphReadMode readMode(); void readMode(GraphReadMode readMode); diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/LoadingMode.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/LoadingMode.java new file mode 100644 index 0000000000..2683761c63 --- /dev/null +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/LoadingMode.java @@ -0,0 +1,113 @@ +/* + * 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; + +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; +import java.util.UUID; + +import org.apache.hugegraph.type.define.GraphMode; +import org.apache.hugegraph.util.E; + +import com.google.common.collect.ImmutableMap; + +/** Same graph-instance ownership; this state is neither persisted nor distributed. */ +public final class LoadingMode { + + private static final int MAX_RELEASED = 1024; + + private final String instance = UUID.randomUUID().toString(); + private final Map active = new HashMap<>(); + private final Map released = new HashMap<>(); + private long epoch; + private volatile GraphMode mode = GraphMode.NONE; + + public GraphMode mode() { + return this.mode; + } + + public synchronized void mode(GraphMode mode) { + Objects.requireNonNull(mode, "mode"); + E.checkState(this.active.isEmpty(), "Loading owners are active; release them before setting graph mode"); + this.mode = mode; + } + + public synchronized Map capability() { + return ImmutableMap.of("protocol", 1, "instance", this.instance, "epoch", this.epoch); + } + + public synchronized Map update(String action, String instance, long epoch, + String token, String principal) { + E.checkArgument("acquire".equals(action) || "release".equals(action), + "Loading owner action must be acquire or release"); + E.checkArgument(this.instance.equals(instance), "Loading owner belongs to another graph instance"); + E.checkArgument(epoch >= 0 && epoch <= this.epoch, "Invalid loading owner epoch"); + E.checkArgument(token != null && token.length() == 36 && UUID.fromString(token).toString().equals(token), + "Loading owner token must be a canonical UUID"); + Objects.requireNonNull(principal, "principal"); + String key = epoch + "/" + token; + String owner = this.active.get(key); + if (owner != null) { + E.checkState(owner.equals(principal), "Loading owner belongs to another authenticated principal"); + if ("acquire".equals(action)) { + return this.receipt(epoch, token, "active"); + } + // Exhaustion must fail before changing an active registration. + E.checkState(epoch != this.epoch || this.released.size() < MAX_RELEASED || + this.epoch < Long.MAX_VALUE, "Loading owner epoch exhausted"); + this.active.remove(key); + if (this.active.isEmpty()) { + this.mode = GraphMode.NONE; + } + } else if ("acquire".equals(action)) { + if (epoch < this.epoch || this.released.containsKey(key)) { + return this.receipt(epoch, token, "released"); + } + E.checkState(this.active.isEmpty() ? this.mode == GraphMode.NONE : this.mode == GraphMode.LOADING, + "Cannot acquire loading ownership of manually set graph mode %s", this.mode); + this.active.put(key, principal); + this.mode = GraphMode.LOADING; + return this.receipt(epoch, token, "active"); + } + // Unknown release cancels an acquire which may still be in transit. + if (epoch == this.epoch) { + String releasedOwner = this.released.get(key); + E.checkState(releasedOwner == null || releasedOwner.equals(principal), + "Loading owner belongs to another authenticated principal"); + if (releasedOwner != null) { + return this.receipt(epoch, token, "released"); + } + if (this.released.size() == MAX_RELEASED) { + E.checkState(this.epoch < Long.MAX_VALUE, "Loading owner epoch exhausted"); + this.epoch++; + this.released.clear(); + // Old nonactive tuples are terminal; active old tuples remain registered. + } else { + this.released.put(key, principal); + } + } + return this.receipt(epoch, token, "released"); + } + + private Map receipt(long epoch, String token, String state) { + return ImmutableMap.builder().put("protocol", 1).put("instance", this.instance) + .put("epoch", epoch).put("token", token).put("state", state) + .put("mode", this.mode).build(); + } +} 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 3cd65b0d8f..300edb4b10 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 @@ -18,6 +18,7 @@ package org.apache.hugegraph.unit; import org.apache.hugegraph.HugeFactoryTest; +import org.apache.hugegraph.unit.core.LoadingModeTest; import org.apache.hugegraph.GraphDropPendingTest; import org.apache.hugegraph.StandardTaskAdmissionTest; import org.apache.hugegraph.DistributedTaskAdmissionTest; @@ -157,6 +158,7 @@ StandardAuthManagerV2Test.class, ContextTaskTest.class, HugeFactoryTest.class, + LoadingModeTest.class, GraphDropPendingTest.class, StandardTaskAdmissionTest.class, DistributedTaskAdmissionTest.class, diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/auth/HugeGraphAuthProxyTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/auth/HugeGraphAuthProxyTest.java index 13216d2164..b16d7b9d5b 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/auth/HugeGraphAuthProxyTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/auth/HugeGraphAuthProxyTest.java @@ -62,6 +62,36 @@ public class HugeGraphAuthProxyTest extends BaseUnitTest { + @Test + public void testLoadingOwnershipRequiresStatusWriteAndUsesAuthenticatedPrincipal() { + HugeGraph graph = Mockito.mock(HugeGraph.class); + HugeConfig config = Mockito.mock(HugeConfig.class); + Mockito.when(graph.name()).thenReturn("tc13_mode_auth"); + Mockito.when(graph.graphSpace()).thenReturn("DEFAULT"); + Mockito.when(graph.spaceGraphName()).thenReturn("tc13_mode_auth"); + Mockito.when(graph.configuration()).thenReturn(config); + Mockito.when(graph.authManager()).thenReturn(Mockito.mock(AuthManager.class)); + Mockito.when(graph.taskScheduler()).thenReturn(Mockito.mock(TaskScheduler.class)); + Mockito.when(config.get(AuthOptions.AUTH_CACHE_EXPIRE)).thenReturn(3600L); + Mockito.when(config.get(AuthOptions.AUTH_CACHE_CAPACITY)).thenReturn(100L); + Mockito.when(config.get(AuthOptions.AUTH_AUDIT_LOG_RATE)).thenReturn(1000D); + HugeGraphAuthProxy proxy = new HugeGraphAuthProxy(graph); + setContext(new HugeGraphAuthProxy.Context(new HugeAuthenticator.User( + "reader", RolePermission.role("DEFAULT", "tc13_mode_auth", HugePermission.READ)))); + Assert.assertThrows(jakarta.ws.rs.ForbiddenException.class, (Assert.ThrowableRunnable) proxy::loadingMode); + Assert.assertThrows(jakarta.ws.rs.ForbiddenException.class, + () -> proxy.loadingMode("acquire", "instance", 0, "token", "forged")); + Mockito.verify(graph, Mockito.never()).loadingMode(); + Mockito.verify(graph, Mockito.never()).loadingMode(Mockito.anyString(), Mockito.anyString(), + Mockito.anyLong(), Mockito.anyString(), Mockito.anyString()); + setContext(new HugeGraphAuthProxy.Context(new HugeAuthenticator.User( + "writer", RolePermission.role("DEFAULT", "tc13_mode_auth", HugePermission.WRITE)))); + proxy.loadingMode(); + proxy.loadingMode("acquire", "instance", 0, "token", "forged"); + Mockito.verify(graph).loadingMode(); + Mockito.verify(graph).loadingMode("acquire", "instance", 0, "token", "writer"); + } + @Test public void testJdk17ReflectionFilters() { Reflection.registerFieldsToFilter(ReflectionFilterTarget.class, "field"); diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/LoadingModeTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/LoadingModeTest.java new file mode 100644 index 0000000000..34b50fa697 --- /dev/null +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/LoadingModeTest.java @@ -0,0 +1,173 @@ +/* + * 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.util.Map; +import java.util.UUID; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; + +import org.apache.hugegraph.LoadingMode; +import org.apache.hugegraph.type.define.GraphMode; +import org.junit.Assert; +import org.junit.Test; + +public class LoadingModeTest { + + @Test + public void testTwoOwnersAndIdempotentRetry() { + LoadingMode mode = new LoadingMode(); + Map capability = mode.capability(); + String a = token(); + String b = token(); + Assert.assertEquals("active", update(mode, capability, a, "acquire").get("state")); + update(mode, capability, a, "acquire"); + update(mode, capability, b, "acquire"); + update(mode, capability, a, "release"); + update(mode, capability, a, "release"); + Assert.assertEquals(GraphMode.LOADING, mode.mode()); + Assert.assertEquals("released", update(mode, capability, a, "acquire").get("state")); + update(mode, capability, b, "release"); + Assert.assertEquals(GraphMode.NONE, mode.mode()); + } + + @Test + public void testUnknownReleaseCancelsLateAcquire() { + LoadingMode mode = new LoadingMode(); + Map capability = mode.capability(); + String token = token(); + update(mode, capability, token, "release"); + Assert.assertEquals("released", update(mode, capability, token, "acquire").get("state")); + Assert.assertEquals(GraphMode.NONE, mode.mode()); + } + + @Test + public void testTerminalCompactionKeepsOldActiveAndRejectsLateAcquire() { + LoadingMode mode = new LoadingMode(); + Map original = mode.capability(); + String b = token(); + String cancelled = token(); + update(mode, original, b, "acquire"); + update(mode, original, cancelled, "release"); + for (int i = 0; i < 2200; i++) { + update(mode, mode.capability(), token(), "release"); + } + Assert.assertTrue(((Number) mode.capability().get("epoch")).longValue() >= 2); + Assert.assertEquals("released", update(mode, original, cancelled, "acquire").get("state")); + Assert.assertEquals("released", update(mode, original, token(), "acquire").get("state")); + Assert.assertEquals("active", update(mode, original, b, "acquire").get("state")); + Assert.assertEquals(GraphMode.LOADING, mode.mode()); + String c = token(); + Map current = mode.capability(); + update(mode, current, c, "acquire"); + update(mode, original, b, "release"); + Assert.assertEquals(GraphMode.LOADING, mode.mode()); + update(mode, current, c, "release"); + Assert.assertEquals(GraphMode.NONE, mode.mode()); + } + + @Test + public void testLegacySetterCannotOverwriteOwnersOrAdoptManualMode() { + LoadingMode mode = new LoadingMode(); + Map capability = mode.capability(); + String a = token(); + for (GraphMode manual : GraphMode.values()) { + if (manual == GraphMode.NONE) { + continue; + } + mode.mode(manual); + Assert.assertThrows(IllegalStateException.class, () -> update(mode, capability, a, "acquire")); + Assert.assertEquals(manual, mode.mode()); + } + mode.mode(GraphMode.NONE); + update(mode, capability, a, "acquire"); + for (GraphMode legacy : GraphMode.values()) { + Assert.assertThrows(IllegalStateException.class, () -> mode.mode(legacy)); + Assert.assertEquals(GraphMode.LOADING, mode.mode()); + } + update(mode, capability, a, "release"); + mode.mode(GraphMode.NONE); + Assert.assertThrows(NullPointerException.class, () -> mode.mode(null)); + } + + @Test + public void testInstanceAndPrincipalBoundaries() { + LoadingMode mode = new LoadingMode(); + Map capability = mode.capability(); + String a = token(); + update(mode, capability, a, "acquire"); + String instance = (String) capability.get("instance"); + Assert.assertThrows(IllegalStateException.class, + () -> mode.update("release", instance, 0, a, "another-user")); + Assert.assertEquals(GraphMode.LOADING, mode.mode()); + Assert.assertThrows(IllegalArgumentException.class, + () -> mode.update("acquire", token(), 0, a, "user")); + Assert.assertThrows(IllegalArgumentException.class, + () -> mode.update("acquire", instance, 1, a, "user")); + update(mode, capability, a, "release"); + } + + @Test + public void testConcurrentAcquireAndLegacySetterAreAtomic() throws Exception { + LoadingMode mode = new LoadingMode(); + Map capability = mode.capability(); + String a = token(); + CountDownLatch start = new CountDownLatch(1); + ExecutorService executor = Executors.newFixedThreadPool(2); + try { + Future acquire = executor.submit(() -> { + Assert.assertTrue(start.await(5, TimeUnit.SECONDS)); + try { + update(mode, capability, a, "acquire"); + return true; + } catch (IllegalStateException expected) { + return false; + } + }); + Future legacy = executor.submit(() -> { + Assert.assertTrue(start.await(5, TimeUnit.SECONDS)); + try { + mode.mode(GraphMode.RESTORING); + return true; + } catch (IllegalStateException expected) { + return false; + } + }); + start.countDown(); + boolean acquired = acquire.get(10, TimeUnit.SECONDS); + Assert.assertNotEquals(acquired, legacy.get(10, TimeUnit.SECONDS)); + Assert.assertEquals(acquired ? GraphMode.LOADING : GraphMode.RESTORING, mode.mode()); + } finally { + executor.shutdownNow(); + Assert.assertTrue(executor.awaitTermination(10, TimeUnit.SECONDS)); + } + } + + private static String token() { + return UUID.randomUUID().toString(); + } + + private static Map update(LoadingMode mode, Map capability, + String token, String action) { + return mode.update(action, (String) capability.get("instance"), + ((Number) capability.get("epoch")).longValue(), token, "user"); + } +} From edf5500c54d53d4d6becdbd2975daa3e12c59cf7 Mon Sep 17 00:00:00 2001 From: imbajin Date: Tue, 13 Oct 2026 14:41:49 +0800 Subject: [PATCH 07/13] fix(server): preserve caller tx when cancel task - Preserve open caller transactions during synchronous cancellation. - Delegate default cancellation cleanup to existing scheduler owners. - Verify queued and running callbacks with committed sentinel readback. - Document caller and worker transaction responsibilities. --- docs/server-runtime.md | 8 ++ .../task/DistributedTaskScheduler.java | 18 ++-- .../hugegraph/task/StandardTaskScheduler.java | 7 +- .../apache/hugegraph/task/TaskCallable.java | 6 +- .../DistributedTaskAdmissionTest.java | 90 +++++++++++++++++++ .../hugegraph/StandardTaskAdmissionTest.java | 59 ++++++++++++ 6 files changed, 179 insertions(+), 9 deletions(-) diff --git a/docs/server-runtime.md b/docs/server-runtime.md index 811e8e3db1..2abacdaebe 100644 --- a/docs/server-runtime.md +++ b/docs/server-runtime.md @@ -463,6 +463,14 @@ thread. A failure while closing one owner must not skip cleanup of other owners server-info resources. Cleanup retains the first exception and attaches later cleanup failures as suppressed exceptions. +Synchronous task cancellation preserves a transaction already open on its caller, +including uncommitted graph writes. Cancellation still runs the normal completion and +status callbacks, but their default cleanup does not close that existing transaction. +The scheduler closes transactions opened by a cancellation callback when it had no +transaction before starting, and a running task retains its worker owner until that +worker's own cleanup finishes. The caller remains responsible for committing or rolling +back its original transaction. + Graph close attempts auth, scheduler and caller-transaction cleanup separately. If a scheduler remains registered or the graph reports active transactions, the graph remains open for a later close attempt. Pending local tasks retain their task database transaction diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/DistributedTaskScheduler.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/DistributedTaskScheduler.java index 0fe0e5b8f2..658596e240 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/DistributedTaskScheduler.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/DistributedTaskScheduler.java @@ -334,15 +334,18 @@ public void cancel(HugeTask task) { String context = TaskManager.getContext(); AtomicBoolean cleaned = new AtomicBoolean(true); TaskManager.ContextCallable owned = new TaskManager.ContextCallable<>(() -> { + boolean callerTxOpen = this.graph.graph().tx().isOpen(); try { this.cancelTask(task, true); return null; } finally { - try { - this.graph.closeTx(); - } catch (RuntimeException | Error error) { - cleaned.set(false); - throw error; + if (!callerTxOpen) { + try { + this.graph.closeTx(); + } catch (RuntimeException | Error error) { + cleaned.set(false); + throw error; + } } } }); @@ -924,10 +927,13 @@ private boolean cancelOwned(PendingTask pending, Callable action, boole pending.callbacks++; } TaskManager.ContextCallable owned = new TaskManager.ContextCallable<>(() -> { + boolean callerTxOpen = this.graph.graph().tx().isOpen(); try { return action.call(); } finally { - this.closeOwner(pending); + if (!callerTxOpen) { + this.closeOwner(pending); + } } }); Callable cancellation = () -> { diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/StandardTaskScheduler.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/StandardTaskScheduler.java index 8d91069553..b53742a54e 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/StandardTaskScheduler.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/StandardTaskScheduler.java @@ -348,6 +348,7 @@ public void initTaskCallable(HugeTask task) { @Override public void cancel(HugeTask task) { E.checkArgumentNotNull(task, "Task can't be null"); + boolean callerTxOpen = this.graph.graph().tx().isOpen(); HugeTask memTask; synchronized (this) { if (task.completed() || task.cancelling()) { @@ -379,8 +380,10 @@ public void cancel(HugeTask task) { if (memTask != null && memTask.isCancelled()) { this.cancelQueuedExecutions(task.id()); } - // Keep the callback registered until its transaction really closes. - this.graph.closeTx(); + // Keep the callback registered until its own transaction really closes. + if (!callerTxOpen) { + this.graph.closeTx(); + } synchronized (this) { this.cancellations--; } diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/TaskCallable.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/TaskCallable.java index 5872acd9dd..9d7c9a6b79 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/TaskCallable.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/TaskCallable.java @@ -56,7 +56,11 @@ public TaskCallable() { } protected void done() { - this.closeTx(); + // Future cancellation runs done() on its caller, whose transaction may predate the callback. + // The scheduler closes callback-owned transactions and the actual worker's transaction separately. + if (this.task == null || !this.task.isCancelled()) { + this.closeTx(); + } } protected void cancelled() { diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/DistributedTaskAdmissionTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/DistributedTaskAdmissionTest.java index bddb0c3ec3..8d1585f9bd 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/DistributedTaskAdmissionTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/DistributedTaskAdmissionTest.java @@ -190,6 +190,96 @@ public void testBusyWorkerLeavesPersistentBacklogInDatabase() throws Exception { } } + @Test + public void testQueuedDefaultCancellationPreservesCallerTransaction() throws Exception { + this.assertDefaultCancellationPreservesCallerTransaction(false); + } + + @Test + public void testRunningDefaultCancellationPreservesCallerTransaction() throws Exception { + this.assertDefaultCancellationPreservesCallerTransaction(true); + } + + private void assertDefaultCancellationPreservesCallerTransaction(boolean running) throws Exception { + try (Fixture fixture = new Fixture("caller_tx_" + running)) { + CountDownLatch started = new CountDownLatch(1); + CountDownLatch finish = new CountDownLatch(1); + if (!running) { + fixture.worker.armed.set(true); + } + HugeTask task = new HugeTask<>(IdGenerator.of(running ? -9999921L : -9999922L), null, + new EphemeralJob() { + @Override + public String type() { + return "test"; + } + + @Override + public Object execute() { + this.graph().vertices().hasNext(); + started.countDown(); + awaitUninterruptibly(finish); + return null; + } + }); + task.type("test"); + task.name("default-callback-caller-tx"); + try { + fixture.scheduler.schedule(task); + Assert.assertTrue((running ? started : fixture.worker.reached) + .await(10L, TimeUnit.SECONDS)); + this.openCallerTransaction(fixture.graph); + fixture.scheduler.cancel(task); + Assert.assertTrue(task.isCancelled()); + this.assertCallerTransactionCommits(fixture); + if (running) { + Assert.assertEquals(1, fixture.scheduler.pendingTasks()); + } + } finally { + finish.countDown(); + fixture.worker.release.countDown(); + } + fixture.assertWorkerClean(); + Assert.assertEquals(running ? 0L : 1L, started.getCount()); + } + } + + @Test + public void testCompletedCancellationPreservesCallerTransaction() throws Exception { + try (Fixture fixture = new Fixture("completed_caller_tx")) { + HugeTask task = task(true, new AtomicBoolean(), 9999923L); + fixture.scheduler.schedule(task).get(10L, TimeUnit.SECONDS); + fixture.assertWorkerClean(); + this.openCallerTransaction(fixture.graph); + fixture.scheduler.cancel(task); + Assert.assertFalse(task.isCancelled()); + this.assertCallerTransactionCommits(fixture); + Assert.assertEquals(0, fixture.scheduler.pendingTasks()); + } + } + + private void openCallerTransaction(HugeGraph graph) { + graph.schema().vertexLabel("caller").useCustomizeStringId().create(); + graph.tx().commit(); + graph.addVertex(org.apache.tinkerpop.gremlin.structure.T.id, "pending", + org.apache.tinkerpop.gremlin.structure.T.label, "caller"); + Assert.assertTrue(graph.tx().isOpen()); + } + + private void assertCallerTransactionCommits(Fixture fixture) throws Exception { + Assert.assertTrue(fixture.graph.tx().isOpen()); + Assert.assertTrue(fixture.graph.vertices("pending").hasNext()); + fixture.graph.tx().commit(); + Assert.assertFalse(fixture.graph.tx().isOpen()); + Assert.assertTrue(fixture.database.submit(() -> { + try { + return fixture.graph.vertices("pending").hasNext(); + } finally { + fixture.params.closeTx(); + } + }).get(10L, TimeUnit.SECONDS)); + } + @Test public void testQueuedOuterCancellationPreservesCallerTransaction() throws Exception { try (Fixture fixture = new Fixture("queued_future")) { diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/StandardTaskAdmissionTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/StandardTaskAdmissionTest.java index 1425693fa3..964b529ada 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/StandardTaskAdmissionTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/StandardTaskAdmissionTest.java @@ -45,6 +45,65 @@ public class StandardTaskAdmissionTest { + @Test + public void testQueuedDefaultCancellationPreservesCallerTransaction() throws Exception { + this.assertDefaultCancellationPreservesCallerTransaction(false); + } + + @Test + public void testRunningDefaultCancellationPreservesCallerTransaction() throws Exception { + this.assertDefaultCancellationPreservesCallerTransaction(true); + } + + private void assertDefaultCancellationPreservesCallerTransaction(boolean running) throws Exception { + try (Fixture fixture = new Fixture("standard_caller_tx_" + running)) { + CountDownLatch occupied = new CountDownLatch(1); + CountDownLatch release = new CountDownLatch(1); + if (!running) { + fixture.worker.submit(() -> { + occupied.countDown(); + await(release); + }); + Assert.assertTrue(occupied.await(10L, TimeUnit.SECONDS)); + } + BlockingJob job = new BlockingJob(); + HugeTask task = fixture.task(job); + try { + fixture.scheduler.schedule(task); + if (running) { + Assert.assertTrue(job.started.await(10L, TimeUnit.SECONDS)); + } + fixture.graph.schema().vertexLabel("caller").useCustomizeStringId().create(); + fixture.graph.tx().commit(); + fixture.graph.addVertex(T.id, "pending", T.label, "caller"); + fixture.scheduler.cancel(task); + Assert.assertTrue(task.isCancelled()); + Assert.assertTrue(fixture.graph.tx().isOpen()); + Assert.assertTrue(fixture.graph.vertices("pending").hasNext()); + fixture.graph.tx().commit(); + Assert.assertFalse(fixture.graph.tx().isOpen()); + Assert.assertTrue(fixture.database.submit(() -> { + try { + return fixture.graph.vertices("pending").hasNext(); + } finally { + fixture.params.closeTx(); + } + }).get(10L, TimeUnit.SECONDS)); + if (running) { + Assert.assertEquals(1, fixture.scheduler.pendingTasks()); + } + } finally { + release.countDown(); + job.release.countDown(); + } + fixture.worker.submit(() -> null).get(10L, TimeUnit.SECONDS); + Assert.assertEquals(0, fixture.scheduler.pendingTasks()); + ThreadLocal owners = Whitebox.getInternalState(fixture.graph.tx(), "transactions"); + Assert.assertNull(fixture.worker.submit(owners::get).get(10L, TimeUnit.SECONDS)); + Assert.assertEquals(running ? 0L : 1L, job.started.getCount()); + } + } + @Test public void testCancelledWorkerFencesAdmissionUntilActualExit() throws Exception { try (Fixture fixture = new Fixture("standard_cancel_worker")) { From ba1dbb916e899f0d49d211ea82b787f5c0ef40b1 Mon Sep 17 00:00:00 2001 From: imbajin Date: Tue, 13 Oct 2026 17:33:57 +0800 Subject: [PATCH 08/13] refactor(server): isolate loading ownership - Keep graph-local ownership, fixed handles and principal checks. - Move RPC and task cleanup changes out of the mode feature. - Preserve current Apache changes and document legacy compatibility. --- docs/server-runtime.md | 8 - .../rpc/RpcServiceConfig4Client.java | 4 - .../rpc/RpcServiceConfig4Server.java | 4 - .../hugegraph/rpc/RpcConsumerConfig.java | 75 +-- .../hugegraph/rpc/RpcProviderConfig.java | 50 +- .../org/apache/hugegraph/rpc/RpcServer.java | 6 +- .../hugegraph/unit/ServerClientTest.java | 109 ---- hugegraph-server/README.md | 4 +- .../hugegraph/auth/HugeGraphAuthProxy.java | 6 - .../java/org/apache/hugegraph/HugeGraph.java | 5 - .../task/DistributedTaskScheduler.java | 18 +- .../hugegraph/task/StandardTaskScheduler.java | 7 +- .../apache/hugegraph/task/TaskCallable.java | 6 +- .../DistributedTaskAdmissionTest.java | 90 --- .../hugegraph/StandardTaskAdmissionTest.java | 59 -- .../unit/core/GraphManagerConfigTest.java | 527 ------------------ 16 files changed, 45 insertions(+), 933 deletions(-) diff --git a/docs/server-runtime.md b/docs/server-runtime.md index 2abacdaebe..811e8e3db1 100644 --- a/docs/server-runtime.md +++ b/docs/server-runtime.md @@ -463,14 +463,6 @@ thread. A failure while closing one owner must not skip cleanup of other owners server-info resources. Cleanup retains the first exception and attaches later cleanup failures as suppressed exceptions. -Synchronous task cancellation preserves a transaction already open on its caller, -including uncommitted graph writes. Cancellation still runs the normal completion and -status callbacks, but their default cleanup does not close that existing transaction. -The scheduler closes transactions opened by a cancellation callback when it had no -transaction before starting, and a running task retains its worker owner until that -worker's own cleanup finishes. The caller remains responsible for committing or rolling -back its original transaction. - Graph close attempts auth, scheduler and caller-transaction cleanup separately. If a scheduler remains registered or the graph reports active transactions, the graph remains open for a later close attempt. Pending local tasks retain their task database transaction diff --git a/hugegraph-commons/hugegraph-common/src/main/java/org/apache/hugegraph/rpc/RpcServiceConfig4Client.java b/hugegraph-commons/hugegraph-common/src/main/java/org/apache/hugegraph/rpc/RpcServiceConfig4Client.java index e0557ebc21..b08d6327d1 100644 --- a/hugegraph-commons/hugegraph-common/src/main/java/org/apache/hugegraph/rpc/RpcServiceConfig4Client.java +++ b/hugegraph-commons/hugegraph-common/src/main/java/org/apache/hugegraph/rpc/RpcServiceConfig4Client.java @@ -32,8 +32,4 @@ default T serviceProxy(String graph, Class clazz) { } void removeAllServiceProxy(); - - default boolean removeServiceProxy(String graph, String interfaceId, Object proxy) { - throw new UnsupportedOperationException("Removing an individual RPC proxy is not supported"); - } } diff --git a/hugegraph-commons/hugegraph-common/src/main/java/org/apache/hugegraph/rpc/RpcServiceConfig4Server.java b/hugegraph-commons/hugegraph-common/src/main/java/org/apache/hugegraph/rpc/RpcServiceConfig4Server.java index 0c9eab15a8..cd0290900e 100644 --- a/hugegraph-commons/hugegraph-common/src/main/java/org/apache/hugegraph/rpc/RpcServiceConfig4Server.java +++ b/hugegraph-commons/hugegraph-common/src/main/java/org/apache/hugegraph/rpc/RpcServiceConfig4Server.java @@ -25,9 +25,5 @@ public interface RpcServiceConfig4Server { void removeService(String serviceId); - default boolean removeService(String serviceId, Object serviceImpl) { - throw new UnsupportedOperationException("Removing an RPC service by implementation is not supported"); - } - void removeAllService(); } diff --git a/hugegraph-commons/hugegraph-rpc/src/main/java/org/apache/hugegraph/rpc/RpcConsumerConfig.java b/hugegraph-commons/hugegraph-rpc/src/main/java/org/apache/hugegraph/rpc/RpcConsumerConfig.java index 1229ca318f..9459ac516a 100644 --- a/hugegraph-commons/hugegraph-rpc/src/main/java/org/apache/hugegraph/rpc/RpcConsumerConfig.java +++ b/hugegraph-commons/hugegraph-rpc/src/main/java/org/apache/hugegraph/rpc/RpcConsumerConfig.java @@ -21,9 +21,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.CopyOnWriteArrayList; -import java.util.concurrent.atomic.AtomicReference; import org.apache.hugegraph.config.HugeConfig; import org.apache.hugegraph.config.RpcOptions; @@ -42,6 +39,8 @@ import com.alipay.sofa.rpc.core.response.SofaResponse; import com.alipay.sofa.rpc.ext.Extension; import com.alipay.sofa.rpc.ext.ExtensionLoaderFactory; +import com.google.common.collect.Lists; +import com.google.common.collect.Maps; import com.google.common.collect.Sets; public class RpcConsumerConfig implements RpcServiceConfig4Client { @@ -60,8 +59,8 @@ public RpcConsumerConfig(HugeConfig config, String remoteUrls) { RpcCommonConfig.initRpcConfigs(config); this.conf = config; this.remoteUrls = remoteUrls; - this.configs = new ConcurrentHashMap<>(); - this.bootstraps = new CopyOnWriteArrayList<>(); + this.configs = Maps.newHashMap(); + this.bootstraps = Lists.newArrayList(); } @Override @@ -71,57 +70,10 @@ public T serviceProxy(String interfaceId) { @Override public T serviceProxy(String graph, String interfaceId) { - String serviceId = graph == null ? interfaceId : interfaceId + ":" + graph; - AtomicReference proxy = new AtomicReference<>(); - this.configs.compute(serviceId, (key, existing) -> { - @SuppressWarnings("unchecked") - ConsumerConfig config = existing == null ? this.consumerConfig(graph, interfaceId) : - (ConsumerConfig) existing; - ConsumerBootstrap bootstrap = Bootstraps.from(config); - try { - proxy.set(bootstrap.refer()); - this.bootstraps.add(bootstrap); - } catch (RuntimeException | Error failure) { - try { - bootstrap.unRefer(); - if (bootstrap.getCluster() != null) { - bootstrap.getCluster().destroy(); - } - } catch (RuntimeException | Error cleanup) { - failure.addSuppressed(cleanup); - } - throw failure; - } - return config; - }); - return proxy.get(); - } - - @Override - public boolean removeServiceProxy(String graph, String interfaceId, Object proxy) { - String serviceId = graph == null ? interfaceId : interfaceId + ":" + graph; - boolean[] removed = {false}; - this.configs.computeIfPresent(serviceId, (key, config) -> { - for (ConsumerBootstrap bootstrap : this.bootstraps) { - if (bootstrap.getConsumerConfig() == config && bootstrap.getProxyIns() == proxy) { - try { - bootstrap.unRefer(); - } finally { - bootstrap.getCluster().destroy(); - } - this.bootstraps.remove(bootstrap); - removed[0] = true; - break; - } - } - for (ConsumerBootstrap bootstrap : this.bootstraps) { - if (bootstrap.getConsumerConfig() == config) { - return config; - } - } - return null; - }); - return removed[0]; + ConsumerConfig config = this.consumerConfig(graph, interfaceId); + ConsumerBootstrap bootstrap = Bootstraps.from(config); + this.bootstraps.add(bootstrap); + return bootstrap.refer(); } @Override @@ -151,7 +103,15 @@ private ConsumerConfig consumerConfig(String graph, serviceId = interfaceId; } - ConsumerConfig consumerConfig = new ConsumerConfig<>(); + @SuppressWarnings("unchecked") + ConsumerConfig consumerConfig = (ConsumerConfig) + this.configs.get(serviceId); + if (consumerConfig != null) { + return consumerConfig; + } + + assert consumerConfig == null; + consumerConfig = new ConsumerConfig<>(); HugeConfig conf = this.conf; String protocol = conf.get(RpcOptions.RPC_PROTOCOL); @@ -178,6 +138,7 @@ private ConsumerConfig consumerConfig(String graph, .setSerialization(conf.get(RpcOptions.RPC_SERIALIZATION)) .setLoadBalancer(loadBalancer); + this.configs.put(serviceId, consumerConfig); return consumerConfig; } diff --git a/hugegraph-commons/hugegraph-rpc/src/main/java/org/apache/hugegraph/rpc/RpcProviderConfig.java b/hugegraph-commons/hugegraph-rpc/src/main/java/org/apache/hugegraph/rpc/RpcProviderConfig.java index 0ce2f17bf3..dafdfcf5a4 100644 --- a/hugegraph-commons/hugegraph-rpc/src/main/java/org/apache/hugegraph/rpc/RpcProviderConfig.java +++ b/hugegraph-commons/hugegraph-rpc/src/main/java/org/apache/hugegraph/rpc/RpcProviderConfig.java @@ -18,15 +18,14 @@ package org.apache.hugegraph.rpc; import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; import com.alipay.sofa.rpc.config.ProviderConfig; -import com.alipay.sofa.rpc.config.ServerConfig; import org.apache.hugegraph.util.E; +import com.google.common.collect.Maps; public class RpcProviderConfig implements RpcServiceConfig4Server { - private final Map> configs = new ConcurrentHashMap<>(); + private final Map> configs = Maps.newHashMap(); @Override public String addService(Class clazz, S serviceImpl) { @@ -55,54 +54,27 @@ private String addService(String graph, providerConfig.setInterfaceId(interfaceId) .setRef(serviceImpl); - E.checkArgument(this.configs.putIfAbsent(serviceId, providerConfig) == null, + E.checkArgument(!this.configs.containsKey(serviceId), "Not allowed to add service already exist: '%s'", serviceId); + this.configs.put(serviceId, providerConfig); return serviceId; } @Override public void removeService(String serviceId) { - boolean[] removed = {false}; - this.configs.computeIfPresent(serviceId, (key, config) -> { - config.unExport(); - removed[0] = true; - return null; - }); - E.checkArgument(removed[0], "The service '%s' doesn't exist", serviceId); - } - - @Override - public boolean removeService(String serviceId, Object serviceImpl) { - boolean[] removed = {false}; - this.configs.computeIfPresent(serviceId, (key, config) -> { - if (config.getRef() != serviceImpl) { - return config; - } - config.unExport(); - removed[0] = true; - return null; - }); - return removed[0]; + ProviderConfig config = this.configs.remove(serviceId); + E.checkArgument(config != null, + "The service '%s' doesn't exist", serviceId); + config.unExport(); } @Override public void removeAllService() { - for (String serviceId : this.configs.keySet()) { - this.configs.computeIfPresent(serviceId, (key, config) -> { - config.unExport(); - return null; - }); - } - } - - void exportAll(ServerConfig server, int timeout) { - for (String serviceId : this.configs.keySet()) { - this.configs.computeIfPresent(serviceId, (key, config) -> { - config.setServer(server).setTimeout(timeout).export(); - return config; - }); + for (ProviderConfig config : this.configs.values()) { + config.unExport(); } + this.configs.clear(); } public Map> configs() { diff --git a/hugegraph-commons/hugegraph-rpc/src/main/java/org/apache/hugegraph/rpc/RpcServer.java b/hugegraph-commons/hugegraph-rpc/src/main/java/org/apache/hugegraph/rpc/RpcServer.java index 1af841c1c7..7791ac8cd5 100644 --- a/hugegraph-commons/hugegraph-rpc/src/main/java/org/apache/hugegraph/rpc/RpcServer.java +++ b/hugegraph-commons/hugegraph-rpc/src/main/java/org/apache/hugegraph/rpc/RpcServer.java @@ -104,7 +104,11 @@ public void exportAll() { return; } int timeout = this.conf.get(RpcOptions.RPC_SERVER_TIMEOUT) * 1000; - this.configs.exportAll(this.serverConfig, timeout); + for (ProviderConfig providerConfig : configs.values()) { + providerConfig.setServer(this.serverConfig) + .setTimeout(timeout) + .export(); + } LOG.info("RpcServer started success on port {}", this.port()); } diff --git a/hugegraph-commons/hugegraph-rpc/src/test/java/org/apache/hugegraph/unit/ServerClientTest.java b/hugegraph-commons/hugegraph-rpc/src/test/java/org/apache/hugegraph/unit/ServerClientTest.java index 24b140b71c..07eb893c7d 100644 --- a/hugegraph-commons/hugegraph-rpc/src/test/java/org/apache/hugegraph/unit/ServerClientTest.java +++ b/hugegraph-commons/hugegraph-rpc/src/test/java/org/apache/hugegraph/unit/ServerClientTest.java @@ -22,12 +22,6 @@ import java.net.ServerSocket; import java.util.ArrayList; import java.util.List; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.Future; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; import org.apache.hugegraph.config.HugeConfig; import org.apache.hugegraph.rpc.RpcClientProvider; @@ -44,7 +38,6 @@ import org.junit.Test; import com.alipay.sofa.rpc.common.RpcOptions; -import com.alipay.sofa.rpc.config.ProviderConfig; import com.alipay.sofa.rpc.core.exception.SofaRpcException; import com.alipay.sofa.rpc.core.exception.SofaRpcRuntimeException; import com.google.common.collect.ImmutableMap; @@ -718,108 +711,6 @@ public void testUnexportNotExistService() { }); } - @Test - public void testRemoveServiceUsesImplementationIdentity() throws Exception { - RpcServer server = new RpcServer(config("server-random")); - GraphHelloServiceImpl original = new GraphHelloServiceImpl("graph"); - RpcProviderConfig providers = server.config(); - String serviceId = providers.addService("graph", HelloService.class, original); - PausedProvider provider = new PausedProvider(); - provider.setId(serviceId).setUniqueId("graph"); - provider.setInterfaceId(HelloService.class.getName()).setRef(original); - providers.configs().put(serviceId, provider); - server.exportAll(); - RpcClientProvider client = clientFor(server); - ExecutorService worker = Executors.newFixedThreadPool(2); - try { - HelloService proxy = client.config().serviceProxy("graph", HelloService.class); - Assert.assertFalse(providers.removeService(serviceId, new GraphHelloServiceImpl("graph"))); - Assert.assertEquals("graph: hello tom!", proxy.hello("tom")); - provider.armed = true; - Future exporting = worker.submit(server::exportAll); - Assert.assertTrue(provider.entered.await(5, TimeUnit.SECONDS)); - CountDownLatch removing = new CountDownLatch(1); - Future removal = worker.submit(() -> { - removing.countDown(); - return providers.removeService(serviceId, original); - }); - Assert.assertTrue(removing.await(5, TimeUnit.SECONDS)); - Assert.assertThrows(TimeoutException.class, () -> removal.get(1, TimeUnit.SECONDS)); - Assert.assertSame(provider, providers.configs().get(serviceId)); - provider.release.countDown(); - exporting.get(5, TimeUnit.SECONDS); - Assert.assertTrue(removal.get(5, TimeUnit.SECONDS)); - Assert.assertFalse(providers.configs().containsKey(serviceId)); - GraphHelloServiceImpl replacement = new GraphHelloServiceImpl("replacement"); - providers.addService("graph", HelloService.class, replacement); - server.exportAll(); - Assert.assertFalse(providers.removeService(serviceId, original)); - Assert.assertSame(replacement, providers.configs().get(serviceId).getRef()); - Assert.assertEquals("replacement: hello tom!", proxy.hello("tom")); - } finally { - provider.release.countDown(); - worker.shutdownNow(); - try { - Assert.assertTrue(worker.awaitTermination(5, TimeUnit.SECONDS)); - } finally { - client.destroy(); - stopServer(server); - } - } - } - - private static class PausedProvider extends ProviderConfig { - - private final CountDownLatch entered = new CountDownLatch(1); - private final CountDownLatch release = new CountDownLatch(1); - private volatile boolean armed; - - @Override - public ProviderConfig setTimeout(int timeout) { - if (this.armed) { - this.entered.countDown(); - try { - E.checkState(this.release.await(5, TimeUnit.SECONDS), "RPC export barrier timed out"); - } catch (InterruptedException failure) { - Thread.currentThread().interrupt(); - throw new IllegalStateException(failure); - } - this.armed = false; - } - return super.setTimeout(timeout); - } - } - - @Test - public void testRemoveServiceProxyUsesProxyIdentity() { - RpcServer server = new RpcServer(config("server-random")); - server.config().addService("graph", HelloService.class, new GraphHelloServiceImpl("graph")); - server.config().addService("other", HelloService.class, new GraphHelloServiceImpl("other")); - server.exportAll(); - RpcClientProvider client = clientFor(server); - try { - RpcConsumerConfig consumers = client.config(); - HelloService original = consumers.serviceProxy("graph", HelloService.class); - HelloService second = consumers.serviceProxy("graph", HelloService.class); - HelloService other = consumers.serviceProxy("other", HelloService.class); - Assert.assertNotSame(original, second); - Assert.assertTrue(consumers.removeServiceProxy("graph", HelloService.class.getName(), original)); - Assert.assertThrows(SofaRpcRuntimeException.class, () -> original.hello("tom")); - Assert.assertEquals("graph: hello tom!", second.hello("tom")); - Assert.assertFalse(consumers.removeServiceProxy("graph", HelloService.class.getName(), original)); - Assert.assertTrue(consumers.removeServiceProxy("graph", HelloService.class.getName(), second)); - List bootstraps = Whitebox.getInternalState(consumers, "bootstraps"); - Assert.assertEquals(1, bootstraps.size()); - HelloService replacement = consumers.serviceProxy("graph", HelloService.class); - Assert.assertFalse(consumers.removeServiceProxy("graph", HelloService.class.getName(), original)); - Assert.assertEquals("graph: hello tom!", replacement.hello("tom")); - Assert.assertEquals("other: hello tom!", other.hello("tom")); - } finally { - client.destroy(); - stopServer(server); - } - } - @Test public void testServerDisabled() { HugeConfig clientConf = config(false); diff --git a/hugegraph-server/README.md b/hugegraph-server/README.md index a2c24f4352..d533d82cf7 100644 --- a/hugegraph-server/README.md +++ b/hugegraph-server/README.md @@ -16,8 +16,8 @@ HugeGraph Server consists of two layers of functionality: the graph engine layer See [launcher GC options](../docs/server-runtime.md#launcher-gc-options) for explicit collector selection and caller JVM tuning. -See [cache RPC graph lifecycle](../docs/server-runtime.md#cache-rpc-graph-lifecycle) for dynamic -graph registration, resource cleanup and notification delivery limits. +See [local loading ownership](../docs/server-runtime.md#local-loading-ownership) for +concurrent import mode protection and its compatibility limits. ## Backend Evolution and Compatibility diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeGraphAuthProxy.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeGraphAuthProxy.java index 3d3ef44ca8..aae2b019d3 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeGraphAuthProxy.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeGraphAuthProxy.java @@ -913,12 +913,6 @@ public void registerRpcServices(RpcServiceConfig4Server serverConfig, this.hugegraph.registerRpcServices(serverConfig, clientConfig); } - @Override - public void unregisterRpcServices() { - this.verifyAdminPermission(); - this.hugegraph.unregisterRpcServices(); - } - @Override public void initBackend() { this.verifyAdminPermission(); diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/HugeGraph.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/HugeGraph.java index d473277181..c5c9cb2340 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/HugeGraph.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/HugeGraph.java @@ -305,11 +305,6 @@ default Predicate searchPredicate(String text) { void registerRpcServices(RpcServiceConfig4Server serverConfig, RpcServiceConfig4Client clientConfig); - default void unregisterRpcServices() { - // Legacy implementations do not participate in the owned RPC cleanup contract. - // Implementations owning graph-scoped RPC resources must override this hook. - } - default List mapPkId2Name(Collection ids) { List names = new ArrayList<>(ids.size()); for (Id id : ids) { diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/DistributedTaskScheduler.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/DistributedTaskScheduler.java index 658596e240..0fe0e5b8f2 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/DistributedTaskScheduler.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/DistributedTaskScheduler.java @@ -334,18 +334,15 @@ public void cancel(HugeTask task) { String context = TaskManager.getContext(); AtomicBoolean cleaned = new AtomicBoolean(true); TaskManager.ContextCallable owned = new TaskManager.ContextCallable<>(() -> { - boolean callerTxOpen = this.graph.graph().tx().isOpen(); try { this.cancelTask(task, true); return null; } finally { - if (!callerTxOpen) { - try { - this.graph.closeTx(); - } catch (RuntimeException | Error error) { - cleaned.set(false); - throw error; - } + try { + this.graph.closeTx(); + } catch (RuntimeException | Error error) { + cleaned.set(false); + throw error; } } }); @@ -927,13 +924,10 @@ private boolean cancelOwned(PendingTask pending, Callable action, boole pending.callbacks++; } TaskManager.ContextCallable owned = new TaskManager.ContextCallable<>(() -> { - boolean callerTxOpen = this.graph.graph().tx().isOpen(); try { return action.call(); } finally { - if (!callerTxOpen) { - this.closeOwner(pending); - } + this.closeOwner(pending); } }); Callable cancellation = () -> { diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/StandardTaskScheduler.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/StandardTaskScheduler.java index b53742a54e..8d91069553 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/StandardTaskScheduler.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/StandardTaskScheduler.java @@ -348,7 +348,6 @@ public void initTaskCallable(HugeTask task) { @Override public void cancel(HugeTask task) { E.checkArgumentNotNull(task, "Task can't be null"); - boolean callerTxOpen = this.graph.graph().tx().isOpen(); HugeTask memTask; synchronized (this) { if (task.completed() || task.cancelling()) { @@ -380,10 +379,8 @@ public void cancel(HugeTask task) { if (memTask != null && memTask.isCancelled()) { this.cancelQueuedExecutions(task.id()); } - // Keep the callback registered until its own transaction really closes. - if (!callerTxOpen) { - this.graph.closeTx(); - } + // Keep the callback registered until its transaction really closes. + this.graph.closeTx(); synchronized (this) { this.cancellations--; } diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/TaskCallable.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/TaskCallable.java index 9d7c9a6b79..5872acd9dd 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/TaskCallable.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/task/TaskCallable.java @@ -56,11 +56,7 @@ public TaskCallable() { } protected void done() { - // Future cancellation runs done() on its caller, whose transaction may predate the callback. - // The scheduler closes callback-owned transactions and the actual worker's transaction separately. - if (this.task == null || !this.task.isCancelled()) { - this.closeTx(); - } + this.closeTx(); } protected void cancelled() { diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/DistributedTaskAdmissionTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/DistributedTaskAdmissionTest.java index 8d1585f9bd..bddb0c3ec3 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/DistributedTaskAdmissionTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/DistributedTaskAdmissionTest.java @@ -190,96 +190,6 @@ public void testBusyWorkerLeavesPersistentBacklogInDatabase() throws Exception { } } - @Test - public void testQueuedDefaultCancellationPreservesCallerTransaction() throws Exception { - this.assertDefaultCancellationPreservesCallerTransaction(false); - } - - @Test - public void testRunningDefaultCancellationPreservesCallerTransaction() throws Exception { - this.assertDefaultCancellationPreservesCallerTransaction(true); - } - - private void assertDefaultCancellationPreservesCallerTransaction(boolean running) throws Exception { - try (Fixture fixture = new Fixture("caller_tx_" + running)) { - CountDownLatch started = new CountDownLatch(1); - CountDownLatch finish = new CountDownLatch(1); - if (!running) { - fixture.worker.armed.set(true); - } - HugeTask task = new HugeTask<>(IdGenerator.of(running ? -9999921L : -9999922L), null, - new EphemeralJob() { - @Override - public String type() { - return "test"; - } - - @Override - public Object execute() { - this.graph().vertices().hasNext(); - started.countDown(); - awaitUninterruptibly(finish); - return null; - } - }); - task.type("test"); - task.name("default-callback-caller-tx"); - try { - fixture.scheduler.schedule(task); - Assert.assertTrue((running ? started : fixture.worker.reached) - .await(10L, TimeUnit.SECONDS)); - this.openCallerTransaction(fixture.graph); - fixture.scheduler.cancel(task); - Assert.assertTrue(task.isCancelled()); - this.assertCallerTransactionCommits(fixture); - if (running) { - Assert.assertEquals(1, fixture.scheduler.pendingTasks()); - } - } finally { - finish.countDown(); - fixture.worker.release.countDown(); - } - fixture.assertWorkerClean(); - Assert.assertEquals(running ? 0L : 1L, started.getCount()); - } - } - - @Test - public void testCompletedCancellationPreservesCallerTransaction() throws Exception { - try (Fixture fixture = new Fixture("completed_caller_tx")) { - HugeTask task = task(true, new AtomicBoolean(), 9999923L); - fixture.scheduler.schedule(task).get(10L, TimeUnit.SECONDS); - fixture.assertWorkerClean(); - this.openCallerTransaction(fixture.graph); - fixture.scheduler.cancel(task); - Assert.assertFalse(task.isCancelled()); - this.assertCallerTransactionCommits(fixture); - Assert.assertEquals(0, fixture.scheduler.pendingTasks()); - } - } - - private void openCallerTransaction(HugeGraph graph) { - graph.schema().vertexLabel("caller").useCustomizeStringId().create(); - graph.tx().commit(); - graph.addVertex(org.apache.tinkerpop.gremlin.structure.T.id, "pending", - org.apache.tinkerpop.gremlin.structure.T.label, "caller"); - Assert.assertTrue(graph.tx().isOpen()); - } - - private void assertCallerTransactionCommits(Fixture fixture) throws Exception { - Assert.assertTrue(fixture.graph.tx().isOpen()); - Assert.assertTrue(fixture.graph.vertices("pending").hasNext()); - fixture.graph.tx().commit(); - Assert.assertFalse(fixture.graph.tx().isOpen()); - Assert.assertTrue(fixture.database.submit(() -> { - try { - return fixture.graph.vertices("pending").hasNext(); - } finally { - fixture.params.closeTx(); - } - }).get(10L, TimeUnit.SECONDS)); - } - @Test public void testQueuedOuterCancellationPreservesCallerTransaction() throws Exception { try (Fixture fixture = new Fixture("queued_future")) { diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/StandardTaskAdmissionTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/StandardTaskAdmissionTest.java index 964b529ada..1425693fa3 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/StandardTaskAdmissionTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/StandardTaskAdmissionTest.java @@ -45,65 +45,6 @@ public class StandardTaskAdmissionTest { - @Test - public void testQueuedDefaultCancellationPreservesCallerTransaction() throws Exception { - this.assertDefaultCancellationPreservesCallerTransaction(false); - } - - @Test - public void testRunningDefaultCancellationPreservesCallerTransaction() throws Exception { - this.assertDefaultCancellationPreservesCallerTransaction(true); - } - - private void assertDefaultCancellationPreservesCallerTransaction(boolean running) throws Exception { - try (Fixture fixture = new Fixture("standard_caller_tx_" + running)) { - CountDownLatch occupied = new CountDownLatch(1); - CountDownLatch release = new CountDownLatch(1); - if (!running) { - fixture.worker.submit(() -> { - occupied.countDown(); - await(release); - }); - Assert.assertTrue(occupied.await(10L, TimeUnit.SECONDS)); - } - BlockingJob job = new BlockingJob(); - HugeTask task = fixture.task(job); - try { - fixture.scheduler.schedule(task); - if (running) { - Assert.assertTrue(job.started.await(10L, TimeUnit.SECONDS)); - } - fixture.graph.schema().vertexLabel("caller").useCustomizeStringId().create(); - fixture.graph.tx().commit(); - fixture.graph.addVertex(T.id, "pending", T.label, "caller"); - fixture.scheduler.cancel(task); - Assert.assertTrue(task.isCancelled()); - Assert.assertTrue(fixture.graph.tx().isOpen()); - Assert.assertTrue(fixture.graph.vertices("pending").hasNext()); - fixture.graph.tx().commit(); - Assert.assertFalse(fixture.graph.tx().isOpen()); - Assert.assertTrue(fixture.database.submit(() -> { - try { - return fixture.graph.vertices("pending").hasNext(); - } finally { - fixture.params.closeTx(); - } - }).get(10L, TimeUnit.SECONDS)); - if (running) { - Assert.assertEquals(1, fixture.scheduler.pendingTasks()); - } - } finally { - release.countDown(); - job.release.countDown(); - } - fixture.worker.submit(() -> null).get(10L, TimeUnit.SECONDS); - Assert.assertEquals(0, fixture.scheduler.pendingTasks()); - ThreadLocal owners = Whitebox.getInternalState(fixture.graph.tx(), "transactions"); - Assert.assertNull(fixture.worker.submit(owners::get).get(10L, TimeUnit.SECONDS)); - Assert.assertEquals(running ? 0L : 1L, job.started.getCount()); - } - } - @Test public void testCancelledWorkerFencesAdmissionUntilActualExit() throws Exception { try (Fixture fixture = new Fixture("standard_cancel_worker")) { diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerConfigTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerConfigTest.java index 30be04035b..b8601c22d5 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerConfigTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/GraphManagerConfigTest.java @@ -17,548 +17,21 @@ package org.apache.hugegraph.unit.core; -import java.lang.reflect.InvocationHandler; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Proxy; -import java.lang.management.ManagementFactory; -import java.lang.management.ThreadInfo; -import java.net.ServerSocket; import java.util.HashMap; -import java.util.List; import java.util.Map; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.concurrent.Future; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.concurrent.atomic.AtomicReference; -import javax.script.ScriptException; -import javax.script.SimpleBindings; - -import org.apache.commons.configuration2.Configuration; import org.apache.commons.configuration2.PropertiesConfiguration; -import org.apache.hugegraph.HugeFactory; -import org.apache.hugegraph.HugeGraph; -import org.apache.hugegraph.StandardHugeGraph; -import org.apache.hugegraph.auth.AuthContext; -import org.apache.hugegraph.auth.HugeAuthenticator; -import org.apache.hugegraph.auth.HugeGraphAuthProxy; -import org.apache.hugegraph.auth.HugePermission; -import org.apache.hugegraph.auth.HugeResource; -import org.apache.hugegraph.auth.RolePermission; -import org.apache.hugegraph.backend.store.BackendFeatures; import org.apache.hugegraph.config.CoreOptions; import org.apache.hugegraph.config.HugeConfig; -import org.apache.hugegraph.config.OptionSpace; -import org.apache.hugegraph.config.RpcOptions; import org.apache.hugegraph.config.ServerOptions; import org.apache.hugegraph.core.GraphManager; import org.apache.hugegraph.event.EventHub; -import org.apache.hugegraph.event.EventListener; -import org.apache.hugegraph.exception.HugeException; -import org.apache.hugegraph.rpc.RpcClientProvider; -import org.apache.hugegraph.rpc.RpcServer; -import org.apache.hugegraph.space.GraphSpace; -import org.apache.hugegraph.task.TaskManager; import org.apache.hugegraph.testutil.Assert; import org.apache.hugegraph.testutil.Whitebox; -import org.apache.hugegraph.util.E; -import org.apache.hugegraph.util.Events; -import org.apache.hugegraph.meta.MetaManager; -import org.apache.tinkerpop.gremlin.server.Settings; -import org.apache.tinkerpop.gremlin.server.util.DefaultGraphManager; -import org.mockito.Mockito; -import org.apache.tinkerpop.gremlin.groovy.jsr223.GremlinGroovyScriptEngine; -import org.junit.AfterClass; -import org.junit.BeforeClass; -import org.junit.Rule; import org.junit.Test; -import org.junit.rules.TemporaryFolder; public class GraphManagerConfigTest { - private static ExecutorService previousEventExecutor; - private static ExecutorService previousTaskDbExecutor; - private static ExecutorService taskDbExecutor; - - @BeforeClass - public static void initEventExecutor() { - // Other suite classes may have destroyed the global EventHub executor. - previousEventExecutor = Whitebox.getInternalState(EventHub.class, "executor"); - Whitebox.setInternalState(EventHub.class, "executor", null); - EventHub.init(2); - previousTaskDbExecutor = Whitebox.getInternalState(TaskManager.instance(), "taskDbExecutor"); - taskDbExecutor = Executors.newSingleThreadExecutor( - runnable -> new Thread(runnable, "task-db-worker-rpc-config-test")); - Whitebox.setInternalState(TaskManager.instance(), "taskDbExecutor", taskDbExecutor); - } - - @AfterClass - public static void restoreEventExecutor() throws InterruptedException { - try { - Assert.assertTrue(EventHub.destroy(5)); - } finally { - Whitebox.setInternalState(EventHub.class, "executor", previousEventExecutor); - taskDbExecutor.shutdown(); - try { - Assert.assertTrue(taskDbExecutor.awaitTermination(5, TimeUnit.SECONDS)); - } finally { - Whitebox.setInternalState(TaskManager.instance(), "taskDbExecutor", previousTaskDbExecutor); - } - } - } - - @Rule - public TemporaryFolder temporary = new TemporaryFolder(); - - @Test - public void testAuthenticatedRpcCleanupPreservesOwnerUntilAdmin() throws Exception { - OptionSpace.register("core", CoreOptions.instance()); - OptionSpace.register("rpc", RpcOptions.instance()); - OptionSpace.register("server", ServerOptions.instance()); - PropertiesConfiguration properties = new PropertiesConfiguration(); - properties.setProperty(ServerOptions.GRAPHS.name(), this.temporary.newFolder("auth").getPath()); - properties.setProperty(ServerOptions.USE_PD.name(), false); - properties.setProperty(ServerOptions.GRAPH_LOAD_FROM_LOCAL_CONFIG.name(), false); - properties.setProperty(ServerOptions.ENABLE_DYNAMIC_CREATE_DROP.name(), true); - properties.setProperty(RpcOptions.RPC_SERVER_HOST.name(), "127.0.0.1"); - properties.setProperty(RpcOptions.RPC_SERVER_PORT.name(), 0); - try (ServerSocket reservation = new ServerSocket(0)) { - properties.setProperty(RpcOptions.RPC_REMOTE_URL.name(), "127.0.0.1:" + reservation.getLocalPort()); - } - EventHub hub = new EventHub("authenticated-rpc-cleanup"); - GraphManager manager = new GraphManager(new HugeConfig(properties), hub); - String originalThreadName = Thread.currentThread().getName(); - AuthContext.setContext(HugeAuthenticator.User.ADMIN.toJson()); - TaskManager.setContext(HugeAuthenticator.User.ADMIN.toJson()); - try { - String factory = "gremlin.graph=" + AuthSharedMemoryGraph.class.getName() + "\nbackend=memory\nstore="; - manager.createGraphLocal("auth_startup", factory + "auth_startup\n"); - Whitebox.invoke(GraphManager.class, "startRpcServer", manager); - HugeGraph owner = manager.createGraphLocal("auth_late", factory + "auth_late\n"); - RpcServer server = Whitebox.getInternalState(manager, "rpcServer"); - RpcClientProvider client = Whitebox.getInternalState(manager, "rpcClient"); - assertRpcResources(server, client, 4); - EventListener rejection = event -> { - throw new IllegalStateException("reject authenticated candidate"); - }; - hub.listen(Events.GRAPH_CREATE, rejection); - try { - Assert.assertThrows(HugeException.class, () -> manager.createGraphLocal( - "auth_rejected", factory + "auth_rejected\n")); - } finally { - hub.unlisten(Events.GRAPH_CREATE, rejection); - } - assertRpcResources(server, client, 4); - TaskManager.resetContext(); - RolePermission execute = RolePermission.role(owner.graphSpace(), owner.name(), HugePermission.EXECUTE); - Assert.assertFalse(RolePermission.isAdmin(execute)); - AuthContext.setContext(new HugeAuthenticator.User("reader", execute).toJson()); - SimpleBindings bindings = new SimpleBindings(); - bindings.put("graph", owner); - GremlinGroovyScriptEngine engine = new GremlinGroovyScriptEngine(); - Assert.assertThrows(ScriptException.class, () -> engine.eval("graph.unregisterRpcServices()", bindings), - error -> Assert.assertContains("Permission denied", error.getMessage())); - assertRpcResources(server, client, 4); - AuthContext.resetContext(); - Assert.assertThrows(IllegalStateException.class, owner::unregisterRpcServices, - error -> Assert.assertContains("Missing authentication context", error.getMessage())); - assertRpcResources(server, client, 4); - HugeGraphAuthProxy.runAsAdmin(owner::unregisterRpcServices); - assertRpcResources(server, client, 2); - AuthContext.setContext(HugeAuthenticator.User.ADMIN.toJson()); - // Use the normal PD-drop branch with explicit admin identity and no placement metadata writes. - Whitebox.setInternalState(manager, "PDExist", true); - Map spaces = Whitebox.getInternalState(manager, "graphSpaces"); - spaces.put("DEFAULT", new GraphSpace("DEFAULT")); - Thread.currentThread().setName("grpc-auth-cleanup-test"); - manager.dropGraph("DEFAULT", "auth_late", false); - Assert.assertTrue(owner.closed()); - assertRpcResources(server, client, 2); - } finally { - Thread.currentThread().setName(originalThreadName); - try { - HugeGraphAuthProxy.runAsAdmin(manager::close); - } finally { - HugeGraphAuthProxy.resetContext(); - TaskManager.resetContext(); - } - } - } - - @Test - public void testAuthenticatedPublicationPreservesCallerScope() throws Exception { - OptionSpace.register("core", CoreOptions.instance()); - OptionSpace.register("rpc", RpcOptions.instance()); - OptionSpace.register("server", ServerOptions.instance()); - PropertiesConfiguration properties = new PropertiesConfiguration(); - properties.setProperty(ServerOptions.GRAPHS.name(), this.temporary.newFolder("lazy").getPath()); - properties.setProperty(ServerOptions.USE_PD.name(), false); - properties.setProperty(ServerOptions.GRAPH_LOAD_FROM_LOCAL_CONFIG.name(), false); - properties.setProperty(RpcOptions.RPC_SERVER_HOST.name(), "127.0.0.1"); - properties.setProperty(RpcOptions.RPC_SERVER_PORT.name(), 0); - try (ServerSocket reservation = new ServerSocket(0)) { - properties.setProperty(RpcOptions.RPC_REMOTE_URL.name(), "127.0.0.1:" + reservation.getLocalPort()); - } - GraphManager manager = new GraphManager(new HugeConfig(properties), new EventHub("lazy-auth-rpc")); - SharedMemoryGraph warm = null; - try { - Whitebox.invoke(GraphManager.class, "startRpcServer", manager); - String space = "rpcscope"; - String name = "lazy_auth"; - PropertiesConfiguration config = new PropertiesConfiguration(); - config.setProperty(CoreOptions.BACKEND.name(), "memory"); - config.setProperty(CoreOptions.STORE.name(), name); - config.setProperty(CoreOptions.GRAPH_SPACE.name(), space); - warm = new SharedMemoryGraph(new HugeConfig(config)); - warm.initBackend(); - HugeGraph owner = new HugeGraphAuthProxy(warm); - RolePermission role = new WriterRole(space, name); - Assert.assertFalse(RolePermission.isAdmin(role)); - String caller = new HugeAuthenticator.User("writer", role).toJson(); - AuthContext.setContext(caller); - TaskManager.setContext(caller); - HugeGraphAuthProxy.setRequestGraphSpace(space); - Whitebox.invoke(GraphManager.class, new Class[]{String.class, HugeGraph.class}, - "publishGraph", manager, owner.spaceGraphName(), owner); - Assert.assertSame(owner, manager.graph(space + "-" + name)); - Assert.assertEquals(caller, AuthContext.getContext()); - Assert.assertEquals(caller, TaskManager.getContext()); - Assert.assertEquals(space, HugeGraphAuthProxy.getRequestGraphSpace()); - Assert.assertEquals("writer", HugeGraphAuthProxy.username()); - RpcServer server = Whitebox.getInternalState(manager, "rpcServer"); - RpcClientProvider client = Whitebox.getInternalState(manager, "rpcClient"); - assertRpcResources(server, client, 2); - Assert.assertThrows(jakarta.ws.rs.ForbiddenException.class, owner::unregisterRpcServices); - assertRpcResources(server, client, 2); - } finally { - HugeGraph retained = warm; - HugeGraphAuthProxy.runAsAdmin(() -> { - manager.close(); - if (retained != null) { - try { - retained.close(); - } catch (Exception failure) { - throw new IllegalStateException(failure); - } finally { - HugeFactory.remove(retained); - } - } - }); - HugeGraphAuthProxy.resetContext(); - TaskManager.resetContext(); - } - } - - private static class WriterRole extends RolePermission { - - WriterRole(String space, String graph) { - this.add(space, graph, HugePermission.READ, HugeResource.ALL_RES); - this.add(space, graph, HugePermission.WRITE, HugeResource.ALL_RES); - } - } - - public static class AuthSharedMemoryGraph { - - public static HugeGraph open(Configuration config) { - return new HugeGraphAuthProxy(new SharedMemoryGraph(new HugeConfig(config))); - } - } - - @Test - public void testLegacyDefaultCleanupDoesNotAbortDropWithoutRpc() throws Exception { - this.checkLegacyDrop(false, false); - } - - @Test - public void testActualCleanupFailureStillPropagates() throws Exception { - this.checkLegacyDrop(true, false); - } - - @Test - public void testClearCleanupFailurePreservesOwnerForRetry() throws Exception { - this.checkLegacyDrop(true, true); - } - - @Test - public void testQuotaUpdateFailurePreservesOwnerAndCountForRetry() throws Exception { - this.checkLegacyDrop(false, false, true); - } - - private void checkLegacyDrop(boolean failCleanup, boolean clear) throws Exception { - this.checkLegacyDrop(failCleanup, clear, false); - } - - private void checkLegacyDrop(boolean failCleanup, boolean clear, boolean failQuota) throws Exception { - OptionSpace.register("core", CoreOptions.instance()); - OptionSpace.register("rpc", RpcOptions.instance()); - OptionSpace.register("server", ServerOptions.instance()); - PropertiesConfiguration properties = new PropertiesConfiguration(); - properties.setProperty(ServerOptions.GRAPHS.name(), this.temporary.newFolder("legacy").getPath()); - properties.setProperty(ServerOptions.GRAPH_LOAD_FROM_LOCAL_CONFIG.name(), false); - properties.setProperty(ServerOptions.USE_PD.name(), false); - properties.setProperty(ServerOptions.ENABLE_DYNAMIC_CREATE_DROP.name(), true); - properties.setProperty(RpcOptions.RPC_SERVER_HOST.name(), ""); - properties.setProperty(RpcOptions.RPC_REMOTE_URL.name(), ""); - EventHub hub = new EventHub("legacy-rpc-cleanup-test"); - GraphManager manager = new GraphManager(new HugeConfig(properties), hub); - HugeGraph delegate = null; - String originalThreadName = Thread.currentThread().getName(); - try { - delegate = manager.createGraphLocal("legacy_cleanup", "gremlin.graph=" + HugeFactory.class.getName() + - "\nbackend=memory\nstore=legacy_cleanup\n"); - HugeGraph owner = delegate; - IllegalStateException failure = new IllegalStateException("actual owner cleanup failed"); - AtomicBoolean failing = new AtomicBoolean(failCleanup); - HugeGraph legacy = (HugeGraph) Proxy.newProxyInstance(HugeGraph.class.getClassLoader(), - new Class[]{HugeGraph.class}, (proxy, method, args) -> { - if ("unregisterRpcServices".equals(method.getName())) { - if (failing.get()) { - throw failure; - } - return InvocationHandler.invokeDefault(proxy, method, args); - } - try { - return method.invoke(owner, args); - } catch (InvocationTargetException error) { - throw error.getCause(); - } - }); - Map graphs = Whitebox.getInternalState(manager, "graphs"); - graphs.put(legacy.spaceGraphName(), legacy); - DefaultGraphManager bindings = new DefaultGraphManager(new Settings()); - bindings.putGraph(legacy.spaceGraphName(), legacy); - CountDownLatch dropped = new CountDownLatch(1); - hub.listen(Events.GRAPH_DROP, event -> { - if (bindings.getGraph(legacy.spaceGraphName()) == legacy) { - try { - bindings.removeGraph(legacy.spaceGraphName()); - } catch (Exception failureToUnbind) { - throw new IllegalStateException(failureToUnbind); - } - } - dropped.countDown(); - return true; - }); - MetaManager meta = Mockito.mock(MetaManager.class); - Whitebox.setInternalState(manager, "metaManager", meta); - RpcServer server = Whitebox.getInternalState(manager, "rpcServer"); - RpcClientProvider client = Whitebox.getInternalState(manager, "rpcClient"); - Assert.assertFalse(server.enabled()); - Assert.assertFalse(client.enabled()); - // Reach the normal PD-drop branch without booting PD or writing placement metadata. - Whitebox.setInternalState(manager, "PDExist", true); - Map spaces = Whitebox.getInternalState(manager, "graphSpaces"); - GraphSpace space = new GraphSpace("DEFAULT"); - space.graphNumberUsed(5); - spaces.put("DEFAULT", space); - if (failQuota) { - Mockito.doThrow(failure).doNothing().when(meta).updateGraphSpaceConfig("DEFAULT", space); - } else { - Thread.currentThread().setName("grpc-legacy-cleanup-test"); - } - if (failCleanup || failQuota) { - Assert.assertThrows(IllegalStateException.class, () -> manager.dropGraph( - "DEFAULT", "legacy_cleanup", clear), caught -> Assert.assertSame(failure, caught)); - Assert.assertEquals(!clear, owner.closed()); - Assert.assertSame(legacy, manager.graph(legacy.spaceGraphName())); - Assert.assertSame(legacy, bindings.getGraph(legacy.spaceGraphName())); - Map factory = Whitebox.getInternalState(HugeFactory.class, "GRAPHS"); - Assert.assertSame(owner, factory.get(legacy.spaceGraphName())); - Assert.assertEquals(1L, dropped.getCount()); - if (failCleanup) { - Mockito.verifyNoInteractions(meta); - } - Assert.assertEquals(5, space.graphNumberUsed()); - failing.set(false); - } - manager.dropGraph("DEFAULT", "legacy_cleanup", clear); - Assert.assertTrue(dropped.await(5, TimeUnit.SECONDS)); - Assert.assertTrue(owner.closed()); - Map factory = Whitebox.getInternalState(HugeFactory.class, "GRAPHS"); - Assert.assertFalse(factory.containsKey(legacy.spaceGraphName())); - Assert.assertNull(bindings.getGraph(legacy.spaceGraphName())); - Assert.assertFalse(manager.graphs().contains(legacy.spaceGraphName())); - if (failQuota) { - Assert.assertEquals(4, space.graphNumberUsed()); - Mockito.verify(meta, Mockito.times(2)).updateGraphSpaceConfig("DEFAULT", space); - } - } finally { - Thread.currentThread().setName(originalThreadName); - manager.close(); - if (delegate != null) { - delegate.close(); - HugeFactory.remove(delegate); - } - } - } - - @Test - public void testLateGraphRpcPublicationRollbackAndReplacement() throws Exception { - OptionSpace.register("core", CoreOptions.instance()); - OptionSpace.register("rpc", RpcOptions.instance()); - OptionSpace.register("server", ServerOptions.instance()); - PropertiesConfiguration properties = new PropertiesConfiguration(); - properties.setProperty(ServerOptions.GRAPHS.name(), this.temporary.newFolder("graphs").getPath()); - properties.setProperty(ServerOptions.USE_PD.name(), false); - properties.setProperty(ServerOptions.GRAPH_LOAD_FROM_LOCAL_CONFIG.name(), false); - properties.setProperty(ServerOptions.ENABLE_DYNAMIC_CREATE_DROP.name(), true); - properties.setProperty(RpcOptions.RPC_SERVER_HOST.name(), "127.0.0.1"); - properties.setProperty(RpcOptions.RPC_SERVER_PORT.name(), 0); - try (ServerSocket reservation = new ServerSocket(0)) { - properties.setProperty(RpcOptions.RPC_REMOTE_URL.name(), "127.0.0.1:" + reservation.getLocalPort()); - } - EventHub hub = new EventHub("rpc-publication-test"); - GraphManager manager = new GraphManager(new HugeConfig(properties), hub); - BarrierRpcServer server = new BarrierRpcServer(new HugeConfig(properties)); - Whitebox.setInternalState(manager, "rpcServer", server); - RpcClientProvider client = Whitebox.getInternalState(manager, "rpcClient"); - ExecutorService initializer = Executors.newFixedThreadPool(2); - HugeGraph startup = null; - HugeGraph replacement = null; - try { - startup = manager.createGraphLocal("rpc_startup", sharedMemoryConfig("rpc_startup")); - Assert.assertEquals(0, server.config().configs().size()); - HugeGraph startupOwner = startup; - AtomicReference initThread = new AtomicReference<>(); - Future initialization = initializer.submit(() -> { - initThread.set(Thread.currentThread()); - try { - Whitebox.invoke(manager.getClass(), "startRpcServer", manager); - } finally { - Whitebox.invoke(StandardHugeGraph.class, "closeCurrentThreadTransaction", startupOwner); - } - }); - Assert.assertTrue(server.entered.await(5, TimeUnit.SECONDS)); - Assert.assertEquals(2, server.config().configs().size()); - AtomicReference publisher = new AtomicReference<>(); - Future publication = initializer.submit(() -> { - publisher.set(Thread.currentThread()); - HugeGraph graph = manager.createGraphLocal("rpc_late", sharedMemoryConfig("rpc_late")); - Whitebox.invoke(StandardHugeGraph.class, "closeCurrentThreadTransaction", graph); - return graph; - }); - awaitManagerGate(publisher, initThread.get()); - Assert.assertFalse(manager.graphs().contains("DEFAULT-rpc_late")); - assertRpcResources(server, client, 2); - server.release.countDown(); - initialization.get(5, TimeUnit.SECONDS); - HugeGraph original = publication.get(5, TimeUnit.SECONDS); - Assert.assertEquals(2, server.exports.get()); - assertRpcResources(server, client, 4); - - EventListener failure = event -> { - throw new IllegalStateException("reject candidate after RPC registration"); - }; - hub.listen(Events.GRAPH_CREATE, failure); - try { - Assert.assertThrows(HugeException.class, () -> manager.createGraphLocal( - "rpc_rejected", sharedMemoryConfig("rpc_rejected"))); - } finally { - hub.unlisten(Events.GRAPH_CREATE, failure); - } - Assert.assertFalse(manager.graphs().contains("DEFAULT-rpc_rejected")); - assertRpcResources(server, client, 4); - - manager.dropGraphLocal("rpc_late"); - assertRpcResources(server, client, 2); - replacement = manager.createGraphLocal("rpc_late", sharedMemoryConfig("rpc_late")); - hub.notifySync(Events.GRAPH_DROP, original); - Assert.assertSame(replacement, manager.graph("DEFAULT-rpc_late")); - assertRpcResources(server, client, 4); - manager.close(); - List bootstraps = Whitebox.getInternalState(client.config(), "bootstraps"); - Assert.assertEquals(0, server.config().configs().size()); - Assert.assertEquals(0, bootstraps.size()); - } finally { - server.release.countDown(); - initializer.shutdownNow(); - initializer.awaitTermination(5, TimeUnit.SECONDS); - manager.close(); - if (startup != null) { - HugeFactory.remove(startup); - } - if (replacement != null) { - HugeFactory.remove(replacement); - } - } - } - - private static void awaitManagerGate(AtomicReference publisher, Thread initializer) throws Exception { - long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(5); - while (System.nanoTime() < deadline) { - Thread thread = publisher.get(); - ThreadInfo info = thread == null ? null : ManagementFactory.getThreadMXBean().getThreadInfo(thread.getId()); - if (info != null && info.getThreadState() == Thread.State.BLOCKED && - info.getLockOwnerId() == initializer.getId()) { - Assert.assertContains(GraphManager.class.getName(), info.getLockName()); - return; - } - Thread.sleep(5); - } - Assert.fail("Graph publisher did not block on the startup manager gate"); - } - - private static class BarrierRpcServer extends RpcServer { - - private final CountDownLatch entered = new CountDownLatch(1); - private final CountDownLatch release = new CountDownLatch(1); - private final AtomicInteger exports = new AtomicInteger(); - - BarrierRpcServer(HugeConfig config) { - super(config); - } - - @Override - public void exportAll() { - if (this.exports.incrementAndGet() == 1) { - this.entered.countDown(); - try { - E.checkState(this.release.await(5, TimeUnit.SECONDS), "RPC export barrier timed out"); - } catch (InterruptedException failure) { - Thread.currentThread().interrupt(); - throw new IllegalStateException(failure); - } - } - super.exportAll(); - } - } - - private static void assertRpcResources(RpcServer server, RpcClientProvider client, int count) { - Assert.assertEquals(count, server.config().configs().size()); - Map configs = Whitebox.getInternalState(client.config(), "configs"); - List bootstraps = Whitebox.getInternalState(client.config(), "bootstraps"); - Assert.assertEquals(count, configs.size()); - Assert.assertEquals(count, bootstraps.size()); - } - - private static String sharedMemoryConfig(String name) { - return "gremlin.graph=" + SharedMemoryGraph.class.getName() + "\nbackend=memory\nstore=" + name + "\n"; - } - - // Exercises cache RPC registration, not shared-backend DML or consistency. - public static class SharedMemoryGraph extends StandardHugeGraph { - - public SharedMemoryGraph(HugeConfig config) { - super(config); - } - - public static SharedMemoryGraph open(Configuration config) { - return new SharedMemoryGraph(new HugeConfig(config)); - } - - @Override - public BackendFeatures backendStoreFeatures() { - BackendFeatures features = super.backendStoreFeatures(); - return (BackendFeatures) Proxy.newProxyInstance(BackendFeatures.class.getClassLoader(), - new Class[]{BackendFeatures.class}, (proxy, method, args) -> - "supportsSharedStorage".equals(method.getName()) ? true : method.invoke(features, args)); - } - } - @Test public void testAttachExplicitLocalResourceLimits() { PropertiesConfiguration properties = new PropertiesConfiguration(); From 623be2fdcdf11e545b93caf76ba6054625bf1a7f Mon Sep 17 00:00:00 2001 From: imbajin Date: Wed, 14 Oct 2026 05:37:51 +0800 Subject: [PATCH 09/13] fix(server): fence privileged loading recovery Require administrator checks before owner recovery. Fence recovery with graph instance, epoch and revision. Map expected loading conflicts to HTTP 409. Document stop and drain prerequisites and cover late owners. --- docs/server-runtime.md | 28 ++++++++- .../hugegraph/api/profile/GraphsAPI.java | 35 +++++++++-- .../hugegraph/auth/HugeGraphAuthProxy.java | 6 ++ .../java/org/apache/hugegraph/HugeGraph.java | 4 ++ .../org/apache/hugegraph/LoadingMode.java | 60 ++++++++++++++++--- .../apache/hugegraph/StandardHugeGraph.java | 7 ++- .../unit/api/space/GraphSpaceAPITest.java | 42 +++++++++++++ .../unit/auth/HugeGraphAuthProxyTest.java | 6 ++ .../hugegraph/unit/core/LoadingModeTest.java | 58 ++++++++++++++++++ 9 files changed, 232 insertions(+), 14 deletions(-) diff --git a/docs/server-runtime.md b/docs/server-runtime.md index 811e8e3db1..49d6732b94 100644 --- a/docs/server-runtime.md +++ b/docs/server-runtime.md @@ -545,7 +545,7 @@ cannot be adopted. Original enum setters reject every request while owners are a including bare LOADING; their original behavior remains available without active owners. `GET /graphspaces/{space}/graphs/{graph}/mode/owners` returns -`{"protocol":1,"instance":"","epoch":0}`. `PUT` on that path accepts +`{"protocol":1,"instance":"","epoch":0,"revision":0}`. `PUT` on that path accepts `{"action":"acquire","instance":"","epoch":0,"token":""}`; `release` uses the same tuple. Receipts echo protocol/instance/epoch/token and return `state` (`active` or `released`) and the effective `mode`. Both operations use existing @@ -562,7 +562,31 @@ Server and graph instance. It does not fence ordinary data requests across load balancers, node failover, graph replacement or Server restart. There is no automatic expiry: after kill -9 or permanent disconnection, stop the import and its in-flight writes before retrying release with the same handle and identity. If that handle is -lost, stop all related imports before reopening the graph instance. +lost, an administrator can recover without that token: + +1. Stop every related import and drain its in-flight writes. Keep ordinary writers + stopped too; the owner protocol does not fence graph data requests. +2. Read the current instance, epoch and revision from the owners endpoint on the same Server. +3. Send `PUT` to that endpoint with + `{"action":"recover","instance":"","epoch":0,"revision":0,"confirm":"I'm sure all imports have stopped"}`. + +Recovery requires administrator permission at both the REST and graph-proxy layers, +the exact current instance/epoch/revision and +the explicit confirmation. It clears owners, advances the epoch and returns NONE; +late acquire/release requests from the retired epoch cannot register an owner again. +The receipt contains the new epoch and `state: recovered`, without tokens. A stale +recovery request returns a conflict and cannot clear an import started in a newer +epoch. Owner registration/release and manual mode changes advance the revision, so +an import started after inspection also rejects the stale recovery request even within +the same epoch. After an uncertain result, inspect current state rather than blindly +retrying with a newly fetched epoch/revision. This operation does not stop or drain imports itself. + +The same recovery procedure can reset manual RESTORING or MERGING after that work +has stopped and drained; restore completion keeps its historical RESTORING mode. +Before ordinary import on an owner-capable Server, explicitly recover or set NONE +only after inspecting the completed work and confirming there are no active imports. +Expected loading-owner and manual-mode conflicts return HTTP 409; invalid requests +return 400 and unauthorized recovery is rejected by the authentication layer. Loaders retain historical ordinary-import and restore behavior with Servers that do not provide this API. Concurrent owner protection applies only when this API is diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/GraphsAPI.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/GraphsAPI.java index 93176abb2d..ac7309d727 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/GraphsAPI.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/GraphsAPI.java @@ -31,6 +31,7 @@ import org.apache.commons.lang3.StringUtils; import org.apache.hugegraph.exception.HugeException; import org.apache.hugegraph.HugeGraph; +import org.apache.hugegraph.LoadingMode; import org.apache.hugegraph.api.API; import org.apache.hugegraph.api.filter.StatusFilter; import org.apache.hugegraph.auth.AuthManager; @@ -56,6 +57,7 @@ import jakarta.annotation.security.RolesAllowed; import jakarta.inject.Singleton; import jakarta.ws.rs.Consumes; +import jakarta.ws.rs.ClientErrorException; import jakarta.ws.rs.DELETE; import jakarta.ws.rs.ForbiddenException; import jakarta.ws.rs.GET; @@ -68,6 +70,7 @@ import jakarta.ws.rs.QueryParam; import jakarta.ws.rs.core.Context; import jakarta.ws.rs.core.SecurityContext; +import jakarta.ws.rs.core.Response; @Path("graphspaces/{graphspace}/graphs") @Singleton @@ -78,6 +81,7 @@ public class GraphsAPI extends API { private static final String CONFIRM_CLEAR = "I'm sure to delete all data"; private static final String CONFIRM_DROP = "I'm sure to drop the graph"; + private static final String CONFIRM_RECOVER = "I'm sure all imports have stopped"; private static final String GRAPH_DESCRIPTION = "description"; private static final String GRAPH_ACTION = "action"; private static final String UPDATE = "update"; @@ -674,7 +678,11 @@ public Map mode(@Context GraphManager manager, E.checkArgument(mode != null, "Graph mode can't be null"); HugeGraph g = graph(manager, graphSpace, name); - g.mode(mode); + try { + g.mode(mode); + } catch (LoadingMode.ConflictException e) { + throw new ClientErrorException(e.getMessage(), Response.Status.CONFLICT, e); + } return ImmutableMap.of("mode", mode); } @@ -713,10 +721,27 @@ public Map loadingMode(@Context GraphManager manager, public Map loadingMode(@Context GraphManager manager, @PathParam("graphspace") String graphSpace, @PathParam("name") String name, - JsonLoadingOwner owner) { + JsonLoadingOwner owner, + @Context SecurityContext securityContext) { E.checkArgument(owner != null && owner.epoch != null, "Loading owner body and epoch are required"); - return graph(manager, graphSpace, name).loadingMode(owner.action, owner.instance, - owner.epoch, owner.token, ""); + if ("recover".equals(owner.action)) { + if (securityContext == null || !securityContext.isUserInRole("admin")) { + throw new ForbiddenException("Loading recovery requires administrator permission"); + } + E.checkArgument(CONFIRM_RECOVER.equals(owner.confirm), + "Stop and drain all related imports, then confirm with '%s'", CONFIRM_RECOVER); + E.checkArgument(owner.revision != null && owner.revision >= 0, + "Current loading state revision is required for recovery"); + } + try { + HugeGraph graph = graph(manager, graphSpace, name); + if ("recover".equals(owner.action)) { + return graph.recoverLoadingMode(owner.instance, owner.epoch, owner.revision); + } + return graph.loadingMode(owner.action, owner.instance, owner.epoch, owner.token, ""); + } catch (LoadingMode.ConflictException e) { + throw new ClientErrorException(e.getMessage(), Response.Status.CONFLICT, e); + } } public static class JsonLoadingOwner { @@ -725,6 +750,8 @@ public static class JsonLoadingOwner { public String instance; public Long epoch; public String token; + public String confirm; + public Long revision; } @PUT diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeGraphAuthProxy.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeGraphAuthProxy.java index aae2b019d3..1967ed0724 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeGraphAuthProxy.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/auth/HugeGraphAuthProxy.java @@ -827,6 +827,12 @@ public Map loadingMode(String action, String instance, long epoc return this.hugegraph.loadingMode(action, instance, epoch, token, context.user().username()); } + @Override + public Map recoverLoadingMode(String instance, long epoch, long revision) { + this.verifyAdminPermission(); + return this.hugegraph.recoverLoadingMode(instance, epoch, revision); + } + @Override public GraphReadMode readMode() { this.verifyStatusPermission(); diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/HugeGraph.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/HugeGraph.java index c5c9cb2340..661453e75f 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/HugeGraph.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/HugeGraph.java @@ -226,6 +226,10 @@ default Map loadingMode(String action, String instance, long epo throw new UnsupportedOperationException("Loading ownership is not supported by this graph implementation"); } + default Map recoverLoadingMode(String instance, long epoch, long revision) { + throw new UnsupportedOperationException("Loading recovery is not supported by this graph implementation"); + } + GraphReadMode readMode(); void readMode(GraphReadMode readMode); diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/LoadingMode.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/LoadingMode.java index 2683761c63..a2ab2c1f95 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/LoadingMode.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/LoadingMode.java @@ -36,6 +36,7 @@ public final class LoadingMode { private final Map active = new HashMap<>(); private final Map released = new HashMap<>(); private long epoch; + private long revision; private volatile GraphMode mode = GraphMode.NONE; public GraphMode mode() { @@ -44,12 +45,31 @@ public GraphMode mode() { public synchronized void mode(GraphMode mode) { Objects.requireNonNull(mode, "mode"); - E.checkState(this.active.isEmpty(), "Loading owners are active; release them before setting graph mode"); + checkState(this.active.isEmpty(), "Loading owners are active; release them before setting graph mode"); + if (this.mode != mode) { + this.advanceRevision(); + } this.mode = mode; } public synchronized Map capability() { - return ImmutableMap.of("protocol", 1, "instance", this.instance, "epoch", this.epoch); + return ImmutableMap.of("protocol", 1, "instance", this.instance, "epoch", this.epoch, + "revision", this.revision); + } + + public synchronized Map recover(String instance, long epoch, long revision) { + E.checkArgument(this.instance.equals(instance), "Loading owner belongs to another graph instance"); + E.checkArgument(epoch >= 0 && revision >= 0, "Invalid loading recovery epoch or revision"); + checkState(epoch == this.epoch && revision == this.revision, + "Loading state changed; inspect it before recovery"); + checkState(this.epoch < Long.MAX_VALUE, "Loading owner epoch exhausted"); + this.advanceRevision(); + this.epoch++; + this.active.clear(); + this.released.clear(); + this.mode = GraphMode.NONE; + return ImmutableMap.builder().putAll(this.capability()) + .put("state", "recovered").put("mode", this.mode).build(); } public synchronized Map update(String action, String instance, long epoch, @@ -64,13 +84,14 @@ public synchronized Map update(String action, String instance, l String key = epoch + "/" + token; String owner = this.active.get(key); if (owner != null) { - E.checkState(owner.equals(principal), "Loading owner belongs to another authenticated principal"); + checkState(owner.equals(principal), "Loading owner belongs to another authenticated principal"); if ("acquire".equals(action)) { return this.receipt(epoch, token, "active"); } // Exhaustion must fail before changing an active registration. - E.checkState(epoch != this.epoch || this.released.size() < MAX_RELEASED || + checkState(epoch != this.epoch || this.released.size() < MAX_RELEASED || this.epoch < Long.MAX_VALUE, "Loading owner epoch exhausted"); + this.advanceRevision(); this.active.remove(key); if (this.active.isEmpty()) { this.mode = GraphMode.NONE; @@ -79,8 +100,9 @@ public synchronized Map update(String action, String instance, l if (epoch < this.epoch || this.released.containsKey(key)) { return this.receipt(epoch, token, "released"); } - E.checkState(this.active.isEmpty() ? this.mode == GraphMode.NONE : this.mode == GraphMode.LOADING, + checkState(this.active.isEmpty() ? this.mode == GraphMode.NONE : this.mode == GraphMode.LOADING, "Cannot acquire loading ownership of manually set graph mode %s", this.mode); + this.advanceRevision(); this.active.put(key, principal); this.mode = GraphMode.LOADING; return this.receipt(epoch, token, "active"); @@ -88,23 +110,47 @@ public synchronized Map update(String action, String instance, l // Unknown release cancels an acquire which may still be in transit. if (epoch == this.epoch) { String releasedOwner = this.released.get(key); - E.checkState(releasedOwner == null || releasedOwner.equals(principal), + checkState(releasedOwner == null || releasedOwner.equals(principal), "Loading owner belongs to another authenticated principal"); if (releasedOwner != null) { return this.receipt(epoch, token, "released"); } if (this.released.size() == MAX_RELEASED) { - E.checkState(this.epoch < Long.MAX_VALUE, "Loading owner epoch exhausted"); + checkState(this.epoch < Long.MAX_VALUE, "Loading owner epoch exhausted"); + if (owner == null) { + this.advanceRevision(); + } this.epoch++; this.released.clear(); // Old nonactive tuples are terminal; active old tuples remain registered. } else { + if (owner == null) { + this.advanceRevision(); + } this.released.put(key, principal); } } return this.receipt(epoch, token, "released"); } + private void advanceRevision() { + checkState(this.revision < Long.MAX_VALUE, "Loading state revision exhausted"); + this.revision++; + } + + private static void checkState(boolean condition, String message, Object... args) { + if (!condition) { + throw new ConflictException(String.format(message, args)); + } + } + + public static class ConflictException extends IllegalStateException { + + public ConflictException(String message) { + super(message); + } + } + private Map receipt(long epoch, String token, String state) { return ImmutableMap.builder().put("protocol", 1).put("instance", this.instance) .put("epoch", epoch).put("token", token).put("state", state) diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/StandardHugeGraph.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/StandardHugeGraph.java index 569e0fb9e0..6bc061819c 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/StandardHugeGraph.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/StandardHugeGraph.java @@ -443,8 +443,8 @@ public GraphMode mode() { @Override public void mode(GraphMode mode) { - LOG.info("Graph {} will work in {} mode", this, mode); this.loadingMode.mode(mode); + LOG.info("Graph {} will work in {} mode", this, mode); } @Override @@ -458,6 +458,11 @@ public Map loadingMode(String action, String instance, long epoc return this.loadingMode.update(action, instance, epoch, token, principal); } + @Override + public Map recoverLoadingMode(String instance, long epoch, long revision) { + return this.loadingMode.recover(instance, epoch, revision); + } + @Override public GraphReadMode readMode() { return this.readMode; diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/space/GraphSpaceAPITest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/space/GraphSpaceAPITest.java index 1a0b585b0c..403b8cb499 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/space/GraphSpaceAPITest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/space/GraphSpaceAPITest.java @@ -26,6 +26,7 @@ import java.util.concurrent.ConcurrentHashMap; import org.apache.hugegraph.HugeGraph; +import org.apache.hugegraph.LoadingMode; import org.apache.hugegraph.api.auth.ManagerAPI; import org.apache.hugegraph.api.profile.GraphsAPI; import org.apache.hugegraph.api.space.GraphSpaceAPI; @@ -43,6 +44,7 @@ import org.apache.hugegraph.space.GraphSpace; import org.apache.hugegraph.testutil.Assert; import org.apache.hugegraph.testutil.Whitebox; +import org.apache.hugegraph.type.define.GraphMode; import org.apache.hugegraph.unit.BaseUnitTest; import org.apache.hugegraph.unit.FakeObjects; import org.apache.tinkerpop.gremlin.structure.Graph; @@ -51,6 +53,7 @@ import org.mockito.Mockito; import jakarta.ws.rs.ForbiddenException; +import jakarta.ws.rs.ClientErrorException; import jakarta.ws.rs.core.SecurityContext; public class GraphSpaceAPITest extends BaseUnitTest { @@ -243,6 +246,45 @@ public void testPdGraphsIncludesLoadedLocalConfigGraphs() { Assert.assertFalse(result.contains(SYS_GRAPH)); } + @Test + public void testLoadingRecoveryConfirmationAndConflictResponse() { + GraphManager manager = Mockito.mock(GraphManager.class); + HugeGraph graph = Mockito.mock(HugeGraph.class); + Mockito.when(manager.graph("DEFAULT", "hugegraph")).thenReturn(graph); + GraphsAPI api = new GraphsAPI(); + GraphsAPI.JsonLoadingOwner request = new GraphsAPI.JsonLoadingOwner(); + request.action = "recover"; + request.instance = "instance"; + request.epoch = 0L; + SecurityContext securityContext = Mockito.mock(SecurityContext.class); + Assert.assertThrows(ForbiddenException.class, + () -> api.loadingMode(manager, "DEFAULT", "hugegraph", request, securityContext)); + Mockito.verify(graph, Mockito.never()).recoverLoadingMode(Mockito.anyString(), Mockito.anyLong(), + Mockito.anyLong()); + Mockito.when(securityContext.isUserInRole("admin")).thenReturn(true); + Assert.assertThrows(IllegalArgumentException.class, + () -> api.loadingMode(manager, "DEFAULT", "hugegraph", request, securityContext)); + Mockito.verify(graph, Mockito.never()).loadingMode(Mockito.anyString(), Mockito.anyString(), + Mockito.anyLong(), Mockito.any(), Mockito.anyString()); + request.confirm = "I'm sure all imports have stopped"; + Assert.assertThrows(IllegalArgumentException.class, + () -> api.loadingMode(manager, "DEFAULT", "hugegraph", request, securityContext)); + request.revision = 0L; + Map receipt = Collections.singletonMap("state", "recovered"); + Mockito.when(graph.recoverLoadingMode("instance", 0, 0)).thenReturn(receipt); + Assert.assertSame(receipt, api.loadingMode(manager, "DEFAULT", "hugegraph", request, securityContext)); + + Mockito.when(graph.recoverLoadingMode("instance", 0, 0)) + .thenThrow(new LoadingMode.ConflictException("stale epoch")); + ClientErrorException ownerConflict = org.junit.Assert.assertThrows(ClientErrorException.class, + () -> api.loadingMode(manager, "DEFAULT", "hugegraph", request, securityContext)); + Assert.assertEquals(409, ownerConflict.getResponse().getStatus()); + Mockito.doThrow(new LoadingMode.ConflictException("active owners")).when(graph).mode(GraphMode.NONE); + ClientErrorException modeConflict = org.junit.Assert.assertThrows(ClientErrorException.class, + () -> api.mode(manager, "DEFAULT", "hugegraph", GraphMode.NONE)); + Assert.assertEquals(409, modeConflict.getResponse().getStatus()); + } + @Test public void testAdminCanListGraphsWithoutGraphReadRole() { GraphManager manager = managerWithLoadedDefaultGraph(ADMIN, true); diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/auth/HugeGraphAuthProxyTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/auth/HugeGraphAuthProxyTest.java index b16d7b9d5b..0756724b1b 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/auth/HugeGraphAuthProxyTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/auth/HugeGraphAuthProxyTest.java @@ -90,6 +90,12 @@ public void testLoadingOwnershipRequiresStatusWriteAndUsesAuthenticatedPrincipal proxy.loadingMode("acquire", "instance", 0, "token", "forged"); Mockito.verify(graph).loadingMode(); Mockito.verify(graph).loadingMode("acquire", "instance", 0, "token", "writer"); + Assert.assertThrows(jakarta.ws.rs.ForbiddenException.class, + () -> proxy.recoverLoadingMode("instance", 0, 0)); + Mockito.verify(graph, Mockito.never()).recoverLoadingMode("instance", 0, 0); + setContext(new HugeGraphAuthProxy.Context(new HugeAuthenticator.User("admin", RolePermission.admin()))); + proxy.recoverLoadingMode("instance", 0, 0); + Mockito.verify(graph).recoverLoadingMode("instance", 0, 0); } @Test diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/LoadingModeTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/LoadingModeTest.java index 34b50fa697..ddfd08309f 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/LoadingModeTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/LoadingModeTest.java @@ -125,6 +125,58 @@ public void testInstanceAndPrincipalBoundaries() { update(mode, capability, a, "release"); } + @Test + public void testRecoveryRetiresOwnersAndLateRequests() { + LoadingMode mode = new LoadingMode(); + Map before = mode.capability(); + String lost = token(); + update(mode, before, lost, "acquire"); + + Map recovered = recover(mode, mode.capability()); + Assert.assertEquals("recovered", recovered.get("state")); + Assert.assertEquals(1L, recovered.get("epoch")); + Assert.assertFalse(recovered.containsKey("token")); + Assert.assertEquals(GraphMode.NONE, mode.mode()); + Assert.assertEquals("released", update(mode, before, lost, "acquire").get("state")); + Assert.assertEquals("released", update(mode, before, token(), "acquire").get("state")); + update(mode, before, lost, "release"); + + Map current = mode.capability(); + update(mode, current, token(), "acquire"); + Assert.assertThrows(IllegalStateException.class, + () -> recover(mode, before)); + Assert.assertEquals(GraphMode.LOADING, mode.mode()); + Assert.assertEquals(1L, mode.capability().get("epoch")); + } + + @Test + public void testRecoveryCannotClearOwnerAddedAfterInspection() { + LoadingMode mode = new LoadingMode(); + Map capability = mode.capability(); + String first = token(); + String second = token(); + update(mode, capability, first, "acquire"); + Map inspected = mode.capability(); + update(mode, inspected, second, "acquire"); + Assert.assertThrows(IllegalStateException.class, + () -> recover(mode, inspected)); + Assert.assertEquals(GraphMode.LOADING, mode.mode()); + update(mode, capability, first, "release"); + Assert.assertEquals(GraphMode.LOADING, mode.mode()); + update(mode, capability, second, "release"); + Assert.assertEquals(GraphMode.NONE, mode.mode()); + } + + @Test + public void testRecoveryAllowsConfirmedManualModeReset() { + LoadingMode mode = new LoadingMode(); + mode.mode(GraphMode.RESTORING); + Map before = mode.capability(); + recover(mode, before); + Assert.assertEquals(GraphMode.NONE, mode.mode()); + Assert.assertEquals("active", update(mode, mode.capability(), token(), "acquire").get("state")); + } + @Test public void testConcurrentAcquireAndLegacySetterAreAtomic() throws Exception { LoadingMode mode = new LoadingMode(); @@ -165,6 +217,12 @@ private static String token() { return UUID.randomUUID().toString(); } + private static Map recover(LoadingMode mode, Map capability) { + return mode.recover((String) capability.get("instance"), + ((Number) capability.get("epoch")).longValue(), + ((Number) capability.get("revision")).longValue()); + } + private static Map update(LoadingMode mode, Map capability, String token, String action) { return mode.update(action, (String) capability.get("instance"), From 7762b90a33c99936ff77bfec177d046de41fc007 Mon Sep 17 00:00:00 2001 From: imbajin Date: Wed, 14 Oct 2026 11:17:34 +0800 Subject: [PATCH 10/13] fix(server): bind loading owners to callers - bind owner requests to the authenticated caller - audit mode and owner transitions under the state lock - cover unwrapped graphs and exclude credentials from logs - document request binding and audit scope --- docs/server-runtime.md | 4 + .../hugegraph/api/profile/GraphsAPI.java | 3 +- .../org/apache/hugegraph/LoadingMode.java | 18 +++++ .../unit/api/space/GraphSpaceAPITest.java | 34 ++++++++ .../hugegraph/unit/core/LoadingModeTest.java | 81 +++++++++++++++++++ 5 files changed, 139 insertions(+), 1 deletion(-) diff --git a/docs/server-runtime.md b/docs/server-runtime.md index 49d6732b94..fcd8c76d12 100644 --- a/docs/server-runtime.md +++ b/docs/server-runtime.md @@ -550,6 +550,10 @@ including bare LOADING; their original behavior remains available without active `release` uses the same tuple. Receipts echo protocol/instance/epoch/token and return `state` (`active` or `released`) and the effective `mode`. Both operations use existing graph-owner/STATUS WRITE permissions; the authenticated principal owns the token. +The request principal is also bound for graphs opened without the authentication proxy. +Each new acquire, actual release and successful recovery writes an INFO audit entry +with the graph instance, previous/resulting mode, epoch and owner counts. Idempotent +retries do not add entries. These audit entries omit tokens and authenticated principal values. Keep one tuple and identity for the whole import attempt, including uncertain acquire outcomes. Release after that import's workers stop. Acquire/release retries are diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/GraphsAPI.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/GraphsAPI.java index ac7309d727..845876a0d0 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/GraphsAPI.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/GraphsAPI.java @@ -738,7 +738,8 @@ public Map loadingMode(@Context GraphManager manager, if ("recover".equals(owner.action)) { return graph.recoverLoadingMode(owner.instance, owner.epoch, owner.revision); } - return graph.loadingMode(owner.action, owner.instance, owner.epoch, owner.token, ""); + return graph.loadingMode(owner.action, owner.instance, owner.epoch, owner.token, + HugeGraphAuthProxy.username()); } catch (LoadingMode.ConflictException e) { throw new ClientErrorException(e.getMessage(), Response.Status.CONFLICT, e); } diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/LoadingMode.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/LoadingMode.java index a2ab2c1f95..02bd6d2fe7 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/LoadingMode.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/LoadingMode.java @@ -24,12 +24,16 @@ import org.apache.hugegraph.type.define.GraphMode; import org.apache.hugegraph.util.E; +import org.apache.hugegraph.util.Log; +import org.slf4j.Logger; import com.google.common.collect.ImmutableMap; /** Same graph-instance ownership; this state is neither persisted nor distributed. */ public final class LoadingMode { + private static final Logger LOG = Log.logger(LoadingMode.class); + private static final int MAX_RELEASED = 1024; private final String instance = UUID.randomUUID().toString(); @@ -63,11 +67,14 @@ public synchronized Map recover(String instance, long epoch, lon checkState(epoch == this.epoch && revision == this.revision, "Loading state changed; inspect it before recovery"); checkState(this.epoch < Long.MAX_VALUE, "Loading owner epoch exhausted"); + GraphMode previousMode = this.mode; + int clearedOwners = this.active.size(); this.advanceRevision(); this.epoch++; this.active.clear(); this.released.clear(); this.mode = GraphMode.NONE; + this.logChange("recover", previousMode, clearedOwners); return ImmutableMap.builder().putAll(this.capability()) .put("state", "recovered").put("mode", this.mode).build(); } @@ -81,6 +88,7 @@ public synchronized Map update(String action, String instance, l E.checkArgument(token != null && token.length() == 36 && UUID.fromString(token).toString().equals(token), "Loading owner token must be a canonical UUID"); Objects.requireNonNull(principal, "principal"); + GraphMode previousMode = this.mode; String key = epoch + "/" + token; String owner = this.active.get(key); if (owner != null) { @@ -105,6 +113,7 @@ public synchronized Map update(String action, String instance, l this.advanceRevision(); this.active.put(key, principal); this.mode = GraphMode.LOADING; + this.logChange("acquire", previousMode, 0); return this.receipt(epoch, token, "active"); } // Unknown release cancels an acquire which may still be in transit. @@ -130,9 +139,18 @@ public synchronized Map update(String action, String instance, l this.released.put(key, principal); } } + if (owner != null) { + this.logChange("release", previousMode, 1); + } return this.receipt(epoch, token, "released"); } + private void logChange(String action, GraphMode previousMode, int clearedOwners) { + LOG.info("Loading ownership {} for graph instance {}: mode {} -> {}, epoch {}, " + + "activeOwners {}, clearedOwners {}", action, this.instance, previousMode, this.mode, + this.epoch, this.active.size(), clearedOwners); + } + private void advanceRevision() { checkState(this.revision < Long.MAX_VALUE, "Loading state revision exhausted"); this.revision++; diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/space/GraphSpaceAPITest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/space/GraphSpaceAPITest.java index 403b8cb499..b044579843 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/space/GraphSpaceAPITest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/space/GraphSpaceAPITest.java @@ -26,6 +26,7 @@ import java.util.concurrent.ConcurrentHashMap; import org.apache.hugegraph.HugeGraph; +import org.apache.hugegraph.HugeFactory; import org.apache.hugegraph.LoadingMode; import org.apache.hugegraph.api.auth.ManagerAPI; import org.apache.hugegraph.api.profile.GraphsAPI; @@ -246,6 +247,39 @@ public void testPdGraphsIncludesLoadedLocalConfigGraphs() { Assert.assertFalse(result.contains(SYS_GRAPH)); } + @Test + public void testUnproxiedLoadingOwnerUsesAuthenticatedPrincipal() throws Exception { + HugeConfig config = FakeObjects.newConfig(); + config.setProperty("backend", "memory"); + config.setProperty("serializer", "text"); + config.setProperty("store", "unproxied_loading_owner"); + HugeGraph graph = HugeFactory.open(config); + try { + GraphManager manager = Mockito.mock(GraphManager.class); + Mockito.when(manager.graph("DEFAULT", "hugegraph")).thenReturn(graph); + GraphsAPI api = new GraphsAPI(); + GraphsAPI.JsonLoadingOwner request = new GraphsAPI.JsonLoadingOwner(); + request.instance = (String) graph.loadingMode().get("instance"); + request.epoch = 0L; + request.token = java.util.UUID.randomUUID().toString(); + request.action = "acquire"; + setContext("alice"); + Assert.assertEquals("active", api.loadingMode(manager, "DEFAULT", "hugegraph", request, null).get("state")); + request.action = "release"; + setContext("bob"); + ClientErrorException conflict = org.junit.Assert.assertThrows(ClientErrorException.class, + () -> api.loadingMode(manager, "DEFAULT", "hugegraph", request, null)); + Assert.assertEquals(409, conflict.getResponse().getStatus()); + Assert.assertEquals(GraphMode.LOADING, graph.mode()); + setContext("alice"); + Assert.assertEquals("released", api.loadingMode(manager, "DEFAULT", "hugegraph", request, null).get("state")); + Assert.assertEquals(GraphMode.NONE, graph.mode()); + } finally { + graph.close(); + HugeFactory.remove(graph); + } + } + @Test public void testLoadingRecoveryConfirmationAndConflictResponse() { GraphManager manager = Mockito.mock(GraphManager.class); diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/LoadingModeTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/LoadingModeTest.java index ddfd08309f..4ff3a8ae2e 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/LoadingModeTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/LoadingModeTest.java @@ -17,6 +17,8 @@ package org.apache.hugegraph.unit.core; +import java.util.ArrayList; +import java.util.List; import java.util.Map; import java.util.UUID; import java.util.concurrent.CountDownLatch; @@ -27,11 +29,90 @@ import org.apache.hugegraph.LoadingMode; import org.apache.hugegraph.type.define.GraphMode; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.core.LogEvent; +import org.apache.logging.log4j.core.LoggerContext; +import org.apache.logging.log4j.core.appender.AbstractAppender; +import org.apache.logging.log4j.core.config.LoggerConfig; +import org.apache.logging.log4j.core.config.Property; import org.junit.Assert; import org.junit.Test; public class LoadingModeTest { + @Test + public void testOwnerAuditRecordsOnlySuccessfulChangesWithoutTokens() { + List events = new ArrayList<>(); + AbstractAppender appender = new AbstractAppender("LoadingModeTestAppender", null, null, + true, Property.EMPTY_ARRAY) { + @Override + public void append(LogEvent event) { + events.add(event.toImmutable()); + } + }; + appender.start(); + LoggerContext context = (LoggerContext) LogManager.getContext(false); + org.apache.logging.log4j.core.config.Configuration configuration = context.getConfiguration(); + String loggerName = LoadingMode.class.getName(); + LoggerConfig previous = configuration.getLoggers().get(loggerName); + LoggerConfig logger = new LoggerConfig(loggerName, Level.INFO, false); + logger.addAppender(appender, Level.INFO, null); + configuration.addLogger(loggerName, logger); + context.updateLoggers(); + try { + LoadingMode mode = new LoadingMode(); + Map capability = mode.capability(); + String first = token(); + String second = token(); + update(mode, capability, first, "acquire"); + update(mode, capability, first, "acquire"); + update(mode, capability, second, "acquire"); + Assert.assertThrows(LoadingMode.ConflictException.class, + () -> mode.update("release", (String) capability.get("instance"), + 0, first, "other-user")); + update(mode, capability, first, "release"); + update(mode, capability, first, "release"); + update(mode, capability, second, "release"); + update(mode, capability, first, "acquire"); + Assert.assertEquals(4, events.size()); + Assert.assertTrue(message(events, 0).contains("mode NONE -> LOADING, epoch 0, activeOwners 1")); + Assert.assertTrue(message(events, 1).contains("mode LOADING -> LOADING, epoch 0, activeOwners 2")); + Assert.assertTrue(message(events, 2).contains("activeOwners 1, clearedOwners 1")); + Assert.assertTrue(message(events, 3).contains("mode LOADING -> NONE, epoch 0, activeOwners 0")); + update(mode, capability, token(), "acquire"); + update(mode, capability, token(), "acquire"); + Map current = mode.capability(); + mode.recover((String) current.get("instance"), 0, ((Number) current.get("revision")).longValue()); + Assert.assertTrue(message(events, 6).contains("mode LOADING -> NONE, epoch 1, " + + "activeOwners 0, clearedOwners 2")); + Assert.assertThrows(LoadingMode.ConflictException.class, + () -> mode.recover((String) capability.get("instance"), 0, 0)); + current = mode.capability(); + mode.recover((String) current.get("instance"), 1, ((Number) current.get("revision")).longValue()); + Assert.assertEquals(8, events.size()); + Assert.assertTrue(message(events, 7).contains("mode NONE -> NONE, epoch 2, " + + "activeOwners 0, clearedOwners 0")); + for (LogEvent event : events) { + Assert.assertEquals(Level.INFO, event.getLevel()); + Assert.assertFalse(event.getMessage().getFormattedMessage().contains(first)); + Assert.assertFalse(event.getMessage().getFormattedMessage().contains(second)); + Assert.assertFalse(event.getMessage().getFormattedMessage().contains("other-user")); + } + } finally { + configuration.removeLogger(loggerName); + if (previous != null) { + configuration.addLogger(loggerName, previous); + } + context.updateLoggers(); + appender.stop(); + } + } + + private static String message(List events, int index) { + return events.get(index).getMessage().getFormattedMessage(); + } + @Test public void testTwoOwnersAndIdempotentRetry() { LoadingMode mode = new LoadingMode(); From 9f3df410af2b5fe1a5b3244bc5a72bfdda5923cf Mon Sep 17 00:00:00 2001 From: imbajin Date: Thu, 15 Oct 2026 01:52:08 +0800 Subject: [PATCH 11/13] fix(server): bound and audit loading owners - include current graph names in ownership audit events - advance recovery revisions on every successful mode write - bound active owners while preserving cleanup and recovery - document admission and terminal epoch behavior --- docs/server-runtime.md | 18 +++-- .../org/apache/hugegraph/LoadingMode.java | 16 +++-- .../apache/hugegraph/StandardHugeGraph.java | 2 +- .../hugegraph/unit/core/LoadingModeTest.java | 68 ++++++++++++++++--- 4 files changed, 83 insertions(+), 21 deletions(-) diff --git a/docs/server-runtime.md b/docs/server-runtime.md index fcd8c76d12..8dc37da0e8 100644 --- a/docs/server-runtime.md +++ b/docs/server-runtime.md @@ -550,16 +550,24 @@ including bare LOADING; their original behavior remains available without active `release` uses the same tuple. Receipts echo protocol/instance/epoch/token and return `state` (`active` or `released`) and the effective `mode`. Both operations use existing graph-owner/STATUS WRITE permissions; the authenticated principal owns the token. +Each live graph admits at most 1024 concurrent owners. At that limit, a new owner +receives HTTP 409 without changing state; existing-owner retries, release and +administrator recovery remain available. Releasing an owner frees a slot. The request principal is also bound for graphs opened without the authentication proxy. Each new acquire, actual release and successful recovery writes an INFO audit entry -with the graph instance, previous/resulting mode, epoch and owner counts. Idempotent +with the graph name, graph instance, previous/resulting mode, epoch and owner counts. Idempotent retries do not add entries. These audit entries omit tokens and authenticated principal values. Keep one tuple and identity for the whole import attempt, including uncertain acquire outcomes. Release after that import's workers stop. Acquire/release retries are idempotent. An unknown release cancels a late acquire; a released handle cannot be reused. Terminal records are bounded to 1024 per epoch. Epoch advancement retains -active old handles and permanently retires all other old handles. +active old handles and permanently retires all other old handles. An initial +acquire prepared before epoch advancement can therefore be rejected even if it +never became active. The client must not automatically refresh that retired handle: +a new token could undo a prior cancellation or administrator recovery. After a +terminal acquire rejection, stop and drain that import attempt before creating a +fresh Loader context to retry. This protection coordinates Loader processes connected directly to the same stable Server and graph instance. It does not fence ordinary data requests across load @@ -580,9 +588,9 @@ the explicit confirmation. It clears owners, advances the epoch and returns NONE late acquire/release requests from the retired epoch cannot register an owner again. The receipt contains the new epoch and `state: recovered`, without tokens. A stale recovery request returns a conflict and cannot clear an import started in a newer -epoch. Owner registration/release and manual mode changes advance the revision, so -an import started after inspection also rejects the stale recovery request even within -the same epoch. After an uncertain result, inspect current state rather than blindly +epoch. Owner registration/release and every successful enum mode write advance the +revision, including a write of the current mode. Either operation after inspection +therefore rejects the stale recovery request even within the same epoch. After an uncertain result, inspect current state rather than blindly retrying with a newly fetched epoch/revision. This operation does not stop or drain imports itself. The same recovery procedure can reset manual RESTORING or MERGING after that work diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/LoadingMode.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/LoadingMode.java index 02bd6d2fe7..11b68f3354 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/LoadingMode.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/LoadingMode.java @@ -21,6 +21,7 @@ import java.util.Map; import java.util.Objects; import java.util.UUID; +import java.util.function.Supplier; import org.apache.hugegraph.type.define.GraphMode; import org.apache.hugegraph.util.E; @@ -34,8 +35,10 @@ public final class LoadingMode { private static final Logger LOG = Log.logger(LoadingMode.class); + private static final int MAX_ACTIVE = 1024; private static final int MAX_RELEASED = 1024; + private final Supplier graphName; private final String instance = UUID.randomUUID().toString(); private final Map active = new HashMap<>(); private final Map released = new HashMap<>(); @@ -43,6 +46,10 @@ public final class LoadingMode { private long revision; private volatile GraphMode mode = GraphMode.NONE; + public LoadingMode(Supplier graphName) { + this.graphName = Objects.requireNonNull(graphName, "graphName"); + } + public GraphMode mode() { return this.mode; } @@ -50,9 +57,7 @@ public GraphMode mode() { public synchronized void mode(GraphMode mode) { Objects.requireNonNull(mode, "mode"); checkState(this.active.isEmpty(), "Loading owners are active; release them before setting graph mode"); - if (this.mode != mode) { - this.advanceRevision(); - } + this.advanceRevision(); this.mode = mode; } @@ -110,6 +115,7 @@ public synchronized Map update(String action, String instance, l } checkState(this.active.isEmpty() ? this.mode == GraphMode.NONE : this.mode == GraphMode.LOADING, "Cannot acquire loading ownership of manually set graph mode %s", this.mode); + checkState(this.active.size() < MAX_ACTIVE, "Loading owner limit %s reached", MAX_ACTIVE); this.advanceRevision(); this.active.put(key, principal); this.mode = GraphMode.LOADING; @@ -146,8 +152,8 @@ public synchronized Map update(String action, String instance, l } private void logChange(String action, GraphMode previousMode, int clearedOwners) { - LOG.info("Loading ownership {} for graph instance {}: mode {} -> {}, epoch {}, " + - "activeOwners {}, clearedOwners {}", action, this.instance, previousMode, this.mode, + LOG.info("Loading ownership {} for graph '{}' instance {}: mode {} -> {}, epoch {}, " + + "activeOwners {}, clearedOwners {}", action, this.graphName.get(), this.instance, previousMode, this.mode, this.epoch, this.active.size(), clearedOwners); } diff --git a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/StandardHugeGraph.java b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/StandardHugeGraph.java index 6bc061819c..551d12556a 100644 --- a/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/StandardHugeGraph.java +++ b/hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/StandardHugeGraph.java @@ -177,7 +177,7 @@ public class StandardHugeGraph implements HugeGraph { private final RamTable ramtable; private volatile boolean started; private volatile boolean closed; - private final LoadingMode loadingMode = new LoadingMode(); + private final LoadingMode loadingMode = new LoadingMode(this::spaceGraphName); private volatile GraphReadMode readMode; private volatile HugeVariables variables; private String graphSpace; diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/LoadingModeTest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/LoadingModeTest.java index 4ff3a8ae2e..2431379c6e 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/LoadingModeTest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/LoadingModeTest.java @@ -26,6 +26,7 @@ import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; import org.apache.hugegraph.LoadingMode; import org.apache.hugegraph.type.define.GraphMode; @@ -61,7 +62,8 @@ public void append(LogEvent event) { configuration.addLogger(loggerName, logger); context.updateLoggers(); try { - LoadingMode mode = new LoadingMode(); + AtomicReference graphName = new AtomicReference<>("test-space-test-graph"); + LoadingMode mode = new LoadingMode(graphName::get); Map capability = mode.capability(); String first = token(); String second = token(); @@ -95,10 +97,17 @@ public void append(LogEvent event) { "activeOwners 0, clearedOwners 0")); for (LogEvent event : events) { Assert.assertEquals(Level.INFO, event.getLevel()); + Assert.assertTrue(event.getMessage().getFormattedMessage().contains( + "graph 'test-space-test-graph' instance " + capability.get("instance"))); Assert.assertFalse(event.getMessage().getFormattedMessage().contains(first)); Assert.assertFalse(event.getMessage().getFormattedMessage().contains(second)); Assert.assertFalse(event.getMessage().getFormattedMessage().contains("other-user")); } + graphName.set("renamed-space-test-graph"); + recover(mode, mode.capability()); + Assert.assertEquals(9, events.size()); + Assert.assertTrue(message(events, 8).contains("graph 'renamed-space-test-graph' instance " + + capability.get("instance"))); } finally { configuration.removeLogger(loggerName); if (previous != null) { @@ -115,7 +124,7 @@ private static String message(List events, int index) { @Test public void testTwoOwnersAndIdempotentRetry() { - LoadingMode mode = new LoadingMode(); + LoadingMode mode = new LoadingMode(() -> "test-space-test-graph"); Map capability = mode.capability(); String a = token(); String b = token(); @@ -132,7 +141,7 @@ public void testTwoOwnersAndIdempotentRetry() { @Test public void testUnknownReleaseCancelsLateAcquire() { - LoadingMode mode = new LoadingMode(); + LoadingMode mode = new LoadingMode(() -> "test-space-test-graph"); Map capability = mode.capability(); String token = token(); update(mode, capability, token, "release"); @@ -142,7 +151,7 @@ public void testUnknownReleaseCancelsLateAcquire() { @Test public void testTerminalCompactionKeepsOldActiveAndRejectsLateAcquire() { - LoadingMode mode = new LoadingMode(); + LoadingMode mode = new LoadingMode(() -> "test-space-test-graph"); Map original = mode.capability(); String b = token(); String cancelled = token(); @@ -167,7 +176,7 @@ public void testTerminalCompactionKeepsOldActiveAndRejectsLateAcquire() { @Test public void testLegacySetterCannotOverwriteOwnersOrAdoptManualMode() { - LoadingMode mode = new LoadingMode(); + LoadingMode mode = new LoadingMode(() -> "test-space-test-graph"); Map capability = mode.capability(); String a = token(); for (GraphMode manual : GraphMode.values()) { @@ -191,7 +200,7 @@ public void testLegacySetterCannotOverwriteOwnersOrAdoptManualMode() { @Test public void testInstanceAndPrincipalBoundaries() { - LoadingMode mode = new LoadingMode(); + LoadingMode mode = new LoadingMode(() -> "test-space-test-graph"); Map capability = mode.capability(); String a = token(); update(mode, capability, a, "acquire"); @@ -208,7 +217,7 @@ public void testInstanceAndPrincipalBoundaries() { @Test public void testRecoveryRetiresOwnersAndLateRequests() { - LoadingMode mode = new LoadingMode(); + LoadingMode mode = new LoadingMode(() -> "test-space-test-graph"); Map before = mode.capability(); String lost = token(); update(mode, before, lost, "acquire"); @@ -232,7 +241,7 @@ public void testRecoveryRetiresOwnersAndLateRequests() { @Test public void testRecoveryCannotClearOwnerAddedAfterInspection() { - LoadingMode mode = new LoadingMode(); + LoadingMode mode = new LoadingMode(() -> "test-space-test-graph"); Map capability = mode.capability(); String first = token(); String second = token(); @@ -250,7 +259,7 @@ public void testRecoveryCannotClearOwnerAddedAfterInspection() { @Test public void testRecoveryAllowsConfirmedManualModeReset() { - LoadingMode mode = new LoadingMode(); + LoadingMode mode = new LoadingMode(() -> "test-space-test-graph"); mode.mode(GraphMode.RESTORING); Map before = mode.capability(); recover(mode, before); @@ -258,9 +267,48 @@ public void testRecoveryAllowsConfirmedManualModeReset() { Assert.assertEquals("active", update(mode, mode.capability(), token(), "acquire").get("state")); } + @Test + public void testSuccessfulSameModeWriteInvalidatesRecoverySnapshot() { + LoadingMode mode = new LoadingMode(() -> "test-space-test-graph"); + for (GraphMode manual : GraphMode.values()) { + mode.mode(manual); + Map inspected = mode.capability(); + mode.mode(manual); + Assert.assertEquals(((Number) inspected.get("revision")).longValue() + 1, + mode.capability().get("revision")); + Assert.assertThrows(LoadingMode.ConflictException.class, () -> recover(mode, inspected)); + Assert.assertEquals(manual, mode.mode()); + } + } + + @Test + public void testOwnerAdmissionLimitPreservesExistingOwnersAndRecovery() { + LoadingMode mode = new LoadingMode(() -> "test-space-test-graph"); + Map capability = mode.capability(); + String first = token(); + update(mode, capability, first, "acquire"); + for (int i = 1; i < 1024; i++) { + update(mode, capability, token(), "acquire"); + } + Map full = mode.capability(); + String rejected = token(); + LoadingMode.ConflictException failure = Assert.assertThrows(LoadingMode.ConflictException.class, + () -> update(mode, capability, rejected, "acquire")); + Assert.assertTrue(failure.getMessage().contains("1024")); + Assert.assertEquals(full, mode.capability()); + Assert.assertEquals(GraphMode.LOADING, mode.mode()); + Assert.assertEquals("active", update(mode, capability, first, "acquire").get("state")); + Assert.assertEquals(full, mode.capability()); + Assert.assertEquals("released", update(mode, capability, first, "release").get("state")); + Assert.assertEquals("active", update(mode, capability, rejected, "acquire").get("state")); + Map recovered = recover(mode, mode.capability()); + Assert.assertEquals(GraphMode.NONE, recovered.get("mode")); + Assert.assertEquals("active", update(mode, mode.capability(), token(), "acquire").get("state")); + } + @Test public void testConcurrentAcquireAndLegacySetterAreAtomic() throws Exception { - LoadingMode mode = new LoadingMode(); + LoadingMode mode = new LoadingMode(() -> "test-space-test-graph"); Map capability = mode.capability(); String a = token(); CountDownLatch start = new CountDownLatch(1); From c605ffcb667fbd35f3c087ec1a1decbe3da05417 Mon Sep 17 00:00:00 2001 From: imbajin Date: Thu, 15 Oct 2026 01:53:28 +0800 Subject: [PATCH 12/13] docs(server): clarify live recovery values - identify the capability response as illustrative - require values from the latest endpoint inspection - avoid relying on a fixed recovery revision --- docs/server-runtime.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/server-runtime.md b/docs/server-runtime.md index 7d51751ae2..3ee335f858 100644 --- a/docs/server-runtime.md +++ b/docs/server-runtime.md @@ -572,7 +572,8 @@ cannot be adopted. Original enum setters reject every request while owners are a including bare LOADING; their original behavior remains available without active owners. `GET /graphspaces/{space}/graphs/{graph}/mode/owners` returns -`{"protocol":1,"instance":"","epoch":0,"revision":0}`. `PUT` on that path accepts +`{"protocol":1,"instance":"","epoch":0,"revision":0}`. This response is illustrative; +recovery must use the actual values returned by the latest GET. `PUT` on that path accepts `{"action":"acquire","instance":"","epoch":0,"token":""}`; `release` uses the same tuple. Receipts echo protocol/instance/epoch/token and return `state` (`active` or `released`) and the effective `mode`. Both operations use existing From eee07ff17dbd144a0361367021fd6263c3e78dfe Mon Sep 17 00:00:00 2001 From: imbajin Date: Thu, 15 Oct 2026 04:20:24 +0800 Subject: [PATCH 13/13] fix(api): enforce global loading recovery role - read the native authenticated administrator role - reject non-admin recovery in auth-disabled graph spaces - cover restricted users, admins and trusted anonymous access --- docs/server-runtime.md | 7 +- .../hugegraph/api/profile/GraphsAPI.java | 10 ++- .../unit/api/space/GraphSpaceAPITest.java | 87 +++++++++++++++++-- 3 files changed, 92 insertions(+), 12 deletions(-) diff --git a/docs/server-runtime.md b/docs/server-runtime.md index 012a845494..586d2d11cf 100644 --- a/docs/server-runtime.md +++ b/docs/server-runtime.md @@ -622,9 +622,10 @@ lost, an administrator can recover without that token: 3. Send `PUT` to that endpoint with `{"action":"recover","instance":"","epoch":0,"revision":0,"confirm":"I'm sure all imports have stopped"}`. -Recovery requires administrator permission at both the REST and graph-proxy layers, -the exact current instance/epoch/revision and -the explicit confirmation. It clears owners, advances the epoch and returns NONE; +Recovery requires the authenticated global administrator role even when graph-space +permission checks are disabled; a proxied graph also enforces administrator permission. +With Server authentication disabled, the existing trusted anonymous administrator semantics apply. +Recovery also requires the exact current instance/epoch/revision and explicit confirmation. It clears owners, advances the epoch and returns NONE; late acquire/release requests from the retired epoch cannot register an owner again. The receipt contains the new epoch and `state: recovered`, without tokens. A stale recovery request returns a conflict and cannot clear an import started in a newer diff --git a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/GraphsAPI.java b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/GraphsAPI.java index 845876a0d0..103c33215f 100644 --- a/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/GraphsAPI.java +++ b/hugegraph-server/hugegraph-api/src/main/java/org/apache/hugegraph/api/profile/GraphsAPI.java @@ -34,10 +34,12 @@ import org.apache.hugegraph.LoadingMode; import org.apache.hugegraph.api.API; import org.apache.hugegraph.api.filter.StatusFilter; +import org.apache.hugegraph.api.filter.AuthenticationFilter.Authorizer; import org.apache.hugegraph.auth.AuthManager; import org.apache.hugegraph.auth.HugeAuthenticator.RequiredPerm; import org.apache.hugegraph.auth.HugeGraphAuthProxy; import org.apache.hugegraph.auth.HugePermission; +import org.apache.hugegraph.auth.RolePermission; import org.apache.hugegraph.config.HugeConfig; import org.apache.hugegraph.core.GraphManager; import org.apache.hugegraph.space.GraphSpace; @@ -69,6 +71,7 @@ import jakarta.ws.rs.Produces; import jakarta.ws.rs.QueryParam; import jakarta.ws.rs.core.Context; +import jakarta.ws.rs.container.ContainerRequestContext; import jakarta.ws.rs.core.SecurityContext; import jakarta.ws.rs.core.Response; @@ -722,10 +725,13 @@ public Map loadingMode(@Context GraphManager manager, @PathParam("graphspace") String graphSpace, @PathParam("name") String name, JsonLoadingOwner owner, - @Context SecurityContext securityContext) { + @Context ContainerRequestContext requestContext) { E.checkArgument(owner != null && owner.epoch != null, "Loading owner body and epoch are required"); if ("recover".equals(owner.action)) { - if (securityContext == null || !securityContext.isUserInRole("admin")) { + // Read the filter's Authorizer rather than Jersey's injected SecurityContext wrapper. + SecurityContext securityContext = requestContext == null ? null : requestContext.getSecurityContext(); + if (!(securityContext instanceof Authorizer) || + !RolePermission.isAdmin(((Authorizer) securityContext).role())) { throw new ForbiddenException("Loading recovery requires administrator permission"); } E.checkArgument(CONFIRM_RECOVER.equals(owner.confirm), diff --git a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/space/GraphSpaceAPITest.java b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/space/GraphSpaceAPITest.java index b044579843..71f45694a4 100644 --- a/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/space/GraphSpaceAPITest.java +++ b/hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/api/space/GraphSpaceAPITest.java @@ -20,6 +20,7 @@ import static org.apache.hugegraph.HugeFactory.SYS_GRAPH; import java.lang.reflect.Method; +import java.net.URI; import java.util.Collections; import java.util.Map; import java.util.Set; @@ -29,6 +30,7 @@ import org.apache.hugegraph.HugeFactory; import org.apache.hugegraph.LoadingMode; import org.apache.hugegraph.api.auth.ManagerAPI; +import org.apache.hugegraph.api.filter.AuthenticationFilter.Authorizer; import org.apache.hugegraph.api.profile.GraphsAPI; import org.apache.hugegraph.api.space.GraphSpaceAPI; import org.apache.hugegraph.auth.AuthManager; @@ -51,11 +53,16 @@ import org.apache.tinkerpop.gremlin.structure.Graph; import org.junit.After; import org.junit.Test; +import org.glassfish.jersey.internal.MapPropertiesDelegate; +import org.glassfish.jersey.server.ContainerRequest; import org.mockito.Mockito; import jakarta.ws.rs.ForbiddenException; import jakarta.ws.rs.ClientErrorException; import jakarta.ws.rs.core.SecurityContext; +import jakarta.ws.rs.container.ContainerRequestContext; +import jakarta.ws.rs.core.MultivaluedHashMap; +import jakarta.ws.rs.core.UriInfo; public class GraphSpaceAPITest extends BaseUnitTest { @@ -280,6 +287,69 @@ public void testUnproxiedLoadingOwnerUsesAuthenticatedPrincipal() throws Excepti } } + @Test + public void testAuthDisabledSpaceRecoveryStillRequiresGlobalAdmin() throws Exception { + HugeConfig config = FakeObjects.newConfig(); + config.setProperty("backend", "memory"); + config.setProperty("serializer", "text"); + config.setProperty("store", "unproxied_loading_recovery"); + HugeGraph graph = HugeFactory.open(config); + try { + Assert.assertFalse(graph instanceof HugeGraphAuthProxy); + String space = "open_space"; + GraphSpace graphSpace = new GraphSpace(space); + graphSpace.auth(false); + GraphManager manager = Mockito.mock(GraphManager.class); + Mockito.when(manager.requireAuthentication()).thenReturn(true); + Mockito.when(manager.graphSpace(space)).thenReturn(graphSpace); + Mockito.when(manager.graph(space, "hugegraph")).thenReturn(graph); + UriInfo uri = Mockito.mock(UriInfo.class); + MultivaluedHashMap parameters = new MultivaluedHashMap<>(); + parameters.putSingle("graphspace", space); + Mockito.when(uri.getPathParameters()).thenReturn(parameters); + Authorizer restricted = new Authorizer(manager, + new HugeAuthenticator.User("bob", RolePermission.none()), uri); + Assert.assertTrue(restricted.isUserInRole("admin")); + Assert.assertFalse(RolePermission.isAdmin(restricted.role())); + URI endpoint = URI.create("http://localhost/graphspaces/open_space/graphs/hugegraph/mode/owners"); + ContainerRequest context = new ContainerRequest(endpoint, endpoint, "PUT", restricted, + new MapPropertiesDelegate()); + GraphsAPI api = new GraphsAPI(); + Map state = graph.loadingMode(); + String instance = (String) state.get("instance"); + String token = java.util.UUID.randomUUID().toString(); + graph.loadingMode("acquire", instance, 0, token, "alice"); + Map before = api.loadingMode(manager, space, "hugegraph"); + GraphsAPI.JsonLoadingOwner request = new GraphsAPI.JsonLoadingOwner(); + request.action = "recover"; + request.instance = instance; + request.epoch = (Long) before.get("epoch"); + request.revision = (Long) before.get("revision"); + request.confirm = "I'm sure all imports have stopped"; + ForbiddenException denied = org.junit.Assert.assertThrows(ForbiddenException.class, + () -> api.loadingMode(manager, space, "hugegraph", request, context)); + Assert.assertEquals(403, denied.getResponse().getStatus()); + Assert.assertEquals(before, graph.loadingMode()); + Assert.assertEquals(GraphMode.LOADING, graph.mode()); + + for (HugeAuthenticator.User user : new HugeAuthenticator.User[]{HugeAuthenticator.User.ADMIN, + HugeAuthenticator.User.ANONYMOUS}) { + Mockito.when(manager.requireAuthentication()).thenReturn(user != HugeAuthenticator.User.ANONYMOUS); + context.setSecurityContext(new Authorizer(manager, user, uri)); + state = graph.loadingMode(); + request.epoch = (Long) state.get("epoch"); + graph.loadingMode("acquire", instance, request.epoch, token, "alice"); + request.revision = (Long) graph.loadingMode().get("revision"); + Assert.assertEquals("recovered", api.loadingMode(manager, space, "hugegraph", request, + context).get("state")); + Assert.assertEquals(GraphMode.NONE, graph.mode()); + } + } finally { + graph.close(); + HugeFactory.remove(graph); + } + } + @Test public void testLoadingRecoveryConfirmationAndConflictResponse() { GraphManager manager = Mockito.mock(GraphManager.class); @@ -290,28 +360,31 @@ public void testLoadingRecoveryConfirmationAndConflictResponse() { request.action = "recover"; request.instance = "instance"; request.epoch = 0L; - SecurityContext securityContext = Mockito.mock(SecurityContext.class); + ContainerRequestContext requestContext = Mockito.mock(ContainerRequestContext.class); + Authorizer authorizer = Mockito.mock(Authorizer.class); + Mockito.when(requestContext.getSecurityContext()).thenReturn(authorizer); + Mockito.when(authorizer.role()).thenReturn(RolePermission.none()); Assert.assertThrows(ForbiddenException.class, - () -> api.loadingMode(manager, "DEFAULT", "hugegraph", request, securityContext)); + () -> api.loadingMode(manager, "DEFAULT", "hugegraph", request, requestContext)); Mockito.verify(graph, Mockito.never()).recoverLoadingMode(Mockito.anyString(), Mockito.anyLong(), Mockito.anyLong()); - Mockito.when(securityContext.isUserInRole("admin")).thenReturn(true); + Mockito.when(authorizer.role()).thenReturn(RolePermission.admin()); Assert.assertThrows(IllegalArgumentException.class, - () -> api.loadingMode(manager, "DEFAULT", "hugegraph", request, securityContext)); + () -> api.loadingMode(manager, "DEFAULT", "hugegraph", request, requestContext)); Mockito.verify(graph, Mockito.never()).loadingMode(Mockito.anyString(), Mockito.anyString(), Mockito.anyLong(), Mockito.any(), Mockito.anyString()); request.confirm = "I'm sure all imports have stopped"; Assert.assertThrows(IllegalArgumentException.class, - () -> api.loadingMode(manager, "DEFAULT", "hugegraph", request, securityContext)); + () -> api.loadingMode(manager, "DEFAULT", "hugegraph", request, requestContext)); request.revision = 0L; Map receipt = Collections.singletonMap("state", "recovered"); Mockito.when(graph.recoverLoadingMode("instance", 0, 0)).thenReturn(receipt); - Assert.assertSame(receipt, api.loadingMode(manager, "DEFAULT", "hugegraph", request, securityContext)); + Assert.assertSame(receipt, api.loadingMode(manager, "DEFAULT", "hugegraph", request, requestContext)); Mockito.when(graph.recoverLoadingMode("instance", 0, 0)) .thenThrow(new LoadingMode.ConflictException("stale epoch")); ClientErrorException ownerConflict = org.junit.Assert.assertThrows(ClientErrorException.class, - () -> api.loadingMode(manager, "DEFAULT", "hugegraph", request, securityContext)); + () -> api.loadingMode(manager, "DEFAULT", "hugegraph", request, requestContext)); Assert.assertEquals(409, ownerConflict.getResponse().getStatus()); Mockito.doThrow(new LoadingMode.ConflictException("active owners")).when(graph).mode(GraphMode.NONE); ClientErrorException modeConflict = org.junit.Assert.assertThrows(ClientErrorException.class,