> On Dec. 19, 2016, 7:46 a.m., Jiang Yan Xu wrote: > > src/master/allocator/mesos/hierarchical.hpp, lines 477-478 > > <https://reviews.apache.org/r/53096/diff/9/?file=1582260#file1582260line477> > > > > Update the comment per the comment about the role of the method. > > > > ``` > > // Helper to update the total resources in the allocator and each of > > the sorters. > > ```
Not each of the sorters but only the role and quota sorters, since the framework sorter's total is a subset of slave's total resources. Correct? > On Dec. 19, 2016, 7:46 a.m., Jiang Yan Xu wrote: > > src/master/allocator/mesos/hierarchical.hpp, line 479 > > <https://reviews.apache.org/r/53096/diff/9/?file=1582260#file1582260line479> > > > > 1. No need for the `_` prefix for private methods. We use it for > > continuations. > > 2. `oldTotal`? Should be the updated total right? i.e., > > > > ``` > > void updateTotal(const SlaveID& slaveId, const Resources& total); > > ``` > > > > I suggested just naming it `total` because I feel intuitively people > > would associate the `total` in a method called `updateTotal` to be "the > > value to update the totals to". What do you think? Well, I was passing the `oldTotal` to be removed, and added `slaves[slaveId].total` (which was updated in the call site). Hence the name `oldTotal`. But I can pass in the new `total` (before updating) and remove `oldTotal` based on `slaves[slaveId].total` and then add new `total`. > On Dec. 19, 2016, 7:46 a.m., Jiang Yan Xu wrote: > > src/master/allocator/mesos/hierarchical.cpp, line 824 > > <https://reviews.apache.org/r/53096/diff/9/?file=1582261#file1582261line824> > > > > Use the same comment: > > > > ``` > > // Update the total resources in the allocator and each of the > > sorters. > > ``` Not all sorters, only the role and quota sorters. - Anindya ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53096/#review159578 ----------------------------------------------------------- On Dec. 12, 2016, 10:35 p.m., Anindya Sinha wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53096/ > ----------------------------------------------------------- > > (Updated Dec. 12, 2016, 10:35 p.m.) > > > Review request for mesos and Jiang Yan Xu. > > > Bugs: MESOS-6444 > https://issues.apache.org/jira/browse/MESOS-6444 > > > Repository: mesos > > > Description > ------- > > We maintain a single copy of shared resource in the role and quota > sorter's total resources. So, when we update these resources, we > remove the previous resources at this agent and add the new resources > at this agent. > > > Diffs > ----- > > src/master/allocator/mesos/hierarchical.hpp > a6424d624864155e1c87a28a63b784512c5c8722 > src/master/allocator/mesos/hierarchical.cpp > a8cbc8db3d1319718765783827f8be1981b56f04 > src/tests/persistent_volume_tests.cpp > b7c313781f3dd9ab4fd68ce23ccc044d50b734d8 > > Diff: https://reviews.apache.org/r/53096/diff/ > > > Testing > ------- > > Tests passed. > > > Thanks, > > Anindya Sinha > >
