[GitHub] markusthoemmes commented on a change in pull request #3789: Refactoring the load balancer with an overflow queue to reduce activation waiting time

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

markusthoemmes commented on a change in pull request #3789: Refactoring the load balancer with an overflow queue to reduce activation waiting time
URL: https://github.com/apache/incubator-openwhisk/pull/3789#discussion_r197350281
 
 

 ##########
 File path: core/controller/src/main/scala/whisk/core/loadBalancer/ShardingContainerPoolBalancer.scala
 ##########
 @@ -133,38 +135,91 @@ class ShardingContainerPoolBalancer(config: WhiskConfig, controllerInstance: Ins
   /** 1. Publish a message to the loadbalancer */
   override def publish(action: ExecutableWhiskActionMetaData, msg: ActivationMessage)(
     implicit transid: TransactionId): Future[Future[Either[ActivationId, WhiskActivation]]] = {
+    val hash = ShardingContainerPoolBalancer.generateHash(msg.user.namespace.name, action.fullyQualifiedName(false))
+    publish(msg.transid, action.limits.timeout.duration.toSeconds.toInt, action.exec.pull, hash, msg, false)
+  }
 
+  def publish(
+    transid: TransactionId,
+    actionTimeoutSeconds: Int,
+    pull: Boolean,
+    hash: Int,
+    msg: ActivationMessage,
+    isRescheduled: Boolean): Future[Future[Either[ActivationId, WhiskActivation]]] = {
+    logging.info(this, s"Publish activation ${msg.activationId} with & isRescheduled= $isRescheduled")
     val (invokersToUse, stepSizes) =
-      if (!action.exec.pull) (schedulingState.managedInvokers, schedulingState.managedStepSizes)
+      if (!pull) (schedulingState.managedInvokers, schedulingState.managedStepSizes)
       else (schedulingState.blackboxInvokers, schedulingState.blackboxStepSizes)
+
     val chosen = if (invokersToUse.nonEmpty) {
-      val hash = ShardingContainerPoolBalancer.generateHash(msg.user.namespace.name, action.fullyQualifiedName(false))
-      val homeInvoker = hash % invokersToUse.size
-      val stepSize = stepSizes(hash % stepSizes.size)
-      ShardingContainerPoolBalancer.schedule(invokersToUse, schedulingState.invokerSlots, homeInvoker, stepSize)
+      // In case of the system isn't in overflow state or it's in the overflow state but the message is rescheduled
+      // we want to call the schedule algorithm to choose an invoker
+      if (!overflowState.get() || (overflowState.get() && isRescheduled)) {
+        val homeInvoker = hash % invokersToUse.size
+        val stepSize = stepSizes(hash % stepSizes.size)
+        ShardingContainerPoolBalancer.schedule(invokersToUse, schedulingState.invokerSlots, homeInvoker, stepSize)
+      } else {
+        // In case of the system is in overflow state and the ActivationMessage isn't rescheduled
+        // return -1 indicates as an index for the invoker
+        Some(new InstanceId(-1))
 
 Review comment:
   Echoing @tysonnorris' argument: I think we should refactor the `schedule` method to return `Some` for a found invoker and `None` for no invoker found. The special case of `no invokers available` could arguably either end up in queueing straight away or rejecting the request. If we want to continue to do the latter, let's move the emptiness check out of `schedule`.

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