[GitHub] rabbah commented on a change in pull request #3232: Making prewarm kind (and count) configurable

2018-05-17 Thread GitBox
rabbah commented on a change in pull request #3232: Making prewarm kind (and 
count) configurable
URL: 
https://github.com/apache/incubator-openwhisk/pull/3232#discussion_r188986673
 
 

 ##
 File path: 
core/invoker/src/main/scala/whisk/core/containerpool/ContainerPool.scala
 ##
 @@ -204,26 +204,25 @@ class ContainerPool(childFactory: ActorRefFactory => 
ActorRef,
* @param kind the kind you want to invoke
* @return the container iff found
*/
-  def takePrewarmContainer(action: ExecutableWhiskAction): Option[(ActorRef, 
ContainerData)] =
-prewarmConfig.flatMap { config =>
-  val kind = action.exec.kind
-  val memory = action.limits.memory.megabytes.MB
-  prewarmedPool
-.find {
-  case (_, PreWarmedData(_, `kind`, `memory`)) => true
-  case _   => false
-}
-.map {
-  case (ref, data) =>
-// Move the container to the usual pool
-freePool = freePool + (ref -> data)
-prewarmedPool = prewarmedPool - ref
-// Create a new prewarm container
-prewarmContainer(config.exec, config.memoryLimit)
+  def takePrewarmContainer(action: ExecutableWhiskAction): Option[(ActorRef, 
ContainerData)] = {
+val kind = action.exec.kind
+val memory = action.limits.memory.megabytes.MB
+prewarmedPool
+  .find {
+case (_, PreWarmedData(_, `kind`, `memory`)) => true
 
 Review comment:
   @tysonnorris with the changes in 
https://github.com/apache/incubator-openwhisk/pull/3669 we can add an 
additional property to the stem cell configuration, _the slop factor_ to allow 
for more sizes to match a particular cell.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rabbah commented on a change in pull request #3232: Making prewarm kind (and count) configurable

2018-05-16 Thread GitBox
rabbah commented on a change in pull request #3232: Making prewarm kind (and 
count) configurable
URL: 
https://github.com/apache/incubator-openwhisk/pull/3232#discussion_r188840143
 
 

 ##
 File path: 
core/invoker/src/main/scala/whisk/core/containerpool/ContainerPool.scala
 ##
 @@ -204,26 +204,25 @@ class ContainerPool(childFactory: ActorRefFactory => 
ActorRef,
* @param kind the kind you want to invoke
* @return the container iff found
*/
-  def takePrewarmContainer(action: ExecutableWhiskAction): Option[(ActorRef, 
ContainerData)] =
-prewarmConfig.flatMap { config =>
-  val kind = action.exec.kind
-  val memory = action.limits.memory.megabytes.MB
-  prewarmedPool
-.find {
-  case (_, PreWarmedData(_, `kind`, `memory`)) => true
-  case _   => false
-}
-.map {
-  case (ref, data) =>
-// Move the container to the usual pool
-freePool = freePool + (ref -> data)
-prewarmedPool = prewarmedPool - ref
-// Create a new prewarm container
-prewarmContainer(config.exec, config.memoryLimit)
+  def takePrewarmContainer(action: ExecutableWhiskAction): Option[(ActorRef, 
ContainerData)] = {
+val kind = action.exec.kind
+val memory = action.limits.memory.megabytes.MB
+prewarmedPool
+  .find {
+case (_, PreWarmedData(_, `kind`, `memory`)) => true
+case _   => false
+  }
+  .map {
+case (ref, data) =>
+  // Move the container to the usual pool
+  freePool = freePool + (ref -> data)
+  prewarmedPool = prewarmedPool - ref
+  // Create a new prewarm container
+  prewarmContainer(action.exec, memory)
 
 Review comment:
   i don't like the use of action.exec here - it leaves the door open to 
leaking init code.
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rabbah commented on a change in pull request #3232: Making prewarm kind (and count) configurable

2018-05-16 Thread GitBox
rabbah commented on a change in pull request #3232: Making prewarm kind (and 
count) configurable
URL: 
https://github.com/apache/incubator-openwhisk/pull/3232#discussion_r188826249
 
 

 ##
 File path: 
core/invoker/src/main/scala/whisk/core/containerpool/ContainerPool.scala
 ##
 @@ -204,26 +204,25 @@ class ContainerPool(childFactory: ActorRefFactory => 
ActorRef,
* @param kind the kind you want to invoke
* @return the container iff found
*/
-  def takePrewarmContainer(action: ExecutableWhiskAction): Option[(ActorRef, 
ContainerData)] =
-prewarmConfig.flatMap { config =>
-  val kind = action.exec.kind
-  val memory = action.limits.memory.megabytes.MB
-  prewarmedPool
-.find {
-  case (_, PreWarmedData(_, `kind`, `memory`)) => true
-  case _   => false
-}
-.map {
-  case (ref, data) =>
-// Move the container to the usual pool
-freePool = freePool + (ref -> data)
-prewarmedPool = prewarmedPool - ref
-// Create a new prewarm container
-prewarmContainer(config.exec, config.memoryLimit)
+  def takePrewarmContainer(action: ExecutableWhiskAction): Option[(ActorRef, 
ContainerData)] = {
+val kind = action.exec.kind
+val memory = action.limits.memory.megabytes.MB
+prewarmedPool
+  .find {
+case (_, PreWarmedData(_, `kind`, `memory`)) => true
 
 Review comment:
   @tysonnorris this is semantic preserving - the stem cell today is only used 
if the memory slot matches exactly. I suggest opening a separate issue to 
configure a memory slop factor for the stem cell matching.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rabbah commented on a change in pull request #3232: Making prewarm kind (and count) configurable

2018-05-02 Thread GitBox
rabbah commented on a change in pull request #3232: Making prewarm kind (and 
count) configurable
URL: 
https://github.com/apache/incubator-openwhisk/pull/3232#discussion_r185493627
 
 

 ##
 File path: common/scala/src/main/resources/application.conf
 ##
 @@ -141,11 +141,11 @@ whisk {
 bypass-pull-for-local-images = false
 local-image-prefix = "whisk"
 }
-# prewarm configuration
-prewarm {
-kind = "nodejs:6"
-count = 2
+
+user-events {
+enabled = false
 
 Review comment:
   what are user events?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rabbah commented on a change in pull request #3232: Making prewarm kind (and count) configurable

2018-03-19 Thread GitBox
rabbah commented on a change in pull request #3232: Making prewarm kind (and 
count) configurable
URL: 
https://github.com/apache/incubator-openwhisk/pull/3232#discussion_r171994115
 
 

 ##
 File path: ansible/group_vars/all
 ##
 @@ -36,6 +36,11 @@ whisk:
 #
 runtimesManifest: "{{ runtimes_manifest | default(lookup('file', '{{ 
openwhisk_home }}/ansible/files/runtimes.json') | from_json) }}"
 
+##
+# Prewarm configuration
+prewarmKind: "{{prewarm_kind | default('nodejs:6')}}"
 
 Review comment:
   @markusthoemmes should we include these as examples?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rabbah commented on a change in pull request #3232: Making prewarm kind (and count) configurable

2018-03-02 Thread GitBox
rabbah commented on a change in pull request #3232: Making prewarm kind (and 
count) configurable
URL: 
https://github.com/apache/incubator-openwhisk/pull/3232#discussion_r171994225
 
 

 ##
 File path: common/scala/src/main/resources/application.conf
 ##
 @@ -118,7 +118,11 @@ whisk {
 bypass-pull-for-local-images = false
 local-image-prefix = "whisk"
 }
-
+# prewarm configuration
+prewarm {
 
 Review comment:
   or above under runtimes.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rabbah commented on a change in pull request #3232: Making prewarm kind (and count) configurable

2018-03-02 Thread GitBox
rabbah commented on a change in pull request #3232: Making prewarm kind (and 
count) configurable
URL: 
https://github.com/apache/incubator-openwhisk/pull/3232#discussion_r171994035
 
 

 ##
 File path: ansible/templates/whisk.properties.j2
 ##
 @@ -35,6 +35,9 @@ whisk.api.vanity.subdomain.parts=1
 
 runtimes.manifest={{ runtimesManifest | to_json }}
 
+prewarm.kind={{ prewarmKind | default('nodejs:6') }}
 
 Review comment:
   @markusthoemmes should we eschew these?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rabbah commented on a change in pull request #3232: Making prewarm kind (and count) configurable

2018-03-02 Thread GitBox
rabbah commented on a change in pull request #3232: Making prewarm kind (and 
count) configurable
URL: 
https://github.com/apache/incubator-openwhisk/pull/3232#discussion_r171994178
 
 

 ##
 File path: common/scala/src/main/resources/application.conf
 ##
 @@ -118,7 +118,11 @@ whisk {
 bypass-pull-for-local-images = false
 local-image-prefix = "whisk"
 }
-
+# prewarm configuration
+prewarm {
 
 Review comment:
   i think we should nest these under `invoker`.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rabbah commented on a change in pull request #3232: Making prewarm kind (and count) configurable

2018-03-02 Thread GitBox
rabbah commented on a change in pull request #3232: Making prewarm kind (and 
count) configurable
URL: 
https://github.com/apache/incubator-openwhisk/pull/3232#discussion_r171994115
 
 

 ##
 File path: ansible/group_vars/all
 ##
 @@ -36,6 +36,11 @@ whisk:
 #
 runtimesManifest: "{{ runtimes_manifest | default(lookup('file', '{{ 
openwhisk_home }}/ansible/files/runtimes.json') | from_json) }}"
 
+##
+# Prewarm configuration
+prewarmKind: "{{prewarm_kind | default('nodejs:6')}}"
 
 Review comment:
   @markusthoemmes should we include these as examples?


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rabbah commented on a change in pull request #3232: Making prewarm kind (and count) configurable

2018-01-29 Thread GitBox
rabbah commented on a change in pull request #3232: Making prewarm kind (and 
count) configurable
URL: 
https://github.com/apache/incubator-openwhisk/pull/3232#discussion_r164492994
 
 

 ##
 File path: ansible/group_vars/all
 ##
 @@ -36,6 +36,11 @@ whisk:
 #
 runtimesManifest: "{{ runtimes_manifest | default(lookup('file', '{{ 
openwhisk_home }}/ansible/files/runtimes.json') | from_json) }}"
 
+##
+# Prewarm configuration
+prewarmKind: "{{prewarm_kind | default('nodejs:6')}}"
 
 Review comment:
   we've increasingly moved away from ansible configs to use pure config: refer 
to this doc 
https://github.com/apache/incubator-openwhisk/blob/master/docs/dev/configuration.md
   
   it makes it more convenient in general for kube and mesos based deployments 
downstream.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] rabbah commented on a change in pull request #3232: Making prewarm kind (and count) configurable

2018-01-29 Thread GitBox
rabbah commented on a change in pull request #3232: Making prewarm kind (and 
count) configurable
URL: 
https://github.com/apache/incubator-openwhisk/pull/3232#discussion_r164493108
 
 

 ##
 File path: ansible/templates/whisk.properties.j2
 ##
 @@ -35,6 +35,9 @@ whisk.api.vanity.subdomain.parts=1
 
 runtimes.manifest={{ runtimesManifest | to_json }}
 
+prewarm.kind={{ prewarmKind | default('nodejs:6') }}
 
 Review comment:
   don't think we'll need these until they play into a test which shouldn't be 
the case.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services