> On March 15, 2020, 9:04 a.m., Qian Zhang wrote: > > src/master/validation.cpp > > Lines 1574-1577 (patched) > > <https://reviews.apache.org/r/72216/diff/4/?file=2213417#file2213417line1574> > > > > Why do we want to return error if `taskCpus` is none? The purpose of > > this `validateResourceLimits()` method should be validating resource limits > > rather than resource requests.
This resource request validation is related to the limits: we don't support tasks which specify a resource limit without an associated resource request. I updated the patch to return a separate error message for that case. In the design doc, we list "best effort" tasks as a non-goal; IMO, a best effort task would correspond to a task with a resource limit and no resource request, so I don't think we intend to support that case. > On March 15, 2020, 9:04 a.m., Qian Zhang wrote: > > src/master/validation.cpp > > Lines 1832-1838 (patched) > > <https://reviews.apache.org/r/72216/diff/4/?file=2213417#file2213417line1832> > > > > The new method `validateResourceLimits` that you introduced has already > > done this validation and that method will be called for both the tasks > > launched via `LAUNCH` operation and the tasks launched via `LAUNCH_GROUP` > > operation, so I think we do not need to do this validation again here. I removed that block from `validateResourceLimits()` after reading your comment on that section, so I'm going to leave this here to catch this case for task groups. - Greg ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72216/#review219933 ----------------------------------------------------------- On March 11, 2020, 7:10 p.m., Greg Mann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72216/ > ----------------------------------------------------------- > > (Updated March 11, 2020, 7:10 p.m.) > > > Review request for mesos, Andrei Budnik and Qian Zhang. > > > Bugs: MESOS-10045 > https://issues.apache.org/jira/browse/MESOS-10045 > > > Repository: mesos > > > Description > ------- > > Added master validation for task resource limits and shared cgroups. > > > Diffs > ----- > > src/master/validation.cpp 084f281eadd65cb8ae0a19b7b7797dc71ccebdd2 > > > Diff: https://reviews.apache.org/r/72216/diff/4/ > > > Testing > ------- > > > Thanks, > > Greg Mann > >
