----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64303/#review193472 -----------------------------------------------------------
Fix it, then Ship it! Looks good! Thanks for spelling out the plan in the comments. I left a few minor edit suggestions to the comments below, I can take care of these for you and get this committed. src/master/allocator/mesos/hierarchical.hpp Lines 456-461 (patched) <https://reviews.apache.org/r/64303/#comment272030> As a general suggestion, we try to use a newline between comment paragraphs, so for example: ``` // Aggregated resource reservations on all agents tied to a // particular role, if any. These are stripped scalar quantities // that contain no meta-data. Used for accounting resource // reservations for quota limit. // // Only roles with non-empty reservations will be stored in the map. ``` src/master/allocator/mesos/hierarchical.hpp Lines 560 (patched) <https://reviews.apache.org/r/64303/#comment272031> No need for parens? src/master/allocator/mesos/hierarchical.hpp Lines 562-568 (patched) <https://reviews.apache.org/r/64303/#comment272032> Thanks for spelling this out! Maybe a little structure to help readability? ``` // TODO(mzhu): Ideally, we want these helpers to instead track the // reservations as *allocated* in the sorters even when the // reservations have not been allocated yet. This will help to: // // (1) Solve the fairness issue when roles with unallocated // reservations may game the allocator (See MESOS-8299). // // (2) Simplify the quota enforcement logic -- the allocator // would no longer need to track reservations separately. ``` - Benjamin Mahler On Dec. 11, 2017, 4:28 a.m., Meng Zhu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64303/ > ----------------------------------------------------------- > > (Updated Dec. 11, 2017, 4:28 a.m.) > > > Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Michael > Park. > > > Repository: mesos > > > Description > ------- > > Resource reservations need to be tracked to make > sure quota limit will not be exceeded in the presence > of resource reservation. > > > Diffs > ----- > > src/master/allocator/mesos/hierarchical.hpp > aae63962bab1a9b50828875492966d851328ced5 > src/master/allocator/mesos/hierarchical.cpp > 9d8b6714d060f67d570c5653798e705248781869 > > > Diff: https://reviews.apache.org/r/64303/diff/3/ > > > Testing > ------- > > > Thanks, > > Meng Zhu > >
