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 2015/10/28 10:32:29 UTC
[1/4] incubator-tinkerpop git commit: TINKERPOP3-910 Clears the
session channel attribute between requests.
Repository: incubator-tinkerpop
Updated Branches:
refs/heads/master 55cdc2dc4 -> b70f5f546
TINKERPOP3-910 Clears the session channel attribute between requests.
If this value (and any other channel attribute) isn't cleared it will hang around until the next request as they are global to the channel. In this case, a first "in-session" request would succeed and put the channel in "session" mode. A future sessionless request would arrive and execute its serialization process in the sessions threads as the channel still had the session state from the previous request. This likely would have been easier to debug if the the onReadWrite stuff had been ThreadLocal as the in-session requests would have been in MANUAL mode and the sessionless request would have yielded a more clear error that the transaction was closed. Instead, because it was AUTO the sessionless request auto-opened the tx.
The tests for this are in the link referenced above. We'll need to merge those tests into this branch at some point to get this all to work together.
Project: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/commit/43f64c2a
Tree: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/tree/43f64c2a
Diff: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/diff/43f64c2a
Branch: refs/heads/master
Commit: 43f64c2a4aad965343ef87ce5407cb0b79d96714
Parents: 2b83c3d
Author: Stephen Mallette <sp...@genoprime.com>
Authored: Fri Oct 23 16:36:37 2015 -0400
Committer: Stephen Mallette <sp...@genoprime.com>
Committed: Fri Oct 23 16:36:37 2015 -0400
----------------------------------------------------------------------
.../gremlin/server/handler/NioGremlinBinaryRequestDecoder.java | 5 +++++
.../gremlin/server/handler/WsGremlinBinaryRequestDecoder.java | 5 +++++
.../gremlin/server/handler/WsGremlinCloseRequestDecoder.java | 5 +++++
.../gremlin/server/handler/WsGremlinTextRequestDecoder.java | 5 +++++
4 files changed, 20 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/43f64c2a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/NioGremlinBinaryRequestDecoder.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/NioGremlinBinaryRequestDecoder.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/NioGremlinBinaryRequestDecoder.java
index c6996a4..622a2f7 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/NioGremlinBinaryRequestDecoder.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/NioGremlinBinaryRequestDecoder.java
@@ -60,6 +60,11 @@ public class NioGremlinBinaryRequestDecoder extends ReplayingDecoder<NioGremlinB
final String contentType = contentTypeFrame.toString(CharsetUtil.UTF_8);
final MessageSerializer serializer = select(contentType, Serializers.DEFAULT_REQUEST_SERIALIZER);
+
+ // it's important to re-initialize these channel attributes as they apply globally to the channel. in
+ // other words, the next request to this channel might not come with the same configuration and mixed
+ // state can carry through from one request to the next
+ channelHandlerContext.channel().attr(StateKey.SESSION).set(null);
channelHandlerContext.channel().attr(StateKey.SERIALIZER).set(serializer);
channelHandlerContext.channel().attr(StateKey.USE_BINARY).set(true);
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/43f64c2a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WsGremlinBinaryRequestDecoder.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WsGremlinBinaryRequestDecoder.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WsGremlinBinaryRequestDecoder.java
index 071eb18..773d33d 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WsGremlinBinaryRequestDecoder.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WsGremlinBinaryRequestDecoder.java
@@ -67,8 +67,13 @@ public class WsGremlinBinaryRequestDecoder extends MessageToMessageDecoder<Binar
final String contentType = contentTypeBytes.toString(UTF8);
final MessageSerializer serializer = select(contentType, Serializers.DEFAULT_REQUEST_SERIALIZER);
+ // it's important to re-initialize these channel attributes as they apply globally to the channel. in
+ // other words, the next request to this channel might not come with the same configuration and mixed
+ // state can carry through from one request to the next
+ channelHandlerContext.channel().attr(StateKey.SESSION).set(null);
channelHandlerContext.channel().attr(StateKey.SERIALIZER).set(serializer);
channelHandlerContext.channel().attr(StateKey.USE_BINARY).set(true);
+
try {
objects.add(serializer.deserializeRequest(messageBytes.discardReadBytes()));
} catch (SerializationException se) {
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/43f64c2a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WsGremlinCloseRequestDecoder.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WsGremlinCloseRequestDecoder.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WsGremlinCloseRequestDecoder.java
index fc6f3cf..44a0906 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WsGremlinCloseRequestDecoder.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WsGremlinCloseRequestDecoder.java
@@ -67,8 +67,13 @@ public class WsGremlinCloseRequestDecoder extends MessageToMessageDecoder<CloseW
final String contentType = contentTypeBytes.toString(UTF8);
final MessageSerializer serializer = select(contentType, Serializers.DEFAULT_REQUEST_SERIALIZER);
+ // it's important to re-initialize these channel attributes as they apply globally to the channel. in
+ // other words, the next request to this channel might not come with the same configuration and mixed
+ // state can carry through from one request to the next
+ channelHandlerContext.channel().attr(StateKey.SESSION).set(null);
channelHandlerContext.channel().attr(StateKey.SERIALIZER).set(serializer);
channelHandlerContext.channel().attr(StateKey.USE_BINARY).set(true);
+
try {
objects.add(serializer.deserializeRequest(messageBytes.discardReadBytes()));
} catch (SerializationException se) {
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/43f64c2a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WsGremlinTextRequestDecoder.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WsGremlinTextRequestDecoder.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WsGremlinTextRequestDecoder.java
index 9316c1b..5e05bc2 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WsGremlinTextRequestDecoder.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WsGremlinTextRequestDecoder.java
@@ -55,8 +55,13 @@ public class WsGremlinTextRequestDecoder extends MessageToMessageDecoder<TextWeb
// the default serializer must be a MessageTextSerializer instance to be compatible with this decoder
final MessageTextSerializer serializer = (MessageTextSerializer) select("application/json", Serializers.DEFAULT_REQUEST_SERIALIZER);
+ // it's important to re-initialize these channel attributes as they apply globally to the channel. in
+ // other words, the next request to this channel might not come with the same configuration and mixed
+ // state can carry through from one request to the next
+ channelHandlerContext.channel().attr(StateKey.SESSION).set(null);
channelHandlerContext.channel().attr(StateKey.SERIALIZER).set(serializer);
channelHandlerContext.channel().attr(StateKey.USE_BINARY).set(false);
+
objects.add(serializer.deserializeRequest(frame.text()));
} catch (SerializationException se) {
objects.add(RequestMessage.INVALID);
[3/4] incubator-tinkerpop git commit: TINKERPOP3-910 Added an assert
as suggested by @pluradj in the PR.
Posted by sp...@apache.org.
TINKERPOP3-910 Added an assert as suggested by @pluradj in the PR.
Project: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/commit/23356d4c
Tree: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/tree/23356d4c
Diff: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/diff/23356d4c
Branch: refs/heads/master
Commit: 23356d4cb27d13745833f9150a283e7a8a2816e1
Parents: 0b57371
Author: Stephen Mallette <sp...@genoprime.com>
Authored: Wed Oct 28 05:30:31 2015 -0400
Committer: Stephen Mallette <sp...@genoprime.com>
Committed: Wed Oct 28 05:30:31 2015 -0400
----------------------------------------------------------------------
.../tinkerpop/gremlin/server/GremlinServerIntegrateTest.java | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/23356d4c/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java
----------------------------------------------------------------------
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 77dccd5..0b4f7cd 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
@@ -608,7 +608,7 @@ public class GremlinServerIntegrateTest extends AbstractGremlinServerIntegration
latch3.countDown();
});
- latch3.await(1500, TimeUnit.MILLISECONDS);
+ assertTrue(latch3.await(1500, TimeUnit.MILLISECONDS));
// Check to see if the transaction is closed.
final CountDownLatch latch4 = new CountDownLatch(1);
[4/4] incubator-tinkerpop git commit: Merge remote-tracking branch
'origin/TINKERPOP3-910'
Posted by sp...@apache.org.
Merge remote-tracking branch 'origin/TINKERPOP3-910'
Project: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/commit/b70f5f54
Tree: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/tree/b70f5f54
Diff: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/diff/b70f5f54
Branch: refs/heads/master
Commit: b70f5f5465d2e91245af7586f4a7fbf738f57939
Parents: 55cdc2d 23356d4
Author: Stephen Mallette <sp...@genoprime.com>
Authored: Wed Oct 28 05:32:13 2015 -0400
Committer: Stephen Mallette <sp...@genoprime.com>
Committed: Wed Oct 28 05:32:13 2015 -0400
----------------------------------------------------------------------
.../handler/NioGremlinBinaryRequestDecoder.java | 5 +
.../handler/WsGremlinBinaryRequestDecoder.java | 5 +
.../handler/WsGremlinCloseRequestDecoder.java | 5 +
.../handler/WsGremlinTextRequestDecoder.java | 5 +
.../server/GremlinDriverIntegrateTest.java | 28 ++++++
.../server/GremlinServerIntegrateTest.java | 100 +++++++++++++++++++
6 files changed, 148 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/b70f5f54/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverIntegrateTest.java
----------------------------------------------------------------------
[2/4] incubator-tinkerpop git commit: added tests around single
channel in session and sessionless requests
Posted by sp...@apache.org.
added tests around single channel in session and sessionless requests
Project: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/commit/0b573712
Tree: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/tree/0b573712
Diff: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/diff/0b573712
Branch: refs/heads/master
Commit: 0b573712c58c3f115a19b4c7542add3d7bbeb8fe
Parents: 43f64c2
Author: Dylan Millikin <dy...@brightzone.fr>
Authored: Mon Oct 26 11:18:50 2015 +0100
Committer: Dylan Millikin <dy...@brightzone.fr>
Committed: Mon Oct 26 13:48:12 2015 +0100
----------------------------------------------------------------------
.../server/GremlinDriverIntegrateTest.java | 28 ++++++
.../server/GremlinServerIntegrateTest.java | 100 +++++++++++++++++++
2 files changed, 128 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/0b573712/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinDriverIntegrateTest.java
----------------------------------------------------------------------
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 575f96e..3e0f821 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
@@ -99,6 +99,7 @@ public class GremlinDriverIntegrateTest extends AbstractGremlinServerIntegration
case "shouldExecuteScriptInSessionOnTransactionalGraph":
case "shouldExecuteSessionlessScriptOnTransactionalGraph":
case "shouldExecuteScriptInSessionOnTransactionalWithManualTransactionsGraph":
+ case "shouldExecuteInSessionAndSessionlessWithoutOpeningTransaction":
deleteDirectory(new File("/tmp/neo4j"));
settings.graphs.put("graph", "conf/neo4j-empty.properties");
break;
@@ -616,6 +617,33 @@ public class GremlinDriverIntegrateTest extends AbstractGremlinServerIntegration
}
@Test
+ public void shouldExecuteInSessionAndSessionlessWithoutOpeningTransaction() throws Exception {
+ assumeNeo4jIsPresent();
+
+ final Cluster cluster = Cluster.build().create();
+ final Client sessionClient = cluster.connect(name.getMethodName());
+ final Client sessionlessClient = cluster.connect();
+
+ //open transaction in session, then add vertex and commit
+ sessionClient.submit("graph.tx().open()").all().get();
+ final Vertex vertexBeforeTx = sessionClient.submit("v=graph.addVertex(\"name\",\"stephen\")").all().get().get(0).getVertex();
+ assertEquals("stephen", vertexBeforeTx.values("name").next());
+ sessionClient.submit("graph.tx().commit()").all().get();
+
+ // check that session transaction is closed
+ final boolean isOpen = sessionClient.submit("graph.tx().isOpen()").all().get().get(0).getBoolean();
+ assertTrue("Transaction should be closed", !isOpen);
+
+ //run a sessionless read
+ sessionlessClient.submit("graph.traversal().V()").all().get();
+
+ // check that session transaction is still closed
+ final boolean isOpenAfterSessionless = sessionClient.submit("graph.tx().isOpen()").all().get().get(0).getBoolean();
+ assertTrue("Transaction should stil be closed", !isOpenAfterSessionless);
+
+ }
+
+ @Test
public void shouldExecuteSessionlessScriptOnTransactionalGraph() throws Exception {
assumeNeo4jIsPresent();
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/0b573712/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java
----------------------------------------------------------------------
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 2e65a56..77dccd5 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
@@ -18,6 +18,7 @@
*/
package org.apache.tinkerpop.gremlin.server;
+import java.io.File;
import org.apache.commons.lang.exception.ExceptionUtils;
import org.apache.log4j.Logger;
import org.apache.tinkerpop.gremlin.driver.Client;
@@ -41,6 +42,7 @@ import org.junit.Before;
import org.junit.Test;
import java.nio.channels.ClosedChannelException;
+import java.util.ArrayList;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.CompletableFuture;
@@ -119,6 +121,10 @@ public class GremlinServerIntegrateTest extends AbstractGremlinServerIntegration
processorSettings.config.put(SessionOpProcessor.CONFIG_SESSION_TIMEOUT, 3000l);
settings.processors.add(processorSettings);
break;
+ case "shouldExecuteInSessionAndSessionlessWithoutOpeningTransactionWithSingleClient":
+ deleteDirectory(new File("/tmp/neo4j"));
+ settings.graphs.put("graph", "conf/neo4j-empty.properties");
+ break;
}
return settings;
@@ -562,4 +568,98 @@ public class GremlinServerIntegrateTest extends AbstractGremlinServerIntegration
cluster.close();
}
}
+
+ @Test
+ public void shouldExecuteInSessionAndSessionlessWithoutOpeningTransactionWithSingleClient() throws Exception {
+ assumeNeo4jIsPresent();
+
+ final SimpleClient client = new WebSocketClient();
+
+ //open a transaction, create a vertex, commit
+ final CountDownLatch latch = new CountDownLatch(1);
+ final RequestMessage OpenRequest = RequestMessage.build(Tokens.OPS_EVAL)
+ .processor("session")
+ .addArg(Tokens.ARGS_SESSION, name.getMethodName())
+ .addArg(Tokens.ARGS_GREMLIN, "graph.tx().open()")
+ .create();
+ client.submit(OpenRequest, (r) -> {
+ latch.countDown();
+ });
+ assertTrue(latch.await(1500, TimeUnit.MILLISECONDS));
+
+ final CountDownLatch latch2 = new CountDownLatch(1);
+ final RequestMessage AddRequest = RequestMessage.build(Tokens.OPS_EVAL)
+ .processor("session")
+ .addArg(Tokens.ARGS_SESSION, name.getMethodName())
+ .addArg(Tokens.ARGS_GREMLIN, "v=graph.addVertex(\"name\",\"stephen\")")
+ .create();
+ client.submit(AddRequest, (r) -> {
+ latch2.countDown();
+ });
+ assertTrue(latch2.await(1500, TimeUnit.MILLISECONDS));
+
+ final CountDownLatch latch3 = new CountDownLatch(1);
+ final RequestMessage CommitRequest = RequestMessage.build(Tokens.OPS_EVAL)
+ .processor("session")
+ .addArg(Tokens.ARGS_SESSION, name.getMethodName())
+ .addArg(Tokens.ARGS_GREMLIN, "graph.tx().commit()")
+ .create();
+ client.submit(CommitRequest, (r) -> {
+ latch3.countDown();
+
+ });
+ latch3.await(1500, TimeUnit.MILLISECONDS);
+
+ // Check to see if the transaction is closed.
+ final CountDownLatch latch4 = new CountDownLatch(1);
+ final AtomicBoolean isOpen = new AtomicBoolean(false);
+ final RequestMessage CheckRequest = RequestMessage.build(Tokens.OPS_EVAL)
+ .processor("session")
+ .addArg(Tokens.ARGS_SESSION, name.getMethodName())
+ .addArg(Tokens.ARGS_GREMLIN, "graph.tx().isOpen()")
+ .create();
+ client.submit(CheckRequest, (r) -> {
+ ArrayList<Boolean> response = (ArrayList) r.getResult().getData();
+ isOpen.set(response.get(0));
+ latch4.countDown();
+ });
+ assertTrue(latch4.await(1500, TimeUnit.MILLISECONDS));
+
+ // make sure no extra message sneak in
+ Thread.sleep(1000);
+
+ assertTrue("Transaction should be closed", !isOpen.get());
+
+ //lets run a sessionless read
+ final CountDownLatch latch5 = new CountDownLatch(1);
+ final RequestMessage sessionlessRequest = RequestMessage.build(Tokens.OPS_EVAL)
+ .addArg(Tokens.ARGS_GREMLIN, "graph.traversal().V()")
+ .create();
+ client.submit(sessionlessRequest, (r) -> {
+ latch5.countDown();
+ });
+ assertTrue(latch5.await(1500, TimeUnit.MILLISECONDS));
+
+ // Check to see if the transaction is still closed.
+ final CountDownLatch latch6 = new CountDownLatch(1);
+ final AtomicBoolean isStillOpen = new AtomicBoolean(false);
+ final RequestMessage CheckAgainRequest = RequestMessage.build(Tokens.OPS_EVAL)
+ .processor("session")
+ .addArg(Tokens.ARGS_SESSION, name.getMethodName())
+ .addArg(Tokens.ARGS_GREMLIN, "graph.tx().isOpen()")
+ .create();
+ client.submit(CheckAgainRequest, (r) -> {
+ ArrayList<Boolean> response = (ArrayList) r.getResult().getData();
+ isStillOpen.set(response.get(0));
+ latch6.countDown();
+ });
+ assertTrue(latch6.await(1500, TimeUnit.MILLISECONDS));
+
+ // make sure no extra message sneak in
+ Thread.sleep(1000);
+
+ assertTrue("Transaction should still be closed", !isStillOpen.get());
+ }
+
+
}