You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openwhisk.apache.org by cs...@apache.org on 2017/09/12 19:36:43 UTC
[incubator-openwhisk] branch master updated: Disable explicit retry
on container http connection. (#2714)
This is an automated email from the ASF dual-hosted git repository.
csantanapr 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 63f92dc Disable explicit retry on container http connection. (#2714)
63f92dc is described below
commit 63f92dc91bba1ba5162d0f144b2743217657bbdd
Author: rodric rabbah <ro...@gmail.com>
AuthorDate: Tue Sep 12 14:36:41 2017 -0500
Disable explicit retry on container http connection. (#2714)
Remove unused type.
---
.../scala/whisk/core/entity/ActivationResult.scala | 1 -
.../core/containerpool/docker/HttpUtils.scala | 17 +++++---
.../docker/test/ContainerConnectionTests.scala | 46 ++++++++++++----------
.../core/entity/test/ActivationResponseTests.scala | 4 +-
4 files changed, 39 insertions(+), 29 deletions(-)
diff --git a/common/scala/src/main/scala/whisk/core/entity/ActivationResult.scala b/common/scala/src/main/scala/whisk/core/entity/ActivationResult.scala
index 495bbc0..51d195d 100644
--- a/common/scala/src/main/scala/whisk/core/entity/ActivationResult.scala
+++ b/common/scala/src/main/scala/whisk/core/entity/ActivationResult.scala
@@ -96,7 +96,6 @@ protected[core] object ActivationResponse extends DefaultJsonProtocol {
* Class of errors for invoker-container communication.
*/
protected[core] sealed abstract class ContainerConnectionError
- protected[core] case class NoHost() extends ContainerConnectionError
protected[core] case class ConnectionError(t: Throwable) extends ContainerConnectionError
protected[core] case class NoResponseReceived() extends ContainerConnectionError
protected[core] case class Timeout() extends ContainerConnectionError
diff --git a/core/invoker/src/main/scala/whisk/core/containerpool/docker/HttpUtils.scala b/core/invoker/src/main/scala/whisk/core/containerpool/docker/HttpUtils.scala
index 574f3c5..a6b53a7 100644
--- a/core/invoker/src/main/scala/whisk/core/containerpool/docker/HttpUtils.scala
+++ b/core/invoker/src/main/scala/whisk/core/containerpool/docker/HttpUtils.scala
@@ -18,11 +18,15 @@
package whisk.core.containerpool.docker
import java.nio.charset.StandardCharsets
-import scala.concurrent.duration.FiniteDuration
+
+import scala.Left
+import scala.Right
import scala.concurrent.duration.DurationInt
+import scala.concurrent.duration.FiniteDuration
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
@@ -32,12 +36,11 @@ import org.apache.http.client.utils.URIBuilder
import org.apache.http.conn.HttpHostConnectException
import org.apache.http.entity.StringEntity
import org.apache.http.impl.client.HttpClientBuilder
+
import spray.json._
import whisk.core.entity.ActivationResponse._
import whisk.core.entity.ByteSize
import whisk.core.entity.size.SizeLong
-import scala.Left
-import scala.Right
/**
* This HTTP client is used only in the invoker to communicate with the action container.
@@ -133,6 +136,7 @@ protected[core] class HttpUtils(hostname: String, timeout: FiniteDuration, maxRe
private val connection = HttpClientBuilder.create
.setDefaultRequestConfig(httpconfig)
.useSystemProperties()
+ .disableAutomaticRetries()
.build
}
@@ -144,11 +148,12 @@ object HttpUtils {
val response = connection.post(endPoint, content, retry = true)
connection.close()
response match {
- case Right(r) => (r.statusCode, Try(r.entity.parseJson.asJsObject).toOption)
- case Left(Timeout()) => throw new java.util.concurrent.TimeoutException()
+ case Right(r) => (r.statusCode, Try(r.entity.parseJson.asJsObject).toOption)
+ case Left(NoResponseReceived()) => throw new IllegalStateException("no response from container")
+ case Left(Timeout()) => throw new java.util.concurrent.TimeoutException()
case Left(ConnectionError(t: java.net.SocketTimeoutException)) =>
throw new java.util.concurrent.TimeoutException()
- case _ => throw new IllegalStateException()
+ case Left(ConnectionError(t)) => throw new IllegalStateException(t.getMessage)
}
}
}
diff --git a/tests/src/test/scala/whisk/core/containerpool/docker/test/ContainerConnectionTests.scala b/tests/src/test/scala/whisk/core/containerpool/docker/test/ContainerConnectionTests.scala
index aaf8895..809165a 100644
--- a/tests/src/test/scala/whisk/core/containerpool/docker/test/ContainerConnectionTests.scala
+++ b/tests/src/test/scala/whisk/core/containerpool/docker/test/ContainerConnectionTests.scala
@@ -17,30 +17,28 @@
package whisk.core.containerpool.docker.test
-import java.time.Instant
import java.nio.charset.StandardCharsets
+import java.time.Instant
import scala.concurrent.duration._
+import org.apache.http.HttpRequest
+import org.apache.http.HttpResponse
+import org.apache.http.entity.StringEntity
+import org.apache.http.localserver.LocalServerTestBase
+import org.apache.http.protocol.HttpContext
+import org.apache.http.protocol.HttpRequestHandler
import org.junit.runner.RunWith
+import org.scalatest.junit.JUnitRunner
import org.scalatest.BeforeAndAfter
import org.scalatest.BeforeAndAfterAll
import org.scalatest.FlatSpec
import org.scalatest.Matchers
-import org.scalatest.junit.JUnitRunner
-
-import org.apache.http.localserver.LocalServerTestBase
-import org.apache.http.protocol.HttpRequestHandler
-import org.apache.http.HttpResponse
-import org.apache.http.HttpRequest
-import org.apache.http.protocol.HttpContext
-import org.apache.http.entity.StringEntity
import spray.json.JsObject
-
-import whisk.core.entity.size._
-import whisk.core.entity.ActivationResponse._
import whisk.core.containerpool.docker.HttpUtils
+import whisk.core.entity.ActivationResponse._
+import whisk.core.entity.size._
/**
* Unit tests for HttpUtils which communicate with containers.
@@ -49,7 +47,7 @@ import whisk.core.containerpool.docker.HttpUtils
class ContainerConnectionTests extends FlatSpec with Matchers with BeforeAndAfter with BeforeAndAfterAll {
var testHang: FiniteDuration = 0.second
- var testStatusOK: Boolean = true
+ var testStatusCode: Int = 200
var testResponse: String = null
val mockServer = new LocalServerTestBase {
@@ -60,7 +58,7 @@ class ContainerConnectionTests extends FlatSpec with Matchers with BeforeAndAfte
if (testHang.length > 0) {
Thread.sleep(testHang.toMillis)
}
- response.setStatusCode(if (testStatusOK) 200 else 500);
+ response.setStatusCode(testStatusCode);
if (testResponse != null) {
response.setEntity(new StringEntity(testResponse, StandardCharsets.UTF_8))
}
@@ -75,7 +73,7 @@ class ContainerConnectionTests extends FlatSpec with Matchers with BeforeAndAfte
before {
testHang = 0.second
- testStatusOK = true
+ testStatusCode = 200
testResponse = null
}
@@ -93,21 +91,29 @@ class ContainerConnectionTests extends FlatSpec with Matchers with BeforeAndAfte
val result = connection.post("/init", JsObject(), retry = true)
val end = Instant.now()
val waited = end.toEpochMilli - start.toEpochMilli
- result.isLeft shouldBe true
+ result shouldBe 'left
waited should be > timeout.toMillis
waited should be < (timeout * 2).toMillis
}
+ it should "handle empty entity response" in {
+ val timeout = 5.seconds
+ val connection = new HttpUtils(hostWithPort, timeout, 1.B)
+ testStatusCode = 204
+ val result = connection.post("/init", JsObject(), retry = true)
+ result shouldBe Left(NoResponseReceived())
+ }
+
it should "not truncate responses within limit" in {
val timeout = 1.minute.toMillis
val connection = new HttpUtils(hostWithPort, timeout.millis, 50.B)
Seq(true, false).foreach { code =>
Seq(null, "", "abc", """{"a":"B"}""", """["a", "b"]""").foreach { r =>
- testStatusOK = code
+ testStatusCode = if (code) 200 else 500
testResponse = r
val result = connection.post("/init", JsObject(), retry = true)
result shouldBe Right {
- ContainerResponse(okStatus = testStatusOK, if (r != null) r else "", None)
+ ContainerResponse(okStatus = code, if (r != null) r else "", None)
}
}
}
@@ -120,11 +126,11 @@ class ContainerConnectionTests extends FlatSpec with Matchers with BeforeAndAfte
val connection = new HttpUtils(hostWithPort, timeout.millis, limit)
Seq(true, false).foreach { code =>
Seq("abc", """{"a":"B"}""", """["a", "b"]""").foreach { r =>
- testStatusOK = code
+ testStatusCode = if (code) 200 else 500
testResponse = r
val result = connection.post("/init", JsObject(), retry = true)
result shouldBe Right {
- ContainerResponse(okStatus = testStatusOK, r.take(limit.toBytes.toInt), Some((r.length.B, limit)))
+ ContainerResponse(okStatus = code, r.take(limit.toBytes.toInt), Some((r.length.B, limit)))
}
}
}
diff --git a/tests/src/test/scala/whisk/core/entity/test/ActivationResponseTests.scala b/tests/src/test/scala/whisk/core/entity/test/ActivationResponseTests.scala
index ed7462a..0324c03 100644
--- a/tests/src/test/scala/whisk/core/entity/test/ActivationResponseTests.scala
+++ b/tests/src/test/scala/whisk/core/entity/test/ActivationResponseTests.scala
@@ -59,7 +59,7 @@ class ActivationResponseTests extends FlatSpec with Matchers {
}
it should "interpret failed init that does not response" in {
- Seq(NoHost(), ConnectionError(new Throwable()), NoResponseReceived(), Timeout())
+ Seq(ConnectionError(new Throwable()), NoResponseReceived(), Timeout())
.map(Left(_))
.foreach { e =>
val ar = processInitResponseContent(e, logger)
@@ -123,7 +123,7 @@ class ActivationResponseTests extends FlatSpec with Matchers {
}
it should "interpret failed run that does not response" in {
- Seq(NoHost(), ConnectionError(new Throwable()), NoResponseReceived(), Timeout())
+ Seq(ConnectionError(new Throwable()), NoResponseReceived(), Timeout())
.map(Left(_))
.foreach { e =>
val ar = processRunResponseContent(e, logger)
--
To stop receiving notification emails like this one, please contact
['"commits@openwhisk.apache.org" <co...@openwhisk.apache.org>'].