-
Notifications
You must be signed in to change notification settings - Fork 38.7k
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
Avoid deep-copying object when possible on kube-apiserver watch path #108252
Avoid deep-copying object when possible on kube-apiserver watch path #108252
Conversation
/hold To address the remaining TODO in the code. |
@p0lyn0mial - FYI |
e2c1ada
to
c1c1661
Compare
I fixed the race that was causing test failures above. |
c1c1661
to
6f14437
Compare
OK - this is now ready for full review. /hold cancel /retest |
Lgtm. Letting @liggitt to take another look. |
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.
/hold
e3f26f0
to
e552789
Compare
e552789
to
e4e0ce8
Compare
@liggitt - PTAL |
/retest |
e4e0ce8
to
d8ddf9a
Compare
/retest |
@liggitt - PTAL when you will have a chance |
staging/src/k8s.io/apiserver/pkg/endpoints/handlers/response.go
Outdated
Show resolved
Hide resolved
d8ddf9a
to
39f141e
Compare
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.
@liggitt - thanks for comments! PTAL
39f141e
to
6573acc
Compare
@liggitt - tests fixed, PTAL |
6573acc
to
779f157
Compare
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.
@liggitt - PTAL
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: liggitt, wojtek-t 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 |
/hold cancel |
Visibly based on #104802 by @sttts
After removing support in kube-apiserver to set selflink kubernetes/enhancements#1164, kube-apiserver is not really modifying the objects that it's getting on the watch path.
As a result, we can optimize the lower layers of kube-apiserver (watchcache) to avoid unnecessary deep-copies of those objects as they are no longer necessary.
/kind cleanup
/priority important-soon
/sig api-machinery