You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by GitBox <gi...@apache.org> on 2020/08/27 23:26:44 UTC
[GitHub] [tinkerpop] divijvaidya opened a new pull request #1318: TINKERPOP-2410 Release server threads waiting on connection if the connection is dead.
divijvaidya opened a new pull request #1318:
URL: https://github.com/apache/tinkerpop/pull/1318
https://issues.apache.org/jira/browse/TINKERPOP-2410
Consider a situation where the server has sent some results back to the client and is waiting for Netty channel to be writable again before sending rest of the results. Let's assume that the client dies (or is closed) at this instant without consuming the entire set of results. In this case, the server threads will continue to wait for the channel to become writable (which it never will since client is dead) until the timeout is hit.
With this change, if the connection becomes inactive, the loop will break and free up the threads.
## Testing
We don't have any test mechanism to check the number of busy server threads. Hence, no tests are added to repro this case. Any suggestions on how to add tests for such scenarios are welcome!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] spmallette commented on pull request #1318: TINKERPOP-2410 Release server threads waiting on connection if the connection is dead.
Posted by GitBox <gi...@apache.org>.
spmallette commented on pull request #1318:
URL: https://github.com/apache/tinkerpop/pull/1318#issuecomment-684120489
I see you brought in the fix from 3.4-dev and the test failure is cleaned up. I'm good with this change - builds with:
`mvn clean install -pl gremlin-server,gremlin-console -DskipIntegrationTests=false -DincludeNeo4j`
VOTE +1 (i assume you won't bring in that merge commit at d632f2a to the release branches and will squash the "changelog" commit)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] divijvaidya merged pull request #1318: TINKERPOP-2410 Release server threads waiting on connection if the connection is dead.
Posted by GitBox <gi...@apache.org>.
divijvaidya merged pull request #1318:
URL: https://github.com/apache/tinkerpop/pull/1318
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] spmallette edited a comment on pull request #1318: TINKERPOP-2410 Release server threads waiting on connection if the connection is dead.
Posted by GitBox <gi...@apache.org>.
spmallette edited a comment on pull request #1318:
URL: https://github.com/apache/tinkerpop/pull/1318#issuecomment-684879607
Just a thought - before you go to merge this, you might want to test it on `master` to be sure all the tests are still ok.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] spmallette commented on pull request #1318: TINKERPOP-2410 Release server threads waiting on connection if the connection is dead.
Posted by GitBox <gi...@apache.org>.
spmallette commented on pull request #1318:
URL: https://github.com/apache/tinkerpop/pull/1318#issuecomment-684879607
Just a thought - before you got to merge this, you might want to test it on `master` to be sure all the tests are still ok.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] spmallette commented on pull request #1318: TINKERPOP-2410 Release server threads waiting on connection if the connection is dead.
Posted by GitBox <gi...@apache.org>.
spmallette commented on pull request #1318:
URL: https://github.com/apache/tinkerpop/pull/1318#issuecomment-684011732
Getting a failing test:
```
[ERROR] Failures:
[ERROR] GremlinServerSessionIntegrateTest.shouldBlockAdditionalRequestsDuringClose:209
Expected: a collection containing "INFO - Rolling back open transactions on graph before killing session: shouldBlockAdditionalRequestsDuringClose\n"
but: was "WARN - Timeout while trying to close connection on shouldBlockAdditionalRequestsDuringClose - force closing - server will close session on shutdown or expiration.\n"
[INFO]
[ERROR] Tests run: 270, Failures: 1, Errors: 0, Skipped: 1
```
I think you just need to rebase since you fixed this on 3.4-dev right?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] divijvaidya merged pull request #1318: TINKERPOP-2410 Release server threads waiting on connection if the connection is dead.
Posted by GitBox <gi...@apache.org>.
divijvaidya merged pull request #1318:
URL: https://github.com/apache/tinkerpop/pull/1318
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org