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

[bitnami/rabbitmq] Fix high CPU usage while idle #11117

Merged
merged 1 commit into from
Jul 12, 2022
Merged

[bitnami/rabbitmq] Fix high CPU usage while idle #11117

merged 1 commit into from
Jul 12, 2022

Conversation

orgads
Copy link
Contributor

@orgads orgads commented Jul 10, 2022

Description of the change

Use REST APIs for liveness/readiness probes, instead of spawning
expensive erlang processes.

Benefits

Negligible CPU usage.

Possible drawbacks

  • Might be less accurate

Applicable issues

Checklist

  • Chart version bumped in Chart.yaml according to semver. This is not necessary when the changes only affect README.md files.
  • Variables are documented in the values.yaml and added to the README.md using readme-generator-for-helm
  • Title of the pull request follows this pattern [bitnami/<name_of_the_chart>] Descriptive title
  • All commits signed off and in agreement of Developer Certificate of Origin (DCO)

@rafariossaa rafariossaa added the verify Execute verification workflow for these changes label Jul 11, 2022
Copy link
Collaborator

@fmulero fmulero left a comment

Choose a reason for hiding this comment

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

Hi @orgads Thanks a lot for your contribution.

I like this approach

Copy link
Collaborator

@fmulero fmulero left a comment

Choose a reason for hiding this comment

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

Hi @orgads, sorry but there is a little failure in your PR. Could you have it a look?

bitnami/rabbitmq/templates/statefulset.yaml Outdated Show resolved Hide resolved
bitnami/rabbitmq/templates/statefulset.yaml Outdated Show resolved Hide resolved
Use REST APIs for liveness/readiness probes, instead of spawning
expensive erlang processes.

Fixes #11116

Signed-off-by: Orgad Shaneh <[email protected]>
Copy link
Collaborator

@fmulero fmulero left a comment

Choose a reason for hiding this comment

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

Great! LGTM. Thanks a lot @orgads

@fmulero fmulero merged commit 73966c6 into bitnami:master Jul 12, 2022
@orgads orgads deleted the rabbitmq-probes branch July 12, 2022 11:22
@mjattie
Copy link
Contributor

mjattie commented Jul 13, 2022

https://www.rabbitmq.com/monitoring.html#deprecations

this change implements a legacy health check. Shouldn't the default health check be one of the recommendations on the mentioned page above, instead of the legacy endpoint?

@fmulero
Copy link
Collaborator

fmulero commented Jul 13, 2022

Hi @mjattie, you are completely right! I should realise it before.

@orgads previous checks were aligned with the RabbitMQ recommendations, so I'll rollback your changes. You can use your custom liveness and readiness probes setting them in your values file.

@orgads
Copy link
Contributor Author

orgads commented Jul 13, 2022

The REST checks are not deprecated, only the liveness probe I used is. I preferred to use the deprecated version, because the current version requires vhost, which is %3f (/) by default, but I think can be customized.

Would you prefer that?

@orgads
Copy link
Contributor Author

orgads commented Jul 13, 2022

I think we can use /api/health/checks/virtual-hosts. See the documentation here.

@fmulero
Copy link
Collaborator

fmulero commented Jul 14, 2022

Thanks @orgads. I agree with you about the CPU usage, but I think is better to keep aligned with RabbitMQ documentation, and report this kind of issues to the upstream project

@orgads
Copy link
Contributor Author

orgads commented Jul 14, 2022

Apparently someone else already asked about this. They don't recommend using the CLI for probes. See here.

vaggeliskls pushed a commit to vaggeliskls/charts that referenced this pull request Jul 21, 2022
Use REST APIs for liveness/readiness probes, instead of spawning
expensive erlang processes.

Fixes bitnami#11116

Signed-off-by: Orgad Shaneh <[email protected]>
Signed-off-by: vaggeliskls <[email protected]>
FraPazGal pushed a commit that referenced this pull request Jul 25, 2022
Use REST APIs for liveness/readiness probes, instead of spawning
expensive erlang processes.

Fixes #11116

Signed-off-by: Orgad Shaneh <[email protected]>
fmulero added a commit that referenced this pull request Aug 10, 2022
* Revert "[bitnami/rabbitmq] Fix high CPU usage while idle (#11117)"

This reverts commit 73966c6.

Signed-off-by: Fran Mulero <[email protected]>

* Version bump

Signed-off-by: Fran Mulero <[email protected]>

Signed-off-by: Fran Mulero <[email protected]>
deprecastor pushed a commit to deprecastor/charts that referenced this pull request Aug 13, 2022
* Revert "[bitnami/rabbitmq] Fix high CPU usage while idle (bitnami#11117)"

This reverts commit 73966c6.

Signed-off-by: Fran Mulero <[email protected]>

* Version bump

Signed-off-by: Fran Mulero <[email protected]>

Signed-off-by: Fran Mulero <[email protected]>
orgads added a commit to orgads/charts that referenced this pull request Apr 17, 2023
Use REST APIs for liveness/readiness probes, instead of spawning
expensive erlang processes.

Reapply of bitnami#11117 and bitnami#11180.

Fixes bitnami#11116.

Signed-off-by: Orgad Shaneh <[email protected]>
(cherry picked from commit 73966c6)
orgads added a commit to orgads/charts that referenced this pull request Apr 27, 2023
Use REST APIs for liveness/readiness probes, instead of spawning
expensive erlang processes.

Reapply of bitnami#11117 and bitnami#11180.

Fixes bitnami#11116.

Signed-off-by: Orgad Shaneh <[email protected]>
(cherry picked from commit 73966c6)
jotamartos added a commit that referenced this pull request May 2, 2023
Use REST APIs for liveness/readiness probes, instead of spawning
expensive erlang processes.

Reapply of #11117 and #11180.

Fixes #11116.

Signed-off-by: Orgad Shaneh <[email protected]>
(cherry picked from commit 73966c6)

Signed-off-by: Juan José Martos <[email protected]>
Co-authored-by: Juan José Martos <[email protected]>
Yaytay pushed a commit to Yaytay/charts that referenced this pull request May 5, 2023
Use REST APIs for liveness/readiness probes, instead of spawning
expensive erlang processes.

Reapply of bitnami#11117 and bitnami#11180.

Fixes bitnami#11116.

Signed-off-by: Orgad Shaneh <[email protected]>
(cherry picked from commit 73966c6)

Signed-off-by: Juan José Martos <[email protected]>
Co-authored-by: Juan José Martos <[email protected]>
Mauraza pushed a commit that referenced this pull request May 9, 2023
Use REST APIs for liveness/readiness probes, instead of spawning
expensive erlang processes.

Reapply of #11117 and #11180.

Fixes #11116.

Signed-off-by: Orgad Shaneh <[email protected]>
(cherry picked from commit 73966c6)

Signed-off-by: Juan José Martos <[email protected]>
Co-authored-by: Juan José Martos <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rabbitmq solved verify Execute verification workflow for these changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bitnami/rabbitmq] RabbitMQ high CPU usage while idle
6 participants