feat: add CPU and memory limits #890
Conversation
✅ Deploy Preview for testcontainers-rust ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
This is a rebase of #776 |
DDtKey
changed the title
|
|
||
| /// The length of a CPU period in microseconds. Default is 100000, this configures how | ||
| /// CFS will schedule the threads for this container. Normally you don't adjust this and | ||
| /// just set the CPU quota or nano CPUs. You might want to set this if you want to increase | ||
| /// or reduce context-switching the container is subjected to. | ||
| pub(crate) cpu_period: Option<i64>, | ||
|
|
||
| /// Microseconds of CPU time that the container can get in a CPU period. | ||
| /// Most users will want to set CPU quota to their desired CPU count * 100000. | ||
| /// For example, to limit a container to 2 CPUs, set CPU quota to 200000. | ||
| /// This is based on the default CPU period of 100000. | ||
| /// If CPU quota is set to 0, the container will not be limited. | ||
| pub(crate) cpu_quota: Option<i64>, | ||
|
|
||
| /// The length of a CPU real-time period in microseconds. Set to 0 to allocate no time allocated to real-time tasks. | ||
| pub(crate) cpu_realtime_period: Option<i64>, | ||
|
|
||
| /// The length of a CPU real-time runtime in microseconds. Set to 0 to allocate no time allocated to real-time tasks. | ||
| pub(crate) cpu_realtime_runtime: Option<i64>, | ||
|
|
||
| /// CPUs in which to allow execution (e.g., `0-3`, `0,1`). | ||
| /// Core pinning should help with performance consistency and context switching in some cases. | ||
| pub(crate) cpuset_cpus: Option<String>, | ||
|
|
||
| /// CPU quota in units of 10<sup>-9</sup> CPUs. This is basically what the --cpus flag turns into, but the | ||
| /// raw value is denominated in billionths of a CPU. cpu_period and cpu_quota give you more control over the scheduler. | ||
| pub nano_cpus: Option<i64>, | ||
|
|
||
| /// Memory limit for the container, the _minimum_ is 6 MiB. | ||
| /// This is the same as `HostConfig::memory`. | ||
| pub(crate) memory: Option<i64>, | ||
|
|
||
| /// Memory reservation, soft limit. Analogous to the JVM's `-Xms` option. | ||
| /// The _minimum_ is 6 MiB. | ||
| /// This is the same as `HostConfig::memory_reservation`. | ||
| pub(crate) memory_reservation: Option<i64>, | ||
|
|
||
| /// Total memory limit (memory + swap). Set as `-1` to enable unlimited swap. | ||
| /// Same 6 MiB minimum as `memory`. | ||
| pub memory_swap: Option<i64>, | ||
|
|
||
| /// Tune a container's memory swappiness behavior. Accepts an integer between 0 and 100. | ||
| pub memory_swappiness: Option<i64>, | ||
|
|
||
| /// Disable OOM Killer for the container. This will not do anything unless -m (memory limit, cf. memory on this struct) is set. | ||
| /// You can disable OOM-killer by writing "1" to memory.oom_control file, as: | ||
| /// ```ignore | ||
| /// echo 1 > memory.oom_control | ||
| /// ``` | ||
| /// This operation is only allowed to the top cgroup of sub-hierarchy. | ||
| /// If OOM-killer is disabled, tasks under cgroup will hang/sleep | ||
| /// in memory cgroup's OOM-waitqueue when they request accountable memory. | ||
| /// https://lwn.net/Articles/432224/ | ||
| pub oom_kill_disable: Option<bool>, | ||
|
|
||
| /// Tune a container's PIDs limit. Set `0` or `-1` for unlimited, or `null` to not change. | ||
| pub pids_limit: Option<i64>, |
There was a problem hiding this comment.
That's quite a lot of options
It feels like it would be nice group some of them to structs (e.g CpuLimits, MemoryLimits - naming is questionable though) and accept typed arguments within ImageExt
This would provide us more granularity and the same time less overloaded trait
There was a problem hiding this comment.
Testcontainers Go maintainer here. Instead of adding so many options, we usually prefer passing a function modifier that has access to the different internal docker types, so the user can, for advanced cases, change all of them if needed: https://golang.testcontainers.org/features/creating_container/#advanced-settings
Please take a look at the config, host config and endpoint settings modifiers.
Of course, I'm not sure how mature is the Rust docker client nowadays.
There was a problem hiding this comment.
I was following the pattern of the existing Rust API. If the Rust maintainers want to refactor that, that's fine, but it's out of scope here. I've been using this fork for the better part of a year because I needed control over these parameters.
There was a problem hiding this comment.
I think that's reasonable and will close many other use-cases at once, otherwise we will end up exposing too many not frequently used docker API details. That's was generally the point in the initial PR
So exposing an ability to pass function-callback seems as good middle-ground.
We can accept Fn(&mut HostConfig) I assume for host config for now, other tweaks can be exposed on demand
There was a problem hiding this comment.
Agreed. That will enable more advanced use cases without increasing the API surface that much
There was a problem hiding this comment.
I'd really like to unfork my dependency. If you want to change the API you're free to do that at any time.
There was a problem hiding this comment.
I apologize 🙏 , but I don’t believe this can be merged in its current state.
Changing the API is typically a breaking change (and a headache with deprecation), and it’s generally advisable to avoid it initially if possible. While merging and making changes immediately before releasing doesn’t offer any benefits.
Furthermore, it currently exposes a lot of parameters that aren’t strictly typed and mostly 1-to-1 mapping for docker client (bollard). That’s why the initial suggestion was to group them into structs.
However, the suggestion with the callback function appears to be a reasonable step here, providing maximum flexibility for users while minimizing the overhead for maintainability.
There was a problem hiding this comment.
I will try to dedicate time and implement it If you don't have capacity to perform the requested changes here.
There was a problem hiding this comment.
Implemented in #899
Now it's possible to set such configs with the following pattern:
.with_host_config_modifier(|host_config| {
host_config.cpu_period = Some(100_000);
host_config.cpu_quota = Some(200_000);
})
Why is everything i64
bitemyapp
force-pushed
the
bitemyapp/please-add-cpu-and-memory-limits-sooner
branch
from
17c00e4 to
375a26b
Compare
Addresses the needs of customizing specific docker params without expossing all available options. Initial discussion is here: #890
This feature addresses the need to customize specific Docker parameters without exposing all available options. For more information, the initial discussion here: #890
|
Closing due to #890 (comment) Thanks for your efforts! It wasn’t lost, but it definitely triggered a positive improvement! |
3 participants
Why is everything i64