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 2021/12/29 19:57:56 UTC
[tinkerpop] branch ci-fix updated: focus on gc issues
This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a commit to branch ci-fix
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
The following commit(s) were added to refs/heads/ci-fix by this push:
new c34a460 focus on gc issues
c34a460 is described below
commit c34a460325310749e707a3743e7fefb46e071738
Author: Stephen Mallette <st...@amazon.com>
AuthorDate: Wed Dec 29 14:57:30 2021 -0500
focus on gc issues
---
.github/workflows/build-test.yml | 2 +-
gremlin-server/conf/neo4j-empty.properties | 6 ++++
.../tinkerpop/gremlin/server/GremlinServer.java | 35 ++++++++++++++++++++--
.../driver/remote/AbstractRemoteGraphProvider.java | 6 ++++
.../AbstractGremlinServerIntegrationTest.java | 8 +++++
.../gremlin/server/GremlinDriverIntegrateTest.java | 4 ++-
.../gremlin/server/GremlinServerIntegrateTest.java | 4 +--
7 files changed, 59 insertions(+), 6 deletions(-)
diff --git a/.github/workflows/build-test.yml b/.github/workflows/build-test.yml
index c561074..ec0b2e3 100644
--- a/.github/workflows/build-test.yml
+++ b/.github/workflows/build-test.yml
@@ -43,7 +43,7 @@ jobs:
- name: Build with Maven
run: |
mvn clean install -pl -:gremlin-javascript,-:gremlin-python,-gremlin-dotnet,-:gremlin-dotnet-source,-:gremlin-dotnet-tests -q -DskipTests -Dci
- mvn verify -pl :gremlin-server -DskipTests -DskipIntegrationTests=false
+ mvn verify -pl :gremlin-server -DskipTests -DskipIntegrationTests=false -DincludeNeo4j
spark-core:
name: spark core
timeout-minutes: 45
diff --git a/gremlin-server/conf/neo4j-empty.properties b/gremlin-server/conf/neo4j-empty.properties
index d1ad61e..f20aa30 100644
--- a/gremlin-server/conf/neo4j-empty.properties
+++ b/gremlin-server/conf/neo4j-empty.properties
@@ -34,6 +34,12 @@ gremlin.neo4j.conf.dbms.auto_index.nodes.enabled=true
gremlin.neo4j.conf.dbms.auto_index.relationships.enabled=true
#gremlin.neo4j.conf.dbms.auto_index.relationships.keys=
+# these memory settings are likely unsuitable for production cases
+gremlin.neo4j.conf.dbms.memory.heap.initial_size=500m
+gremlin.neo4j.conf.dbms.memory.heap.max_size=500m
+gremlin.neo4j.conf.dbms.memory.pagecache.size=1m
+gremlin.neo4j.conf.dbms.tx_state.memory_allocation=ON_HEAP
+
# uncomment the following to enable Bolt on the specified port
# gremlin.neo4j.conf.dbms.connector.0.type=BOLT
# gremlin.neo4j.conf.dbms.connector.0.enabled=true
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 2a1adf7..1e16ea1 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
@@ -39,12 +39,15 @@ import org.apache.tinkerpop.gremlin.server.util.LifeCycleHook;
import org.apache.tinkerpop.gremlin.server.util.MetricManager;
import org.apache.tinkerpop.gremlin.server.util.ServerGremlinExecutor;
import org.apache.tinkerpop.gremlin.server.util.ThreadFactoryUtil;
+import org.apache.tinkerpop.gremlin.structure.Graph;
+import org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerGraph;
import org.apache.tinkerpop.gremlin.util.Gremlin;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.lang.reflect.UndeclaredThrowableException;
+import java.util.Set;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutorService;
@@ -296,16 +299,44 @@ public class GremlinServer {
logger.warn("Timeout waiting for boss/worker thread pools to shutdown - continuing with shutdown process.");
}
+ // close TraversalSource and Graph instances - there aren't guarantees that closing Graph will close all
+ // spawned TraversalSource instances so both should be closed directly and independently.
if (serverGremlinExecutor != null) {
- serverGremlinExecutor.getGraphManager().getGraphNames().forEach(gName -> {
+ final Set<String> traversalSourceNames = serverGremlinExecutor.getGraphManager().getTraversalSourceNames();
+ traversalSourceNames.forEach(traversalSourceName -> {
+ logger.debug("Closing GraphTraversalSource instance [{}]", traversalSourceName);
+ try {
+ serverGremlinExecutor.getGraphManager().getTraversalSource(traversalSourceName).close();
+ } catch (Exception ex) {
+ logger.warn(String.format("Exception while closing GraphTraversalSource instance [%s]", traversalSourceName), ex);
+ } finally {
+ logger.info("Closed GraphTraversalSource instance [{}]", traversalSourceName);
+ }
+
+ try {
+ serverGremlinExecutor.getGraphManager().removeTraversalSource(traversalSourceName);
+ } catch (Exception ex) {
+ logger.warn(String.format("Exception while removing GraphTraversalSource instance [%s] from GraphManager", traversalSourceName), ex);
+ }
+ });
+
+ final Set<String> graphNames = serverGremlinExecutor.getGraphManager().getGraphNames();
+ graphNames.forEach(gName -> {
logger.debug("Closing Graph instance [{}]", gName);
try {
- serverGremlinExecutor.getGraphManager().getGraph(gName).close();
+ final Graph graph = serverGremlinExecutor.getGraphManager().getGraph(gName);
+ graph.close();
} catch (Exception ex) {
logger.warn(String.format("Exception while closing Graph instance [%s]", gName), ex);
} finally {
logger.info("Closed Graph instance [{}]", gName);
}
+
+ try {
+ serverGremlinExecutor.getGraphManager().removeGraph(gName);
+ } catch (Exception ex) {
+ logger.warn(String.format("Exception while removing Graph instance [%s] from GraphManager", gName), ex);
+ }
});
}
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/driver/remote/AbstractRemoteGraphProvider.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/driver/remote/AbstractRemoteGraphProvider.java
index e86a546..696669f 100644
--- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/driver/remote/AbstractRemoteGraphProvider.java
+++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/driver/remote/AbstractRemoteGraphProvider.java
@@ -237,6 +237,12 @@ public abstract class AbstractRemoteGraphProvider extends AbstractGraphProvider
@Override
public void close() throws Exception {
try {
+ cluster.close();
+ } catch (Exception ex) {
+ throw new RuntimeException(ex);
+ }
+
+ try {
stopServer();
} catch (Exception ex) {
throw new RuntimeException(ex);
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/AbstractGremlinServerIntegrationTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/AbstractGremlinServerIntegrationTest.java
index e49c85a..798eb2f 100644
--- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/AbstractGremlinServerIntegrationTest.java
+++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/AbstractGremlinServerIntegrationTest.java
@@ -18,7 +18,9 @@
*/
package org.apache.tinkerpop.gremlin.server;
+import org.apache.tinkerpop.gremlin.neo4j.structure.Neo4jGraph;
import org.apache.tinkerpop.gremlin.server.op.OpLoader;
+import org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerGraph;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
@@ -130,9 +132,15 @@ public abstract class AbstractGremlinServerIntegrationTest {
}
public void stopServer() throws Exception {
+ // calling close() on TinkerGraph does not free resources quickly enough. adding a clear() call let's gc
+ // cleanup earlier
+ server.getServerGremlinExecutor().getGraphManager().getAsBindings().values().stream()
+ .filter(g -> g instanceof TinkerGraph).forEach(g -> ((TinkerGraph) g).clear());
+
if (server != null) {
server.stop().join();
}
+
// reset the OpLoader processors so that they can get reconfigured on startup - Settings may have changed
// between tests
OpLoader.reset();
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverIntegrateTest.java
index 74f1bdb..15b3004 100644
--- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverIntegrateTest.java
+++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverIntegrateTest.java
@@ -669,6 +669,8 @@ public class GremlinDriverIntegrateTest extends AbstractGremlinServerIntegration
assertThat("Should contain 2 results", results.size() == 2);
assertThat("The numeric result should be 1", results.contains(1L));
assertThat("The string result contain label person", results.contains("person"));
+
+ executor.shutdown();
} finally {
cluster.close();
}
@@ -1892,7 +1894,7 @@ public class GremlinDriverIntegrateTest extends AbstractGremlinServerIntegration
logger.info("Verifying driver cannot connect to server.");
client.submit("g").all().get(500, TimeUnit.MILLISECONDS);
fail("Should throw an exception.");
- } catch (RuntimeException re) {
+ } catch (Exception re) {
// Client would have no active connections to the host, hence it would encounter a timeout
// trying to find an alive connection to the host.
assertThat(re.getCause(), instanceOf(NoHostAvailableException.class));
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java
index 45ada8d..4a95ae3 100644
--- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java
+++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java
@@ -1098,7 +1098,7 @@ public class GremlinServerIntegrateTest extends AbstractGremlinServerIntegration
traversal.hasNext();
// start a separate thread to iterate
- final Thread t = new Thread(traversal::iterate);
+ final Thread t = new Thread(traversal::iterate, name.getMethodName());
t.start();
// blocks here until traversal iteration is complete
@@ -1142,7 +1142,7 @@ public class GremlinServerIntegrateTest extends AbstractGremlinServerIntegration
traversal.hasNext();
// start a separate thread to iterate
- final Thread t = new Thread(traversal::iterate);
+ final Thread t = new Thread(traversal::iterate, name.getMethodName());
t.start();
// blocks here until traversal iteration is complete