> On Oct. 26, 2015, 8:27 a.m., Qian Zhang wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 996
> > <https://reviews.apache.org/r/39401/diff/5/?file=1105060#file1105060line996>
> >
> >     These newly added code makes allocate() a huge method (more than 200 
> > lines), maybe move these codes into a separate method?

Absolutely! The reason why it's not done is because we have already planned 
(but not yet scheduled) an allocator refactoring. Let me add a `TODO` for now 
in order to increase the pressure on ourselves ; ).


> On Oct. 26, 2015, 8:27 a.m., Qian Zhang wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 1017-1020
> > <https://reviews.apache.org/r/39401/diff/5/?file=1105060#file1105060line1017>
> >
> >     Why do we put these code inside the framework sorters foreach loop? I 
> > do not see it is related to framework.
> >     If we really want to put these code here, then I think we also need to 
> > recalculate roleAllocatedResources every time when we allocate some 
> > resources to a framework of the role, and once the quota for the role is 
> > satifised, break.

There can be multiple frameworks in a role, hence quota may get satisfied after 
we allocate resources to some frameworks.

> then I think we also need to recalculate roleAllocatedResources every time 
> when we allocate some resources to a framework

But we do that at the end of the loop, right?


> On Oct. 26, 2015, 8:27 a.m., Qian Zhang wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 1055-1057
> > <https://reviews.apache.org/r/39401/diff/5/?file=1105060#file1105060line1055>
> >
> >     Since we know the guarentee of each quota'ed role and the gap between 
> > it and role's current allocation, I think it might be better to do the 
> > "find-grained" allocation, i.e., only allocate resources to fill the gap. 
> > Otherwise, we may run into the situation that we allocate too much resource 
> > to satisfy a role's guarentee but another role's guarentee can not be 
> > satisfied.

Yep, this can be the case and it was a hot debate during the design phase. I 
have decided to choose this approach because it's follows the least surprise 
principle for people who already familiar with DRF. I think we'll have to 
revisit the strategy anyway and (most probably) introduce `Quota.limit`. I 
think this is fine for MVP.


> On Oct. 26, 2015, 8:27 a.m., Qian Zhang wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1058
> > <https://reviews.apache.org/r/39401/diff/5/?file=1105060#file1105060line1058>
> >
> >     We also have this code ```offerable[frameworkId][slaveId] = 
> > resources;``` in the following DRF allocation stage, so that means for a 
> > framework, the resources we allocate to it here will be overwritten by the 
> > the resources we allocate to it in DRF allocation stage? This seems not 
> > correct, maybe change the code in DRF allocation stage from "=" to "+="?

Good catch!


> On Oct. 26, 2015, 8:27 a.m., Qian Zhang wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1097
> > <https://reviews.apache.org/r/39401/diff/5/?file=1105060#file1105060line1097>
> >
> >     If we *continue* from here, that means the following DRF allocation 
> > stage will be skipped? I think that is not what we want. So I guess it 
> > should be:
> >     ```
> >     if (allocatable(resources)) {
> >       // Lay aside the resources.
> >       laidAsideResources[slaveId] = resources;
> >       slaves[slaveId].allocated += resources;
> >     }
> >     ```

I think you're right.


- Alexander


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39401/#review103958
-----------------------------------------------------------


On Oct. 23, 2015, 4:38 p.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39401/
> -----------------------------------------------------------
> 
> (Updated Oct. 23, 2015, 4:38 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3718
>     https://issues.apache.org/jira/browse/MESOS-3718
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 
> 
> Diff: https://reviews.apache.org/r/39401/diff/
> 
> 
> Testing
> -------
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Reply via email to