This is an automated email from the ASF dual-hosted git repository.
dubeejw pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-openwhisk.git
The following commit(s) were added to refs/heads/master by this push:
new 1ed7568 Remove obsolete filter when determining container to remove.
(#3130)
1ed7568 is described below
commit 1ed7568800630059e12f849544af2734301f72b9
Author: rodric rabbah <rod...@gmail.com>
AuthorDate: Sat Dec 23 20:47:51 2017 -0500
Remove obsolete filter when determining container to remove. (#3130)
While collecting a container to remove, the filter checks the action and
invocation namespace and excludes those from the candidate list. But this check
is not necessary - any container in the free pool matching the action and
namespace would be reused and the remove method should not be called in this
case.
---
.../whisk/core/containerpool/ContainerPool.scala | 22 +++++++++-------------
.../containerpool/test/ContainerPoolTests.scala | 22 +++++-----------------
2 files changed, 14 insertions(+), 30 deletions(-)
diff --git
a/core/invoker/src/main/scala/whisk/core/containerpool/ContainerPool.scala
b/core/invoker/src/main/scala/whisk/core/containerpool/ContainerPool.scala
index 5177026..1fa6b3a 100644
--- a/core/invoker/src/main/scala/whisk/core/containerpool/ContainerPool.scala
+++ b/core/invoker/src/main/scala/whisk/core/containerpool/ContainerPool.scala
@@ -95,7 +95,7 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
}
.orElse {
// Remove a container and create a new one for the given job
- ContainerPool.remove(r.action, r.msg.user.namespace, freePool).map
{ toDelete =>
+ ContainerPool.remove(freePool).map { toDelete =>
removeContainer(toDelete)
takePrewarmContainer(r.action).getOrElse {
createContainer()
@@ -200,9 +200,9 @@ object ContainerPool {
* @param idles a map of idle containers, awaiting work
* @return a container if one found
*/
- def schedule[A](action: ExecutableWhiskAction,
- invocationNamespace: EntityName,
- idles: Map[A, ContainerData]): Option[(A, ContainerData)] = {
+ protected[containerpool] def schedule[A](action: ExecutableWhiskAction,
+ invocationNamespace: EntityName,
+ idles: Map[A, ContainerData]):
Option[(A, ContainerData)] = {
idles.find {
case (_, WarmedData(_, `invocationNamespace`, `action`, _)) => true
case _ => false
@@ -210,21 +210,17 @@ object ContainerPool {
}
/**
- * Finds the best container to remove to make space for the job passed to
run.
+ * Finds the oldest previously used container to remove to make space for
the job passed to run.
*
- * Determines the least recently used Free container in the pool.
+ * NOTE: This method is never called to remove an action that is in the pool
already,
+ * since this would be picked up earlier in the scheduler and the container
reused.
*
- * @param action the action that wants to get a container
- * @param invocationNamespace the namespace, that wants to run the action
* @param pool a map of all free containers in the pool
* @return a container to be removed iff found
*/
- def remove[A](action: ExecutableWhiskAction,
- invocationNamespace: EntityName,
- pool: Map[A, ContainerData]): Option[A] = {
- // Try to find a Free container that is initialized with any OTHER action
+ protected[containerpool] def remove[A](pool: Map[A, ContainerData]):
Option[A] = {
val freeContainers = pool.collect {
- case (ref, w: WarmedData) if (w.action != action ||
w.invocationNamespace != invocationNamespace) => ref -> w
+ case (ref, w: WarmedData) => ref -> w
}
if (freeContainers.nonEmpty) {
diff --git
a/tests/src/test/scala/whisk/core/containerpool/test/ContainerPoolTests.scala
b/tests/src/test/scala/whisk/core/containerpool/test/ContainerPoolTests.scala
index 25b17b6..a607f5b 100644
---
a/tests/src/test/scala/whisk/core/containerpool/test/ContainerPoolTests.scala
+++
b/tests/src/test/scala/whisk/core/containerpool/test/ContainerPoolTests.scala
@@ -380,30 +380,18 @@ class ContainerPoolObjectTests extends FlatSpec with
Matchers with MockFactory {
behavior of "ContainerPool remove()"
it should "not provide a container if pool is empty" in {
- ContainerPool.remove(createAction(), standardNamespace, Map()) shouldBe
None
+ ContainerPool.remove(Map()) shouldBe None
}
it should "not provide a container from busy pool with non-warm containers"
in {
val pool = Map('none -> noData(), 'pre -> preWarmedData())
- ContainerPool.remove(createAction(), standardNamespace, pool) shouldBe None
+ ContainerPool.remove(pool) shouldBe None
}
- it should "not provide a container from pool with one single free container
with the same action and namespace" in {
+ it should "provide a container from pool with one single free container" in {
val data = warmedData()
val pool = Map('warm -> data)
-
- // same data --> no removal
- ContainerPool.remove(data.action, data.invocationNamespace, pool) shouldBe
None
-
- // different action, same namespace --> remove
- ContainerPool.remove(createAction(data.action.name + "butDifferent"),
data.invocationNamespace, pool) shouldBe Some(
- 'warm)
-
- // different namespace, same action --> remove
- ContainerPool.remove(data.action, differentNamespace, pool) shouldBe
Some('warm)
-
- // different everything --> remove
- ContainerPool.remove(createAction(data.action.name + "butDifferent"),
differentNamespace, pool) shouldBe Some('warm)
+ ContainerPool.remove(pool) shouldBe Some('warm)
}
it should "provide oldest container from busy pool with multiple containers"
in {
@@ -414,6 +402,6 @@ class ContainerPoolObjectTests extends FlatSpec with
Matchers with MockFactory {
val pool = Map('first -> first, 'second -> second, 'oldest -> oldest)
- ContainerPool.remove(createAction(), standardNamespace, pool) shouldBe
Some('oldest)
+ ContainerPool.remove(pool) shouldBe Some('oldest)
}
}
--
To stop receiving notification emails like this one, please contact
['"commits@openwhisk.apache.org" <commits@openwhisk.apache.org>'].
