This is an automated email from the ASF dual-hosted git repository. dubeejw pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/incubator-openwhisk.git
The following commit(s) were added to refs/heads/master by this push: new cd91b4f Make HttpUtils actually use only one connection and harden connection release. (#3818) cd91b4f is described below commit cd91b4fc024b16b9cc89b1aef77deb494da939e2 Author: Markus Thömmes <markusthoem...@me.com> AuthorDate: Mon Jul 2 16:31:38 2018 +0200 Make HttpUtils actually use only one connection and harden connection release. (#3818) When not specifying any ConnectionManager for the HttpClient used by HttpUtils, it defaults to an arbitrary PoolingManager with 5 connections. This can hide subtle bugs in connection management, since we expect it to only have one connection to the specific container, we'll default to the BasicConnectionManager. Furthermore, there are subtle issues when an entity returned by a HttpRequest is not consumed fully (which could happen if it gets truncated or content-length is not returned correctly). That can cause the ConnectionPool (which shouldn't be there in the first place) to go stale. We also never closed the entity's stream. --- .../src/main/scala/whisk/core/containerpool/HttpUtils.scala | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/common/scala/src/main/scala/whisk/core/containerpool/HttpUtils.scala b/common/scala/src/main/scala/whisk/core/containerpool/HttpUtils.scala index d9bdbdc..4ff205d 100644 --- a/common/scala/src/main/scala/whisk/core/containerpool/HttpUtils.scala +++ b/common/scala/src/main/scala/whisk/core/containerpool/HttpUtils.scala @@ -29,17 +29,16 @@ import scala.util.control.NoStackTrace import scala.util.Failure import scala.util.Success import scala.util.Try - import org.apache.commons.io.IOUtils import org.apache.http.HttpHeaders import org.apache.http.client.config.RequestConfig import org.apache.http.client.methods.HttpPost import org.apache.http.client.methods.HttpRequestBase -import org.apache.http.client.utils.URIBuilder +import org.apache.http.client.utils.{HttpClientUtils, URIBuilder} import org.apache.http.conn.HttpHostConnectException import org.apache.http.entity.StringEntity import org.apache.http.impl.client.HttpClientBuilder -import org.apache.http.impl.conn.PoolingHttpClientConnectionManager +import org.apache.http.impl.conn.{BasicHttpClientConnectionManager, PoolingHttpClientConnectionManager} import spray.json._ import whisk.common.Logging import whisk.common.TransactionId @@ -116,7 +115,8 @@ protected class HttpUtils(hostname: String, timeout: FiniteDuration, maxResponse Left(NoResponseReceived()) } - response.close() + // Fully consumes the entity and closes the response + HttpClientUtils.closeQuietly(response) containerResponse } recoverWith { // The route to target socket as well as the target socket itself may need some time to be available - @@ -167,7 +167,7 @@ protected class HttpUtils(hostname: String, timeout: FiniteDuration, maxResponse // Increase max total connections (default is 20) cm.setMaxTotal(maxConcurrent) cm - } else null) //set the Pooling connection manager IFF maxConcurrent > 1 + } else new BasicHttpClientConnectionManager()) // set the Pooling connection manager IFF maxConcurrent > 1 .useSystemProperties() .disableAutomaticRetries() .build