Skip to content

deploy: add -automaxprocs to reduce CPU and memory resources #5292

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
May 7, 2025
Merged

deploy: add -automaxprocs to reduce CPU and memory resources #5292

merged 2 commits into from
May 7, 2025

Conversation

@nixpanic
Copy link
Member

With the new -automaxprocs commandline parameter, Ceph-CSI will adjust
the GOMAXPROCS environment variable for the Golang runtime. The values
are based on the CPU quota that is given to the process. This can reduce
the number of threads that the Golang runtime spawns, which affects the
require amount of memory as well.

Fixes: #5228

Show available bot commands

These commands are normally not required, but in case of issues, leave any of
the following bot commands in an otherwise empty comment in this PR:

  • /retest ci/centos/<job-name>: retest the <job-name> after unrelated
    failure (please report the failure too!)

@nixpanic
Copy link
Member Author

/test ci/centos/mini-e2e-helm/k8s-1.31

@nixpanic nixpanic requested a review from a team April 28, 2025 15:16
@nixpanic nixpanic force-pushed the automaxprocs branch 2 times, most recently from 088694c to a824688 Compare April 28, 2025 16:43
@nixpanic nixpanic added component/rbd Issues related to RBD component/deployment Helm chart, kubernetes templates and configuration Issues/PRs labels Apr 28, 2025
Rakshith-R
Rakshith-R reviewed Apr 29, 2025
View reviewed changes
Copy link
Contributor

@Rakshith-R Rakshith-R left a comment

Choose a reason for hiding this comment

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

Lets set this options to true by default in direct k8s deployment yamls too ?
https://github.com/ceph/ceph-csi/tree/devel/deploy

@nixpanic
Copy link
Member Author

Lets set this options to true by default in direct k8s deployment yamls too ? https://github.com/ceph/ceph-csi/tree/devel/deploy

Hey @Rakshith-R,

There is no resources section there, so it would not be of much use? Or do you expect uses to use the manual deploy files and add extra sections like that?

@nixpanic nixpanic requested a review from a team April 30, 2025 12:51
black-dragon74
black-dragon74 previously approved these changes Apr 30, 2025
View reviewed changes
@Rakshith-R
Copy link
Contributor

Lets set this options to true by default in direct k8s deployment yamls too ? https://github.com/ceph/ceph-csi/tree/devel/deploy

Hey @Rakshith-R,

There is no resources section there, so it would not be of much use? Or do you expect uses to use the manual deploy files and add extra sections like that?

Yes, users maybe adding the resource contraints manually ?

We'll need to add this option at Ceph-CSI-operator too ?

@nixpanic
Copy link
Member Author

nixpanic commented May 2, 2025

We'll need to add this option at Ceph-CSI-operator too ?

If it set a resource quota for the Ceph-CSI container, then it would be good, yes.

@nixpanic nixpanic requested a review from Rakshith-R May 2, 2025 15:32
@mergify
Copy link
Contributor

mergify bot commented May 6, 2025

This pull request now has conflicts with the target branch. Could you please resolve conflicts and force push the corrected changes? 🙏

Rakshith-R
Rakshith-R previously approved these changes May 6, 2025
View reviewed changes
@mergify mergify bot dismissed stale reviews from Rakshith-R and black-dragon74 May 7, 2025 07:06

Pull request has been modified.

@nixpanic
Copy link
Member Author

nixpanic commented May 7, 2025

Hi @Rakshith-R and @black-dragon74 , please review again, the merge conflict has been resolved.

@nixpanic
Copy link
Member Author

nixpanic commented May 7, 2025

@Mergifyio queue

@mergify
Copy link
Contributor

mergify bot commented May 7, 2025 •
edited
Loading

queue

🛑 The pull request has been removed from the queue default

The merge conditions cannot be satisfied due to failing checks.

You can take a look at Queue: Embarked in merge queue check runs for more details about the failure.

@mergify mergify bot force-pushed the automaxprocs branch from e577db4 to 577d48a Compare May 7, 2025 10:57
@mergify mergify bot added the ok-to-test Label to trigger E2E tests label May 7, 2025
@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.30

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.31

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.30

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-rbd

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.31

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.32

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.30

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.31

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.32

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.32

@ceph-csi-bot ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label May 7, 2025
@mergify
Copy link
Contributor

mergify bot commented May 7, 2025

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

You may have to fix your CI before adding the pull request to the queue again.
If you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

@nixpanic
Copy link
Member Author

nixpanic commented May 7, 2025

/retest ci/centos/mini-e2e/k8s-1.30

@nixpanic
Copy link
Member Author

nixpanic commented May 7, 2025

@Mergifyio requeue

/retest ci/centos/mini-e2e/k8s-1.30

Failed with:

[FAILED] backend images not matching kubernetes resource count,image count 1 kubernetes resource count 0
  backend image Info:
   [csi-vol-117161eb-316d-4945-af87-a0fd89748598]
   images information and status Pool: replicapool, Image: csi-vol-117161eb-316d-4945-af87-a0fd89748598, Info: {"name":"csi-vol-117161eb-316d-4945-af87-a0fd89748598","id":"1661f8c7f17a","size":1073741824,"objects":256,"order":22,"object_size":4194304,"snapshot_count":0,"block_name_prefix":"rbd_data.1661f8c7f17a","format":2,"features":["layering"],"op_features":[],"flags":[],"create_timestamp":"Wed May  7 11:39:57 2025","access_timestamp":"Wed May  7 11:39:57 2025","modify_timestamp":"Wed May  7 11:39:57 2025"}
  , Status: {"watchers":[]}

logs

@mergify
Copy link
Contributor

mergify bot commented May 7, 2025

requeue

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically

nixpanic added 2 commits May 7, 2025 16:30
@nixpanic @mergify
deploy: add -automaxprocs to reduce CPU and memory resources
6f44df7 
With the new `-automaxprocs` commandline parameter, Ceph-CSI will adjust
the GOMAXPROCS environment variable for the Golang runtime. The values
are based on the CPU quota that is given to the process. This can reduce
the number of threads that the Golang runtime spawns, which affects the
require amount of memory as well.

Updates: ceph#5228
Signed-off-by: Niels de Vos <[email protected]>
@nixpanic @mergify
deploy: include -automaxprocs to apply configured resource quota
1826d67 
When the Ceph-CSI provisioner/plugin is configured with CPU resource
quota, the GOMAXPROCS environment variable should get set as well.
Without the `-automaxprocs` commandline argument, it is possible that
there are way too many threads started by the Go runtime.

Signed-off-by: Niels de Vos <[email protected]>
@mergify mergify bot force-pushed the automaxprocs branch from 577d48a to 1826d67 Compare May 7, 2025 16:30
@mergify mergify bot added the ok-to-test Label to trigger E2E tests label May 7, 2025
@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.30

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.30

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.32

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.30

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.32

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-cephfs

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.31

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.32

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-rbd

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.31

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.31

@ceph-csi-bot ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label May 7, 2025
@mergify mergify bot merged commit d52dc2c into ceph:devel May 7, 2025
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@black-dragon74 black-dragon74 black-dragon74 approved these changes

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

Assignees

No one assigned

Labels

component/deployment Helm chart, kubernetes templates and configuration Issues/PRs component/rbd Issues related to RBD

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

allow adding to environment of pods in charts or autotune GOMAXPROCS

4 participants

@nixpanic @Rakshith-R @ceph-csi-bot @black-dragon74