VPA: Allow CPU startup boost unboost mechanism to be quicker/reactive

[maxcao13]

Which component are you using?:

/area vertical-pod-autoscaler

Is your feature request designed to solve a problem? If so describe the problem this feature should solve.:

The current alpha implementation of VPA CPU Startup Boost relies on the updater interval in order to "unboost" values back to the original manifest values. This most of the time results in a delay to when cpu values get unboosted, which is probably not the behaviour that users expect from just reading the enhancement/documentation. I would expect that as soon as the durationSeconds is hit, or that the pod becomes Ready, that the boost would end immediately.

Describe the solution you'd like.:

I want to propose making the cpu unboosting mechanism event driven and reactive, instead of relying on the set updater-interval that is defaulted to 60 seconds. I understand that requires somewhat of a different architecture than currently stands, so this requires careful implementation as to not make these new code paths blow up memory/CPU usage of the Updater.

Thinking to the future, I think there could be value in making the entire updater code reactive to pod recommendations changing as a whole. This would allow VPA updates to happen faster and can allow updates to spread out more over time, and allow the Updater to be able to do work throughout it's lifecycle, instead of sit idly between intervals. But that is out of scope of this issue.

A side note here, if there is not a recommendation existing, then the unboost should go back to the original value, but if there does exist a recommendation, it falls back to that value as is documented in the AEP. However, if we unboost back to the original value, and 1 second later the updater has a run tick which sees a new recommendation, we can technically be wasting an API call. But I think this is a worthy tradeoff.

Describe any alternative solutions you've considered.:

The workaround for this is to just have a really small updater interval, and don't use VPA for applying recommendations at all (updateMode=Off so that we don't throttle the updater), but that's a lot of user intervention for a seemingly simple mechanism.

Additional context.:

None. Open to ideas, or maybe reasons why we don't want to do this.

[omerap12]

We've actually discussed this offline and agree that we want to modernize the VPA components - specifically making the updater more reactive. We are totally supportive of this direction.

However, since this would involve a major end-to-end refactor, I’m not sure if any of us have the capacity to take it on right now.

/cc @adrianmoisey
/triage accepted

[adrianmoisey]

+1 to everything already said

I want to propose making the cpu unboosting mechanism event driven and reactive, instead of relying on the set updater-interval that is defaulted to 60 seconds. I understand that requires somewhat of a different architecture than currently stands, so this requires careful implementation as to not make these new code paths blow up memory/CPU usage of the Updater.

Thinking to the future, I think there could be value in making the entire updater code reactive to pod recommendations changing as a whole. This would allow VPA updates to happen faster and can allow updates to spread out more over time, and allow the Updater to be able to do work throughout it's lifecycle, instead of sit idly between intervals. But that is out of scope of this issue.

Agree to all of this, however, I think most of what Omer and I had spoken about was to make it reactive to the VPA resource.
What you're asking for is making it reactive to the Pod resource. I could be wrong, but I think CPU unboost is the only current event we need to handle from a Pod resource (may be Recreate some time after Eviction fails, in the case of InPlaceOrRecreate is another candidate).

I think what this all means is that it may be possible to neatly add a Pod handler to the current VPA codebase to handle CPU unboost, without the need to refactor everything just yet.

[maxcao13]

Thanks for your thoughts all! I'm glad this is a direction that we want to move towards to modernize the repo.

I think what this all means is that it may be possible to neatly add a Pod handler to the current VPA codebase to handle CPU unboost, without the need to refactor everything just yet.

Yes, I was thinking we could somehow try to introduce this just for Pods for now and just for the CPU unboost portion of the updater, which can serve as an opening in the future to migrate all the components to controller-runtime and create reconcilers and such.

However, since this would involve a major end-to-end refactor, I’m not sure if any of us have the capacity to take it on right now.

I can assign this to myself and have a go at it :-)

/assign

[voelzmo]

Can you elaborate a bit on the impact if we didn't change the behavior? Given what I understood from the original problem statement

This most of the time results in a delay to when cpu values get unboosted, which is probably not the behaviour that users expect from just reading the enhancement/documentation. I would expect that as soon as the durationSeconds is hit, or that the pod becomes Ready, that the boost would end immediately.

Is thinking correct that we're talking about 59 seconds in the worst case here? If you need vpa-updater to react more quickly, you could also reduce the --updater-interval to be less than 60s, right?
This assumes that if you're very time sensitive regarding how long it takes updater to react, you probably would want this for each and every change and not only specifically for the CPU boost/unboost.

I'm asking all this, because the rough sketch in the above post sounds like a pretty big change to how the updater works for a so far not entirely clear benefit.