----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/70716/#review215551 -----------------------------------------------------------
Ship it! src/master/allocator/mesos/hierarchical.hpp Lines 121-124 (original), 121-126 (patched) <https://reviews.apache.org/r/70716/#comment302281> Could probably just declare these two together with one comment, that would be a bit more succinct / readable IMO src/master/allocator/mesos/hierarchical.cpp Lines 1446-1450 (patched) <https://reviews.apache.org/r/70716/#comment302282> We could contain this logic a bit more without having the 'limits' variable in the top level scope? ``` roles[role].quotaLimits = [&]() { protobuf::Map<string, Value::Scalar> limits; foreach (const Resource& r, quota.info.guarantee()) { limits[r.name()] = r.scalar(); } return ResourceLimits(limits); }(); ``` src/master/allocator/mesos/hierarchical.cpp Line 2559 (original), 2570-2571 (patched) <https://reviews.apache.org/r/70716/#comment302283> How about: ``` return roles.contains(role) && (!roles.at(role).quotaGuarantees.empty() || !roles.at(role).quotaLimits.empty()); ``` - Benjamin Mahler On May 24, 2019, noon, Meng Zhu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/70716/ > ----------------------------------------------------------- > > (Updated May 24, 2019, noon) > > > Review request for mesos, Andrei Sekretenko and Benjamin Mahler. > > > Repository: mesos > > > Description > ------- > > Currently, resource quota acts as both guarantees and limits. > This patch adds a field `quotaLimits` in the role struct. > This paves the way for enforcing guarantees and limits > separately in the allocator. > > > Diffs > ----- > > src/master/allocator/mesos/hierarchical.hpp > 71a9656fb934bf9ac58e3165254ea49cb09efa8b > src/master/allocator/mesos/hierarchical.cpp > 40c8363afddccdd5275ca06318a8cc2cc6fa21af > > > Diff: https://reviews.apache.org/r/70716/diff/2/ > > > Testing > ------- > > make check > > > Thanks, > > Meng Zhu > >
