-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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 clearing objects in conversion funcs #24241
Conversation
Functionally a noop, so I don't think we need a changelog entry here. Tested on our biggest clusters as well, and we see no change. cc @aanm |
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. I checked our other conversion functions and they are not doing the clearing either, so this seems sane to me.
cc @aanm
cc: @marseel , you'll be interested on this kubernetes/kubernetes#115658 |
This removes the behavior of mutating the objects received from the client-go library. To begin with there isn't really any benefit from doing so, given we don't store the object afterwards, and it will be ready for gc when it leaves the scope inside client-go. client-go can possibly return the same pointer twice here, to trigger eg. both an object update delta and then a DeletedFinalStateUnknown delta with the same pointer. For more info, see the issue 115658 in the kubernetes/kubernetes repo on github. Signed-off-by: Odin Ugedal <ougedal@palantir.com> Signed-off-by: Odin Ugedal <odin@uged.al>
Rebasing and running tests |
4f7db1d
to
8b6efad
Compare
/test |
Sure looks like this also landed in v1.11 by now. |
This removes the behavior of mutating the objects received from the client-go library. To begin with there isn't really any benefit from doing so, given we don't store the object afterwards, and it will be ready for gc when it leaves the scope inside client-go. client-go can possibly return the same pointer twice here, to trigger eg. both an object update delta and then a DeletedFinalStateUnknown delta with the same pointer.
For more info, see the issue 115658 in the kubernetes/kubernetes repo on github. (avoiding to link directly since it spamms the original issue).
Please ensure your pull request adheres to the following guidelines:
description and a
Fixes: #XXX
line if the commit addresses a particularGitHub issue.
Fixes: <commit-id>
tag, thenplease add the commit author[s] as reviewer[s] to this issue.