Proposal: Support Readiness Healthchecks

[ameowlia]

✨A big thank you to @mariash who has done great work to map out this problem and solution ✨

Problem

With the current implementation of application healthchecks, when the application healthcheck detects that an app instance (AI) is unhealthy, then Diego will stop the AI, delete the AI, and reschedule a new AI.

This is too aggressive from some users. There could be many reasons why a single request could fail but the next passes just fine. Killing a instance after a single failed healthcheck is seen a too aggressive. Additionally, many applications have a warm up period where they are not ready to receive requests until they populate their cache. Cloud Foundry currently cannot receive a signal that the app is running correctly, but not yet ready to serve request.

Proposed Solution Summary

We intend to support readiness healthchecks. (This was requested previously in this issue.) This would be an additional healthcheck that app developers could configure. When the readiness healthcheck passes, the app is marked "ready" and the app will be routable. When the readiness healthcheck fails, the app is marked as "not ready" and its route will be removed from gorouter's route table. This new readiness healthcheck will give users a healthcheck option that is less drastic than the current option.

Architecture overview

This feature will require changes in the following releases

• CF CLI
• Cloud Controller
• Diego
• Routing

1. The cloud controller will store this new data, before passing it onto the BBS as part of the desired LRP.
2. The Diego executor will see these new readiness healthchecks on the desired LRP and will run the healthchecker binary in the app container with configuration provided.
3. When the readiness healthcheck succeeds, the container will be marked as "ready". When the readiness healthcheck fails, the container will be marked as "not ready".
4. When the route emitter gets route information, it will inspect if the AI is ready or not ready. It will emit registration or unregistration messages as appropriate for the gorouter to consume

CC Design

Users will be able to set the healthcheck via the app manifest.

applications:
- name: test-app
  processes:
  - type: web
    health-check-http-endpoint: /health
    health-check-invocation-timeout: 2
    health-check-type: http
    timeout: 80
    readiness-health-check-http-endpoint: /health      # 👈 new property
    readiness-health-check-invocation-timeout: 2       # 👈 new property
    readiness-health-check-type: http                           # 👈 new property

LRP Design

The readiness healthcheck data will be apart of the desired LRP object

"check_definition": {
    "checks": [
      {
        "http_check": {
          "port": 8080,
          "path": "/health",
          "request_timeout_ms": 10000
        },
      }
    ],
    "readiness_checks": [                                               # 👈 new property
      {
        "tcp_check": {         
          "port": 8080,
          "connect_timeout_ms": 10000
        },
      }
    ],
    "log_source": ""
  },

👉 This work is ongoing. All comments and concerns are welcomed from the community. Either add a comment here or reach out in slack in #wg-app-runtime-platform.

[ameowlia]

❓ Why did I open this in cf-deployment when likely nothing will have to happen in this repo?
Because this work will cross many repo boundaries and this seems like the best place to document a large change 😄

[domdom82]

@ameowlia Thanks for this I like it a lot!
So, I wondered what will happen with the original health check then? Will it still stop and reschedule the AI after one failed check? Assume yes, this probably means that users need to map their original health checks as a readiness check now ("I can't accept requests just yet.") and add another logic to bind the regular health check to ("I am technically alive.").

Otherwise the old logic will stay in place and there won't be any real benefit as the app is still deemed unhealthy and is removed / rescheduled as before.

[beyhan]

@ameowlia @mariash thanks for the great proposal. It definitely makes sense but I didn't have time to look into it for feedback yet. I definitely will provide. @ameowlia I think that this should be an RFC like cloudfoundry/community#591 because as you mentioned will have impact on multiple WGs and we need to have technical discussion and decisions as described in RFC.

[ameowlia]

Thanks for this comment @domdom82,

Your understanding of the situation is correct. With this proposal the original health check will stay they same; it will still stop and reschedule the AI after one failed check[1]. We will definitely add docs to make sure the users understand how to use this feature to best take advantage of its capabilities. And we can also send the docs for you to review (once they exist).

[1] We have considered adding some more configuration here. For example, "if my app fails 2 out of 5 times, then it should fail, but not if it fails 1 out of 5 times". However, we decided that was a different track of work and we are not committing to it at this time.

[PlamenDoychev]

Hi folks, i really like the idea we even have a direct stakeholder for this feature. Several years ago we tried to implement something on our own, but now i am really happy to see that there is a community effort in place.

[philippthun]

Would this have an effect on the state of the process stats object?

[ameowlia]

@philippthun - We plan on making a new property on that object (name TBD). So it will not affect the state property. If you have any concerns around this, we would love to hear them!

[beyhan]

To my understanding this will help also for applications with instances which execute CPU intensive tasks from time to time and request processing could be blocked during that time on those instances. Those type of applications configure none http healthchecks types to not get killed because of failed healthchecks. Now with the readiness healthchecks those instances can be taken out from the request dispatching by the GoRouter when they fail the readiness healthcheck because of the CPU intensive task which will improve the overall end user experience. Is my understanding here correct or do I miss something.

Otherwise the proposed change is backwards compatible from CF users' perspective and doesn't look to be an invasive change in CF itself which I really like.

P.S. My previous comment to make this an RFC is still valid :-) and if you decide to go for an RFC I can help if needed.

[JuergenSu]

Just an idea, how about emitting a standard metric having the number of ready instances for an app. this metric could be used in case of autoscaling or manual scaling decisions and even in case of monitoring and alarming as it might happen that you have 10 instances but all of them are overloaded and not ready…

[marcohelmerich]

Basically we were missing that feature for as long as we are using cloudfoundry :-) It would help us in many use-cases from "not-so-cloud-native" applications who need time for cache warming or other data-transformation/warming tasks to avoiding consecutive app crashes in overload situations.

[ameowlia]

Thank you all for your valuable feedback!

Closing this issue in preference of this RFC per @beyhan 's suggestion.