knative / serving

Kubernetes-based, scale-to-zero, request-driven compute
https://knative.dev/docs/serving/
Apache License 2.0
5.57k stars 1.16k forks source link

Vertical Pod Autoscaling Beta #2929

Closed josephburnett closed 4 years ago

josephburnett commented 5 years ago

Design Proposal

Background

The default autoscaling strategy for a Knative Revision is concurrency-based, meaning the Autoscaler tries to maintain a target number of requests in-flight on average for all pods. The default concurrency target is 100. Knative also provides a default resource request to Kubernetes of 500 millicpu.

However 1/2 a cpu might not be the right resource request for an application handling 100 requests at a time. The developer can add resource requests to the Revision template, but a more "serverless" solution would be to adjust the resource requests according to the observed resource usage. That's what the Kubernetes Vertical Pod Autoscaler does.

Previous Work

We previously implemented vertical pod autoscaling in Knative by creating an Alpha VPA resource for each Revision. This had a few shortcomings that have been addressed:

  1. Knative needs at least Beta dependencies. VPA has since release a Beta implementation.
  2. Our usage of VPA resources was tightly coupled with the Revision controller. @mattmoor pulled out the autoscaling concerns of Knative Serving into an internal CRD, PodAutoscaler. So we can put the VPA implementation in the autoscaling controller.
  3. We didn't have a way to configure / enable VPA per Revision. We only had cluster-level controls at the time. Since then, I've added support of configuring autoscaling properties through annotations. We can use that to turn VPA on / off and configure in a pass-though manner. This was one of the reason @mattmoor ripped out the Alpha VPA implementation.
  4. We didn't have E2E tests of VPA integration. This was largely because it had to be turned on for the whole cluster, we weren't ready to do. Now we can turn it on per-revision under test.
  5. Minimum recommendations were too large. This was a bug in the VPA implementation which applied a minimum cpu request of 200 millicpu to all containers instead of dividing it among the pod's containers. So with a concurrency of 1 and a Hello World workload, VPA recommendations were too small. They have since fixed that bug and the Knative concurrency target default is much higher (100).

(See Vertical Pod Autoscaling integration with Alpha API)

There is another shortcoming that this design proposal intends to address:

  1. Knative Revisions should be safe to rollback to. So we create a VPA per Revision. (Otherwise, the new code would "taint" the recommendation of the old Revision.) However, that means each Revision is learning from scratch what resource recommendations it should make. We need to carry forward the recommendation from the previous Revision (if the Service mode implies such a hierarchy).

Proposal

I propose that when the autoscaling.knative.dev/vpaEnable: true annotation is present on a PodAutoscaler (inherited from Revision template) and the Revision's Container does not include resource requests, then the PodAutoscaler controller will create a autoscaling.k8s.io/v1beta1 VPA object.

Periodically (every 30 minutes at resync) the PodAutoscaler controller will back-propagate the resource recommendations from the VPA status to the PodAutoscaler status under a new field (in red below) resourceRecommendations.

When the Revision is scaled to zero and all pods are gone, the Revision or PodAutoscaler will update the Deployment's template with the recommended resource requests and limits. This ensures that if the Revision is invoked after a long period of time (> 2 weeks) the pods will start with the resources they left off with, even if the VPA webhook doesn't have confidence to update the requests.

image

When a new Revision is created, iff it is controlled by a Service in RunLatest or Rollout modes, and both Revisions have VPA enabled, the recommendations from the previous VPA will be used for the Revision.

mattmoor commented 5 years ago

This is better than how the VPA works today (AIUI), but I don't think that the PA controller should alter the Deployment (the Revision controller would simply undo it). I think we should surface the recommendation in the PA's status and have the Revision controller incorporate the recommendation into the desired Deployment, which would prompt it to perform the update.

