8000 perf: optimize the rabbitmq health check by conet · Pull Request #3037 · libretime/libretime · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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

Merged
merged 4 commits into from
Jun 22, 2024
Merged

perf: optimize the rabbitmq health check #3037

merged 4 commits into from
Jun 22, 2024

Conversation

conet
Copy link
Contributor
@conet conet commented Jun 19, 2024

Description

Running rabbitmq-diagnostics every 30s is a resource intensive operation wasting cpu cycles

This 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.

@conet conet changed the title Use a more resource efficient health check perf: Use a more resource efficient health check Jun 19, 2024
@conet conet changed the title perf: Use a more resource efficient health check perf: Use a more resource efficient health check for rabbitmq Jun 19, 2024
@conet conet changed the title perf: Use a more resource efficient health check for rabbitmq perf: use a more resource efficient health check for rabbitmq Jun 19, 2024
@jooola
Copy link
Contributor
jooola commented Jun 22, 2024

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.

@conet
Copy link
Contributor Author
conet commented Jun 22, 2024

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:

$ docker compose exec rabbitmq bash
# time for in in {1..100}; do rabbitmq-diagnostics -q ping; done
Ping succeeded
Ping succeeded
...
Ping succeeded

real	0m40.225s
user	2m33.354s
sys	0m17.481s
# time for in in {1..100}; do nc -z 127.0.0.1 5672; done
real	0m0.027s
user	0m0.018s
sys	0m0.010s

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 rabbitmq-diagnostics -q ping is really bad for a command that is run every 30s (the default healthcheck interval), it's worse if one runs multiple rabbitmq sevices. I'm not sure what is the lost functionality here, is there a case when rabbitmq is down but it keeps the amqp port open? Anyway whoever recommended rabbitmq-diagnostics -q ping as a good healtcheck probe (I found stack overflow threads about this) is responsible for a lot of CPU usage out there.

@jooola jooola changed the title perf: use a more resource efficient health check for rabbitmq perf: optimize the rabbitmq health check Jun 22, 2024
@jooola
Copy link
Contributor
jooola commented Jun 22, 2024

Thanks for the data!

Merging now

@jooola jooola merged commit 9684214 into libretime:main Jun 22, 2024
28 of 32 checks passed
jooola pushed a commit that referenced this pull request Jun 22, 2024
🤖 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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0