Fix HTTP connection leak on Datastore errors#186
Fix HTTP connection leak on Datastore errors#186eddavisson merged 1 commit intogoogleapis:masterfrom
Conversation
| content.close(); | ||
| } catch(IOException e) { | ||
| logger.fine(String.format("closing content stream failed: %s",e)); | ||
| } |
There was a problem hiding this comment.
I think we can also fail to close the stream in the try block that starts at line 231. What about instead adding a try-finally around the code at line 185. That should ensure the stream is closed no matter what happens in this method.
There was a problem hiding this comment.
I updated the pull request based on your suggestion, and also modified the IOUtils.copy line to avoid a potential double-close.
eddavisson
left a comment
There was a problem hiding this comment.
Looks good, just one nit.
| throw makeException(url, methodName, httpResponse.getContent(), | ||
| httpResponse.getContentType(), httpResponse.getContentCharset(), null, | ||
| httpResponse.getStatusCode()); | ||
| try(InputStream content = httpResponse.getContent()) { |
There was a problem hiding this comment.
Add a blank space after try.
|
@danieln100 we hear you. We've released v1.5.1, which includes this fix. Give it a spin and let us know! |
|
@wsh Awesome thanks. I deleted my question before you answered me because I wanted to move it to a separate issue but just for context I asked how would I able to use this change since we are experiencing consistent spikes in latency and we think its due to a http connection leak. What is the best way to use this with the Google Cloud Java Client for Datastore, I am using version 1.8 and looking at the pom.xml file for that version it looks like they are still using Thanks again |
|
@danieln100 google-cloud-java/google-cloud-datastore 1.8.0 uses version 1.4.1 of this library. (Seems like you're looking at the proto dependencies.) At least in theory, specifying 1.5.1 might work but, now that this pull request has been merged, 1.5.1 will also be included in the next release of google-cloud-java. Your call. 😄 |
Properly close HTTP response input stream on Datastore errors (fixes #185)