[GitHub] [tinkerpop] divijvaidya commented on pull request #1727: Release msg in finally block on HttpGremlinEndpointHandler

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

divijvaidya commented on PR #1727:
URL: https://github.com/apache/tinkerpop/pull/1727#issuecomment-1163376060

   The existing code does not release in a `finally` block because we want to release the buffer as soon as possible to reclaim memory sooner rather than at the end. After your change, the memory will be released after the execution of the request.
   Hence, the change will have implication on number of incoming requests that the server can handle contained by memory.
   
   Having said that, I agree with your code change for the sake of code readability and decreased likely hood of introducing bugs. The largest component in message is the query which should be of finite length so we don't have to worry about each message retaining a very large segment of memory.
   
   Can you please add a unit test for this class where we can check for code execution across different paths and validate that memory is not being freed at the end of it? That test would fail before your change and pass afterwards.


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

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org