----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55359/#review164101 -----------------------------------------------------------
LGTM. Just need to take another look after rebasing to ship it. src/master/allocator/mesos/hierarchical.cpp (lines 682 - 683) <https://reviews.apache.org/r/55359/#comment236137> "contained" is ambiguous, I think it's better if we convey the idea of "at least one copy". How about: ``` Ensure that offered resources contain at least one copy of the consumed shared resource (guaranteed by master validation). ``` src/master/allocator/mesos/hierarchical.cpp (lines 696 - 697) <https://reviews.apache.org/r/55359/#comment236123> So `additional` now encompasses multiple launch operations but it feels like it's still valuable to log the taskIds in this call for debugging? src/master/allocator/mesos/hierarchical.cpp (lines 732 - 733) <https://reviews.apache.org/r/55359/#comment236135> So now we have two reasons: ``` The agent's total resources shouldn't contain: 1. The additionally allocated shared resources. 2. `AllocationInfo` as set in `updatedOfferedResources`. ``` Maybe spell them out like this? - Jiang Yan Xu On Feb. 3, 2017, 2:01 p.m., Anindya Sinha wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55359/ > ----------------------------------------------------------- > > (Updated Feb. 3, 2017, 2:01 p.m.) > > > Review request for mesos, Benjamin Mahler and Jiang Yan Xu. > > > Bugs: MESOS-6444 > https://issues.apache.org/jira/browse/MESOS-6444 > > > Repository: mesos > > > Description > ------- > > We handle shared resources for `LAUNCH` operations separately in the > `updateAllocation()` API to add additional copies of shared resources. > But the updates to the allocations in the allocator and sorters > can be consolidated together. > > > Diffs > ----- > > src/master/allocator/mesos/hierarchical.cpp > 5f540569043df9d2bb75416c8c36bb4dd7bd68a1 > > Diff: https://reviews.apache.org/r/55359/diff/ > > > Testing > ------- > > Tests passed. > > > Thanks, > > Anindya Sinha > >
