openwhisk-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From csantan...@apache.org
Subject [incubator-openwhisk] branch master updated: Disable explicit retry on container http connection. (#2714)
Date Tue, 12 Sep 2017 19:36:43 GMT
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 <rodric@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" <commits@openwhisk.apache.org>'].

Mime
View raw message