> On Dec. 18, 2014, 9:38 p.m., Ben Mahler wrote: > > src/master/master.cpp, lines 2073-2094 > > <https://reviews.apache.org/r/28720/diff/8/?file=796131#file796131line2073> > > > > Ok, this tripped me up a bit, given there are a few invariants here we > > might want to write down explicitly: > > > > ``` > > // We allow frameworks to implicitly acquire persistent disks > > // through task and executor resources. This means that we need > > // to infer these implicit disk acquisition transformations on the > > // offered resources, so that we can validate resource usage. > > // > > // NOTE: ResourceValidator ensures that there are no duplicate > > // persistence IDs per role in 'resources'. > > // > > // NOTE: 'offeredResources' will not contain duplicate persistence > > // IDs per role, given we do not construct such offers. > > Resources::CompositeTransformation transformation; > > foreach (const Resource& disk, resources.persistentDisks()) { > > if (!offeredResources.contains(disk)) { > > // This is an implicit acquisition, the framework is not allowed > > // to mutate an offered persistent disk, so we need to check the > > // offered resources for this persistence ID within the role. > > // > > // TODO(jieyu): We need to ensure this persistence ID within the > > // role does not clash with any in-use persistent disks on the > > // slave. > > ... > > } > > } > > ```
Thanks! Added. > On Dec. 18, 2014, 9:38 p.m., Ben Mahler wrote: > > src/master/master.cpp, line 2096 > > <https://reviews.apache.org/r/28720/diff/8/?file=796131#file796131line2096> > > > > How about we move the comment below up here: > > > > ``` > > // Validate that the offered resources are sufficient for launching > > this task/executor. > > // First we must apply the implicit transformations. > > ``` Done. > On Dec. 18, 2014, 9:38 p.m., Ben Mahler wrote: > > src/master/master.cpp, lines 2767-2769 > > <https://reviews.apache.org/r/28720/diff/8/?file=796131#file796131line2767> > > > > Can you remove the NOTE on this one? Doesn't seem like a "gotcha" IMHO. Removed. - Jie ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/28720/#review65537 ----------------------------------------------------------- On Dec. 18, 2014, 7:59 p.m., Jie Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/28720/ > ----------------------------------------------------------- > > (Updated Dec. 18, 2014, 7:59 p.m.) > > > Review request for mesos and Ben Mahler. > > > Bugs: MESOS-2030 > https://issues.apache.org/jira/browse/MESOS-2030 > > > Repository: mesos-git > > > Description > ------- > > Introduced an abstraction for mutating resources. Acquiring persistent disk > is one type of resources mutation. > > Infer persistent disk acquisitions from resources and check resource usage > against adjusted total resources. > > Adjusted the calculation of unused resources in _launchTasks by considering > persistent disk acquisition. > > > Diffs > ----- > > src/master/master.cpp 0c98b5184dec4c6db87f7a6ae7993b83f9b9b4c9 > src/tests/resource_offers_tests.cpp > e13b6c5460d9e6729843c40bed9e4d4e3f76d5d3 > > Diff: https://reviews.apache.org/r/28720/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jie Yu > >
