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 2020/04/02 06:13:18 UTC
[GitHub] [openwhisk] ningyougang opened a new pull request #4871: Adjust
prewarm container dynamically
ningyougang opened a new pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871
<!--- Provide a concise summary of your changes in the Title -->
## Description
<!--- Provide a detailed description of your changes. -->
<!--- Include details of what problem you are solving and how your changes are tested. -->
As we already know, when invoker starts up, will create some prewarmed containers in prewarmedpool in advance.
If the prewarmed containers are not used for sometime, the prewarmed containers will exist forever.
it may lead to `waste of resources`
so there has need to provide some mechanism to save memory resources, e.g.
* If the prewarmed containers are not used for sometime, should delete them automatically.
* When cold start happened, we should supplement a prewarmed container which matched the kind/memory in runtime.
## Related issue and scope
<!--- Please include a link to a related issue if there is one. -->
https://github.com/apache/openwhisk/issues/4725
## My changes affect the following components
<!--- Select below all system components are affected by your change. -->
<!--- Enter an `x` in all applicable boxes. -->
- [ ] API
- [ ] Controller
- [ ] Message Bus (e.g., Kafka)
- [ ] Loadbalancer
- [x] Invoker
- [ ] Intrinsic actions (e.g., sequences, conductors)
- [ ] Data stores (e.g., CouchDB)
- [ ] Tests
- [ ] Deployment
- [ ] CLI
- [ ] General tooling
- [ ] Documentation
## Types of changes
<!--- What types of changes does your code introduce? Use `x` in all the boxes that apply: -->
- [ ] Bug fix (generally a non-breaking change which closes an issue).
- [x] Enhancement or new feature (adds new functionality).
- [ ] Breaking change (a bug fix or enhancement which changes existing behavior).
## Checklist:
<!--- Please review the points below which help you make sure you've covered all aspects of the change you're making. -->
- [x] I signed an [Apache CLA](https://github.com/apache/openwhisk/blob/master/CONTRIBUTING.md).
- [x] I reviewed the [style guides](https://github.com/apache/openwhisk/wiki/Contributing:-Git-guidelines#code-readiness) and followed the recommendations (Travis CI will check :).
- [ ] I added tests to cover my changes.
- [ ] My changes require further changes to the documentation.
- [ ] I updated the documentation where necessary.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] tysonnorris commented on issue #4871: Adjust prewarm
container dynamically
Posted by GitBox <gi...@apache.org>.
tysonnorris commented on issue #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#issuecomment-611229152
Please also update the `ContainerPool.emitMetrics()` function to include count per stemcell config, instead of just aggregate count of all stemcells - since with fluctuating numbers, these values will get very confusing.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r402680381
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -82,6 +85,27 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
backfillPrewarms(true)
+ // check periodically every 1 minute, delete prewarmed container if unused for sometime
+ context.system.scheduler.schedule(10.seconds, 1.minute) {
+ prewarmConfig.foreach { config =>
+ val kind = config.exec.kind
+ val memory = config.memoryLimit
+ val ttlMinutes = config.ttlMinutes
+ val containers = prewarmedPool.filter { warmInfo =>
+ warmInfo match {
+ case (_, PreWarmedData(_, `kind`, `memory`, _)) => true
+ case _ => false
+ }
+ }
+ for ((container, data) <- containers) {
+ if (Duration.between(data.lastUsed, Instant.now).compareTo(Duration.ofMinutes(ttlMinutes)) > 0) {
Review comment:
Yes, agree with you.
I introduced a new mechanism to avoid `the cold start`
added `threshold`, `increment` in `runtime.json`
after prewarmed containers are deleted due to unused.
if action for that kind/memory's activation number >= `threshold` in last one minute,
create `increment` prewarmed containers.
refer to: https://github.com/apache/openwhisk/pull/4871/commits/2fd4dd1b677d65a101c954655742ea1860cbb29a
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] tysonnorris commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
tysonnorris commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r407852723
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -80,8 +86,59 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
//periodically emit metrics (don't need to do this for each message!)
context.system.scheduler.schedule(30.seconds, 10.seconds, self, EmitMetrics)
+ // Key is `kind,memory,time`, value is the number of cold Start in minute
+ var coldStartCount = immutable.Map.empty[String, Int]
+
backfillPrewarms(true)
+ // check periodically every 1 minute, delete prewarmed container if unused for sometime
+ context.system.scheduler.schedule(10.seconds, 1.minute, self, DeleteUnusedPrewarmedContainer)
+
+ // check periodically for the cold start and create some increment containers automatically if activation >= threshold
+ context.system.scheduler.schedule(1.minute, 1.minute, self, SupplementPrewarmedContainer)
Review comment:
Use same scheduled message to do both delete + supplement
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] style95 commented on a change in pull request #4871:
Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
style95 commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r402192334
##########
File path: ansible/files/runtimes.json
##########
@@ -58,7 +58,8 @@
"stemCells": [
{
"count": 2,
- "memory": "256 MB"
+ "memory": "256 MB",
+ "ttlMinutes": 10
Review comment:
How about adding more fine granularity by changing the unit to second?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r406674460
##########
File path: ansible/files/runtimes.json
##########
@@ -58,7 +58,10 @@
"stemCells": [
{
"count": 2,
- "memory": "256 MB"
+ "memory": "256 MB",
+ "ttlMinutes": 10,
+ "threshold": 5,
+ "increment": 1
Review comment:
Already fixed
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] style95 commented on a change in pull request #4871:
Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
style95 commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r407478832
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -80,8 +86,59 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
//periodically emit metrics (don't need to do this for each message!)
context.system.scheduler.schedule(30.seconds, 10.seconds, self, EmitMetrics)
+ // Key is `kind,memory,time`, value is the number of cold Start in minute
+ var coldStartCount = immutable.Map.empty[String, Int]
Review comment:
It seems this map can be used in multiple threads, should we use `concurrent.map` instead?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r402677602
##########
File path: ansible/files/runtimes.json
##########
@@ -58,7 +58,8 @@
"stemCells": [
{
"count": 2,
- "memory": "256 MB"
+ "memory": "256 MB",
+ "ttlMinutes": 10
Review comment:
It seems unit of second is very small, and the unit of hour is too big.
so i choosed the unit of minute.
Does it make sense if we use unit of second?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r405958096
##########
File path: ansible/files/runtimes.json
##########
@@ -58,7 +58,8 @@
"stemCells": [
{
"count": 2,
- "memory": "256 MB"
+ "memory": "256 MB",
+ "ttlMinutes": 10
Review comment:
@tysonnorris ,got it
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r402680381
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -82,6 +85,27 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
backfillPrewarms(true)
+ // check periodically every 1 minute, delete prewarmed container if unused for sometime
+ context.system.scheduler.schedule(10.seconds, 1.minute) {
+ prewarmConfig.foreach { config =>
+ val kind = config.exec.kind
+ val memory = config.memoryLimit
+ val ttlMinutes = config.ttlMinutes
+ val containers = prewarmedPool.filter { warmInfo =>
+ warmInfo match {
+ case (_, PreWarmedData(_, `kind`, `memory`, _)) => true
+ case _ => false
+ }
+ }
+ for ((container, data) <- containers) {
+ if (Duration.between(data.lastUsed, Instant.now).compareTo(Duration.ofMinutes(ttlMinutes)) > 0) {
Review comment:
Yes, agree with you.
I introduced a new mechanism to `avoid the cold start`
added `threshold`, `increment` in `runtime.json`
after prewarmed containers are deleted due to unused.
if action for that kind/memory's activation number >= `threshold` in previous one minute,
create `increment` prewarmed containers.
refer to the new added commit: https://github.com/apache/openwhisk/pull/4871/commits/8c36f33b2c2eddd266fcd0418b21da3078639ba5
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] tysonnorris commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
tysonnorris commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r405849373
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -80,8 +85,49 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
//periodically emit metrics (don't need to do this for each message!)
context.system.scheduler.schedule(30.seconds, 10.seconds, self, EmitMetrics)
+ // Key is `kind,memory,time`, value is the number of cold Start in minute
+ val coldStartCount = TrieMap[String, AtomicInteger]()
Review comment:
Actually, I would probably not create a separate map here, but rather add `expiration:Option[Deadline]` to `PreWarmedData` class. Then you can track expiration AND more easily filter to find expired prewarms.
EDIT: sorry, to be clear: the expiration is required to remove unused ones, but there is a separate issue that needs to be tracked which is prewam "miss", where the config matches prewarm config, but no prewarm was available. For the miss, you can use an immutable.Map that is reset every minute (or configurable time period), without tracking timestamps or expirations there.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r402677602
##########
File path: ansible/files/runtimes.json
##########
@@ -58,7 +58,8 @@
"stemCells": [
{
"count": 2,
- "memory": "256 MB"
+ "memory": "256 MB",
+ "ttlMinutes": 10
Review comment:
It seems unit of second is very small, and the unit of hour is too big.
so i chosed the unit of minute (second < minute < hour).
Does it make sense if we use unit of second?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] tysonnorris commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
tysonnorris commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r407854790
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerProxy.scala
##########
@@ -146,7 +146,7 @@ case class PreWarmedData(override val container: Container,
kind: String,
override val memoryLimit: ByteSize,
override val activeActivationCount: Int = 0)
- extends ContainerStarted(container, Instant.EPOCH, memoryLimit, activeActivationCount)
+ extends ContainerStarted(container, Instant.now(), memoryLimit, activeActivationCount)
Review comment:
overloading the lastUse property is confusing - I would move that property to ContainerInUse trait, and create a dedicated property in PrewarmedData for tracking expiration Instant or Deadline, so that every scheduled period can easily check (without calculation of expiration) whether the expiration is in the past.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] tysonnorris commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
tysonnorris commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r405854040
##########
File path: ansible/files/runtimes.json
##########
@@ -58,7 +58,10 @@
"stemCells": [
{
"count": 2,
- "memory": "256 MB"
+ "memory": "256 MB",
+ "ttlMinutes": 10,
+ "threshold": 5,
+ "increment": 1
Review comment:
In this scenario, I think "count" behaves like "initialCount" -right? so now I think we should also have a "maxCount" so that additional prewarms cannot be created past the maxCount limit.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r402680381
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -82,6 +85,27 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
backfillPrewarms(true)
+ // check periodically every 1 minute, delete prewarmed container if unused for sometime
+ context.system.scheduler.schedule(10.seconds, 1.minute) {
+ prewarmConfig.foreach { config =>
+ val kind = config.exec.kind
+ val memory = config.memoryLimit
+ val ttlMinutes = config.ttlMinutes
+ val containers = prewarmedPool.filter { warmInfo =>
+ warmInfo match {
+ case (_, PreWarmedData(_, `kind`, `memory`, _)) => true
+ case _ => false
+ }
+ }
+ for ((container, data) <- containers) {
+ if (Duration.between(data.lastUsed, Instant.now).compareTo(Duration.ofMinutes(ttlMinutes)) > 0) {
Review comment:
Yes, agree with you.
I introduced a new mechanism to `avoid the cold start`
added `threshold`, `increment` in `runtime.json`
after prewarmed containers are deleted due to unused.
if action for that kind/memory's activation number >= `threshold` in previous one minute, create `increment` prewarmed containers.
refer to the new added commit: https://github.com/apache/openwhisk/pull/4871/commits/8c36f33b2c2eddd266fcd0418b21da3078639ba5
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r407831844
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -80,8 +86,59 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
//periodically emit metrics (don't need to do this for each message!)
context.system.scheduler.schedule(30.seconds, 10.seconds, self, EmitMetrics)
+ // Key is `kind,memory,time`, value is the number of cold Start in minute
+ var coldStartCount = immutable.Map.empty[String, Int]
+
backfillPrewarms(true)
+ // check periodically every 1 minute, delete prewarmed container if unused for sometime
+ context.system.scheduler.schedule(10.seconds, 1.minute, self, DeleteUnusedPrewarmedContainer)
+
+ // check periodically for the cold start and create some increment containers automatically if activation >= threshold
+ context.system.scheduler.schedule(1.minute, 1.minute, self, SupplementPrewarmedContainer)
+
+ def deleteUnusedPrewarmedContainer(): Unit = {
+ prewarmConfig.foreach { config =>
+ val kind = config.exec.kind
+ val memory = config.memoryLimit
+ val ttlSeconds = config.ttl.toSeconds
+ val containers = prewarmedPool.filter { warmInfo =>
+ warmInfo match {
+ case (_, PreWarmedData(_, `kind`, `memory`, _)) => true
+ case _ => false
+ }
+ }
+ for ((container, data) <- containers) {
+ if (JDuration.between(data.lastUsed, Instant.now).compareTo(JDuration.ofSeconds(ttlSeconds)) > 0) {
+ // Don't recover a new one under this situation
+ container ! RemovePreWarmedContainer
+ }
+ }
+ }
+ }
+
+ def supplementPrewarmedContainer() = {
+ prewarmConfig.foreach { config =>
+ val kind = config.exec.kind
+ val memory = config.memoryLimit
+ val calendar = Calendar.getInstance()
+ calendar.add(Calendar.MINUTE, -1)
+ val lastDate = new SimpleDateFormat("yyyy-MM-dd-HH:mm").format(calendar.getTime)
+ val key = s"${kind},${memory.toMB},${lastDate}"
+ coldStartCount.get(key) match {
Review comment:
If we changed from `immutable.Map` to `TrieMap`, we can use `coldStartCount.remove`
But here, coldStartCount can only be accessed via single thread due to actor, i think use `immutable.Map` is ok.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r402680381
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -82,6 +85,27 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
backfillPrewarms(true)
+ // check periodically every 1 minute, delete prewarmed container if unused for sometime
+ context.system.scheduler.schedule(10.seconds, 1.minute) {
+ prewarmConfig.foreach { config =>
+ val kind = config.exec.kind
+ val memory = config.memoryLimit
+ val ttlMinutes = config.ttlMinutes
+ val containers = prewarmedPool.filter { warmInfo =>
+ warmInfo match {
+ case (_, PreWarmedData(_, `kind`, `memory`, _)) => true
+ case _ => false
+ }
+ }
+ for ((container, data) <- containers) {
+ if (Duration.between(data.lastUsed, Instant.now).compareTo(Duration.ofMinutes(ttlMinutes)) > 0) {
Review comment:
Yes, agree with you.
I introduced a new mechanism to avoid `the cold start`
added `threshold`, `increment` in `runtime.json`
after prewarmed containers are deleted due to unused.
if action for that kind/memory's activation number >= `threshold` in last one minute,
create `increment` prewarmed containers.
refer to: https://github.com/apache/openwhisk/pull/4871/commits/8c36f33b2c2eddd266fcd0418b21da3078639ba5
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang edited a comment on issue #4871: Adjust
prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
ningyougang edited a comment on issue #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#issuecomment-612709277
Regarding this pr's travis-ci's `system test`, reported below error
```
TASK [controller : wait until the Controller in this host is up and running] ***
Sunday 12 April 2020 10:02:41 +0000 (0:00:01.132) 0:00:55.659 **********
FAILED - RETRYING: wait until the Controller in this host is up and running (12 retries left).
FAILED - RETRYING: wait until the Controller in this host is up and running (11 retries left).
FAILED - RETRYING: wait until the Controller in this host is up and running (10 retries left).
FAILED - RETRYING: wait until the Controller in this host is up and running (9 retries left).
FAILED - RETRYING: wait until the Controller in this host is up and running (8 retries left).
FAILED - RETRYING: wait until the Controller in this host is up and running (7 retries left).
FAILED - RETRYING: wait until the Controller in this host is up and running (6 retries left).
FAILED - RETRYING: wait until the Controller in this host is up and running (5 retries left).
FAILED - RETRYING: wait until the Controller in this host is up and running (4 retries left).
FAILED - RETRYING: wait until the Controller in this host is up and running (3 retries left).
FAILED - RETRYING: wait until the Controller in this host is up and running (2 retries left).
FAILED - RETRYING: wait until the Controller in this host is up and running (1 retries left).
fatal: [controller0]: FAILED! => {"attempts": 12, "changed": false, "content": "", "msg": "Status code was -1 and not [200]: Request failed: <urlopen error [Errno 111] Connection refused>", "redirected": false, "status": -1, "url": "https://172.17.0.1:10001/ping"}
Status code was -1 and not [200]: Request failed: <urlopen error [Errno
111] Connection refused>
```
But i run in local's severl envs, the controller started successfully.
I don't know why controller start failed in travis-ci, can check the travis-ci's controller logs?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] style95 commented on a change in pull request #4871:
Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
style95 commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r407480887
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -340,6 +418,47 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
newContainer ! Start(exec, memoryLimit)
}
+ /** Create a new prewarm container if currentCount doesn't reach maxCount */
+ def prewarmContainerIfpossible(kind: String, memoryLimit: ByteSize, count: Int): Unit = {
+ prewarmConfig
+ .filter { config =>
+ kind == config.exec.kind && memoryLimit == config.memoryLimit
+ }
+ .foreach { config =>
+ val currentCount = prewarmedPool.count {
+ case (_, PreWarmedData(_, `kind`, `memoryLimit`, _)) => true //done starting
+ case _ => false //started but not finished starting
+ }
+ val startingCount = prewarmStartingPool.count(p => p._2._1 == kind && p._2._2 == memoryLimit)
+ val containerCount = currentCount + startingCount
+ if (containerCount < config.maxCount) {
+ val createNumber = if (config.maxCount - containerCount > count) count else config.maxCount - containerCount
+ 1 to createNumber foreach { _ =>
+ prewarmContainer(config.exec, config.memoryLimit)
+ }
+ }
+ }
+ }
+
+ /** statistics the cold start */
+ def countColdStart(kind: String, memoryLimit: ByteSize): Unit = {
+ prewarmConfig
+ .filter { config =>
+ kind == config.exec.kind && memoryLimit == config.memoryLimit
+ }
+ .foreach { _ =>
+ val time = new SimpleDateFormat("yyyy-MM-dd-HH:mm").format(Calendar.getInstance().getTime)
+ val key = s"${kind},${memoryLimit.toMB},${time}"
Review comment:
Same with the above.
A typed key is preferred.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] tysonnorris commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
tysonnorris commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r407851955
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -80,8 +86,59 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
//periodically emit metrics (don't need to do this for each message!)
context.system.scheduler.schedule(30.seconds, 10.seconds, self, EmitMetrics)
+ // Key is `kind,memory,time`, value is the number of cold Start in minute
+ var coldStartCount = immutable.Map.empty[String, Int]
Review comment:
immutable.Map should be fine if it is only modified during actor message processing
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] style95 commented on a change in pull request #4871:
Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
style95 commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r402203935
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -82,6 +85,27 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
backfillPrewarms(true)
+ // check periodically every 1 minute, delete prewarmed container if unused for sometime
+ context.system.scheduler.schedule(10.seconds, 1.minute) {
+ prewarmConfig.foreach { config =>
+ val kind = config.exec.kind
+ val memory = config.memoryLimit
+ val ttlMinutes = config.ttlMinutes
+ val containers = prewarmedPool.filter { warmInfo =>
+ warmInfo match {
+ case (_, PreWarmedData(_, `kind`, `memory`, _)) => true
+ case _ => false
+ }
+ }
+ for ((container, data) <- containers) {
+ if (Duration.between(data.lastUsed, Instant.now).compareTo(Duration.ofMinutes(ttlMinutes)) > 0) {
Review comment:
If there is no activation for prewarm kind for the given TTL time, it will always cause a cold start next time.
Also, even if we can add more prewarm containers when cold starts happen I think it may not fit well for some workloads. If loads spike for some runtime kind, it will cause a lot of cold starts. Also, it will try to create more prewarm containers at the same time. As we know, docker operation is quite expensive and it will end up with a huge number of container creation along with long wait-time.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] style95 commented on a change in pull request #4871:
Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
style95 commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r407477787
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -80,8 +86,59 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
//periodically emit metrics (don't need to do this for each message!)
context.system.scheduler.schedule(30.seconds, 10.seconds, self, EmitMetrics)
+ // Key is `kind,memory,time`, value is the number of cold Start in minute
+ var coldStartCount = immutable.Map.empty[String, Int]
+
backfillPrewarms(true)
+ // check periodically every 1 minute, delete prewarmed container if unused for sometime
+ context.system.scheduler.schedule(10.seconds, 1.minute, self, DeleteUnusedPrewarmedContainer)
+
+ // check periodically for the cold start and create some increment containers automatically if activation >= threshold
+ context.system.scheduler.schedule(1.minute, 1.minute, self, SupplementPrewarmedContainer)
+
+ def deleteUnusedPrewarmedContainer(): Unit = {
+ prewarmConfig.foreach { config =>
+ val kind = config.exec.kind
+ val memory = config.memoryLimit
+ val ttlSeconds = config.ttl.toSeconds
+ val containers = prewarmedPool.filter { warmInfo =>
+ warmInfo match {
+ case (_, PreWarmedData(_, `kind`, `memory`, _)) => true
+ case _ => false
+ }
+ }
+ for ((container, data) <- containers) {
+ if (JDuration.between(data.lastUsed, Instant.now).compareTo(JDuration.ofSeconds(ttlSeconds)) > 0) {
+ // Don't recover a new one under this situation
+ container ! RemovePreWarmedContainer
+ }
+ }
+ }
+ }
+
+ def supplementPrewarmedContainer() = {
+ prewarmConfig.foreach { config =>
+ val kind = config.exec.kind
+ val memory = config.memoryLimit
+ val calendar = Calendar.getInstance()
+ calendar.add(Calendar.MINUTE, -1)
+ val lastDate = new SimpleDateFormat("yyyy-MM-dd-HH:mm").format(calendar.getTime)
+ val key = s"${kind},${memory.toMB},${lastDate}"
+ coldStartCount.get(key) match {
Review comment:
Can we use `coldStartCount.remove(key)`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on issue #4871: Adjust prewarm
container dynamically
Posted by GitBox <gi...@apache.org>.
ningyougang commented on issue #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#issuecomment-612709277
Regarding this pr's `system test pipeline`, reported below error
```
TASK [controller : wait until the Controller in this host is up and running] ***
Sunday 12 April 2020 10:02:41 +0000 (0:00:01.132) 0:00:55.659 **********
FAILED - RETRYING: wait until the Controller in this host is up and running (12 retries left).
FAILED - RETRYING: wait until the Controller in this host is up and running (11 retries left).
FAILED - RETRYING: wait until the Controller in this host is up and running (10 retries left).
FAILED - RETRYING: wait until the Controller in this host is up and running (9 retries left).
FAILED - RETRYING: wait until the Controller in this host is up and running (8 retries left).
FAILED - RETRYING: wait until the Controller in this host is up and running (7 retries left).
FAILED - RETRYING: wait until the Controller in this host is up and running (6 retries left).
FAILED - RETRYING: wait until the Controller in this host is up and running (5 retries left).
FAILED - RETRYING: wait until the Controller in this host is up and running (4 retries left).
FAILED - RETRYING: wait until the Controller in this host is up and running (3 retries left).
FAILED - RETRYING: wait until the Controller in this host is up and running (2 retries left).
FAILED - RETRYING: wait until the Controller in this host is up and running (1 retries left).
fatal: [controller0]: FAILED! => {"attempts": 12, "changed": false, "content": "", "msg": "Status code was -1 and not [200]: Request failed: <urlopen error [Errno 111] Connection refused>", "redirected": false, "status": -1, "url": "https://172.17.0.1:10001/ping"}
Status code was -1 and not [200]: Request failed: <urlopen error [Errno
111] Connection refused>
```
But i run in local's severl envs, the controller started successfully.
I don't know why controller start failed in travis-ci, can check the travis-ci's controller logs?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r406679822
##########
File path: ansible/files/runtimes.json
##########
@@ -58,7 +58,8 @@
"stemCells": [
{
"count": 2,
- "memory": "256 MB"
+ "memory": "256 MB",
+ "ttlMinutes": 10
Review comment:
Already changed to `Duration`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r407832098
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -80,8 +86,59 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
//periodically emit metrics (don't need to do this for each message!)
context.system.scheduler.schedule(30.seconds, 10.seconds, self, EmitMetrics)
+ // Key is `kind,memory,time`, value is the number of cold Start in minute
+ var coldStartCount = immutable.Map.empty[String, Int]
+
backfillPrewarms(true)
+ // check periodically every 1 minute, delete prewarmed container if unused for sometime
+ context.system.scheduler.schedule(10.seconds, 1.minute, self, DeleteUnusedPrewarmedContainer)
+
+ // check periodically for the cold start and create some increment containers automatically if activation >= threshold
+ context.system.scheduler.schedule(1.minute, 1.minute, self, SupplementPrewarmedContainer)
+
+ def deleteUnusedPrewarmedContainer(): Unit = {
+ prewarmConfig.foreach { config =>
+ val kind = config.exec.kind
+ val memory = config.memoryLimit
+ val ttlSeconds = config.ttl.toSeconds
+ val containers = prewarmedPool.filter { warmInfo =>
+ warmInfo match {
+ case (_, PreWarmedData(_, `kind`, `memory`, _)) => true
+ case _ => false
+ }
+ }
+ for ((container, data) <- containers) {
+ if (JDuration.between(data.lastUsed, Instant.now).compareTo(JDuration.ofSeconds(ttlSeconds)) > 0) {
+ // Don't recover a new one under this situation
+ container ! RemovePreWarmedContainer
+ }
+ }
+ }
+ }
+
+ def supplementPrewarmedContainer() = {
+ prewarmConfig.foreach { config =>
+ val kind = config.exec.kind
+ val memory = config.memoryLimit
+ val calendar = Calendar.getInstance()
+ calendar.add(Calendar.MINUTE, -1)
+ val lastDate = new SimpleDateFormat("yyyy-MM-dd-HH:mm").format(calendar.getTime)
+ val key = s"${kind},${memory.toMB},${lastDate}"
Review comment:
ok
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] style95 commented on a change in pull request #4871:
Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
style95 commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r407479611
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -313,12 +391,12 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
}
val startingCount = prewarmStartingPool.count(p => p._2._1 == kind && p._2._2 == memory)
val containerCount = currentCount + startingCount
- if (containerCount < config.count) {
+ if (containerCount < config.initialCount) {
Review comment:
`config.initialCount` is being used multiple times.
How about introducing a local variable for this?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] tysonnorris commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
tysonnorris commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r405849373
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -80,8 +85,49 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
//periodically emit metrics (don't need to do this for each message!)
context.system.scheduler.schedule(30.seconds, 10.seconds, self, EmitMetrics)
+ // Key is `kind,memory,time`, value is the number of cold Start in minute
+ val coldStartCount = TrieMap[String, AtomicInteger]()
Review comment:
Actually, I would probably not create a separate map here, but rather add `expiration:Option[Deadline]` to `PreWarmedData` class. Then you can track expiration AND more easily filter to find expired prewarms.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r402677602
##########
File path: ansible/files/runtimes.json
##########
@@ -58,7 +58,8 @@
"stemCells": [
{
"count": 2,
- "memory": "256 MB"
+ "memory": "256 MB",
+ "ttlMinutes": 10
Review comment:
It seems unit of second is very small, and the unit of hour is too big.
so i choosed the unit of minute (second < minute < hour).
Does it make sense if we use unit of second?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r406678421
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -82,6 +85,27 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
backfillPrewarms(true)
+ // check periodically every 1 minute, delete prewarmed container if unused for sometime
+ context.system.scheduler.schedule(10.seconds, 1.minute) {
+ prewarmConfig.foreach { config =>
+ val kind = config.exec.kind
+ val memory = config.memoryLimit
+ val ttlMinutes = config.ttlMinutes
+ val containers = prewarmedPool.filter { warmInfo =>
+ warmInfo match {
+ case (_, PreWarmedData(_, `kind`, `memory`, _)) => true
+ case _ => false
+ }
+ }
+ for ((container, data) <- containers) {
+ if (Duration.between(data.lastUsed, Instant.now).compareTo(Duration.ofMinutes(ttlMinutes)) > 0) {
Review comment:
@style95 already fixed
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] tysonnorris commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
tysonnorris commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r407857906
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -80,8 +86,59 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
//periodically emit metrics (don't need to do this for each message!)
context.system.scheduler.schedule(30.seconds, 10.seconds, self, EmitMetrics)
+ // Key is `kind,memory,time`, value is the number of cold Start in minute
+ var coldStartCount = immutable.Map.empty[String, Int]
+
backfillPrewarms(true)
+ // check periodically every 1 minute, delete prewarmed container if unused for sometime
+ context.system.scheduler.schedule(10.seconds, 1.minute, self, DeleteUnusedPrewarmedContainer)
Review comment:
For future work, I would also consider:
- Does this invoker support only blackbox? If so, it can safely scale down to 0 prewarms, since blackbox are never prewarmed.
- Can new prewarm configs be completely inferred? So that if lots of actions of a particular config arrive, they eventually get benefits of having prewarms.
- Can blackbox then be allowed to have prewarms, if they receive enough traffic at this invoker?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] style95 commented on a change in pull request #4871:
Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
style95 commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r408085296
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -80,8 +86,59 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
//periodically emit metrics (don't need to do this for each message!)
context.system.scheduler.schedule(30.seconds, 10.seconds, self, EmitMetrics)
+ // Key is `kind,memory,time`, value is the number of cold Start in minute
+ var coldStartCount = immutable.Map.empty[String, Int]
Review comment:
oh ok I thought it was being accessed by a scheduler and actor thread at the same time.
But it was not.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r406678503
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -340,6 +401,41 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
newContainer ! Start(exec, memoryLimit)
}
+ /** Create a new prewarm container if currentCount doesn't reach maximum */
+ def prewarmContainerIfpossible(kind: String, memoryLimit: ByteSize, count: Int): Unit = {
+ prewarmConfig
+ .filter { config =>
+ kind == config.exec.kind && memoryLimit == config.memoryLimit
+ }
+ .foreach { config =>
+ val currentCount = prewarmedPool.count {
+ case (_, PreWarmedData(_, `kind`, `memoryLimit`, _)) => true //done starting
+ case _ => false //started but not finished starting
+ }
+ val startingCount = prewarmStartingPool.count(p => p._2._1 == kind && p._2._2 == memoryLimit)
+ val containerCount = currentCount + startingCount
+ if (containerCount < config.count) {
+ val createNumber = if (config.count - containerCount > count) count else config.count - containerCount
+ 1 to createNumber foreach { _ =>
+ prewarmContainer(config.exec, config.memoryLimit)
+ }
+ }
+ }
+ }
+
+ /** statistics the cold start */
+ def countColdStart(kind: String, memoryLimit: ByteSize): Unit = {
+ prewarmConfig
+ .filter { config =>
+ kind == config.exec.kind && memoryLimit == config.memoryLimit
+ }
+ .foreach { _ =>
+ val time = new SimpleDateFormat("yyyy-MM-dd-HH:mm").format(Calendar.getInstance().getTime)
+ val key = s"${kind},${memoryLimit.toMB},${time}"
+ coldStartCount.getOrElseUpdate(key, new AtomicInteger(0)).getAndIncrement()
Review comment:
Already fixed
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r406674372
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -80,8 +85,49 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
//periodically emit metrics (don't need to do this for each message!)
context.system.scheduler.schedule(30.seconds, 10.seconds, self, EmitMetrics)
+ // Key is `kind,memory,time`, value is the number of cold Start in minute
+ val coldStartCount = TrieMap[String, AtomicInteger]()
Review comment:
Already fixed
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] tysonnorris commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
tysonnorris commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r407855935
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -80,8 +86,59 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
//periodically emit metrics (don't need to do this for each message!)
context.system.scheduler.schedule(30.seconds, 10.seconds, self, EmitMetrics)
+ // Key is `kind,memory,time`, value is the number of cold Start in minute
+ var coldStartCount = immutable.Map.empty[String, Int]
+
backfillPrewarms(true)
+ // check periodically every 1 minute, delete prewarmed container if unused for sometime
+ context.system.scheduler.schedule(10.seconds, 1.minute, self, DeleteUnusedPrewarmedContainer)
+
+ // check periodically for the cold start and create some increment containers automatically if activation >= threshold
+ context.system.scheduler.schedule(1.minute, 1.minute, self, SupplementPrewarmedContainer)
+
+ def deleteUnusedPrewarmedContainer(): Unit = {
+ prewarmConfig.foreach { config =>
+ val kind = config.exec.kind
+ val memory = config.memoryLimit
+ val ttlSeconds = config.ttl.toSeconds
+ val containers = prewarmedPool.filter { warmInfo =>
+ warmInfo match {
+ case (_, PreWarmedData(_, `kind`, `memory`, _)) => true
+ case _ => false
+ }
+ }
+ for ((container, data) <- containers) {
+ if (JDuration.between(data.lastUsed, Instant.now).compareTo(JDuration.ofSeconds(ttlSeconds)) > 0) {
+ // Don't recover a new one under this situation
+ container ! RemovePreWarmedContainer
Review comment:
Can you just use Remove message here?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r402680381
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -82,6 +85,27 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
backfillPrewarms(true)
+ // check periodically every 1 minute, delete prewarmed container if unused for sometime
+ context.system.scheduler.schedule(10.seconds, 1.minute) {
+ prewarmConfig.foreach { config =>
+ val kind = config.exec.kind
+ val memory = config.memoryLimit
+ val ttlMinutes = config.ttlMinutes
+ val containers = prewarmedPool.filter { warmInfo =>
+ warmInfo match {
+ case (_, PreWarmedData(_, `kind`, `memory`, _)) => true
+ case _ => false
+ }
+ }
+ for ((container, data) <- containers) {
+ if (Duration.between(data.lastUsed, Instant.now).compareTo(Duration.ofMinutes(ttlMinutes)) > 0) {
Review comment:
Yes, agree with you.
I introduced a new mechanism to avoid `the cold start`
added `threshold`, `increment` in `runtime.json`
after prewarmed containers are deleted due to unused.
if action for that kind/memory's activation number >= `threshold` in last one minute,
create `increment` prewarmed containers.
refer to the new added commit: https://github.com/apache/openwhisk/pull/4871/commits/8c36f33b2c2eddd266fcd0418b21da3078639ba5
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r407830255
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -80,8 +86,59 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
//periodically emit metrics (don't need to do this for each message!)
context.system.scheduler.schedule(30.seconds, 10.seconds, self, EmitMetrics)
+ // Key is `kind,memory,time`, value is the number of cold Start in minute
+ var coldStartCount = immutable.Map.empty[String, Int]
+
backfillPrewarms(true)
+ // check periodically every 1 minute, delete prewarmed container if unused for sometime
+ context.system.scheduler.schedule(10.seconds, 1.minute, self, DeleteUnusedPrewarmedContainer)
Review comment:
Regarding `So how about introducing a minimum limit as well?`
You mean add `minCount` in runtime.json
Regarding `We can configure the minimum number of prewarm containers while controlling the ratio of them based on the usage.`
What's mean? the minimum numbe can be changed on some cases?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] tysonnorris commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
tysonnorris commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r405846826
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -80,8 +85,49 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
//periodically emit metrics (don't need to do this for each message!)
context.system.scheduler.schedule(30.seconds, 10.seconds, self, EmitMetrics)
+ // Key is `kind,memory,time`, value is the number of cold Start in minute
+ val coldStartCount = TrieMap[String, AtomicInteger]()
Review comment:
this can be a `var immutable.Map` instead, i think?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] tysonnorris commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
tysonnorris commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r407856103
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerProxy.scala
##########
@@ -375,6 +377,8 @@ class ContainerProxy(factory: (TransactionId,
case Event(Remove, data: PreWarmedData) => destroyContainer(data)
+ case Event(RemovePreWarmedContainer, data: PreWarmedData) => destoryPreWarmedContainer(data)
Review comment:
I think using Remove message and existing destroyContainer() function is simpler?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on issue #4871: Adjust prewarm
container dynamically
Posted by GitBox <gi...@apache.org>.
ningyougang commented on issue #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#issuecomment-612781434
@style95 @tysonnorris
Have any other review comments?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] style95 commented on a change in pull request #4871:
Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
style95 commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r407477307
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -80,8 +86,59 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
//periodically emit metrics (don't need to do this for each message!)
context.system.scheduler.schedule(30.seconds, 10.seconds, self, EmitMetrics)
+ // Key is `kind,memory,time`, value is the number of cold Start in minute
+ var coldStartCount = immutable.Map.empty[String, Int]
+
backfillPrewarms(true)
+ // check periodically every 1 minute, delete prewarmed container if unused for sometime
+ context.system.scheduler.schedule(10.seconds, 1.minute, self, DeleteUnusedPrewarmedContainer)
Review comment:
So the main idea is removing unused prewarm containers every 1 minute and create more prewarm containers based on the usage during the last 1 minute.
I think it is a good starting point.
This is a kind of heuristic approach expecting the future based on history
One thing to consider is, we can't exactly expect the required prewarm types.
Any kind of activation can come at any time.
If an action is sparsely invoked or invoked with a bigger interval than prewarm TTL, it will always trigger a cold-start.
So how about introducing a minimum limit as well?
We can configure the minimum number of prewarm containers while controlling the ratio of them based on the usage.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang opened a new pull request #4871: Adjust
prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
ningyougang opened a new pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871
<!--- Provide a concise summary of your changes in the Title -->
## Description
<!--- Provide a detailed description of your changes. -->
<!--- Include details of what problem you are solving and how your changes are tested. -->
As we already know, when invoker starts up, will create some prewarmed containers in prewarmedpool in advance.
If the prewarmed containers are not used for sometime, the prewarmed containers will exist forever.
it may lead to `waste of resources`
so there has need to provide some mechanism to save memory resources, e.g.
* If the prewarmed containers are not used for sometime, should delete them automatically.
* When cold start happened, we should supplement a prewarmed container which matched the kind/memory in runtime.
Node: Current, i didn't add test cases, after some reviews, i will add it.
## Related issue and scope
<!--- Please include a link to a related issue if there is one. -->
https://github.com/apache/openwhisk/issues/4725#issuecomment-572824267
## My changes affect the following components
<!--- Select below all system components are affected by your change. -->
<!--- Enter an `x` in all applicable boxes. -->
- [ ] API
- [ ] Controller
- [ ] Message Bus (e.g., Kafka)
- [ ] Loadbalancer
- [x] Invoker
- [ ] Intrinsic actions (e.g., sequences, conductors)
- [ ] Data stores (e.g., CouchDB)
- [x] Tests
- [ ] Deployment
- [ ] CLI
- [ ] General tooling
- [ ] Documentation
## Types of changes
<!--- What types of changes does your code introduce? Use `x` in all the boxes that apply: -->
- [ ] Bug fix (generally a non-breaking change which closes an issue).
- [x] Enhancement or new feature (adds new functionality).
- [ ] Breaking change (a bug fix or enhancement which changes existing behavior).
## Checklist:
<!--- Please review the points below which help you make sure you've covered all aspects of the change you're making. -->
- [x] I signed an [Apache CLA](https://github.com/apache/openwhisk/blob/master/CONTRIBUTING.md).
- [x] I reviewed the [style guides](https://github.com/apache/openwhisk/wiki/Contributing:-Git-guidelines#code-readiness) and followed the recommendations (Travis CI will check :).
- [x] I added tests to cover my changes.
- [ ] My changes require further changes to the documentation.
- [ ] I updated the documentation where necessary.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] tysonnorris commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
tysonnorris commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r405846072
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -340,6 +401,41 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
newContainer ! Start(exec, memoryLimit)
}
+ /** Create a new prewarm container if currentCount doesn't reach maximum */
+ def prewarmContainerIfpossible(kind: String, memoryLimit: ByteSize, count: Int): Unit = {
+ prewarmConfig
+ .filter { config =>
+ kind == config.exec.kind && memoryLimit == config.memoryLimit
+ }
+ .foreach { config =>
+ val currentCount = prewarmedPool.count {
+ case (_, PreWarmedData(_, `kind`, `memoryLimit`, _)) => true //done starting
+ case _ => false //started but not finished starting
+ }
+ val startingCount = prewarmStartingPool.count(p => p._2._1 == kind && p._2._2 == memoryLimit)
+ val containerCount = currentCount + startingCount
+ if (containerCount < config.count) {
+ val createNumber = if (config.count - containerCount > count) count else config.count - containerCount
+ 1 to createNumber foreach { _ =>
+ prewarmContainer(config.exec, config.memoryLimit)
+ }
+ }
+ }
+ }
+
+ /** statistics the cold start */
+ def countColdStart(kind: String, memoryLimit: ByteSize): Unit = {
+ prewarmConfig
+ .filter { config =>
+ kind == config.exec.kind && memoryLimit == config.memoryLimit
+ }
+ .foreach { _ =>
+ val time = new SimpleDateFormat("yyyy-MM-dd-HH:mm").format(Calendar.getInstance().getTime)
+ val key = s"${kind},${memoryLimit.toMB},${time}"
+ coldStartCount.getOrElseUpdate(key, new AtomicInteger(0)).getAndIncrement()
Review comment:
Does this need AtomicInteger? Container allocation should happen in actor message processing, which is single threaded
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] style95 commented on a change in pull request #4871:
Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
style95 commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r407479057
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -271,6 +336,13 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
//backfill prewarms on every ContainerRemoved, just in case
backfillPrewarms(false) //in case a prewarm is removed due to health failure or crash
+ // prewarmed container got removed
+ case PreWarmedContainerRemoved =>
+ prewarmedPool.get(sender()).foreach { _ =>
+ logging.info(this, "prewarmed container is deleted due to unused for long time")
Review comment:
Would be great to include the type of prewarm container.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] tysonnorris commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
tysonnorris commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r405850071
##########
File path: ansible/files/runtimes.json
##########
@@ -58,7 +58,8 @@
"stemCells": [
{
"count": 2,
- "memory": "256 MB"
+ "memory": "256 MB",
+ "ttlMinutes": 10
Review comment:
Why not FiniteDuration? So that operators can make the value whatever is appropriate for them.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] tysonnorris commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
tysonnorris commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r407850871
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -80,8 +86,59 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
//periodically emit metrics (don't need to do this for each message!)
context.system.scheduler.schedule(30.seconds, 10.seconds, self, EmitMetrics)
+ // Key is `kind,memory,time`, value is the number of cold Start in minute
+ var coldStartCount = immutable.Map.empty[String, Int]
+
backfillPrewarms(true)
+ // check periodically every 1 minute, delete prewarmed container if unused for sometime
+ context.system.scheduler.schedule(10.seconds, 1.minute, self, DeleteUnusedPrewarmedContainer)
+
+ // check periodically for the cold start and create some increment containers automatically if activation >= threshold
+ context.system.scheduler.schedule(1.minute, 1.minute, self, SupplementPrewarmedContainer)
+
+ def deleteUnusedPrewarmedContainer(): Unit = {
+ prewarmConfig.foreach { config =>
+ val kind = config.exec.kind
+ val memory = config.memoryLimit
+ val ttlSeconds = config.ttl.toSeconds
+ val containers = prewarmedPool.filter { warmInfo =>
+ warmInfo match {
+ case (_, PreWarmedData(_, `kind`, `memory`, _)) => true
+ case _ => false
+ }
+ }
+ for ((container, data) <- containers) {
+ if (JDuration.between(data.lastUsed, Instant.now).compareTo(JDuration.ofSeconds(ttlSeconds)) > 0) {
+ // Don't recover a new one under this situation
+ container ! RemovePreWarmedContainer
+ }
+ }
+ }
+ }
+
+ def supplementPrewarmedContainer() = {
+ prewarmConfig.foreach { config =>
+ val kind = config.exec.kind
+ val memory = config.memoryLimit
+ val calendar = Calendar.getInstance()
+ calendar.add(Calendar.MINUTE, -1)
+ val lastDate = new SimpleDateFormat("yyyy-MM-dd-HH:mm").format(calendar.getTime)
+ val key = s"${kind},${memory.toMB},${lastDate}"
+ coldStartCount.get(key) match {
Review comment:
+1 for immutable.Map
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] tysonnorris commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
tysonnorris commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r407850743
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -80,8 +86,59 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
//periodically emit metrics (don't need to do this for each message!)
context.system.scheduler.schedule(30.seconds, 10.seconds, self, EmitMetrics)
+ // Key is `kind,memory,time`, value is the number of cold Start in minute
+ var coldStartCount = immutable.Map.empty[String, Int]
+
backfillPrewarms(true)
+ // check periodically every 1 minute, delete prewarmed container if unused for sometime
+ context.system.scheduler.schedule(10.seconds, 1.minute, self, DeleteUnusedPrewarmedContainer)
Review comment:
It is true that without a minimum, the prewarms may all go to zero, which is what you need if you have blackbox dedicated invokers if you don't want any prewarms running there.
So the stemcell config could have:
initialCount : this should probably be whatever the value is used today for number of prewarms of that config.
minCount : 0 is required, if you want to prevent case where unused prewarms are wasting resources, but it implies that you will get a cold start for that config on that invoker.
maxCount : this is an upper limit on max number of prewarms for that config.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] tysonnorris commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
tysonnorris commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r405854330
##########
File path: ansible/files/runtimes.json
##########
@@ -58,7 +58,10 @@
"stemCells": [
{
"count": 2,
- "memory": "256 MB"
+ "memory": "256 MB",
+ "ttlMinutes": 10,
+ "threshold": 5,
+ "increment": 1
Review comment:
We could consider renaming "count" to "initialCount", but I'm not sure this is an issue for backwards compatibility.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang closed pull request #4871: Adjust prewarm
container dynamically
Posted by GitBox <gi...@apache.org>.
ningyougang closed pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r402680381
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -82,6 +85,27 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
backfillPrewarms(true)
+ // check periodically every 1 minute, delete prewarmed container if unused for sometime
+ context.system.scheduler.schedule(10.seconds, 1.minute) {
+ prewarmConfig.foreach { config =>
+ val kind = config.exec.kind
+ val memory = config.memoryLimit
+ val ttlMinutes = config.ttlMinutes
+ val containers = prewarmedPool.filter { warmInfo =>
+ warmInfo match {
+ case (_, PreWarmedData(_, `kind`, `memory`, _)) => true
+ case _ => false
+ }
+ }
+ for ((container, data) <- containers) {
+ if (Duration.between(data.lastUsed, Instant.now).compareTo(Duration.ofMinutes(ttlMinutes)) > 0) {
Review comment:
Yes, agree with you.
I introduced a new mechanism to `avoid the cold start`
added `threshold`, `increment` in `runtime.json`
after prewarmed containers are deleted due to unused.
if action for that kind/memory's activation number >= `threshold` in last one minute,
create `increment` prewarmed containers.
refer to the new added commit: https://github.com/apache/openwhisk/pull/4871/commits/8c36f33b2c2eddd266fcd0418b21da3078639ba5
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] style95 commented on a change in pull request #4871:
Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
style95 commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r407478173
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -80,8 +86,59 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
//periodically emit metrics (don't need to do this for each message!)
context.system.scheduler.schedule(30.seconds, 10.seconds, self, EmitMetrics)
+ // Key is `kind,memory,time`, value is the number of cold Start in minute
+ var coldStartCount = immutable.Map.empty[String, Int]
+
backfillPrewarms(true)
+ // check periodically every 1 minute, delete prewarmed container if unused for sometime
+ context.system.scheduler.schedule(10.seconds, 1.minute, self, DeleteUnusedPrewarmedContainer)
+
+ // check periodically for the cold start and create some increment containers automatically if activation >= threshold
+ context.system.scheduler.schedule(1.minute, 1.minute, self, SupplementPrewarmedContainer)
+
+ def deleteUnusedPrewarmedContainer(): Unit = {
+ prewarmConfig.foreach { config =>
+ val kind = config.exec.kind
+ val memory = config.memoryLimit
+ val ttlSeconds = config.ttl.toSeconds
+ val containers = prewarmedPool.filter { warmInfo =>
+ warmInfo match {
+ case (_, PreWarmedData(_, `kind`, `memory`, _)) => true
+ case _ => false
+ }
+ }
+ for ((container, data) <- containers) {
+ if (JDuration.between(data.lastUsed, Instant.now).compareTo(JDuration.ofSeconds(ttlSeconds)) > 0) {
+ // Don't recover a new one under this situation
+ container ! RemovePreWarmedContainer
+ }
+ }
+ }
+ }
+
+ def supplementPrewarmedContainer() = {
+ prewarmConfig.foreach { config =>
+ val kind = config.exec.kind
+ val memory = config.memoryLimit
+ val calendar = Calendar.getInstance()
+ calendar.add(Calendar.MINUTE, -1)
+ val lastDate = new SimpleDateFormat("yyyy-MM-dd-HH:mm").format(calendar.getTime)
+ val key = s"${kind},${memory.toMB},${lastDate}"
Review comment:
I think it would be better to use a typed key such as a case class rather than a String key with multiple fields.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] style95 commented on a change in pull request #4871:
Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
style95 commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r407479823
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -340,6 +418,47 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
newContainer ! Start(exec, memoryLimit)
}
+ /** Create a new prewarm container if currentCount doesn't reach maxCount */
+ def prewarmContainerIfpossible(kind: String, memoryLimit: ByteSize, count: Int): Unit = {
+ prewarmConfig
+ .filter { config =>
+ kind == config.exec.kind && memoryLimit == config.memoryLimit
+ }
+ .foreach { config =>
+ val currentCount = prewarmedPool.count {
+ case (_, PreWarmedData(_, `kind`, `memoryLimit`, _)) => true //done starting
+ case _ => false //started but not finished starting
+ }
+ val startingCount = prewarmStartingPool.count(p => p._2._1 == kind && p._2._2 == memoryLimit)
+ val containerCount = currentCount + startingCount
+ if (containerCount < config.maxCount) {
+ val createNumber = if (config.maxCount - containerCount > count) count else config.maxCount - containerCount
+ 1 to createNumber foreach { _ =>
+ prewarmContainer(config.exec, config.memoryLimit)
+ }
+ }
+ }
+ }
+
+ /** statistics the cold start */
+ def countColdStart(kind: String, memoryLimit: ByteSize): Unit = {
+ prewarmConfig
+ .filter { config =>
+ kind == config.exec.kind && memoryLimit == config.memoryLimit
+ }
+ .foreach { _ =>
+ val time = new SimpleDateFormat("yyyy-MM-dd-HH:mm").format(Calendar.getInstance().getTime)
+ val key = s"${kind},${memoryLimit.toMB},${time}"
+ coldStartCount.get(key) match {
+ case Some(value) => coldStartCount = coldStartCount + (key -> (value + 1))
+ case None => coldStartCount = coldStartCount + (key -> 1)
+ }
+ for ((k, v) <- coldStartCount) {
+ logging.info(this, s"===statistics the cold start, k: ${k}, v: ${v}")
Review comment:
Removing `===`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] style95 commented on a change in pull request #4871:
Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
style95 commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r407480681
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -340,6 +418,47 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
newContainer ! Start(exec, memoryLimit)
}
+ /** Create a new prewarm container if currentCount doesn't reach maxCount */
+ def prewarmContainerIfpossible(kind: String, memoryLimit: ByteSize, count: Int): Unit = {
+ prewarmConfig
+ .filter { config =>
+ kind == config.exec.kind && memoryLimit == config.memoryLimit
+ }
+ .foreach { config =>
+ val currentCount = prewarmedPool.count {
+ case (_, PreWarmedData(_, `kind`, `memoryLimit`, _)) => true //done starting
+ case _ => false //started but not finished starting
+ }
+ val startingCount = prewarmStartingPool.count(p => p._2._1 == kind && p._2._2 == memoryLimit)
+ val containerCount = currentCount + startingCount
+ if (containerCount < config.maxCount) {
+ val createNumber = if (config.maxCount - containerCount > count) count else config.maxCount - containerCount
+ 1 to createNumber foreach { _ =>
+ prewarmContainer(config.exec, config.memoryLimit)
+ }
+ }
+ }
+ }
+
+ /** statistics the cold start */
+ def countColdStart(kind: String, memoryLimit: ByteSize): Unit = {
Review comment:
It seems this is to increase a count for the number of cold-starts.
How about renaming this to something more intuitive such as `incrementColdStartCount`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r407832030
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -80,8 +86,59 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
//periodically emit metrics (don't need to do this for each message!)
context.system.scheduler.schedule(30.seconds, 10.seconds, self, EmitMetrics)
+ // Key is `kind,memory,time`, value is the number of cold Start in minute
+ var coldStartCount = immutable.Map.empty[String, Int]
Review comment:
Oh, really, i think `coldStartCount` can only be accessed in single thread.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [openwhisk] ningyougang commented on a change in pull request
#4871: Adjust prewarm container dynamically
Posted by GitBox <gi...@apache.org>.
ningyougang commented on a change in pull request #4871: Adjust prewarm container dynamically
URL: https://github.com/apache/openwhisk/pull/4871#discussion_r402680381
##########
File path: core/invoker/src/main/scala/org/apache/openwhisk/core/containerpool/ContainerPool.scala
##########
@@ -82,6 +85,27 @@ class ContainerPool(childFactory: ActorRefFactory => ActorRef,
backfillPrewarms(true)
+ // check periodically every 1 minute, delete prewarmed container if unused for sometime
+ context.system.scheduler.schedule(10.seconds, 1.minute) {
+ prewarmConfig.foreach { config =>
+ val kind = config.exec.kind
+ val memory = config.memoryLimit
+ val ttlMinutes = config.ttlMinutes
+ val containers = prewarmedPool.filter { warmInfo =>
+ warmInfo match {
+ case (_, PreWarmedData(_, `kind`, `memory`, _)) => true
+ case _ => false
+ }
+ }
+ for ((container, data) <- containers) {
+ if (Duration.between(data.lastUsed, Instant.now).compareTo(Duration.ofMinutes(ttlMinutes)) > 0) {
Review comment:
Yes, agree with you.
I am also thinking about it how to avoid `cold start` after ttl time and new activations come.
One option is:
Use `minCount` and `maxCount` together instead of `count`
minCount >= 0, maxCount >=minCount (it is decided by user)
When invoker starts up, creates `maxCount` prewarmed containers.
afer ttl time, can delete some unused containers but need to keep the number of prewarm containers equal with `minCount`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services