You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hugegraph.apache.org by je...@apache.org on 2022/06/13 12:55:37 UTC
[incubator-hugegraph] 01/01: fix: can't shutdown when starting with exception
This is an automated email from the ASF dual-hosted git repository.
jermy pushed a commit to branch cannt-shutdown-on-exception
in repository https://gitbox.apache.org/repos/asf/incubator-hugegraph.git
commit 43aa3559e6cfa15c34f2c161b9263eaffbab8481
Author: Zhangmei Li <li...@baidu.com>
AuthorDate: Mon Jun 13 20:01:13 2022 +0800
fix: can't shutdown when starting with exception
Change-Id: If312aa4368bfd7af540818bc299ed219f4e9f335
---
.../com/baidu/hugegraph/core/GraphManager.java | 33 ++++++++++++++--------
.../baidu/hugegraph/server/ApplicationConfig.java | 21 +++++++++-----
.../baidu/hugegraph/task/ServerInfoManager.java | 8 +++++-
.../java/com/baidu/hugegraph/cmd/InitStore.java | 22 +++++++++------
.../com/baidu/hugegraph/dist/HugeGraphServer.java | 16 ++++++++---
.../backend/store/rocksdb/RocksDBStore.java | 16 +++++++++--
6 files changed, 81 insertions(+), 35 deletions(-)
diff --git a/hugegraph-api/src/main/java/com/baidu/hugegraph/core/GraphManager.java b/hugegraph-api/src/main/java/com/baidu/hugegraph/core/GraphManager.java
index 749e350aa..f5e164327 100644
--- a/hugegraph-api/src/main/java/com/baidu/hugegraph/core/GraphManager.java
+++ b/hugegraph-api/src/main/java/com/baidu/hugegraph/core/GraphManager.java
@@ -98,7 +98,11 @@ public final class GraphManager {
this.rpcClient = new RpcClientProvider(conf);
this.eventHub = hub;
this.conf = conf;
+ }
+ public void init() {
+ E.checkArgument(this.graphs.isEmpty(),
+ "GraphManager has been initialized before");
this.listenChanges();
this.loadGraphs(ConfigUtil.scanGraphsDir(this.graphsDir));
@@ -111,10 +115,10 @@ public final class GraphManager {
// Raft will load snapshot firstly then launch election and replay log
this.waitGraphsReady();
- this.checkBackendVersionOrExit(conf);
- this.serverStarted(conf);
+ this.checkBackendVersionOrExit(this.conf);
+ this.serverStarted(this.conf);
- this.addMetrics(conf);
+ this.addMetrics(this.conf);
}
public void loadGraphs(Map<String, String> graphConfs) {
@@ -124,7 +128,7 @@ public final class GraphManager {
HugeFactory.checkGraphName(name, "rest-server.properties");
try {
this.loadGraph(name, graphConfPath);
- } catch (RuntimeException e) {
+ } catch (Throwable e) {
LOG.error("Graph '{}' can't be loaded: '{}'",
name, graphConfPath, e);
}
@@ -215,12 +219,12 @@ public final class GraphManager {
}
public void rollbackAll() {
- this.graphs.values().forEach(graph -> {
+ for (Graph graph : this.graphs.values()) {
if (graph.features().graph().supportsTransactions() &&
graph.tx().isOpen()) {
graph.tx().rollback();
}
- });
+ }
}
public void rollback(final Set<String> graphSourceNamesToCloseTxOn) {
@@ -228,12 +232,12 @@ public final class GraphManager {
}
public void commitAll() {
- this.graphs.values().forEach(graph -> {
+ for (Graph graph : this.graphs.values()) {
if (graph.features().graph().supportsTransactions() &&
graph.tx().isOpen()) {
graph.tx().commit();
}
- });
+ }
}
public void commit(final Set<String> graphSourceNamesToCloseTxOn) {
@@ -257,6 +261,13 @@ public final class GraphManager {
}
public void close() {
+ for (Graph graph : this.graphs.values()) {
+ try {
+ graph.close();
+ } catch (Throwable e) {
+ LOG.warn("Failed to close graph '{}'", graph, e);
+ }
+ }
this.destroyRpcServer();
this.unlistenChanges();
}
@@ -369,10 +380,10 @@ public final class GraphManager {
private void waitGraphsReady() {
com.alipay.remoting.rpc.RpcServer remotingRpcServer =
this.remotingRpcServer();
- this.graphs.keySet().forEach(name -> {
- HugeGraph graph = this.graph(name);
+ for (String graphName : this.graphs.keySet()) {
+ HugeGraph graph = this.graph(graphName);
graph.waitReady(remotingRpcServer);
- });
+ }
}
private void checkBackendVersionOrExit(HugeConfig config) {
diff --git a/hugegraph-api/src/main/java/com/baidu/hugegraph/server/ApplicationConfig.java b/hugegraph-api/src/main/java/com/baidu/hugegraph/server/ApplicationConfig.java
index 96eb8cd36..22e8d66e0 100644
--- a/hugegraph-api/src/main/java/com/baidu/hugegraph/server/ApplicationConfig.java
+++ b/hugegraph-api/src/main/java/com/baidu/hugegraph/server/ApplicationConfig.java
@@ -19,12 +19,6 @@
package com.baidu.hugegraph.server;
-import io.swagger.v3.jaxrs2.integration.resources.OpenApiResource;
-import io.swagger.v3.oas.annotations.OpenAPIDefinition;
-import io.swagger.v3.oas.annotations.info.Contact;
-import io.swagger.v3.oas.annotations.info.Info;
-import jakarta.ws.rs.ApplicationPath;
-
import org.apache.tinkerpop.gremlin.server.util.MetricManager;
import org.glassfish.hk2.api.Factory;
import org.glassfish.hk2.api.MultiException;
@@ -47,6 +41,12 @@ import com.baidu.hugegraph.version.CoreVersion;
import com.codahale.metrics.MetricRegistry;
import com.codahale.metrics.jersey3.InstrumentedResourceMethodApplicationListener;
+import io.swagger.v3.jaxrs2.integration.resources.OpenApiResource;
+import io.swagger.v3.oas.annotations.OpenAPIDefinition;
+import io.swagger.v3.oas.annotations.info.Contact;
+import io.swagger.v3.oas.annotations.info.Info;
+import jakarta.ws.rs.ApplicationPath;
+
@ApplicationPath("/")
@OpenAPIDefinition(
info =
@@ -127,7 +127,14 @@ public class ApplicationConfig extends ResourceConfig {
@Override
public void onEvent(ApplicationEvent event) {
if (event.getType() == this.eventInited) {
- GraphManagerFactory.this.manager = new GraphManager(conf, hub);
+ GraphManager manager = new GraphManager(conf, hub);
+ try {
+ manager.init();
+ } catch (Throwable e) {
+ manager.close();
+ throw e;
+ }
+ GraphManagerFactory.this.manager = manager;
} else if (event.getType() == this.eventDestroyed) {
if (GraphManagerFactory.this.manager != null) {
GraphManagerFactory.this.manager.close();
diff --git a/hugegraph-core/src/main/java/com/baidu/hugegraph/task/ServerInfoManager.java b/hugegraph-core/src/main/java/com/baidu/hugegraph/task/ServerInfoManager.java
index cdae8d3fc..ce7e0e3b0 100644
--- a/hugegraph-core/src/main/java/com/baidu/hugegraph/task/ServerInfoManager.java
+++ b/hugegraph-core/src/main/java/com/baidu/hugegraph/task/ServerInfoManager.java
@@ -319,7 +319,13 @@ public class ServerInfoManager {
}
private HugeServerInfo removeSelfServerInfo() {
- if (this.graph.initialized()) {
+ /*
+ * Check this.selfServerId != null to avoid graph.initialized() call.
+ * NOTE: graph.initialized() may throw exception if we can't connect to
+ * backend store, initServerInfo() is not called in this case, so
+ * this.selfServerId is null at this time.
+ */
+ if (this.selfServerId != null && this.graph.initialized()) {
return this.removeServerInfo(this.selfServerId);
}
return null;
diff --git a/hugegraph-dist/src/main/java/com/baidu/hugegraph/cmd/InitStore.java b/hugegraph-dist/src/main/java/com/baidu/hugegraph/cmd/InitStore.java
index cd45bfa24..30ba9a0dd 100644
--- a/hugegraph-dist/src/main/java/com/baidu/hugegraph/cmd/InitStore.java
+++ b/hugegraph-dist/src/main/java/com/baidu/hugegraph/cmd/InitStore.java
@@ -87,9 +87,8 @@ public class InitStore {
for (HugeGraph graph : graphs) {
graph.close();
}
+ HugeFactory.shutdown(30L);
}
-
- HugeFactory.shutdown(30L);
}
private static HugeGraph initGraph(String configPath) throws Exception {
@@ -99,13 +98,18 @@ public class InitStore {
config.setProperty(CoreOptions.RAFT_MODE.name(), "false");
HugeGraph graph = (HugeGraph) GraphFactory.open(config);
- BackendStoreInfo backendStoreInfo = graph.backendStoreInfo();
- if (backendStoreInfo.exists()) {
- LOG.info("Skip init-store due to the backend store of '{}' " +
- "had been initialized", graph.name());
- backendStoreInfo.checkVersion();
- } else {
- initBackend(graph);
+ try {
+ BackendStoreInfo backendStoreInfo = graph.backendStoreInfo();
+ if (backendStoreInfo.exists()) {
+ LOG.info("Skip init-store due to the backend store of '{}' " +
+ "had been initialized", graph.name());
+ backendStoreInfo.checkVersion();
+ } else {
+ initBackend(graph);
+ }
+ } catch (Throwable e) {
+ graph.close();
+ throw e;
}
return graph;
}
diff --git a/hugegraph-dist/src/main/java/com/baidu/hugegraph/dist/HugeGraphServer.java b/hugegraph-dist/src/main/java/com/baidu/hugegraph/dist/HugeGraphServer.java
index 602f20ecd..02a8583c6 100644
--- a/hugegraph-dist/src/main/java/com/baidu/hugegraph/dist/HugeGraphServer.java
+++ b/hugegraph-dist/src/main/java/com/baidu/hugegraph/dist/HugeGraphServer.java
@@ -76,7 +76,6 @@ public class HugeGraphServer {
} catch (Throwable t) {
LOG.error("HugeRestServer stop error: ", t);
}
- HugeFactory.shutdown(30L);
throw e;
} finally {
System.setSecurityManager(securityManager);
@@ -118,12 +117,19 @@ public class HugeGraphServer {
HugeGraphServer.register();
- HugeGraphServer server = new HugeGraphServer(args[0], args[1]);
+ HugeGraphServer server;
+ try {
+ server = new HugeGraphServer(args[0], args[1]);
+ } catch (Throwable e) {
+ HugeFactory.shutdown(30L);
+ throw e;
+ }
/*
- * HugeFactory.shutdown hook may be invoked before server stop,
+ * Remove HugeFactory.shutdown and let HugeGraphServer.stop() do it.
+ * NOTE: HugeFactory.shutdown hook may be invoked before server stop,
* causes event-hub can't execute notification events for another
- * shutdown executor such as gremling-stop-shutdown
+ * shutdown executor such as gremlin-stop-shutdown
*/
HugeFactory.removeShutdownHook();
@@ -132,8 +138,10 @@ public class HugeGraphServer {
LOG.info("HugeGraphServer stopping");
server.stop();
LOG.info("HugeGraphServer stopped");
+
serverStopped.complete(null);
}, "hugegraph-server-shutdown"));
+ // Wait for server-shutdown and server-stopped
serverStopped.get();
}
}
diff --git a/hugegraph-rocksdb/src/main/java/com/baidu/hugegraph/backend/store/rocksdb/RocksDBStore.java b/hugegraph-rocksdb/src/main/java/com/baidu/hugegraph/backend/store/rocksdb/RocksDBStore.java
index cd14d3e6d..40720f3f8 100644
--- a/hugegraph-rocksdb/src/main/java/com/baidu/hugegraph/backend/store/rocksdb/RocksDBStore.java
+++ b/hugegraph-rocksdb/src/main/java/com/baidu/hugegraph/backend/store/rocksdb/RocksDBStore.java
@@ -246,18 +246,28 @@ public abstract class RocksDBStore extends AbstractBackendStore<Session> {
}));
}
}
- this.waitOpenFinish(futures, openPool);
+
+ try {
+ this.waitOpenFinished(futures);
+ } finally {
+ this.shutdownOpenPool(openPool);
+ }
}
- private void waitOpenFinish(List<Future<?>> futures,
- ExecutorService openPool) {
+ private void waitOpenFinished(List<Future<?>> futures) {
for (Future<?> future : futures) {
try {
future.get();
} catch (Throwable e) {
+ if (e.getCause() instanceof ConnectionException) {
+ throw new ConnectionException("Failed to open RocksDB store", e);
+ }
throw new BackendException("Failed to open RocksDB store", e);
}
}
+ }
+
+ private void shutdownOpenPool(ExecutorService openPool) {
if (openPool.isShutdown()) {
return;
}