[tinkerpop] 01/01: Merge branch '3.6-dev'
This is an automated email from the ASF dual-hosted git repository. colegreer pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/tinkerpop.git commit 6d55564560bb2210f78e8e0a0bea0e9ce41bd6a1 Merge: fed731ee56 379fa8b7b0 Author: Cole Greer AuthorDate: Mon Jun 12 15:49:26 2023 -0700 Merge branch '3.6-dev'
[tinkerpop] branch 3.6-dev updated (57bca85227 -> 379fa8b7b0)
This is an automated email from the ASF dual-hosted git repository. colegreer pushed a change to branch 3.6-dev in repository https://gitbox.apache.org/repos/asf/tinkerpop.git from 57bca85227 Merge branch '3.5-dev' into 3.6-dev add ac7b4ef5b5 CTR fix typo in python translator tests for 3.5 new 379fa8b7b0 Merge branch '3.5-dev' into 3.6-dev 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. Summary of changes:
[tinkerpop] branch master updated (fed731ee56 -> 6d55564560)
This is an automated email from the ASF dual-hosted git repository. colegreer pushed a change to branch master in repository https://gitbox.apache.org/repos/asf/tinkerpop.git from fed731ee56 Merge branch '3.6-dev' add ac7b4ef5b5 CTR fix typo in python translator tests for 3.5 add 379fa8b7b0 Merge branch '3.5-dev' into 3.6-dev new 6d55564560 Merge branch '3.6-dev' 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. Summary of changes:
[tinkerpop] 01/01: Merge branch '3.5-dev' into 3.6-dev
This is an automated email from the ASF dual-hosted git repository. colegreer pushed a commit to branch 3.6-dev in repository https://gitbox.apache.org/repos/asf/tinkerpop.git commit 379fa8b7b05bd48265cd25a602a66f896455d445 Merge: 57bca85227 ac7b4ef5b5 Author: Cole Greer AuthorDate: Mon Jun 12 15:47:38 2023 -0700 Merge branch '3.5-dev' into 3.6-dev
[tinkerpop] branch 3.5-dev updated: CTR fix typo in python translator tests for 3.5
This is an automated email from the ASF dual-hosted git repository.
colegreer pushed a commit to branch 3.5-dev
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
The following commit(s) were added to refs/heads/3.5-dev by this push:
new ac7b4ef5b5 CTR fix typo in python translator tests for 3.5
ac7b4ef5b5 is described below
commit ac7b4ef5b55e625f89f04933470ea209d1e0e76d
Author: Cole Greer
AuthorDate: Mon Jun 12 15:45:39 2023 -0700
CTR fix typo in python translator tests for 3.5
---
gremlin-python/src/main/python/tests/process/test_translator.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/gremlin-python/src/main/python/tests/process/test_translator.py
b/gremlin-python/src/main/python/tests/process/test_translator.py
index 5f48cc9a07..783d694af7 100644
--- a/gremlin-python/src/main/python/tests/process/test_translator.py
+++ b/gremlin-python/src/main/python/tests/process/test_translator.py
@@ -355,11 +355,11 @@ class TestTraversalStrategies(object):
"g.withStrategies(new
VertexProgramStrategy()).V().shortestPath().with('~tinkerpop.shortestPath.target',__.has('name','peter'))"])
# 99
-tests.append([g.V().has("p1", starting_with("foo")),
+tests.append([g.V().has("p1", startingWith("foo")),
"g.V().has('p1',startingWith('foo'))"])
# 100
-tests.append([g.V().has("p1", ending_with("foo")),
+tests.append([g.V().has("p1", endingWith("foo")),
"g.V().has('p1',endingWith('foo'))"])
# 101
[GitHub] [tinkerpop] lyndonbauto commented on pull request #2090: TINKERPOP-2958 Added cancel for query timeout task when query completes.
lyndonbauto commented on PR #2090:
URL: https://github.com/apache/tinkerpop/pull/2090#issuecomment-1587954864
@kenhuuu
> Hi Lyndon, is it possible for you to share what kind of testing you were
running that showed this issue?
Hey Ken, yeah sure, this isn't pretty code but here it is:
```
package com.aerospike.firefly.server;
import com.aerospike.firefly.Server;
import org.apache.tinkerpop.gremlin.driver.Cluster;
import org.apache.tinkerpop.gremlin.driver.remote.DriverRemoteConnection;
import
org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource;
import org.apache.tinkerpop.gremlin.structure.Property;
import org.junit.Test;
import java.util.ArrayList;
import java.util.List;
import java.util.Random;
import java.util.Timer;
import java.util.TimerTask;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.stream.Collectors;
import static org.apache.tinkerpop.gremlin.driver.Tokens.ARGS_EVAL_TIMEOUT;
import static
org.apache.tinkerpop.gremlin.process.traversal.AnonymousTraversalSource.traversal;
public class TestServer {
private static final long SECOND = 1000;
private static final long MINUTE = 60 * SECOND;
private static final long TEST_DURATION = 4 * MINUTE;
private long totalExecuted = 0;
Server server;
@Test
public void testServer() throws Exception {
server = Server.main(new String[]
{"../conf/firefly-gremlin-server-local.yaml"});
HEAP_REPORTING_TIMER.schedule(new HeapReport(), 0, 5000);
exposeLeak();
}
private static final Timer HEAP_REPORTING_TIMER = new Timer(true);
private class HeapReport extends TimerTask {
@Override
public void run() {
System.gc();
System.gc();
System.gc();
System.out.printf("Heap size: %d MB%n",
Runtime.getRuntime().totalMemory() / 1024 / 1024);
System.out.printf("Executed %d queries%n", totalExecuted);
}
}
public void exposeLeak() throws Exception {
final long startTime = System.currentTimeMillis();
Cluster cluster =
Cluster.build().addContactPoint("localhost").port(8182).create();
DriverRemoteConnection drc = DriverRemoteConnection.using(cluster,
"g");
// final DriverRemoteConnection drc =
DriverRemoteConnection.using("localhost", 8182, "g");
final GraphTraversalSource g = traversal().withRemote(drc);
final ExecutorService executorService =
Executors.newFixedThreadPool(16);
g.with(ARGS_EVAL_TIMEOUT, 1L).V().drop().iterate();
for (int i = 0; i < 25000; i++) {
g.addV("randomVertex").
property("last_seen", String.format("%d",
System.currentTimeMillis())).
property("something", String.format("%d", i)).
property("otherThing", String.format("%d", i)).
property("anotherThing", String.format("%d", i)).
property("yetAnotherThing", String.format("%d", i)).
property("andAnotherThing", String.format("%d", i)).
property("andYetAnotherThing", String.format("%d", i)).
iterate();
}
final List ids = g.V().id().toList();
final List>> futures = new
ArrayList<>();
while (System.currentTimeMillis() - startTime <= TEST_DURATION) {
while (futures.size() > 500) {
Thread.sleep(100);
final List>>
completedFutures =
futures.stream().filter(Future::isDone).collect(Collectors.toList());
completedFutures.forEach(f -> {
try {
f.get();
} catch (Exception e) {
e.printStackTrace();
}
});
totalExecuted += completedFutures.size();
futures.removeAll(completedFutures);
}
final Random rand = new Random();
final List idz = new ArrayList<>();
for (int i = 0; i < 9; i++) {
idz.add(ids.get(rand.nextInt(ids.size(;
}
futures.add(executorService.submit(() ->
g.V(idz).properties("last_seen").toList()));
}
futures.forEach(f-> {
try {
f.get();
} catch (Exception e) {
e.printStackTrace();
}
});
drc.close();
Thread.sleep(3);
}
}
```
The server code is:
```
package com.aerospike.firefly;
import
[GitHub] [tinkerpop] kenhuuu commented on pull request #2090: TINKERPOP-2958 Added cancel for query timeout task when query completes.
kenhuuu commented on PR #2090: URL: https://github.com/apache/tinkerpop/pull/2090#issuecomment-1587936932 Hi Lyndon, is it possible for you to share what kind of testing you were running that showed this issue? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [tinkerpop] codecov-commenter commented on pull request #2090: TINKERPOP-2958 Added cancel for query timeout task when query completes.
codecov-commenter commented on PR #2090: URL: https://github.com/apache/tinkerpop/pull/2090#issuecomment-1587890366 ## [Codecov](https://app.codecov.io/gh/apache/tinkerpop/pull/2090?src=pr=h1_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) Report > Merging [#2090](https://app.codecov.io/gh/apache/tinkerpop/pull/2090?src=pr=desc_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) (21eda20) into [3.5-dev](https://app.codecov.io/gh/apache/tinkerpop/commit/d90e6f788523c591bd45aa9f0d48ec82cc2c42ee?el=desc_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) (d90e6f7) will **decrease** coverage by `0.01%`. > The diff coverage is `72.72%`. > :exclamation: Current head 21eda20 differs from pull request most recent head 3f2ed90. Consider uploading reports for the commit 3f2ed90 to get more accurate results ```diff @@ Coverage Diff @@ ## 3.5-dev#2090 +/- ## = - Coverage 69.92% 69.91% -0.01% - Complexity 8978 8980 +2 = Files865 841 -24 Lines 4094937508-3441 Branches5442 5445 +3 = - Hits 2863326224-2409 + Misses 10407 9553 -854 + Partials1909 1731 -178 ``` | [Impacted Files](https://app.codecov.io/gh/apache/tinkerpop/pull/2090?src=pr=tree_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) | Coverage Δ | | |---|---|---| | [.../gremlin/server/op/session/SessionOpProcessor.java](https://app.codecov.io/gh/apache/tinkerpop/pull/2090?src=pr=tree_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache#diff-Z3JlbWxpbi1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3RpbmtlcnBvcC9ncmVtbGluL3NlcnZlci9vcC9zZXNzaW9uL1Nlc3Npb25PcFByb2Nlc3Nvci5qYXZh) | `7.41% <0.00%> (-0.03%)` | :arrow_down: | | [...mlin/server/op/traversal/TraversalOpProcessor.java](https://app.codecov.io/gh/apache/tinkerpop/pull/2090?src=pr=tree_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache#diff-Z3JlbWxpbi1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3RpbmtlcnBvcC9ncmVtbGluL3NlcnZlci9vcC90cmF2ZXJzYWwvVHJhdmVyc2FsT3BQcm9jZXNzb3IuamF2YQ==) | `49.74% <80.00%> (-0.26%)` | :arrow_down: | | [...a/org/apache/tinkerpop/gremlin/server/Context.java](https://app.codecov.io/gh/apache/tinkerpop/pull/2090?src=pr=tree_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache#diff-Z3JlbWxpbi1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3RpbmtlcnBvcC9ncmVtbGluL3NlcnZlci9Db250ZXh0LmphdmE=) | `84.33% <100.00%> (+1.23%)` | :arrow_up: | ... and [29 files with indirect coverage changes](https://app.codecov.io/gh/apache/tinkerpop/pull/2090/indirect-changes?src=pr=tree-more_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [tinkerpop] lyndonbauto commented on pull request #2090: TINKERPOP-2958 Added cancel for query timeout task when query completes.
lyndonbauto commented on PR #2090: URL: https://github.com/apache/tinkerpop/pull/2090#issuecomment-1587887574 > Can similar problem occur in `GremlinExecutor.eval`? I looked and as far as I can tell, this issue is limited to the two places I fixed it, however I am not well versed with this part of the gremlin code base so I may be wrong. > Are you planning to open separate PR for 3.5 and 3.6? This code is identical in 3.5 and 3.6 so my plan was to merge this to 3.5-dev then merge 3.5-dev into 3.6-dev. I just noticed I was pointed at master but meant to be pointed at 3.5-dev in this PR. @spmallette is this the right merging strategy? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [tinkerpop] vkagamlyk commented on pull request #2090: TINKERPOP-2958 Added cancel for query timeout task when query completes.
vkagamlyk commented on PR #2090: URL: https://github.com/apache/tinkerpop/pull/2090#issuecomment-1587881620 Can similar problem occur in `GremlinExecutor.eval`? Are you planning to open separate PR for 3.5 and 3.6? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[tinkerpop] branch TINKERPOP-2958 updated (21eda2074c -> 3f2ed900e6)
This is an automated email from the ASF dual-hosted git repository. lyndonb pushed a change to branch TINKERPOP-2958 in repository https://gitbox.apache.org/repos/asf/tinkerpop.git from 21eda2074c TINKERPOP-2958 Added cancel for query timeout task when query completes. add 3f2ed900e6 TINKERPOP-2958 Fixing comment. No new revisions were added by this update. Summary of changes: .../tinkerpop/gremlin/server/op/traversal/TraversalOpProcessor.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
[GitHub] [tinkerpop] lyndonbauto opened a new pull request, #2090: TINKERPOP-2958 Added cancel for query timeout task when query completes.
lyndonbauto opened a new pull request, #2090: URL: https://github.com/apache/tinkerpop/pull/2090 I noticed extreme memory usage in recent versions of TinkerPop. After some investigation I found that this was because the task that is scheduled to cancel the query if it doesn't complete in time is not cancelled when the query completes. This means that if the timeout is large and you have high throughput on the graph, memory starts adding up. On my laptop with Aerospike Graph, I was seeing ~1 GB per second growth in memory until it hit about 10 GB and then my system would slow down and eventually OOME. I added a cancel for this in this PR to fix this issue and tested it and the memory growth does not happen. (I tested with changes on 3.6-dev but then brought those to 3.5-dev since the problem is rooted there). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@tinkerpop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[tinkerpop] branch TINKERPOP-2958 updated: TINKERPOP-2958 Added cancel for query timeout task when query completes.
This is an automated email from the ASF dual-hosted git repository.
lyndonb pushed a commit to branch TINKERPOP-2958
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
The following commit(s) were added to refs/heads/TINKERPOP-2958 by this push:
new 21eda2074c TINKERPOP-2958 Added cancel for query timeout task when
query completes.
21eda2074c is described below
commit 21eda2074c753d73df9a3839ba54b300b9a6864c
Author: lyndon
AuthorDate: Mon Jun 12 11:18:57 2023 -0700
TINKERPOP-2958 Added cancel for query timeout task when query completes.
---
CHANGELOG.asciidoc | 1 +
.../apache/tinkerpop/gremlin/server/Context.java | 23 ++
.../server/op/session/SessionOpProcessor.java | 13 ++--
.../server/op/traversal/TraversalOpProcessor.java | 11 +--
4 files changed, 44 insertions(+), 4 deletions(-)
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 14b1e8f724..07122d2b70 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -29,6 +29,7 @@
image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
* Upgraded `gremlin-go` to Go 1.20.
* Improved the python Translator class
* Added `gremlin-java8.bat` file as a workaround to allow loading the console
using Java 8 on Windows.
+* Fixed a bug in `gremlin-server` where timeout tasks were not cancelled and
could cause very large memory usage when timeout is large.
[[release-3-5-6]]
=== TinkerPop 3.5.6 (Release Date: May 1, 2023)
diff --git
a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/Context.java
b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/Context.java
index ad15772509..38f2f25966 100644
---
a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/Context.java
+++
b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/Context.java
@@ -35,6 +35,7 @@ import org.slf4j.LoggerFactory;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.atomic.AtomicBoolean;
/**
@@ -55,6 +56,9 @@ public class Context {
private final RequestContentType requestContentType;
private final Object gremlinArgument;
private final AtomicBoolean startedResponse = new AtomicBoolean(false);
+private ScheduledFuture timeoutExecutor = null;
+private boolean timeoutExecutorGrabbed = false;
+private final Object timeoutExecutorLock = new Object();
/**
* The type of the request as determined by the contents of {@link
Tokens#ARGS_GREMLIN}.
@@ -92,6 +96,25 @@ public class Context {
this.requestTimeout = determineTimeout();
}
+public void setTimeoutExecutor(final ScheduledFuture timeoutExecutor) {
+synchronized (timeoutExecutorLock) {
+this.timeoutExecutor = timeoutExecutor;
+
+// Timeout was grabbed before we got here, this means the query
executed before the timeout was created.
+if (timeoutExecutorGrabbed) {
+// Cancel the timeout.
+this.timeoutExecutor.cancel(true);
+}
+}
+}
+
+public ScheduledFuture getTimeoutExecutor() {
+synchronized (timeoutExecutorLock) {
+timeoutExecutorGrabbed = true;
+return this.timeoutExecutor;
+}
+}
+
/**
* The timeout for the request. If the request is a script it examines the
script for a timeout setting using
* {@code with()}. If that is not found then it examines the request
itself to see if the timeout is provided by
diff --git
a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/SessionOpProcessor.java
b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/SessionOpProcessor.java
index 4077ffdb11..3d17786be9 100644
---
a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/SessionOpProcessor.java
+++
b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/SessionOpProcessor.java
@@ -65,6 +65,7 @@ import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.Future;
import java.util.concurrent.FutureTask;
+import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
import java.util.function.Function;
import java.util.function.Supplier;
@@ -466,6 +467,14 @@ public class SessionOpProcessor extends
AbstractEvalOpProcessor {
} finally {
// todo: timer matter???
//timerContext.stop();
+
+// There is a race condition that this query may have finished
before the timeoutFuture was created,
+// though this is very unlikely. This is handled in the
settor, if this has already been grabbed.
+// If we passed this point and the setter hasn't been called,
it
[tinkerpop] branch TINKERPOP-2958 created (now d90e6f7885)
This is an automated email from the ASF dual-hosted git repository. lyndonb pushed a change to branch TINKERPOP-2958 in repository https://gitbox.apache.org/repos/asf/tinkerpop.git at d90e6f7885 Merge pull request #2086 No new revisions were added by this update.
