Executes conditional mutations in a thread pool #5184
Conversation
Conditional mutations currently execute in the thrift thread pool which can grow unbounded in size. Having an unbounded number of conditional mutations executing concurrently could degrade a tablet servers health in some cases. This change adds threads pools for executing conditional updates. This will help limit the CPU and memory used by executing conditional mutations. Conditional mutations can also contain data as part of the mutation that needs to be checked, if this is large that could still cause memory issues. This change more limits the memory used by reading current tablet data to check the condition. Limiting the memory used by conditional mutations waiting to execute is something that would need to somehow be controlled by thrift. So this change protects memory and CPU resources for conditional mutations that are executing but does not protect memory for ones that are waiting to execute. To protect memory for those waiting to execute would need to end the current practice of letting the thrift thread pool grow unbounded, but that is a long existing problem.
There was a problem hiding this comment.
LGTM, in terms of a default size, picking a static number is fine but I'm also wondering if it would be possible to make the default dynamic. There's going to be a wide range of servers in terms of how much cpu/memory they have so having the default scale up or down by detecting the resources could be nice. But it's definitely simpler to just make users configure what they want.
Co-authored-by: Christopher L. Shannon <cshannon@apache.org>
Co-authored-by: Christopher L. Shannon <cshannon@apache.org>
Adjusted the defaults from 16 threads to 64 threads since it used to be unlimited thought a higher default would be good. Can use the metrics to monitor the utilization of the thread pool and adjust it based on those. |
Conditional mutations currently execute in the thrift thread pool which can grow unbounded in size. Having an unbounded number of conditional mutations executing concurrently could degrade a tablet servers health in some cases. This change adds threads pools for executing conditional updates. This will help limit the CPU and memory used by executing conditional mutations. Conditional mutations can also contain data as part of the mutation that needs to be checked, if this is large that could still cause memory issues. This change more limits the memory used by reading current tablet data to check the condition. Limiting the memory used by conditional mutations waiting to execute is something that would need to somehow be controlled by thrift. So this change protects memory and CPU resources for conditional mutations that are executing but does not protect memory for ones that are waiting to execute. To protect memory for those waiting to execute would need to end the current practice of letting the thrift thread pool grow unbounded, but that is a long existing problem.