Skip to content

Optimizations for controller memory usage#907

Merged
mergify[bot] merged 5 commits intocsi-addons:mainfrom
black-dragon74:fix-exc-mem
Dec 2, 2025
Merged

Optimizations for controller memory usage#907
mergify[bot] merged 5 commits intocsi-addons:mainfrom
black-dragon74:fix-exc-mem

Conversation

@black-dragon74
Copy link
Copy Markdown
Member

@black-dragon74 black-dragon74 commented Nov 24, 2025 •
edited
Loading

This PR aims to optimize memory consumption and allocation for csi-addons controller. This goal is achieved mainly by:

  • Disabling cache for resources that we do not need frequently.
  • Setting up shared informers on every resource that we Get/List frequently (VAs / PVs)
  • A new simplified PVC controller without the overheads that came with old way of operation, mainly:
    • Supports and enabled only when schedule-precedence is set to storageclass.
    • Uses deterministic naming for resources to avoid manual housekeeping of stale resources**
    • Ditches the extra field indexers on created resources (side-effect of above)
    • No update operations on PVCs, state-to-reconcile is always calculated with SC as source of truth.
  • A little housekeeping by moving common functions/variables into their own controller/utils package.

Closes: #815

@black-dragon74 black-dragon74 self-assigned this Nov 24, 2025
@black-dragon74 black-dragon74 added the DNM Do Not Merge label Nov 24, 2025
@black-dragon74 black-dragon74 force-pushed the fix-exc-mem branch 2 times, most recently from 76eac37 to ae4b739 Compare November 24, 2025 12:15
@Madhu-1
Copy link
Copy Markdown
Member

Madhu-1 commented Nov 24, 2025

@black-dragon74 can you please share the result of the memory optimization after this change?

@black-dragon74 black-dragon74 removed the DNM Do Not Merge label Dec 1, 2025
@black-dragon74 black-dragon74 force-pushed the fix-exc-mem branch 2 times, most recently from 5c4f7e6 to 58044be Compare December 1, 2025 09:18
@black-dragon74
Copy link
Copy Markdown
Member Author

black-dragon74 commented Dec 1, 2025

Testing

I was able to run initial test with the new PVC reconciler. Created 100 encrypted PVCs with 100 deployments, 1:1 mapped.

Old reconciler stats:

Initial mem: 125M, max memory: 161M
Max CPU: 73m

New reconciler stats:

Initial mem: 48M, max mem: 61M
Max CPU: 32m

The following operations were performed:

- Annotate SC for EKR, with schedule: @weekly
- Update SC anno with schedule: * * * * *
- Watch memory go up and ensure KR succeeds.
- Annotate SC with ann enable: false
- Wait for resources to be GCed
- Monitor memory usage go down (in new reconciler, it is back to initial stats)

Here is a video with the tests.

Rakshith-R
Rakshith-R reviewed Dec 2, 2025
View reviewed changes
Comment thread internal/controller/csiaddons/pvc_new_controller.go Outdated
Rakshith-R
Rakshith-R reviewed Dec 2, 2025
View reviewed changes
Comment thread internal/controller/csiaddons/pvc_new_controller.go Outdated
Madhu-1
Madhu-1 reviewed Dec 2, 2025
View reviewed changes
Comment thread cmd/manager/main.go
Comment thread internal/controller/csiaddons/reclaimspacejob_controller.go
Comment thread internal/controller/utils/annotations.go
Comment thread internal/controller/utils/annotations_test.go
Comment thread internal/controller/utils/predicates.go
Comment thread internal/controller/utils/predicates.go
Comment thread internal/controller/utils/reclaimspace_keyrotation.go
Comment thread internal/controller/utils/reclaimspace_keyrotation_utils.go Outdated
Comment thread internal/controller/utils/reclaimspace_keyrotation.go
@black-dragon74
controller: move commonly used functions to utils package
6a06f44 
This patch moves the variables and functions common across
controllers into its own `utils` package.

The related tests have been moved as well.

Signed-off-by: Niraj Yadav <niryadav@redhat.com>
@black-dragon74
controller: add a new PVC controller - used with storageclass precedence
f69b130 
This patch adds a new controller for PVC objects which differs from
existing controller in the following main ways:

- Only supports annotations on StorageClasses
- Sets us a silent watch on PV objects to optimize lookups.
- Uses deterministic naming for child resources to avoid manual house
  keeping.

Signed-off-by: Niraj Yadav <niryadav@redhat.com>
@black-dragon74
manager: Disable cache for Pods, NS and enabled new PVC controller
1ccf4c1 
when SchedulePrecedence is set to `StorageClass`

Signed-off-by: Niraj Yadav <niryadav@redhat.com>
@black-dragon74
controller: watch for VAs silently to optimize lookups
b7d5de9 
This patch adds a silent watch on VolumeAttachment objects
to avoid unexpected memory consumption due to `List` calls.

It is okay to setup a watch in two places as the Informer is shared
and managed by the Manager.

Signed-off-by: Niraj Yadav <niryadav@redhat.com>
@black-dragon74
Copy link
Copy Markdown
Member Author

black-dragon74 commented Dec 2, 2025 •
edited
Loading

The tracker for shim removal (assigned to me): #918

@black-dragon74
controller: add shim to garbage collect old resources
0b93f39 
This patch adds a shim layer that is responsible for cleaning
up the old resources so that we do not have two CronJobs created
for the same underlying PVC.

Signed-off-by: Niraj Yadav <niryadav@redhat.com>
@black-dragon74 black-dragon74 mentioned this pull request Dec 2, 2025
Closed
@Rakshith-R
Copy link
Copy Markdown
Member

Rakshith-R commented Dec 2, 2025

@Mergifyio queue

@mergify
Copy link
Copy Markdown

mergify Bot commented Dec 2, 2025 •
edited
Loading

Merge Queue Status Beta

✅ The pull request has been merged

This pull request spent 1 hour 28 minutes 39 seconds in the queue, including 1 hour 28 minutes 26 seconds running CI.
The checks were run on draft #919.

Required conditions to merge

@mergify mergify Bot added the queued label Dec 2, 2025
mergify Bot added a commit that referenced this pull request Dec 2, 2025
@mergify
Merge of #907
d3377b4
@mergify mergify Bot mentioned this pull request Dec 2, 2025
Closed
11 tasks
@mergify
Copy link
Copy Markdown

mergify Bot commented Dec 2, 2025 •
edited
Loading

queue

✅ The pull request has been merged automatically

Details

The pull request has been merged automatically at 40a8a6c

@mergify mergify Bot merged commit 40a8a6c into csi-addons:main Dec 2, 2025
15 checks passed
@mergify mergify Bot removed the queued label Dec 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Madhu-1 Madhu-1 Madhu-1 approved these changes

@Rakshith-R Rakshith-R Rakshith-R approved these changes

@iPraveenParihar iPraveenParihar Awaiting requested review from iPraveenParihar

@Nikhil-Ladha Nikhil-Ladha Awaiting requested review from Nikhil-Ladha

@nixpanic nixpanic Awaiting requested review from nixpanic

@yati1998 yati1998 Awaiting requested review from yati1998

Assignees

@black-dragon74 black-dragon74

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Optimize memory footprint of csi-addons

3 participants

@black-dragon74 @Madhu-1 @Rakshith-R