> On July 4, 2016, 8:01 a.m., Guangya Liu wrote: > > src/master/allocator/sorter/drf/sorter.cpp, lines 272-275 > > <https://reviews.apache.org/r/49376/diff/3/?file=1435880#file1435880line272> > > > > How about adjust the order a bit as following? > > > > CHECK(allocations[name].resources[slaveId].contains(resources)); > > > > const Resources resourcesQuantity = > > resources.createStrippedScalarQuantity(); > > CHECK(allocations[name].scalarQuantities.contains(resourcesQuantity)); > > Neil Conway wrote: > Personally, I prefer grouping the two `CHECKs` together because they are > semantically related. > > Guangya Liu wrote: > I'm ok either way, the only advantage of updating > `CHECK(allocations[name].resources[slaveId].contains(resources));` to be > checked first can avoid the case of `const Resources resourcesQuantity = > resources.createStrippedScalarQuantity();` was calculated but > `CHECK(allocations[name].resources[slaveId].contains(resources));` failed.
True, but I don't think that is worth optimizing for -- if the `CHECK` fails, we're not going to continue executing the program. - Neil ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49376/#review140620 ----------------------------------------------------------- On July 3, 2016, 8:36 a.m., Neil Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49376/ > ----------------------------------------------------------- > > (Updated July 3, 2016, 8:36 a.m.) > > > Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and > Michael Park. > > > Bugs: MESOS-5698 > https://issues.apache.org/jira/browse/MESOS-5698 > > > Repository: mesos > > > Description > ------- > > Added assertions to DRFSorter. > > > Diffs > ----- > > src/master/allocator/sorter/drf/sorter.cpp > 967290d4d1100208900b4b724422c3218abc23cb > > Diff: https://reviews.apache.org/r/49376/diff/ > > > Testing > ------- > > These assertions DO NOT PASS. They are conceptually correct, however -- after > r/49377 they pass on `make check`. > > > Thanks, > > Neil Conway > >
