You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openwhisk.apache.org by cb...@apache.org on 2017/06/22 13:58:04 UTC

[incubator-openwhisk] 02/03: Retype scheduling code to get rid of unsafeness.

This is an automated email from the ASF dual-hosted git repository.

cbickel pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-openwhisk.git

commit 85e8c8f1d49dac3c04fe0f58480d652f52729c54
Author: Markus Thoemmes <ma...@de.ibm.com>
AuthorDate: Tue Jun 6 14:38:18 2017 +0200

    Retype scheduling code to get rid of unsafeness.
    
    The pool used to look up the data of a container it has just chosen/created which is unnecessary. Removed also a non-reachable error-condition.
    
    Also: Take out unnecessary prewarmConfig checking.
---
 .../whisk/core/containerpool/ContainerPool.scala   | 64 +++++++++++-----------
 .../containerpool/test/ContainerPoolTests.scala    |  6 +-
 2 files changed, 34 insertions(+), 36 deletions(-)

diff --git a/core/invoker/src/main/scala/whisk/core/containerpool/ContainerPool.scala b/core/invoker/src/main/scala/whisk/core/containerpool/ContainerPool.scala
index 012373f..9a796ee 100644
--- a/core/invoker/src/main/scala/whisk/core/containerpool/ContainerPool.scala
+++ b/core/invoker/src/main/scala/whisk/core/containerpool/ContainerPool.scala
@@ -100,17 +100,12 @@ class ContainerPool(
             } else None
 
             container match {
-                case Some(actor) =>
-                    freePool.get(actor) match {
-                        case Some(data) =>
-                            busyPool.update(actor, data)
-                            freePool.remove(actor)
-                            actor ! r // forwards the run request to the container
-                        case None =>
-                            logging.error(this, "actor data not found")
-                            self ! r
-                    }
-                case None => self ! r
+                case Some((actor, data)) =>
+                    busyPool.update(actor, data)
+                    freePool.remove(actor)
+                    actor ! r // forwards the run request to the container
+                case None =>
+                    self ! r
             }
 
         // Container is free to take more work
@@ -133,15 +128,17 @@ class ContainerPool(
     }
 
     /** Creates a new container and updates state accordingly. */
-    def createContainer() = {
+    def createContainer(): (ActorRef, ContainerData) = {
         val ref = childFactory(context)
-        freePool.update(ref, NoData())
-        ref
+        val data = NoData()
+        freePool.update(ref, data)
+
+        (ref, data)
     }
 
     /** Creates a new prewarmed container */
     def prewarmContainer(exec: CodeExec[_], memoryLimit: ByteSize) =
-        prewarmConfig.foreach(config => childFactory(context) ! Start(exec, memoryLimit))
+        childFactory(context) ! Start(exec, memoryLimit)
 
     /**
      * Takes a prewarm container out of the prewarmed pool
@@ -150,23 +147,24 @@ class ContainerPool(
      * @param kind the kind you want to invoke
      * @return the container iff found
      */
-    def takePrewarmContainer(action: ExecutableWhiskAction) = prewarmConfig.flatMap { config =>
-        val kind = action.exec.kind
-        val memory = action.limits.memory.megabytes.MB
-        prewarmedPool.find {
-            case (_, PreWarmedData(_, `kind`, `memory`)) => true
-            case _                                       => false
-        }.map {
-            case (ref, data) =>
-                // Move the container to the usual pool
-                freePool.update(ref, data)
-                prewarmedPool.remove(ref)
-                // Create a new prewarm container
-                prewarmContainer(config.exec, config.memoryLimit)
-
-                ref
+    def takePrewarmContainer(action: ExecutableWhiskAction): Option[(ActorRef, ContainerData)] =
+        prewarmConfig.flatMap { config =>
+            val kind = action.exec.kind
+            val memory = action.limits.memory.megabytes.MB
+            prewarmedPool.find {
+                case (_, PreWarmedData(_, `kind`, `memory`)) => true
+                case _                                       => false
+            }.map {
+                case (ref, data) =>
+                    // Move the container to the usual pool
+                    freePool.update(ref, data)
+                    prewarmedPool.remove(ref)
+                    // Create a new prewarm container
+                    prewarmContainer(config.exec, config.memoryLimit)
+
+                    (ref, data)
+            }
         }
-    }
 
     /** Removes a container and updates state accordingly. */
     def removeContainer(toDelete: ActorRef) = {
@@ -192,11 +190,11 @@ object ContainerPool {
      * @param idles a map of idle containers, awaiting work
      * @return a container if one found
      */
-    def schedule[A](action: ExecutableWhiskAction, invocationNamespace: EntityName, idles: Map[A, ContainerData]): Option[A] = {
+    def schedule[A](action: ExecutableWhiskAction, invocationNamespace: EntityName, idles: Map[A, ContainerData]): Option[(A, ContainerData)] = {
         idles.find {
             case (_, WarmedData(_, `invocationNamespace`, `action`, _)) => true
             case _ => false
-        }.map(_._1)
+        }
     }
 
     /**
diff --git a/tests/src/test/scala/whisk/core/containerpool/test/ContainerPoolTests.scala b/tests/src/test/scala/whisk/core/containerpool/test/ContainerPoolTests.scala
index 7a3454b..e262525 100644
--- a/tests/src/test/scala/whisk/core/containerpool/test/ContainerPoolTests.scala
+++ b/tests/src/test/scala/whisk/core/containerpool/test/ContainerPoolTests.scala
@@ -340,7 +340,7 @@ class ContainerPoolObjectTests extends FlatSpec with Matchers with MockFactory {
         val pool = Map('name -> data)
 
         // copy to make sure, referencial equality doesn't suffice
-        ContainerPool.schedule(data.action.copy(), data.invocationNamespace, pool) shouldBe Some('name)
+        ContainerPool.schedule(data.action.copy(), data.invocationNamespace, pool) shouldBe Some('name, data)
     }
 
     it should "reuse an applicable warm container from idle pool with several applicable containers" in {
@@ -349,7 +349,7 @@ class ContainerPoolObjectTests extends FlatSpec with Matchers with MockFactory {
             'first -> data,
             'second -> data)
 
-        ContainerPool.schedule(data.action.copy(), data.invocationNamespace, pool) should contain oneOf ('first, 'second)
+        ContainerPool.schedule(data.action.copy(), data.invocationNamespace, pool) should (be(Some('first, data)) or be(Some('second, data)))
     }
 
     it should "reuse an applicable warm container from idle pool with several different containers" in {
@@ -359,7 +359,7 @@ class ContainerPoolObjectTests extends FlatSpec with Matchers with MockFactory {
             'pre -> preWarmedData(),
             'warm -> matchingData)
 
-        ContainerPool.schedule(matchingData.action.copy(), matchingData.invocationNamespace, pool) shouldBe Some('warm)
+        ContainerPool.schedule(matchingData.action.copy(), matchingData.invocationNamespace, pool) shouldBe Some('warm, matchingData)
     }
 
     it should "not reuse a container from idle pool with non-warm containers" in {

-- 
To stop receiving notification emails like this one, please contact
"commits@openwhisk.apache.org" <co...@openwhisk.apache.org>.