[GitHub] markusthoemmes commented on a change in pull request #2827: Notify the user if a container is killed due to memory exhaustion.

[gi...@git.apache.org]

markusthoemmes commented on a change in pull request #2827: Notify the user if a container is killed due to memory exhaustion.
URL: https://github.com/apache/incubator-openwhisk/pull/2827#discussion_r142586734
 
 

 ##########
 File path: core/invoker/src/main/scala/whisk/core/containerpool/docker/DockerContainer.scala
 ##########
 @@ -142,6 +142,44 @@ class DockerContainer(protected val id: ContainerId, protected val addr: Contain
     docker.rm(id)
   }
 
+  private def isOomKilled(retries: Int = (waitForOomState / filePollInterval).toInt)(
+    implicit transid: TransactionId): Future[Boolean] = {
+    docker.isOomKilled(id)(TransactionId.invoker).flatMap { killed =>
+      if (killed) Future.successful(killed)
+      else if (retries > 0) akka.pattern.after(filePollInterval, as.scheduler)(isOomKilled(retries - 1))
+      else Future.successful(killed)
+    }
+  }
+
+  override protected def callContainer(path: String, body: JsObject, timeout: FiniteDuration, retry: Boolean = false)(
+    implicit transid: TransactionId): Future[RunResult] = {
+    val started = Instant.now()
+    val http = httpConnection.getOrElse {
+      val conn = new HttpUtils(s"${addr.host}:${addr.port}", timeout, 1.MB)
+      httpConnection = Some(conn)
+      conn
+    }
+    Future {
+      http.post(path, body, retry)
+    }.flatMap { response =>
+      val finished = Instant.now()
+
+      response.left
+        .map {
+          case t: ConnectionError =>
+            isOomKilled().map {
+              case true =>
+                logging.info(this, "connection abruptly terminated, checking for memory exhaustion")
+                MemoryExhausted()
+              case false => t
+            }
+          case t => Future.successful(t)
+        }
+        .fold(_.map(Left(_)), right => Future.successful(Right(right)))
 
 Review comment:
   Any suggestions? It is to transform `Either[Future[ContainerConnectionError], ContainerResponse]` to `Future[Either[ContainerConnectionError, ContainerResponse]]`
   
   I implemented it using just one `fold` (vs. using `left.map` and a `fold`). It's possible, but I found it more clear the way it is now.
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services