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 2016/06/29 19:22:07 UTC

[GitHub] tinkerpop pull request #352: TINKERPOP-1352 Fixed logic in ConnectionPool th...

GitHub user spmallette opened a pull request:

    https://github.com/apache/tinkerpop/pull/352

    TINKERPOP-1352 Fixed logic in ConnectionPool that was preventing it from growing.

    https://issues.apache.org/jira/browse/TINKERPOP-1352
    
    Also, fixed a bug (an invalid FastNoSuchElementException)  in Gremlin Server that would only appear when the server paused writes to an overwhelmed client. No new tests to validate as the driver doesn't have a good way to inspect the pool size. Had to test manually, however
    
    ```text
    mvn clean install && mvn verify -pl gremlin-server -DskipIntegrationTests=false
    ```
    
    passed so there were no regressions. 
    
    VOTE +1
    


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

    $ git pull https://github.com/apache/tinkerpop TINKERPOP-1352

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

    https://github.com/apache/tinkerpop/pull/352.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 #352
    
----

----


---
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] tinkerpop issue #352: TINKERPOP-1352 Fixed logic in ConnectionPool that was ...

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

    https://github.com/apache/tinkerpop/pull/352
  
    Just pushed a fix. It was more a problem with the configuration of the client in the test than a bug. I think the recent fixes in this branch made some things suddenly "work" and it caused these side cases to start squeezing out problems. I've had several successful runs at this point with no troubles so I think that both tests might be solid now.
    
    While I was in the debugger, I noticed one final thing that I don't really like that I'd like to resolved, so reviewers may want to hang on to their votes for a little bit longer for this one.


---
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] tinkerpop issue #352: TINKERPOP-1352 Fixed logic in ConnectionPool that was ...

Posted by ramzioueslati <gi...@git.apache.org>.
Github user ramzioueslati commented on the issue:

    https://github.com/apache/tinkerpop/pull/352
  
    Successful pass


---
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] tinkerpop issue #352: TINKERPOP-1352 Fixed logic in ConnectionPool that was ...

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

    https://github.com/apache/tinkerpop/pull/352
  
    Just pushed what I think is the final fixes for this issue. Note that this branch also contains the fix for 
    
    https://issues.apache.org/jira/browse/TINKERPOP-1351
    
    Please feel free to test/review/vote at this point. Thanks.


---
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] tinkerpop issue #352: TINKERPOP-1352 Fixed logic in ConnectionPool that was ...

Posted by robertdale <gi...@git.apache.org>.
Github user robertdale commented on the issue:

    https://github.com/apache/tinkerpop/pull/352
  
    Passed for me. I'll run it a few more times to be sure.


---
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] tinkerpop issue #352: TINKERPOP-1352 Fixed logic in ConnectionPool that was ...

Posted by robertdale <gi...@git.apache.org>.
Github user robertdale commented on the issue:

    https://github.com/apache/tinkerpop/pull/352
  
    Using (have to ignore hadoop/spark)  mvn clean install -pl gremlin-shaded,gremlin-core,gremlin-test,gremlin-groovy,gremlin-groovy-test,tinkergraph-gremlin,neo4j-gremlin,gremlin-driver,gremlin-console,gremlin-server,gremlin-archetype && mvn verify -pl gremlin-server -DskipIntegrationTests=false -DincludeNeo4j
    
    GremlinServerSessionIntegrateTest.shouldEnsureSessionBindingsAreThreadSafe consistently fails for me.  The tp31 tip does not fail.
    
    No such property: a for class: Script2
    
    I added a get() where the vars are set to ensure it happens before the rest of the threads. Does not appear to be a race condition.
    
    [console.txt](https://github.com/apache/tinkerpop/files/341746/console.txt)
    



---
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] tinkerpop issue #352: TINKERPOP-1352 Fixed logic in ConnectionPool that was ...

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

    https://github.com/apache/tinkerpop/pull/352
  
    wow - now this one is failing:
    
    ```text
    GremlinDriverIntegrateTest.shouldBeThreadSafeToUseOneClient:994
    ```
    
    only fails when run in maven - works in intellij on its own - wonderful. i fixed one non-deterministic test and ended up with a new one for my troubles.


