----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53982/#review157091 -----------------------------------------------------------
Overall seems okay, but there no discussion of _why_ I would want to use rlimits to constrain the resource usage of a task, compared with other methods of resource isolation (e.g., `posix/cpu`, `posix/mem`, etc.) Other things it would be good to spell out: * The resource limit for a task is different than the amount of resources the task is allocated by Mesos. Presumably it is up to the user to ensure that the `rlimits_info` in their `ContainerInfo` is set appropriately, right? As an operator, I can't require that all tasks are run under a certain rlimit, right? * If the `mesos-agent` is started under a resource limit, what happens? i.e., presumably we can launch tasks with higher hard rlimits iff `mesos-agent` process has sufficient priviledges to do so. * If the containerizer fails to set the resource limits for a task, what happens? Presumably the task launch fails; how is this signaled to the framework? docs/posix_rlimits.md (line 3) <https://reviews.apache.org/r/53982/#comment227511> s/The isolators/This isolator/ ? docs/posix_rlimits.md (line 4) <https://reviews.apache.org/r/53982/#comment227534> "adds support for setting POSIX resource limits (rlimits) for containers ..." docs/posix_rlimits.md (line 7) <https://reviews.apache.org/r/53982/#comment227537> "POSIX rlimits can be used to control the resources that a process can consume." docs/posix_rlimits.md (line 8) <https://reviews.apache.org/r/53982/#comment227541> "Resource limits are typically set at boot time and inherited when a child process is forked from a parent process; resource limits can also be modified via `setrlimit(2)`." docs/posix_rlimits.md (line 11) <https://reviews.apache.org/r/53982/#comment227539> Comma after "shells" docs/posix_rlimits.md (line 14) <https://reviews.apache.org/r/53982/#comment227543> "tuple" seems like unnecessary jargon here. "A resource limit consists of a _soft_ and a _hard_ limit. The soft limit specifies the effective ..." docs/posix_rlimits.md (line 15) <https://reviews.apache.org/r/53982/#comment227544> The difference between hard and soft limits is unclear here. Would be good to elaborate on what purpose the different types of limits serve (e.g., soft limits are useful when the limiter and limited process can be assumed to be cooperative; hard limits are more useful as a security mechanism). docs/posix_rlimits.md (line 22) <https://reviews.apache.org/r/53982/#comment227546> "enables interpretation" seems vague here. The point is that by using this isolator, resource limits specified in the task's `ContainerInfo` will be applied to the task's container, right? If so, I'd say that directly and avoid phrases like "interpreting rlimits". Also mention that rlimits are specified via the `rlimit_info` field of `ContainerInfo`. Wouldn't hurt to give an example of doing this. docs/posix_rlimits.md (line 47) <https://reviews.apache.org/r/53982/#comment227547> Comma after "types". docs/posix_rlimits.md (line 48) <https://reviews.apache.org/r/53982/#comment227549> s/In addition many limits defined on Linux/Linux defines a number of additional resource limits that are not included in the POSIX standard. These are also supported by this isolator; see the definition of `RLimitInfo::RLimit` for the list of currently exposed types." Is there a reason to not include all the rlimit types in this document? The list is probably not going to change very often. - Neil Conway On Nov. 22, 2016, 1:27 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53982/ > ----------------------------------------------------------- > > (Updated Nov. 22, 2016, 1:27 p.m.) > > > Review request for mesos, Jie Yu and Neil Conway. > > > Bugs: MESOS-6427 > https://issues.apache.org/jira/browse/MESOS-6427 > > > Repository: mesos > > > Description > ------- > > Added documentation for posix/rlimit isolator. > > > Diffs > ----- > > docs/mesos-containerizer.md 2bff35f6361f760a9001a249d2c01bbbc9e72932 > docs/posix_rlimits.md PRE-CREATION > > Diff: https://reviews.apache.org/r/53982/diff/ > > > Testing > ------- > > Tested with github markdown renderer, and with build setup from `site/`. > > > Thanks, > > Benjamin Bannier > >
