Re: Review Request 71858: WIP: Set resource limits when launching executor container.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71858/ --- (Updated Jan. 20, 2020, 10:30 a.m.) Review request for mesos, Andrei Budnik

Re: Review Request 71858: WIP: Set resource limits when launching executor container.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71858/ --- (Updated Jan. 19, 2020, 10:51 a.m.) Review request for mesos, Andrei Budnik

Re: Review Request 71858: WIP: Set resource limits when launching executor container.

> On Jan. 8, 2020, 5:43 a.m., Greg Mann wrote: > > src/slave/slave.cpp > > Lines 3140-3201 (patched) > > > > > > I think maybe this logic could be easier to read if we do something > > like: > > > > ``` >

Re: Review Request 71858: WIP: Set resource limits when launching executor container.

> On Jan. 7, 2020, 9:43 p.m., Greg Mann wrote: > > src/slave/slave.cpp > > Lines 3623 (patched) > > > > > > Should this be an `Option`? So that we can only set > > `containerConfig.limits` when limits have

Re: Review Request 71858: WIP: Set resource limits when launching executor container.

> On Jan. 7, 2020, 9:43 p.m., Greg Mann wrote: > > src/slave/slave.cpp > > Lines 3140-3201 (patched) > > > > > > I think maybe this logic could be easier to read if we do something > > like: > > > > ``` >

Re: Review Request 71858: WIP: Set resource limits when launching executor container.

> On Jan. 8, 2020, 5:43 a.m., Greg Mann wrote: > > src/slave/slave.cpp > > Lines 3140-3201 (patched) > > > > > > I think maybe this logic could be easier to read if we do something > > like: > > > > ``` >

Re: Review Request 71858: WIP: Set resource limits when launching executor container.

> On Jan. 7, 2020, 9:43 p.m., Greg Mann wrote: > > src/slave/slave.cpp > > Lines 3623 (patched) > > > > > > Should this be an `Option`? So that we can only set > > `containerConfig.limits` when limits have

Re: Review Request 71858: WIP: Set resource limits when launching executor container.

> On Jan. 7, 2020, 9:43 p.m., Greg Mann wrote: > > src/slave/slave.cpp > > Lines 3140-3201 (patched) > > > > > > I think maybe this logic could be easier to read if we do something > > like: > > > > ``` >

Re: Review Request 71858: WIP: Set resource limits when launching executor container.

> On Jan. 8, 2020, 5:43 a.m., Greg Mann wrote: > > src/slave/slave.cpp > > Lines 3623 (patched) > > > > > > Should this be an `Option`? So that we can only set > > `containerConfig.limits` when limits have

Re: Review Request 71858: WIP: Set resource limits when launching executor container.

> On Jan. 8, 2020, 5:43 a.m., Greg Mann wrote: > > src/slave/slave.cpp > > Lines 3623 (patched) > > > > > > Should this be an `Option`? So that we can only set > > `containerConfig.limits` when limits have

Re: Review Request 71858: WIP: Set resource limits when launching executor container.

> On Jan. 8, 2020, 5:43 a.m., Greg Mann wrote: > > src/slave/slave.cpp > > Lines 3140-3201 (patched) > > > > > > I think maybe this logic could be easier to read if we do something > > like: > > > > ``` >

Re: Review Request 71858: WIP: Set resource limits when launching executor container.

> On Jan. 7, 2020, 9:43 p.m., Greg Mann wrote: > > src/slave/slave.cpp > > Lines 3140-3201 (patched) > > > > > > I think maybe this logic could be easier to read if we do something > > like: > > > > ``` >

Re: Review Request 71858: WIP: Set resource limits when launching executor container.

> On Jan. 7, 2020, 9:43 p.m., Greg Mann wrote: > > src/slave/slave.cpp > > Lines 3623 (patched) > > > > > > Should this be an `Option`? So that we can only set > > `containerConfig.limits` when limits have

Re: Review Request 71858: WIP: Set resource limits when launching executor container.

> On Jan. 7, 2020, 9:43 p.m., Greg Mann wrote: > > src/slave/slave.cpp > > Lines 3137 (patched) > > > > > > Our style guide prefers trailing underscores in cases like this, could > > you use `task_` instead of

Re: Review Request 71858: WIP: Set resource limits when launching executor container.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71858/ --- (Updated Jan. 8, 2020, 10:40 a.m.) Review request for mesos, Andrei Budnik and

Re: Review Request 71858: WIP: Set resource limits when launching executor container.

> On Jan. 8, 2020, 5:43 a.m., Greg Mann wrote: > > src/slave/slave.cpp > > Lines 3116 (patched) > > > > > > Are we using `-1` for this? I thought the plan was to use `INFINITY`? Yeah, that's why I marked this

Re: Review Request 71858: WIP: Set resource limits when launching executor container.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71858/ --- (Updated Jan. 8, 2020, 10:08 a.m.) Review request for mesos, Andrei Budnik and

Re: Review Request 71858: WIP: Set resource limits when launching executor container.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71858/#review219164 --- src/slave/slave.cpp Lines 126 (patched)

Re: Review Request 71858: WIP: Set resource limits when launching executor container.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71858/#review219154 --- src/slave/slave.cpp Lines 3115 (patched)

Re: Review Request 71858: WIP: Set resource limits when launching executor container.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71858/ --- (Updated Dec. 31, 2019, 5:08 p.m.) Review request for mesos, Andrei Budnik and

Re: Review Request 71858: WIP: Set resource limits when launching executor container.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71858/ --- (Updated Dec. 27, 2019, 4:03 p.m.) Review request for mesos, Andrei Budnik and

Re: Review Request 71858: WIP: Set resource limits when launching executor container.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71858/ --- (Updated Dec. 20, 2019, 8:44 p.m.) Review request for mesos, Andrei Budnik and

Re: Review Request 71858: WIP: Set resource limits when launching executor container.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71858/#review218904 --- Patch looks great! Reviews applied: [71855, 71856, 71858]