A couple of lingering concerns:

  1. Recommendations don't carry over from one Revision to the next (as a baseline to start from), which could lead to rocky rollouts as the new Revision is recalibrated.
  2. I believe a reason VPA doesn't update the deployment is due to a potential high volume of churn, is the interval of new recommendations configurable?
mattmoor commented 5 years ago

thanks for writing this up, @josephburnett

josephburnett commented 5 years ago

I think we should surface the recommendation in the PA's status and have the Revision controller incorporate the recommendation into the desired Deployment, which would prompt it to perform the update.

Agree.

Recommendations don't carry over from one Revision to the next (as a baseline to start from), which could lead to rocky rollouts as the new Revision is recalibrated.

They will. Maybe you missed this last sentence under the diagram:

When a new Revision is created, iff it is controlled by a Service in RunLatest or Rollout modes, and both Revisions have VPA enabled, the recommendations from the previous VPA will be used for the Revision.

mattmoor commented 5 years ago

(doing some triage with @mdemirhan ) Putting out of scope for "v1" with the guidance that we can chase this, but GAing this "feature" is separate (and perhaps beyond) GAing the core part of serving.

josephburnett commented 5 years ago

I believe a reason VPA doesn't update the deployment is due to a potential high volume of churn, is the interval of new recommendations configurable?

We need to back-propagate the VPA recommendations to the Deployment at lower rate than the recommendations are updated. Just when it will be likely that the VPA webhook will fail / elect-not-to update Pods. That is, after scale-to-zero.

knative-housekeeping-robot commented 4 years ago

Issues go stale after 90 days of inactivity. Mark the issue as fresh by adding the comment /remove-lifecycle stale. Stale issues rot after an additional 30 days of inactivity and eventually close. If this issue is safe to close now please do so by adding the comment /close.

Send feedback to Knative Productivity Slack channel or file an issue in knative/test-infra.

/lifecycle stale

tbarrella commented 4 years ago

/remove-lifecycle stale

knative-housekeeping-robot commented 4 years ago

Issues go stale after 90 days of inactivity. Mark the issue as fresh by adding the comment /remove-lifecycle stale. Stale issues rot after an additional 30 days of inactivity and eventually close. If this issue is safe to close now please do so by adding the comment /close.

Send feedback to Knative Productivity Slack channel or file an issue in knative/test-infra.

/lifecycle stale

knative-housekeeping-robot commented 4 years ago

Stale issues rot after 30 days of inactivity. Mark the issue as fresh by adding the comment /remove-lifecycle rotten. Rotten issues close after an additional 30 days of inactivity. If this issue is safe to close now please do so by adding the comment /close.

Send feedback to Knative Productivity Slack channel or file an issue in knative/test-infra.

/lifecycle rotten

knative-housekeeping-robot commented 4 years ago

Rotten issues close after 30 days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle rotten.

Send feedback to Knative Productivity Slack channel or file an issue in knative/test-infra.

/close

knative-prow-robot commented 4 years ago

@knative-housekeeping-robot: Closing this issue.

In response to [this](https://github.com/knative/serving/issues/2929#issuecomment-630773067): >Rotten issues close after 30 days of inactivity. >Reopen the issue with `/reopen`. >Mark the issue as fresh by adding the comment `/remove-lifecycle rotten`. > >Send feedback to [Knative Productivity Slack channel](https://knative.slack.com/messages/CCSNR4FCH) or file an issue in [knative/test-infra](https://github.com/knative/test-infra/issues/new). > >/close Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
yashbhutwala commented 4 years ago

/reopen /remove-lifecycle rotten

knative-prow-robot commented 4 years ago

@yashbhutwala: You can't reopen an issue/PR unless you authored it or you are a collaborator.

In response to [this](https://github.com/knative/serving/issues/2929#issuecomment-630819020): >/reopen >/remove-lifecycle rotten Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
mdemirhan commented 4 years ago

@vagababov @markusthoemmes is this something Autoscaling WG tracking and want to keep open?

vagababov commented 4 years ago

Currently it's not on our charter.