[GitHub] tysonnorris commented on a change in pull request #3232: Making prewarm kind (and count) configurable

[GitBox <gi...@apache.org>]

tysonnorris commented on a change in pull request #3232: Making prewarm kind (and count) configurable
URL: https://github.com/apache/incubator-openwhisk/pull/3232#discussion_r187685673
 
 

 ##########
 File path: core/invoker/src/main/scala/whisk/core/containerpool/ContainerPool.scala
 ##########
 @@ -204,26 +204,25 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
    * @param kind the kind you want to invoke
    * @return the container iff found
    */
-  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 = freePool + (ref -> data)
-            prewarmedPool = prewarmedPool - ref
-            // Create a new prewarm container
-            prewarmContainer(config.exec, config.memoryLimit)
+  def takePrewarmContainer(action: ExecutableWhiskAction): Option[(ActorRef, ContainerData)] = {
+    val kind = action.exec.kind
+    val memory = action.limits.memory.megabytes.MB
+    prewarmedPool
+      .find {
+        case (_, PreWarmedData(_, `kind`, `memory`)) => true
 
 Review comment:
   This will use the prewarm IFF the prewarm container is an exact match for memory req. Is that intended? 
   
   I wonder if it should be `if prewarm-memory > action memory > prewarm-memory - memory-match-threshold`
   So that
   * there DOES NOT need to be an exact match of action memory req + prewarm memory
   * there  DOES need to be a "close match", dictated by a configurable memory-match-threshold (so that a 20MB action does not land in a 4GB prewarm)
   
   I'm not sure that configuration should be global vs per-prewarm config (which would require additional change in the manifest)
   
   WDYT?

----------------------------------------------------------------
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