[openwhisk] branch master updated: Take revision into consideration when choose warm container (#5233)

[st...@apache.org]

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

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


The following commit(s) were added to refs/heads/master by this push:
     new 7fdc24613 Take revision into consideration when choose warm container (#5233)
7fdc24613 is described below

commit 7fdc246137162e937e54435e1f27d8d95e8e89d5
Author: jiangpch <ji...@navercorp.com>
AuthorDate: Tue May 24 12:52:55 2022 +0800

    Take revision into consideration when choose warm container (#5233)
---
 .../scheduler/container/ContainerManager.scala     |  2 +-
 .../container/test/ContainerManagerTests.scala     | 61 +++++++++++++++++++++-
 2 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/core/scheduler/src/main/scala/org/apache/openwhisk/core/scheduler/container/ContainerManager.scala b/core/scheduler/src/main/scala/org/apache/openwhisk/core/scheduler/container/ContainerManager.scala
index 52cfe69eb..eaeb70be4 100644
--- a/core/scheduler/src/main/scala/org/apache/openwhisk/core/scheduler/container/ContainerManager.scala
+++ b/core/scheduler/src/main/scala/org/apache/openwhisk/core/scheduler/container/ContainerManager.scala
@@ -251,7 +251,7 @@ class ContainerManager(jobManagerFactory: ActorRefFactory => ActorRef,
   // Filter out messages which can use warmed container
   private def filterWarmedCreations(msgs: List[ContainerCreationMessage]) = {
     msgs.filter { msg =>
-      val warmedPrefix = containerPrefix(ContainerKeys.warmedPrefix, msg.invocationNamespace, msg.action)
+      val warmedPrefix = containerPrefix(ContainerKeys.warmedPrefix, msg.invocationNamespace, msg.action, Some(msg.revision))
       val chosenInvoker = warmedContainers
         .filter(!inProgressWarmedContainers.values.toSeq.contains(_))
         .find { container =>
diff --git a/tests/src/test/scala/org/apache/openwhisk/core/scheduler/container/test/ContainerManagerTests.scala b/tests/src/test/scala/org/apache/openwhisk/core/scheduler/container/test/ContainerManagerTests.scala
index 569a66141..6c17346da 100644
--- a/tests/src/test/scala/org/apache/openwhisk/core/scheduler/container/test/ContainerManagerTests.scala
+++ b/tests/src/test/scala/org/apache/openwhisk/core/scheduler/container/test/ContainerManagerTests.scala
@@ -278,7 +278,7 @@ class ContainerManagerTests
     )
     expectGetInvokers(mockEtcd, invokers)
     expectGetInvokers(mockEtcd, invokers)
-    expectGetInvokers(mockEtcd, invokers) // this test case will run `getPrefix` twice
+    expectGetInvokers(mockEtcd, invokers) // this test case will run `getPrefix` twice, and another one for warmup
 
     val mockJobManager = TestProbe()
     val mockWatcher = TestProbe()
@@ -377,6 +377,65 @@ class ContainerManagerTests
     receiver.expectMsg(s"invoker0-$msg1")
   }
 
+  it should "not try warmed containers if revision is unmatched" in {
+    val mockEtcd = mock[EtcdClient]
+
+    // for test, only invoker2 is healthy, so that no-warmed creations can be only sent to invoker2
+    val invokers: List[InvokerHealth] = List(
+      InvokerHealth(InvokerInstanceId(0, userMemory = testMemory, tags = Seq.empty[String]), Unhealthy),
+      InvokerHealth(InvokerInstanceId(1, userMemory = testMemory, tags = Seq.empty[String]), Unhealthy),
+      InvokerHealth(InvokerInstanceId(2, userMemory = testMemory, tags = Seq.empty[String]), Healthy),
+    )
+    expectGetInvokers(mockEtcd, invokers)
+    expectGetInvokers(mockEtcd, invokers) // one for warmup
+
+    val mockJobManager = TestProbe()
+    val mockWatcher = TestProbe()
+    val receiver = TestProbe()
+
+    val manager =
+      system.actorOf(ContainerManager
+        .props(factory(mockJobManager), mockMessaging(Some(receiver.ref)), testsid, mockEtcd, config, mockWatcher.ref))
+
+    // there are 1 warmed container for `test-namespace/test-action` but with a different revision
+    manager ! WatchEndpointInserted(
+      ContainerKeys.warmedPrefix,
+      ContainerKeys.warmedContainers(
+        testInvocationNamespace,
+        testfqn,
+        DocRevision("2-testRev"),
+        InvokerInstanceId(0, userMemory = 0.bytes),
+        ContainerId("fake")),
+      "",
+      true)
+
+    val msg =
+      ContainerCreationMessage(
+        TransactionId.testing,
+        testInvocationNamespace,
+        testfqn,
+        testRevision,
+        actionMetadata,
+        testsid,
+        schedulerHost,
+        rpcPort)
+
+    // it should not reuse the warmed container
+    manager ! ContainerCreation(List(msg), 128.MB, testInvocationNamespace)
+
+    // ignore warmUp message
+    receiver.ignoreMsg {
+      case s: String => s.contains("warmUp")
+    }
+
+    // it should be scheduled to the sole health invoker: invoker2
+    receiver.expectMsg(s"invoker2-$msg")
+
+    mockJobManager.expectMsgPF() {
+      case RegisterCreationJob(`msg`) => true
+    }
+  }
+
   it should "rescheduling container creation" in {
     val mockEtcd = mock[EtcdClient]
     (mockEtcd