Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: can't shutdown when starting with exception #1902

Merged
merged 2 commits into from
Jul 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,11 @@ public GraphManager(HugeConfig conf, EventHub hub) {
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));
Expand All @@ -111,10 +115,10 @@ public GraphManager(HugeConfig conf, EventHub hub) {
// 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) {
Expand All @@ -124,7 +128,7 @@ public void loadGraphs(Map<String, String> graphConfs) {
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);
}
Expand Down Expand Up @@ -215,25 +219,25 @@ public Serializer serializer(Graph g) {
}

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) {
closeTx(graphSourceNamesToCloseTxOn, Transaction.Status.ROLLBACK);
}

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) {
Expand All @@ -257,6 +261,13 @@ public AuthManager authManager() {
}

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();
}
Expand Down Expand Up @@ -369,10 +380,10 @@ private void loadGraph(String name, String graphConfPath) {
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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -47,6 +41,12 @@
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 =
Expand Down Expand Up @@ -127,7 +127,14 @@ public GraphManagerFactory(HugeConfig conf, EventHub hub) {
@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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,13 @@ private HugeServerInfo serverInfo(Id server) {
}

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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,8 @@ public static void main(String[] args) throws Exception {
for (HugeGraph graph : graphs) {
graph.close();
}
HugeFactory.shutdown(30L);
}

HugeFactory.shutdown(30L);
}

private static HugeGraph initGraph(String configPath) throws Exception {
Expand All @@ -99,13 +98,18 @@ private static HugeGraph initGraph(String configPath) throws Exception {
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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ public HugeGraphServer(String gremlinServerConf, String restServerConf)
} catch (Throwable t) {
LOG.error("HugeRestServer stop error: ", t);
}
HugeFactory.shutdown(30L);
throw e;
} finally {
System.setSecurityManager(securityManager);
Expand Down Expand Up @@ -118,12 +117,19 @@ public static void main(String[] args) throws Exception {

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();

Expand All @@ -132,8 +138,10 @@ public static void main(String[] args) throws Exception {
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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -246,18 +246,28 @@ public synchronized void open(HugeConfig config) {
}));
}
}
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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import java.util.Iterator;
import java.util.List;

import com.google.common.collect.ImmutableList;
import org.apache.commons.configuration2.BaseConfiguration;
import org.apache.commons.configuration2.Configuration;
import org.apache.commons.configuration2.PropertiesConfiguration;
Expand All @@ -36,11 +35,11 @@

import com.baidu.hugegraph.HugeException;
import com.baidu.hugegraph.HugeGraph;
import com.baidu.hugegraph.backend.BackendException;
import com.baidu.hugegraph.backend.id.IdGenerator;
import com.baidu.hugegraph.backend.store.BackendStoreInfo;
import com.baidu.hugegraph.backend.store.rocksdb.RocksDBOptions;
import com.baidu.hugegraph.config.CoreOptions;
import com.baidu.hugegraph.exception.ConnectionException;
import com.baidu.hugegraph.exception.ExistedException;
import com.baidu.hugegraph.schema.EdgeLabel;
import com.baidu.hugegraph.schema.IndexLabel;
Expand All @@ -50,6 +49,7 @@
import com.baidu.hugegraph.testutil.Assert;
import com.baidu.hugegraph.testutil.Utils;
import com.baidu.hugegraph.type.define.NodeRole;
import com.google.common.collect.ImmutableList;

public class MultiGraphsTest {

Expand Down Expand Up @@ -333,7 +333,7 @@ public void testCreateGraphsWithMultiDisksForRocksDB() {
g1.clearBackend();

final HugeGraph[] g2 = new HugeGraph[1];
Assert.assertThrows(BackendException.class, () -> {
Assert.assertThrows(ConnectionException.class, () -> {
g2[0] = openGraphWithBackend("g2", "rocksdb", "binary",
"rocksdb.data_disks",
"[g/range_int_index:rocksdb-index1]");
Expand All @@ -348,7 +348,7 @@ public void testCreateGraphsWithMultiDisksForRocksDB() {
});

final HugeGraph[] g3 = new HugeGraph[1];
Assert.assertThrows(BackendException.class, () -> {
Assert.assertThrows(ConnectionException.class, () -> {
g3[0] = openGraphWithBackend("g3", "rocksdb", "binary",
"rocksdb.data_disks",
"[g/secondary_index:/]");
Expand Down