You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by sp...@apache.org on 2020/03/25 13:31:42 UTC
[tinkerpop] branch TINKERPOP-2353 created (now dfff3c6)
This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a change to branch TINKERPOP-2353
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git.
at dfff3c6 TINKERPOP-2353 Added some checks for null in shutdown of server
This branch includes the following new commits:
new dfff3c6 TINKERPOP-2353 Added some checks for null in shutdown of server
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[tinkerpop] 01/01: TINKERPOP-2353 Added some checks for null in
shutdown of server
Posted by sp...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a commit to branch TINKERPOP-2353
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit dfff3c61ff4550f672f62b499d1ed6ce15402c31
Author: Stephen Mallette <sp...@genoprime.com>
AuthorDate: Wed Mar 25 09:29:29 2020 -0400
TINKERPOP-2353 Added some checks for null in shutdown of server
The null checks seem necessary as failures in GraphManager initialization can leave Gremlin Server in a state where it is partially setup so that when shutdown starts those components that were not initialized end up being null and calls to their shutdown methods end up failing.
---
CHANGELOG.asciidoc | 1 +
.../tinkerpop/gremlin/server/GremlinServer.java | 58 ++++++++++++----------
2 files changed, 33 insertions(+), 26 deletions(-)
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 7b9ec50..3b5ff78 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -26,6 +26,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
* Added `trustStoreType` such that keystore and truststore can be of different types in the Java driver.
* Added session support to gremlin-javascript.
+* Fixed bug in Gremlin Server shutdown if failures occurred during `GraphManager` initialization.
* Modified Gremlin Server to close the session when the channel itself is closed.
* Added `maxWaitForClose` configuration option to the Java driver.
* Deprecated `maxWaitForSessionClose` in the Java driver.
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/GremlinServer.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/GremlinServer.java
index 333dcdd..50afdb2 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/GremlinServer.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/GremlinServer.java
@@ -95,7 +95,7 @@ public class GremlinServer {
this.settings = settings;
provideDefaultForGremlinPoolSize(settings);
this.isEpollEnabled = settings.useEpollEventLoop && SystemUtils.IS_OS_LINUX;
- if(settings.useEpollEventLoop && !SystemUtils.IS_OS_LINUX){
+ if (settings.useEpollEventLoop && !SystemUtils.IS_OS_LINUX){
logger.warn("cannot use epoll in non-linux env, falling back to NIO");
}
@@ -105,19 +105,20 @@ public class GremlinServer {
// if linux os use epoll else fallback to nio based eventloop
// epoll helps in reducing GC and has better performance
// http://netty.io/wiki/native-transports.html
- if(isEpollEnabled){
+ if (isEpollEnabled){
bossGroup = new EpollEventLoopGroup(settings.threadPoolBoss, threadFactoryBoss);
} else {
bossGroup = new NioEventLoopGroup(settings.threadPoolBoss, threadFactoryBoss);
}
final ThreadFactory threadFactoryWorker = ThreadFactoryUtil.create("worker-%d");
- if(isEpollEnabled) {
+ if (isEpollEnabled) {
workerGroup = new EpollEventLoopGroup(settings.threadPoolWorker, threadFactoryWorker);
- }else {
+ } else {
workerGroup = new NioEventLoopGroup(settings.threadPoolWorker, threadFactoryWorker);
}
+ // use the ExecutorService returned from ServerGremlinExecutor as it might be initialized there
serverGremlinExecutor = new ServerGremlinExecutor(settings, gremlinExecutorService, workerGroup);
this.gremlinExecutorService = serverGremlinExecutor.getGremlinExecutorService();
@@ -161,7 +162,7 @@ public class GremlinServer {
channelizer.init(serverGremlinExecutor);
b.group(bossGroup, workerGroup)
.childHandler(channelizer);
- if(isEpollEnabled){
+ if (isEpollEnabled){
b.channel(EpollServerSocketChannel.class);
} else{
b.channel(NioServerSocketChannel.class);
@@ -251,7 +252,7 @@ public class GremlinServer {
logger.info("Shutting down thread pools.");
try {
- gremlinExecutorService.shutdown();
+ if (gremlinExecutorService != null) gremlinExecutorService.shutdown();
} finally {
logger.debug("Shutdown Gremlin thread pool.");
}
@@ -269,18 +270,21 @@ public class GremlinServer {
// channel is shutdown as are the thread pools - time to kill graphs as nothing else should be acting on them
new Thread(() -> {
- serverGremlinExecutor.getHooks().forEach(hook -> {
- logger.info("Executing shutdown {}", LifeCycleHook.class.getSimpleName());
- try {
- hook.onShutDown(new LifeCycleHook.Context(logger));
- } catch (UnsupportedOperationException | UndeclaredThrowableException uoe) {
- // if the user doesn't implement onShutDown the scriptengine will throw
- // this exception. it can safely be ignored.
- }
- });
+ if (serverGremlinExecutor != null) {
+ serverGremlinExecutor.getHooks().forEach(hook -> {
+ logger.info("Executing shutdown {}", LifeCycleHook.class.getSimpleName());
+ try {
+ hook.onShutDown(new LifeCycleHook.Context(logger));
+ } catch (UnsupportedOperationException | UndeclaredThrowableException uoe) {
+ // if the user doesn't implement onShutDown the scriptengine will throw
+ // this exception. it can safely be ignored.
+ }
+ });
+ }
try {
- gremlinExecutorService.awaitTermination(30000, TimeUnit.MILLISECONDS);
+ if (gremlinExecutorService != null)
+ gremlinExecutorService.awaitTermination(30000, TimeUnit.MILLISECONDS);
} catch (InterruptedException ie) {
logger.warn("Timeout waiting for Gremlin thread pool to shutdown - continuing with shutdown process.");
}
@@ -291,16 +295,18 @@ public class GremlinServer {
logger.warn("Timeout waiting for boss/worker thread pools to shutdown - continuing with shutdown process.");
}
- serverGremlinExecutor.getGraphManager().getGraphNames().forEach(gName -> {
- logger.debug("Closing Graph instance [{}]", gName);
- try {
- serverGremlinExecutor.getGraphManager().getGraph(gName).close();
- } catch (Exception ex) {
- logger.warn(String.format("Exception while closing Graph instance [%s]", gName), ex);
- } finally {
- logger.info("Closed Graph instance [{}]", gName);
- }
- });
+ if (serverGremlinExecutor != null) {
+ serverGremlinExecutor.getGraphManager().getGraphNames().forEach(gName -> {
+ logger.debug("Closing Graph instance [{}]", gName);
+ try {
+ serverGremlinExecutor.getGraphManager().getGraph(gName).close();
+ } catch (Exception ex) {
+ logger.warn(String.format("Exception while closing Graph instance [%s]", gName), ex);
+ } finally {
+ logger.info("Closed Graph instance [{}]", gName);
+ }
+ });
+ }
// kills reporter threads. this is a last bit of cleanup that can be done. typically, the jvm is headed
// for shutdown which would obviously kill the reporters, but when it isn't they just keep reporting.