----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71020/#review216411 -----------------------------------------------------------
Fix it, then Ship it! I assume we'll track the missing pieces with additional tickets or leave the existing ticket open until we have them? src/master/quota_handler.cpp Lines 94 (patched) <https://reviews.apache.org/r/71020/#comment303633> This seems like a transitive property that doesn't need to be spelled out? sum of children's guarantees <= parent's guarantee <= parent's limit It seems somewhat unnatural to directly check: sum of children's guarantees <= parent's limit rather than check: sum of children's guarantees <= parent's limit && guarantee <= limit src/master/quota_handler.cpp Lines 467-515 (patched) <https://reviews.apache.org/r/71020/#comment303636> Hm.. is this code copied from the original quota handler? Is your plan to have this be the single path and the old api goes through this? if so, can you clarify with a TODO that this is duplicated from the original quota handler and the plan is to have that go through this path? src/master/quota_handler.cpp Lines 472 (patched) <https://reviews.apache.org/r/71020/#comment303634> up-to-date s/the// src/master/quota_handler.cpp Lines 485 (patched) <https://reviews.apache.org/r/71020/#comment303637> BadRequest vs Conflict below seems a bit inconsitent? They're both violations of the constraints? - Benjamin Mahler On July 7, 2019, 10:12 p.m., Meng Zhu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71020/ > ----------------------------------------------------------- > > (Updated July 7, 2019, 10:12 p.m.) > > > Review request for mesos and Benjamin Mahler. > > > Bugs: MESOS-9812 > https://issues.apache.org/jira/browse/MESOS-9812 > > > Repository: mesos > > > Description > ------- > > The overcommit check validates that the total quota guarantees in > the cluster is contained by the cluster capacity. > > The hierarchical inclusion check validates that the sum of > children's guarantees is contained by the parent guarantee. > > Further validation is needed for: > > - Check a role's limit is less than its current consumption. > - Check a role's limit is less than its parent's limit. > - Check the sum of children's guarantees are less than its > parent's limit. > > > Diffs > ----- > > src/master/quota_handler.cpp f9c743170461d8d83180db20c917d3842241d4df > > > Diff: https://reviews.apache.org/r/71020/diff/1/ > > > Testing > ------- > > make check > > tests in r/71022/ > > > Thanks, > > Meng Zhu > >
