New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use protobuf for kubectl top #96655
Use protobuf for kubectl top #96655
Conversation
This seems like it would be ok, but we don't set content type anywhere else in kubectl currently. Maybe a topic for the next sig-cli meeting. /cc @soltysh |
kubectl uses normally generic client that doesn't support proto. |
/sig cli |
That's not necessarily true, we're not using generic client any more at least. Most commands create ad-hoc client which has all the necessary capabilities including support for proto, if needed. @serathius what is the benefit for kubectl users behind this change? |
Yeah, it's a good topic to have. |
Switching from JSON to proto allows Metrics Server to save around 30% of CPU and 15% of memory used. By saving on JSON encoding we plan to improve freshness of metrics, from 1 minute to 15 seconds. With this we both improve autoscaling quality and usefulness of |
/cc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note from SIG-CLI meeting:
This change will require a flag option in kubectl and a warning message that in the next release it will be enabled by default. Other than that people is exited about this change.
@soltysh @serathius please let me know if I missed anything.
85c044c
to
655deb8
Compare
655deb8
to
8697d12
Compare
8697d12
to
560394d
Compare
/retest |
2 similar comments
/retest |
/retest |
ping @soltysh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approve
/triage accepted |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: serathius, soltysh The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
What type of PR is this?
/kind feature
What this PR does / why we need it:
Encoding JSON takes around 30% of CPU and 15% of memory used by Metrics Server. Switching to proto will allow large savings in resources needed for MS.