---
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] tinkerpop pull request #352: TINKERPOP-1352 Fixed logic in ConnectionPool th...

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

    https://github.com/apache/tinkerpop/pull/352


---
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] tinkerpop issue #352: TINKERPOP-1352 Fixed logic in ConnectionPool that was ...

Posted by robertdale <gi...@git.apache.org>.
Github user robertdale commented on the issue:

    https://github.com/apache/tinkerpop/pull/352
  
    5 consecutive passes


---
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] tinkerpop issue #352: TINKERPOP-1352 Fixed logic in ConnectionPool that was ...

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

    https://github.com/apache/tinkerpop/pull/352
  
    nice @robertdale - thanks for taking the time to review. always a big help when folks look over the PRs and provide feedback.


---
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] tinkerpop issue #352: TINKERPOP-1352 Fixed logic in ConnectionPool that was ...

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

    https://github.com/apache/tinkerpop/pull/352
  
    uh - i wasn't aware of any kerberos setup required. does that happen consistently?


---
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] tinkerpop issue #352: TINKERPOP-1352 Fixed logic in ConnectionPool that was ...

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

    https://github.com/apache/tinkerpop/pull/352
  
    @robertdale i think i fixed the problem. i've been hunting that issue forever, but have never been able to witness it with any kind of consistent nature to unravel the problem. i guess my earlier changes in this branch allowed me to see semi-consistently (once every few runs) which was painful but good enough to get to the bottom of it. it would be great if you could try the build again - thanks for testing it out.


---
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] tinkerpop issue #352: TINKERPOP-1352 Fixed logic in ConnectionPool that was ...

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

    https://github.com/apache/tinkerpop/pull/352
  
    nice find @robertdale - i pushed a fix that removed that assert. that whole test case is deprecated anyway, so i'm not too worried about it. 
    
    @ramzioueslati care to pull the branch again and give it another go?


---
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] tinkerpop issue #352: TINKERPOP-1352 Fixed logic in ConnectionPool that was ...

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

    https://github.com/apache/tinkerpop/pull/352
  
    Code looks good, `docker/build.sh -t -i -n` succeeded.
    
    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] tinkerpop issue #352: TINKERPOP-1352 Fixed logic in ConnectionPool that was ...

Posted by okram <gi...@git.apache.org>.
Github user okram commented on the issue:

    https://github.com/apache/tinkerpop/pull/352
  
    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] tinkerpop issue #352: TINKERPOP-1352 Fixed logic in ConnectionPool that was ...

Posted by robertdale <gi...@git.apache.org>.
Github user robertdale commented on the issue:

    https://github.com/apache/tinkerpop/pull/352
  
    Can you provide the stacktrace (from the surefire report)?  What OS, java versions?


---
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] tinkerpop issue #352: TINKERPOP-1352 Fixed logic in ConnectionPool that was ...

Posted by ramzioueslati <gi...@git.apache.org>.
Github user ramzioueslati commented on the issue:

    https://github.com/apache/tinkerpop/pull/352
  
    I tried 3 times with the same result


---
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] tinkerpop issue #352: TINKERPOP-1352 Fixed logic in ConnectionPool that was ...

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

    https://github.com/apache/tinkerpop/pull/352
  
    hmm - you're right - i'm getting that now too. i wonder why i didn't see that yesterday. investigating now.


---
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] tinkerpop issue #352: TINKERPOP-1352 Fixed logic in ConnectionPool that was ...

