You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@openwhisk.apache.org by GitBox <gi...@apache.org> on 2022/11/01 01:59:40 UTC

[GitHub] [openwhisk] bdoyle0182 commented on a diff in pull request #5344: optimize scheduling decision when there are stale activations

bdoyle0182 commented on code in PR #5344:
URL: https://github.com/apache/openwhisk/pull/5344#discussion_r1010006968


##########
tests/src/test/scala/org/apache/openwhisk/core/scheduler/queue/test/SchedulingDecisionMakerTests.scala:
##########
@@ -683,6 +683,54 @@ class SchedulingDecisionMakerTests
     testProbe.expectMsg(DecisionResults(AddContainer, 2))
   }
 
+  it should "add more containers when there are stale messages and non-stale messages and both message classes need more containers" in {
+    val decisionMaker = system.actorOf(SchedulingDecisionMaker.props(testNamespace, action, schedulingConfig))
+    val testProbe = TestProbe()
+
+    val msg = QueueSnapshot(
+      initialized = true,
+      incomingMsgCount = new AtomicInteger(0),
+      currentMsgCount = 5,
+      existingContainerCount = 2,
+      inProgressContainerCount = 0,
+      staleActivationNum = 2,
+      existingContainerCountInNamespace = 2,
+      inProgressContainerCountInNamespace = 0,
+      averageDuration = Some(1000), // the average duration exists
+      limit = 10,
+      stateName = Running,
+      recipient = testProbe.ref)
+
+    decisionMaker ! msg
+
+    //should add two for the stale messages and one to increase tps of non-stale available messages
+    testProbe.expectMsg(DecisionResults(AddContainer, 3))
+  }
+
+  it should "add more containers when there are stale messages and non-stale messages have needed tps" in {
+    val decisionMaker = system.actorOf(SchedulingDecisionMaker.props(testNamespace, action, schedulingConfig))
+    val testProbe = TestProbe()
+
+    val msg = QueueSnapshot(
+      initialized = true,
+      incomingMsgCount = new AtomicInteger(0),
+      currentMsgCount = 5,
+      existingContainerCount = 2,
+      inProgressContainerCount = 0,
+      staleActivationNum = 2,
+      existingContainerCountInNamespace = 2,
+      inProgressContainerCountInNamespace = 0,
+      averageDuration = Some(50), // the average duration gives container throughput of 2

Review Comment:
   Isn't the default stale threshold 100ms so 100 / 50 = 2. Is that a mistake should the `staleThreshold` really be 1000ms?



##########
tests/src/test/scala/org/apache/openwhisk/core/scheduler/queue/test/SchedulingDecisionMakerTests.scala:
##########
@@ -683,6 +683,54 @@ class SchedulingDecisionMakerTests
     testProbe.expectMsg(DecisionResults(AddContainer, 2))
   }
 
+  it should "add more containers when there are stale messages and non-stale messages and both message classes need more containers" in {
+    val decisionMaker = system.actorOf(SchedulingDecisionMaker.props(testNamespace, action, schedulingConfig))
+    val testProbe = TestProbe()
+
+    val msg = QueueSnapshot(
+      initialized = true,
+      incomingMsgCount = new AtomicInteger(0),
+      currentMsgCount = 5,
+      existingContainerCount = 2,
+      inProgressContainerCount = 0,
+      staleActivationNum = 2,
+      existingContainerCountInNamespace = 2,
+      inProgressContainerCountInNamespace = 0,
+      averageDuration = Some(1000), // the average duration exists
+      limit = 10,
+      stateName = Running,
+      recipient = testProbe.ref)
+
+    decisionMaker ! msg
+
+    //should add two for the stale messages and one to increase tps of non-stale available messages
+    testProbe.expectMsg(DecisionResults(AddContainer, 3))
+  }
+
+  it should "add more containers when there are stale messages and non-stale messages have needed tps" in {
+    val decisionMaker = system.actorOf(SchedulingDecisionMaker.props(testNamespace, action, schedulingConfig))
+    val testProbe = TestProbe()
+
+    val msg = QueueSnapshot(
+      initialized = true,
+      incomingMsgCount = new AtomicInteger(0),
+      currentMsgCount = 5,
+      existingContainerCount = 2,
+      inProgressContainerCount = 0,
+      staleActivationNum = 2,
+      existingContainerCountInNamespace = 2,
+      inProgressContainerCountInNamespace = 0,
+      averageDuration = Some(50), // the average duration gives container throughput of 2

Review Comment:
   Isn't the default stale threshold 100ms so 100 / 50 = 2. Is that a mistake should the `staleThreshold` configuration really be 1000ms?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@openwhisk.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org