You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tinkerpop.apache.org by spmallette <gi...@git.apache.org> on 2015/10/26 14:18:00 UTC

[GitHub] incubator-tinkerpop pull request: TINKERPOP3-910 - In session tran...

GitHub user spmallette opened a pull request:

    https://github.com/apache/incubator-tinkerpop/pull/122

    TINKERPOP3-910 - In session transaction opened from sessionless request

    https://issues.apache.org/jira/browse/TINKERPOP3-910
    
    Test via:
    
    ```text
    mvn clean install -DincludeNeo4j
    mvn clean -DskipIntegrationTests=false -DincludeNeo4j --projects gremlin-server verify
    ```
    
    The summary for this change is in the comments of this commit:
    
    https://github.com/apache/incubator-tinkerpop/commit/43f64c2a4aad965343ef87ce5407cb0b79d96714
    
    VOTE: +1

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/apache/incubator-tinkerpop TINKERPOP3-910

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-tinkerpop/pull/122.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #122
    
----
commit 43f64c2a4aad965343ef87ce5407cb0b79d96714
Author: Stephen Mallette <sp...@genoprime.com>
Date:   2015-10-23T20:36:37Z

    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.

commit 0b573712c58c3f115a19b4c7542add3d7bbeb8fe
Author: Dylan Millikin <dy...@brightzone.fr>
Date:   2015-10-26T10:18:50Z

    added tests around single channel in session and sessionless requests

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-tinkerpop pull request: TINKERPOP3-910 - In session tran...

Posted by dkuppitz <gi...@git.apache.org>.
Github user dkuppitz commented on the pull request:

    https://github.com/apache/incubator-tinkerpop/pull/122#issuecomment-151215336
  
    Simple code changes, tests passed for me, everything looking good.
    
    Vote: +1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-tinkerpop pull request: TINKERPOP3-910 - In session tran...

Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the pull request:

    https://github.com/apache/incubator-tinkerpop/pull/122#issuecomment-151774888
  
    RESULT: 4 +1 (3 binding, 1 non-binding) - merging


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-tinkerpop pull request: TINKERPOP3-910 - In session tran...

Posted by pluradj <gi...@git.apache.org>.
Github user pluradj commented on a diff in the pull request:

    https://github.com/apache/incubator-tinkerpop/pull/122#discussion_r43218601
  
    --- Diff: gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java ---
    @@ -562,4 +568,98 @@ public void shouldBlockWhenMaxConnectionsExceeded() throws Exception {
                 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);
    --- End diff --
    
    Should this be `assertTrue(latch3.await(1500, TimeUnit.MILLISECONDS));` ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-tinkerpop pull request: TINKERPOP3-910 - In session tran...

Posted by PommeVerte <gi...@git.apache.org>.
Github user PommeVerte commented on the pull request:

    https://github.com/apache/incubator-tinkerpop/pull/122#issuecomment-151133769
  
    non-binding +1 here. This resolves client side failing tests.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-tinkerpop pull request: TINKERPOP3-910 - In session tran...

Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on a diff in the pull request:

    https://github.com/apache/incubator-tinkerpop/pull/122#discussion_r43231045
  
    --- Diff: gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java ---
    @@ -562,4 +568,98 @@ public void shouldBlockWhenMaxConnectionsExceeded() throws Exception {
                 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);
    --- End diff --
    
    I can add that.  `SimpleClient` stinks - we need this done: https://issues.apache.org/jira/browse/TINKERPOP3-916


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-tinkerpop pull request: TINKERPOP3-910 - In session tran...

Posted by pluradj <gi...@git.apache.org>.
Github user pluradj commented on the pull request:

    https://github.com/apache/incubator-tinkerpop/pull/122#issuecomment-151734434
  
    +1 Ran `mvn clean install -DincludeNeo4j` and then `mvn verify -DskipIntegrationTests=false -DincludeNeo4j` successfully. Looks good to me.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-tinkerpop pull request: TINKERPOP3-910 - In session tran...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-tinkerpop/pull/122


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-tinkerpop pull request: TINKERPOP3-910 - In session tran...

Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the pull request:

    https://github.com/apache/incubator-tinkerpop/pull/122#issuecomment-151642701
  
    @pluradj do you have time to help out with a vote here for gremlin server stuff?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---