Posted by ramzioueslati <gi...@git.apache.org>.
Github user ramzioueslati commented on the issue:

    https://github.com/apache/tinkerpop/pull/352
  
    Hi Stephen
    
    I tried to run the test :
    ```
    mvn clean install && mvn verify -pl gremlin-server -DskipIntegrationTests=false
    ```
    
    It failed :
    ```
    Failed tests: 
      GremlinServerAuthOldIntegrateTest.shouldFailAuthenticateWithPlainTextNoCredentials:135 
    Expected: a string starting with "Invalid name provided"
         but: was "No valid credentials provided (Mechanism level: Failed to find any Kerberos tgt)"
    
    Tests run: 152, Failures: 1, Errors: 0, Skipped: 13
    ```
    
    Do I need to do any kerberos configuration ?


---
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] tinkerpop issue #352: TINKERPOP-1352 Fixed logic in ConnectionPool that was ...

Posted by ramzioueslati <gi...@git.apache.org>.
Github user ramzioueslati commented on the issue:

    https://github.com/apache/tinkerpop/pull/352
  
    Will do right away.
    
    For the record here is the stacktrace:
    ```
    Running org.apache.tinkerpop.gremlin.server.GremlinServerAuthOldIntegrateTest
    [WARN] org.apache.tinkerpop.gremlin.server.auth.SimpleAuthenticator - Using {} configuration option which is deprecated - prefer including the location of the credentials graph data in the TinkerGraph config file.
    [WARN] org.apache.tinkerpop.gremlin.server.auth.SimpleAuthenticator - Using {} configuration option which is deprecated - prefer including the location of the credentials graph data in the TinkerGraph config file.
    [WARN] org.apache.tinkerpop.gremlin.server.auth.SimpleAuthenticator - Using {} configuration option which is deprecated - prefer including the location of the credentials graph data in the TinkerGraph config file.
    [WARN] org.apache.tinkerpop.gremlin.server.AbstractChannelizer - Enabling SSL with self-signed certificate (NOT SUITABLE FOR PRODUCTION)
    [WARN] org.apache.tinkerpop.gremlin.server.auth.SimpleAuthenticator - Using {} configuration option which is deprecated - prefer including the location of the credentials graph data in the TinkerGraph config file.
    [WARN] org.apache.tinkerpop.gremlin.driver.Cluster - SSL configured without a trustCertChainFile and thus trusts all certificates without verification (not suitable for production)
    [WARN] org.apache.tinkerpop.gremlin.driver.Cluster - SSL configured without a trustCertChainFile and thus trusts all certificates without verification (not suitable for production)
    [WARN] org.apache.tinkerpop.gremlin.server.auth.SimpleAuthenticator - Using {} configuration option which is deprecated - prefer including the location of the credentials graph data in the TinkerGraph config file.
    [WARN] org.apache.tinkerpop.gremlin.server.auth.SimpleAuthenticator - Using {} configuration option which is deprecated - prefer including the location of the credentials graph data in the TinkerGraph config file.
    [ERROR] org.apache.tinkerpop.gremlin.driver.Handler$GremlinResponseHandler - Could not process the response
    javax.security.sasl.SaslException: GSS initiate failed [Caused by GSSException: No valid credentials provided (Mechanism level: Failed to find any Kerberos tgt)]
            at com.sun.security.sasl.gsskerb.GssKrb5Client.evaluateChallenge(GssKrb5Client.java:211)
            at org.apache.tinkerpop.gremlin.driver.Handler$GremlinSaslAuthenticationHandler.evaluateChallenge(Handler.java:123)
            at org.apache.tinkerpop.gremlin.driver.Handler$GremlinSaslAuthenticationHandler.channelRead0(Handler.java:93)
            at org.apache.tinkerpop.gremlin.driver.Handler$GremlinSaslAuthenticationHandler.channelRead0(Handler.java:66)
            at io.netty.channel.SimpleChannelInboundHandler.channelRead(SimpleChannelInboundHandler.java:105)
            at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:307)
            at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:293)
            at io.netty.handler.codec.MessageToMessageDecoder.channelRead(MessageToMessageDecoder.java:103)
            at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:307)
            at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:293)
            at org.apache.tinkerpop.gremlin.driver.handler.WebSocketClientHandler.channelRead0(WebSocketClientHandler.java:90)
            at io.netty.channel.SimpleChannelInboundHandler.channelRead(SimpleChannelInboundHandler.java:105)
            at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:307)
            at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:293)
            at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:276)
            at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:263)
            at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:307)
            at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:293)
            at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:840)
            at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:131)
            at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:511)
            at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:468)
            at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:382)
            at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:354)
            at io.netty.util.concurrent.SingleThreadEventExecutor$2.run(SingleThreadEventExecutor.java:112)
            at java.lang.Thread.run(Thread.java:745)
    Caused by: GSSException: No valid credentials provided (Mechanism level: Failed to find any Kerberos tgt)
            at sun.security.jgss.krb5.Krb5InitCredential.getInstance(Krb5InitCredential.java:147)
            at sun.security.jgss.krb5.Krb5MechFactory.getCredentialElement(Krb5MechFactory.java:122)
            at sun.security.jgss.krb5.Krb5MechFactory.getMechanismContext(Krb5MechFactory.java:187)
            at sun.security.jgss.GSSManagerImpl.getMechanismContext(GSSManagerImpl.java:224)
            at sun.security.jgss.GSSContextImpl.initSecContext(GSSContextImpl.java:212)
            at sun.security.jgss.GSSContextImpl.initSecContext(GSSContextImpl.java:179)
            at com.sun.security.sasl.gsskerb.GssKrb5Client.evaluateChallenge(GssKrb5Client.java:192)
            ... 25 more
    [WARN] org.apache.tinkerpop.gremlin.server.auth.SimpleAuthenticator - Using {} configuration option which is deprecated - prefer including the location of the credentials graph data in the TinkerGraph config file.
    [WARN] org.apache.tinkerpop.gremlin.server.auth.SimpleAuthenticator - Using {} configuration option which is deprecated - prefer including the location of the credentials graph data in the TinkerGraph config file.
    [WARN] org.apache.tinkerpop.gremlin.server.auth.SimpleAuthenticator - Using {} configuration option which is deprecated - prefer including the location of the credentials graph data in the TinkerGraph config file.
    [WARN] org.apache.tinkerpop.gremlin.server.AbstractChannelizer - Enabling SSL with self-signed certificate (NOT SUITABLE FOR PRODUCTION)
    [WARN] org.apache.tinkerpop.gremlin.server.auth.SimpleAuthenticator - Using {} configuration option which is deprecated - prefer including the location of the credentials graph data in the TinkerGraph config file.
    Tests run: 10, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 62.071 sec <<< FAILURE! - in org.apache.tinkerpop.gremlin.server.GremlinServerAuthOldIntegrateTest
    shouldFailAuthenticateWithPlainTextNoCredentials(org.apache.tinkerpop.gremlin.server.GremlinServerAuthOldIntegrateTest)  Time elapsed: 4.58 sec  <<< FAILURE!
    java.lang.AssertionError:
    Expected: a string starting with "Invalid name provided"
         but: was "No valid credentials provided (Mechanism level: Failed to find any Kerberos tgt)"
            at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
            at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:8)
            at org.apache.tinkerpop.gremlin.server.GremlinServerAuthOldIntegrateTest.shouldFailAuthenticateWithPlainTextNoCredentials(GremlinServerAuthOldIntegrateTest.java:135)
    ```


---
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] tinkerpop issue #352: TINKERPOP-1352 Fixed logic in ConnectionPool that was ...

Posted by robertdale <gi...@git.apache.org>.
Github user robertdale commented on the issue:

    https://github.com/apache/tinkerpop/pull/352
  
    @spmallette  I think the GremlinServerAuthOldIntegrateTest.shouldFailAuthenticateWithPlainTextNoCredentials() should remove the assertion startsWith("Invalid name provided").  The GremlinServerAuthIntegrateTest.shouldFailAuthenticateWithPlainTextNoCredentials() test does not look at the string, just the exception, and it appears to be passing.
    
    I believe the string in the exception may change based on the local kerberos configuration.



---
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.
---