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/03/12 17:00:02 UTC
[GitHub] [openwhisk] selfxp opened a new pull request #4858: Controller
endpoint for checking the controller readiness
selfxp opened a new pull request #4858: Controller endpoint for checking the controller readiness
URL: https://github.com/apache/openwhisk/pull/4858
This PR adds a new endpoint `<controller>/invokers/ready` that returns a `200` https status code when the proportion of healthy vs unhealthy invokers is higher than a predefined value, and `500` otherwise.
This endpoint can be used in a readiness probe and will allow adding the controller to the pool only once "enough" invokers are already available.
## My changes affect the following components
<!--- Select below all system components are affected by your change. -->
<!--- Enter an `x` in all applicable boxes. -->
- [ ] API
- [x] Controller
- [ ] Message Bus (e.g., Kafka)
- [ ] Loadbalancer
- [ ] 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 :).
- [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
#4858: Controller endpoint for checking the controller readiness
Posted by GitBox <gi...@apache.org>.
tysonnorris commented on a change in pull request #4858: Controller endpoint for checking the controller readiness
URL: https://github.com/apache/openwhisk/pull/4858#discussion_r392489810
##########
File path: core/controller/src/main/resources/reference.conf
##########
@@ -33,5 +33,6 @@ whisk {
controller {
protocol: http
interface: "0.0.0.0"
+ readiness-fraction: 50%
Review comment:
If you remove the `.toOption` (see other comment), this default I guess should be 100%?
----------------------------------------------------------------
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] rabbah commented on a change in pull request #4858:
Controller endpoint for checking the controller readiness
Posted by GitBox <gi...@apache.org>.
rabbah commented on a change in pull request #4858: Controller endpoint for checking the controller readiness
URL: https://github.com/apache/openwhisk/pull/4858#discussion_r391793249
##########
File path: core/controller/src/main/scala/org/apache/openwhisk/core/controller/Controller.scala
##########
@@ -151,6 +153,16 @@ class Controller(val instance: ControllerInstanceId,
.invokerHealth()
.map(_.count(_.status == InvokerState.Healthy).toJson)
}
+ } ~ path("ready") {
+ onSuccess(loadBalancer.invokerHealth()) { invokersHealth =>
+ val all = invokersHealth.size
+ val healthy = invokersHealth.count(_.status == InvokerState.Healthy)
+ val ready = Controller.readyState(all, healthy, Controller.readinessThreshold.getOrElse(1))
+ if (ready)
+ complete(s"healthy $healthy/$all")
Review comment:
can you make these a JSON response for consistency with all controller responses?
----------------------------------------------------------------
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
#4858: Controller endpoint for checking the controller readiness
Posted by GitBox <gi...@apache.org>.
tysonnorris commented on a change in pull request #4858: Controller endpoint for checking the controller readiness
URL: https://github.com/apache/openwhisk/pull/4858#discussion_r392468423
##########
File path: core/controller/src/main/scala/org/apache/openwhisk/core/controller/Controller.scala
##########
@@ -44,7 +44,7 @@ import org.apache.openwhisk.http.{BasicHttpService, BasicRasService}
import org.apache.openwhisk.spi.SpiLoader
import scala.concurrent.ExecutionContext.Implicits
-import scala.concurrent.duration.DurationInt
+import scala.concurrent.duration.{DurationInt}
Review comment:
nit - no braces needed 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] tysonnorris commented on a change in pull request
#4858: Controller endpoint for checking the controller readiness
Posted by GitBox <gi...@apache.org>.
tysonnorris commented on a change in pull request #4858: Controller endpoint for checking the controller readiness
URL: https://github.com/apache/openwhisk/pull/4858#discussion_r392489276
##########
File path: core/controller/src/main/scala/org/apache/openwhisk/core/controller/Controller.scala
##########
@@ -172,6 +184,7 @@ object Controller {
protected val protocol = loadConfigOrThrow[String]("whisk.controller.protocol")
protected val interface = loadConfigOrThrow[String]("whisk.controller.interface")
+ protected val readinessThreshold = loadConfig[Double]("whisk.controller.readiness-fraction").toOption
Review comment:
No need for `.toOption` - the default is in the application.conf
----------------------------------------------------------------
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] selfxp merged pull request #4858: Controller endpoint
for checking the controller readiness
Posted by GitBox <gi...@apache.org>.
selfxp merged pull request #4858: Controller endpoint for checking the controller readiness
URL: https://github.com/apache/openwhisk/pull/4858
----------------------------------------------------------------
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] selfxp commented on a change in pull request #4858:
Controller endpoint for checking the controller readiness
Posted by GitBox <gi...@apache.org>.
selfxp commented on a change in pull request #4858: Controller endpoint for checking the controller readiness
URL: https://github.com/apache/openwhisk/pull/4858#discussion_r393437709
##########
File path: core/controller/src/main/scala/org/apache/openwhisk/core/controller/Controller.scala
##########
@@ -151,6 +153,16 @@ class Controller(val instance: ControllerInstanceId,
.invokerHealth()
.map(_.count(_.status == InvokerState.Healthy).toJson)
}
+ } ~ path("ready") {
+ onSuccess(loadBalancer.invokerHealth()) { invokersHealth =>
Review comment:
I was thinking of doing that originally but that implied writing an Exception Handler and though it would not make a lot of sense of doing that for only one endpoint
----------------------------------------------------------------
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
#4858: Controller endpoint for checking the controller readiness
Posted by GitBox <gi...@apache.org>.
tysonnorris commented on a change in pull request #4858: Controller endpoint for checking the controller readiness
URL: https://github.com/apache/openwhisk/pull/4858#discussion_r392478033
##########
File path: core/controller/src/main/scala/org/apache/openwhisk/core/controller/Controller.scala
##########
@@ -151,6 +153,16 @@ class Controller(val instance: ControllerInstanceId,
.invokerHealth()
.map(_.count(_.status == InvokerState.Healthy).toJson)
}
+ } ~ path("ready") {
+ onSuccess(loadBalancer.invokerHealth()) { invokersHealth =>
Review comment:
minor style suggestion (take it or leave it): since non-success is a 500, you might be able to simplify this to:
```
complete {
loadBalancer
.invokerHealth()
.map(
invokersHealth =>
Controller.readyState(
invokersHealth.size,
invokersHealth.count(_.status == InvokerState.Healthy),
Controller.readinessThreshold.getOrElse(1)))
}
```
Then, make Controller.readinessThreshold return a JsObject or throw an exception with the desired error message.
----------------------------------------------------------------
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