Scale celery memory use to system's available memory #2407
Conversation
Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
There was a problem hiding this comment.
I think this is good. Does celery reserve only a percentage of available memory. We wouldn’t want it to take it all.
There was a problem hiding this comment.
It for sure won't take up all of the memory. I don't know the exact proportion at the moment given the custom celery settings we have.
There was a problem hiding this comment.
Can we add some checks to ensure there’s some kind of cap? Is the underlying issue that celery isn’t grabbing enough memory?
There was a problem hiding this comment.
I just added a flat 30% cap of the total memory to keep it simple.
Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
…otal virtual memory Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
| #Get a subset of the maximum procesess available using a ratio, not exceeding a maximum value | ||
| def determine_worker_processes(ratio,maximum): | ||
| return min(round(max_process_estimate * ratio),maximum) | ||
|
|
There was a problem hiding this comment.
We need to either add a min of 1 to this, or return an error if that the system does not have enough memory to run Augur because a system with 8 GB of RAM with Augur configured to use 30 percent of virtual memory will currently run 2 scheduling processes, 1 core process, 0 secondary processes, and 0 facade processes. Note: With a 30 percent usage around 8.2 GB is what is needed to for 2 scheduling processes and 1 of all the others. At 20 percent usage 12.3 GB is needed
There was a problem hiding this comment.
I think we should bump it up to at least 40% and then add a minimum process of 1.
There was a problem hiding this comment.
@sgoggins was suggesting to drop the limit to 20% to account for the rabbitmq, redis, and database memory usage. I also don't think 40% is a reasonable number for larger servers where multiple instances are being run. With this, I think it should be configurable as a number between 0 and 1. Then we can just raise an error if not enough memory is allocated to it, and notify them to change the config. This allows for the flexibility of small systems where 1 augur instance is the only thing running, and larger systems where there are many instances running
There was a problem hiding this comment.
We already have a hard limit on the maximum level of processes each worker can have for larger instances. I think for now a default cap of 20-30% would be fine. I think it is a good idea to put the percentage in the config though.
There was a problem hiding this comment.
Yeah I didn't notice that before, this sounds good to me
Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
Signed-off-by: Isaac Milarsky <imilarsky@gmail.com>
Description
Notes for Reviewers
These changes should help to preserve some memory on systems with smaller amounts of it without hampering collection on larger memory instances.
Signed commits