Skip to content
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

Improve memory usage of Istio manager #852

Merged
merged 11 commits into from
Jun 3, 2024

Conversation

barchw
Copy link
Contributor

@barchw barchw commented May 29, 2024

Description

Changes proposed in this pull request:

  • Remove deepcopy from sidecar restart to decrease memory usage
  • Remove unneeded caches

Related issues

@barchw barchw requested a review from a team as a code owner May 29, 2024 14:12
@kyma-bot kyma-bot added cla: yes Indicates the PR's author has signed the CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 29, 2024
@barchw barchw changed the title Remove deepcopy from sidecar restart Improve memory usage of Istio manager Jun 3, 2024
for _, pod := range podList.Items {
if evaluator.RequiresProxyRestart(pod) {
outputPodsList.Items = append(outputPodsList.Items, *pod.DeepCopy())
outputPodsList.Items = append(outputPodsList.Items, pod)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a micro-optimization since the pod objects are not that big, but I think we will create multiple backing arrays, since the capacity of the slices will increase multiple times. That will lead to multiple arrays being kept in memory until the outputPodList isn't referenced any longer and can be garbage collected.
We could initialize the outputPodsList.Items with a capacity derived from podList.Items length.

@@ -107,6 +109,16 @@ func main() {
HealthProbeBindAddress: flagVar.probeAddr,
LeaderElection: flagVar.enableLeaderElection,
LeaderElectionID: "76223278.kyma-project.io",
Client: client.Options{
Cache: &client.CacheOptions{
DisableFor: []client.Object{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a comment that explains why we disable the cache for this types?

@barchw barchw requested a review from a team as a code owner June 3, 2024 11:31
@kyma-bot kyma-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 3, 2024
@barchw barchw changed the base branch from main to release-1.6 June 3, 2024 11:32
@barchw barchw changed the base branch from release-1.6 to main June 3, 2024 11:32
@kyma-bot kyma-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 3, 2024
@triffer triffer self-requested a review June 3, 2024 12:06
@kyma-bot kyma-bot added the lgtm Looks good to me! label Jun 3, 2024
@kyma-bot kyma-bot merged commit 6582277 into kyma-project:main Jun 3, 2024
24 checks passed
barchw added a commit to barchw/istio that referenced this pull request Jun 4, 2024
* Use distinct path for Istio telemetries

* Remove deepcopy from gathering pods for restart

* Revert unneded

* Remove caching for objects

* Post review

* Use distinct path for Istio telemetries

* Remove deepcopy from gathering pods for restart

* Revert unneded

* Remove caching for objects

* Post review
kyma-bot pushed a commit that referenced this pull request Jun 4, 2024
* Improve memory usage of Istio manager (#852)

* Use distinct path for Istio telemetries

* Remove deepcopy from gathering pods for restart

* Revert unneded

* Remove caching for objects

* Post review

* Use distinct path for Istio telemetries

* Remove deepcopy from gathering pods for restart

* Revert unneded

* Remove caching for objects

* Post review

* Run tests on Gardener live (#851)

* Deprecate kyma provision gardener (#825)

* Deprecate kyma provision gardener

* fixes

* use relative yaml path

* fix yaml path

again

* force k8s version to be a string

* try delaying before requesting kubeconfig

* don't hibernate

* fix varaible name

* remove sleep

* remove unused GARDENER_GARDENLINUX_VERSION

* fix Gardener provisioning (#835)

* Use the default Gardener network addresses (#840)

* Use the default Gardener network addresses

* aws

* [skip ci]

* Don't skip ci

* Dummy

---------

Co-authored-by: Patryk Strugacz <werdes72@users.noreply.github.com>
Co-authored-by: Piotr Halama <piotr.halama@sap.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Indicates the PR's author has signed the CLA. lgtm Looks good to me! size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants