Uploaded image for project: 'TinkerPop'
  1. TinkerPop
  2. TINKERPOP-1350

Server locks when submitting parallel requests on session

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 3.1.2-incubating
    • Fix Version/s: 3.1.4, 3.2.2
    • Component/s: server
    • Labels:
      None

      Description

      This really is only a problem when there is some form of long blocking script submitted and only on a session when done in parallel, like:

      final ResultSet first = client.submit(
                      "Object mon1 = 'mon1';\n" +
                      "synchronized (mon1) {\n" +
                      "    mon1.wait();\n" +
                      "} ");
      
      final ResultSet second = client.submit(
                      "Object mon2 = 'mon2';\n" +
                              "synchronized (mon2) {\n" +
                              "    mon2.wait();\n" +
                              "}");
      

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user spmallette opened a pull request:

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

          TINKERPOP-1350 Fixed a problem with how timeouts were scheduled in GremlinExecutor

          https://issues.apache.org/jira/browse/TINKERPOP-1350

          Made timeouts schedule closer to when execution of a script actually occurred. All together, this change helps prevent sessions from locking when submitting multiple parallel requests that have the potential to block.

          All good with `$ mvn clean install && mvn verify -pl gremlin-server -DskipIntegrationTests=false
          `

          VOTE +1

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

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

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

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


          commit cd0e58bb769faf06ddfee9473649c7b53ba1b50c
          Author: Stephen Mallette <spmva@genoprime.com>
          Date: 2016-06-28T20:19:54Z

          Fixed a problem with how timeouts were scheduled in GremlinExecutor

          Made timeouts schedule closer to when execution of a script actually occurred. All together, this change helps prevent sessions from locking when submitting multiple parallel requests that have the potential to block.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user spmallette opened a pull request: https://github.com/apache/tinkerpop/pull/350 TINKERPOP-1350 Fixed a problem with how timeouts were scheduled in GremlinExecutor https://issues.apache.org/jira/browse/TINKERPOP-1350 Made timeouts schedule closer to when execution of a script actually occurred. All together, this change helps prevent sessions from locking when submitting multiple parallel requests that have the potential to block. All good with `$ mvn clean install && mvn verify -pl gremlin-server -DskipIntegrationTests=false ` VOTE +1 You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1350 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/350.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 #350 commit cd0e58bb769faf06ddfee9473649c7b53ba1b50c Author: Stephen Mallette <spmva@genoprime.com> Date: 2016-06-28T20:19:54Z Fixed a problem with how timeouts were scheduled in GremlinExecutor Made timeouts schedule closer to when execution of a script actually occurred. All together, this change helps prevent sessions from locking when submitting multiple parallel requests that have the potential to block.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user dkuppitz commented on the issue:

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

          VOTE: +1

          Show
          githubbot ASF GitHub Bot added a comment - Github user dkuppitz commented on the issue: https://github.com/apache/tinkerpop/pull/350 VOTE: +1
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user okram commented on the issue:

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

          VOTE +1

          Show
          githubbot ASF GitHub Bot added a comment - Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/350 VOTE +1
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

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

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/tinkerpop/pull/350
          Hide
          spmallette stephen mallette added a comment -

          This one didn't appear to be quite fixed. Additional testing showed that it was fixed only because the test assumed two parallel requests when the problem actually showed when the number of parallel requests was greater than the number of threads in the gremlin server worker pool.

          Show
          spmallette stephen mallette added a comment - This one didn't appear to be quite fixed. Additional testing showed that it was fixed only because the test assumed two parallel requests when the problem actually showed when the number of parallel requests was greater than the number of threads in the gremlin server worker pool.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user spmallette opened a pull request:

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

          TINKERPOP-1350 was never quite fixed in 3.1.3.

          Unfortunately, more testing revealed that the fix for the problem was only masked by the changes in 3.1.3.

          Changed response encoding to not use the session executor when the session has an error condition it is trying to serialize. This should be fine as there is no need to serialized an error condition as part of a transaction and thus no need to have the session thread to do it. That in turn frees up the worker executor to serialize and cancel long run jobs in the session. Removed recommendations for submitting parallel requests on a session from docs.

          Works in full with `mvn clean install && mvn verify -pl gremlin-server -DskipIntegrationTests=false`

          VOTE +1

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

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

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

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


          commit 57ef449ded52e5c11072e7a626eebcfcc13889c4
          Author: Stephen Mallette <spmva@genoprime.com>
          Date: 2016-08-02T19:59:19Z

          TINKERPOP-1350 was never quite fixed in 3.1.3.

          Changed response encoding to not use the session executor when the session has an error condition it is trying to serialize. This should be fine as there is no need to serialized an error condition as part of a transaction and thus no need to have the session thread to do it. That in turn frees up the worker executor to serialize and cancel long run jobs in the session. Removed recommendations for submitting parallel requests on a session from docs.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user spmallette opened a pull request: https://github.com/apache/tinkerpop/pull/367 TINKERPOP-1350 was never quite fixed in 3.1.3. Unfortunately, more testing revealed that the fix for the problem was only masked by the changes in 3.1.3. Changed response encoding to not use the session executor when the session has an error condition it is trying to serialize. This should be fine as there is no need to serialized an error condition as part of a transaction and thus no need to have the session thread to do it. That in turn frees up the worker executor to serialize and cancel long run jobs in the session. Removed recommendations for submitting parallel requests on a session from docs. Works in full with `mvn clean install && mvn verify -pl gremlin-server -DskipIntegrationTests=false` VOTE +1 You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1350 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/367.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 #367 commit 57ef449ded52e5c11072e7a626eebcfcc13889c4 Author: Stephen Mallette <spmva@genoprime.com> Date: 2016-08-02T19:59:19Z TINKERPOP-1350 was never quite fixed in 3.1.3. Changed response encoding to not use the session executor when the session has an error condition it is trying to serialize. This should be fine as there is no need to serialized an error condition as part of a transaction and thus no need to have the session thread to do it. That in turn frees up the worker executor to serialize and cancel long run jobs in the session. Removed recommendations for submitting parallel requests on a session from docs.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user twilmes commented on the issue:

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

          Code and doc updates look good.

          VOTE: +1

          Show
          githubbot ASF GitHub Bot added a comment - Github user twilmes commented on the issue: https://github.com/apache/tinkerpop/pull/367 Code and doc updates look good. VOTE: +1
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user okram commented on the issue:

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

          VOTE +1.

          Show
          githubbot ASF GitHub Bot added a comment - Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/367 VOTE +1.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

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

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/tinkerpop/pull/367
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user spmallette commented on the issue:

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

          Rebased and force pushed to resolve merge conflicts on tp31. merged

          Show
          githubbot ASF GitHub Bot added a comment - Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/367 Rebased and force pushed to resolve merge conflicts on tp31. merged

            People

            • Assignee:
              spmallette stephen mallette
              Reporter:
              spmallette stephen mallette
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development