> On March 17, 2014, 4:28 a.m., Adam B wrote: > > Good start, Ashutosh. I think validating the sanity of the resources flag > > is vitally important, but I have my concerns about whether this is best > > done in slave/main.cpp, or in Containerizer or Slave itself. I think we > > should also discuss the desired behavior when an invalid resource flag > > value is provided. Should we exit without allowing the slave to start, or > > should we default to a sane value? > > Just a few questions and comments: > > - At the top of the review page, just below "Submitter: ASHUTOSH JAIN", > > please set "Branch: master" and "Bugs: MESOS-969". > > - As for testing, did you manually test that your changes fixed the problem > > you originally ran into? Can you write a unit test to represent that? > > ASHUTOSH JAIN wrote: > 1. Initially I did the check in containerizer.cpp and if flags were more > than the main memory then I set mem to total memory of system. But, this lead > to failure of DFR Allocation test and Resource Reservation tests so Bmahler > suggested to do this in main.cpp . > 2. I did a manual testing to verify the changes. I can try writing unit > test with some help from your side. Just point me to how to proceed > (preferably some example to how to write a test and where to write it)
Adam, to your point of exiting vs defaulting: we do not want to modify resources that are specified explicitly in the flags. That is, if an operator asks the slave to allocate 8GB, and the machine has 6GB, it would be preferable to let them know immediately rather than silently offer only 6GB. Otherwise, this could prove very confusing for operators. However, we can use definitely use defaults if the resources are unspecified. - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/19263/#review37335 ----------------------------------------------------------- On March 17, 2014, 5:21 p.m., ASHUTOSH JAIN wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/19263/ > ----------------------------------------------------------- > > (Updated March 17, 2014, 5:21 p.m.) > > > Review request for mesos, Adam B, Adam Berry, and Ben Mahler. > > > Bugs: MESOS-969 > https://issues.apache.org/jira/browse/MESOS-969 > > > Repository: mesos-git > > > Description > ------- > > resources (mem) provided by user not being checked > > link to issue: https://issues.apache.org/jira/browse/MESOS-969 > > > Diffs > ----- > > src/slave/main.cpp 8c2b70c > > Diff: https://reviews.apache.org/r/19263/diff/ > > > Testing > ------- > > make check > > > Thanks, > > ASHUTOSH JAIN > >
