[GitHub] markusthoemmes commented on a change in pull request #3747: Memory based loadbalancing

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

markusthoemmes commented on a change in pull request #3747: Memory based loadbalancing
URL: https://github.com/apache/incubator-openwhisk/pull/3747#discussion_r196359970
 
 

 ##########
 File path: core/invoker/src/main/scala/whisk/core/containerpool/ContainerPool.scala
 ##########
 @@ -91,62 +95,89 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
     // their requests and send them back to the pool for rescheduling (this may happen if "docker" operations
     // fail for example, or a container has aged and was destroying itself when a new request was assigned)
     case r: Run =>
-      val createdContainer = if (busyPool.size < poolConfig.maxActiveContainers) {
+      // Only process request, if there are no other requests waiting for free slots, or if the current request is the next request to process
+      if (runBuffer.isEmpty || runBuffer.dequeueOption.exists(_._1.msg == r.msg)) {
+        val createdContainer =
+          if (busyPool
+                .map(_._2.memoryLimit.toMB)
+                .sum + r.action.limits.memory.megabytes <= poolConfig.userMemory.toMB) {
+            // Schedule a job to a warm container
+            ContainerPool
+              .schedule(r.action, r.msg.user.namespace.name, freePool)
+              .map(container => {
+                (container, "warm")
+              })
+              .orElse {
+                if (busyPool
+                      .map(_._2.memoryLimit.toMB)
+                      .sum + freePool.map(_._2.memoryLimit.toMB).sum < poolConfig.userMemory.toMB) {
 
 Review comment:
   I think we need to add the action's memory limit to this condition as well.
   
   On a broader note: Should we make this a method?
   
   ```scala
   def hasCapacityFor(pool: Map[ActorRef, ContainerData], memory: ByteSize): Boolean =
     pool.map(_._2.memoryLimit.toMB).sum + memory.toMB <= poolConfig.userMemory.toMB)
   ```
   
   Would then be usable like `hasCapacityFor(busyPool ++ freePool, r.action.limits.memory.megabytes)`

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