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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71858/ --- (Updated March 16, 2020, 5:04 p.m.) Review request for mesos, Andrei Budnik

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71858/#review219881 --- Fix it, then Ship it! src/slave/slave.cpp Lines 191 (patched)

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

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

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71858/#review219775 --- Ship it! Ship It! - Greg Mann On Feb. 25, 2020, 1:46 a.m.,

Re: Review Request 71858: 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: 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: 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: Set resource limits when launching executor container.

> On Feb. 28, 2020, 5:57 p.m., Greg Mann wrote: > > src/slave/slave.cpp > > Lines 3262 (patched) > > > > > > We have validation which ensures that executor CPU and mem are always > > greater than or equal to the

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

> On Feb. 29, 2020, 1:57 a.m., Greg Mann wrote: > > src/slave/slave.cpp > > Lines 3262 (patched) > > > > > > We have validation which ensures that executor CPU and mem are always > > greater than or equal to the

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

> On Feb. 29, 2020, 1:55 a.m., Greg Mann wrote: > > src/slave/slave.cpp > > Lines 3201-3204 (original), 3285-3291 (patched) > > > > > > I was just thinking about this more... IIUC, we actually want to always > >

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

> On Feb. 29, 2020, 1:38 a.m., Greg Mann wrote: > > src/slave/slave.cpp > > Lines 3201-3204 (original), 3285-3291 (patched) > > > > > > So it looks like here, we will only set limits for the executor if they > >

Re: Review Request 71858: 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: Set resource limits when launching executor container.

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

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

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71858/#review219696 --- src/slave/slave.cpp Lines 3201-3204 (original), 3285-3291

Re: Review Request 71858: 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: Set resource limits when launching executor container.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71858/#review219692 --- src/slave/slave.cpp Lines 3201-3204 (original), 3285-3291

Re: Review Request 71858: 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: 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: Set resource limits when launching executor container.

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

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

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