-
-
Notifications
You must be signed in to change notification settings - Fork 231
perf: optimize the rabbitmq health check #3037
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
Conversation
Running rabbitmq-diagnostics every 30s is a resource intensive operation wasting cpu cycles
Hey thanks for the contribution! Could you provide us the data that back your proposal up? I don't see this as a huge performance improvement, and we lose some functionality. Unless you have a real scenario where this is causing a problem, I doubt we will merge this. You would also use a docker-compose.override.yml file to use your own health check command. |
I have graphs from machines running 20+ compositions and with this change the load and cpu usage was actually reduced to half. I'm not looking to have this PR merged, I'm happy to use this optimization on our deployments but I wanted to share this with the community, a 50% cpu usage reduction ads up. I will not share the graphs but I can share this easily reproducible test:
Everyone is free to run whatever they want but I would say that one of the goals of any good project is to ship optimum defaults. Running |
Thanks for the data! Merging now |
🤖 I have created a release *beep* *boop* --- ## [4.2.0](4.1.0...4.2.0) (2024-06-22) ### Features * **legacy:** add current date macro to string block criteria ([#3013](#3013)) ([451652b](451652b)) * **legacy:** add filename block criteria ([#3015](#3015)) ([4642b6c](4642b6c)) ### Bug Fixes * pin pip version to <24.1 to allow installing pytz (celery) ([#3043](#3043)) ([646bc81](646bc81)) * playlist allocates inaccurate time to smartblocks ([#3026](#3026)) ([2b43e51](2b43e51)) ### Performance Improvements * optimize the api image health check ([#3038](#3038)) ([d99d6e1](d99d6e1)) * optimize the rabbitmq health check ([#3037](#3037)) ([9684214](9684214)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Description
Running
rabbitmq-diagnostics
every 30s is a resource intensive operation wasting cpu cyclesThis is a new feature:
No
I have updated the documentation to reflect these changes:
Not necessary
Testing Notes
What I did:
We reduced the CPU load generated by the composition to about half of the previous value
How you can replicate my testing:
Run the current composition for a while, record cpu usage then run the new health check and observe much lower cpu usage. Or simply compare the execution time of the two commands in the rabbitmq container. The second command does not perform all checks but I think it is good enough considering the performance gain.