[GitHub] [tinkerpop] divijvaidya opened a new pull request #1082: TINKERPOP-2169 Responses exceeding maxContentLength cause subsequent queries to hang

[GitBox <gi...@apache.org>]

divijvaidya opened a new pull request #1082: TINKERPOP-2169 Responses exceeding maxContentLength cause subsequent queries to hang
URL: https://github.com/apache/tinkerpop/pull/1082
 
 
   https://issues.apache.org/jira/browse/TINKERPOP-2169
   https://issues.apache.org/jira/browse/TINKERPOP-2173
   
   Similar pull request associated with master branch: #1081  
   
   This pull request corresponds to branch tp33.
   
   **Problem**
   1. On receiving a CorruptedFrameException, the current code does not decrement the borrowed count on the connection and marks the connection for destruction. However, the connection never gets destroyed since the it can only be destroyed when the borrowed count is 0. This leads to unnecessary connections waiting in the bin to be cleared.
   2. If a connection is serving multiple requests and one of the requests gets a CorruptedFrameException, Netty closes the underlying channel and thus, other requests on the same channel receive a ChannelClosed exception. The current code marks the server host as unavailable (thus closing out other connections for the host as well) on a ChannelClosed exception whereas that is not necessarily true. Marking the host as unavailable is too aggressive in this scenario.
   
   ** Changes**
   1. Connection.isDead() logic now consists of checking the underlying channel.
   2. If a connection is dead, destroy the connection, irrespective of the number of borrowed items. Informing the inflight (pending) request futures about this is already done by the response handler.
   3. If a connection is returned to the pool and it is dead, replace the connection. Do not consider the host as unavailable since a single connection dead does not imply a dead host.
   4. Add a warning log whenever a host is marked as unavailable.
   5. Replace the connection on ClosedChannel exception.
   6. Reset the logging level in the integration tests after each test execution.
   
   **Testing**
   1. Added a test to reproduce the leak (Problem#1). The test fails before the fixes and passes after the fixes.
   2. `mvn verify -DskipIntegrationTests=false` & `mvn verify install` is successful for gremlin-server & gremlin-driver
   

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


With regards,
Apache Git Services