mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Qian Zhang" <zhang...@cn.ibm.com>
Subject Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.
Date Mon, 26 Oct 2015 08:27:01 GMT

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



src/master/allocator/mesos/hierarchical.cpp (line 996)
<https://reviews.apache.org/r/39401/#comment162066>

    These newly added code makes allocate() a huge method (more than 200 lines), maybe move
these codes into a separate method?



src/master/allocator/mesos/hierarchical.cpp (lines 1017 - 1020)
<https://reviews.apache.org/r/39401/#comment162085>

    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.



src/master/allocator/mesos/hierarchical.cpp (lines 1055 - 1057)
<https://reviews.apache.org/r/39401/#comment162144>

    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.



src/master/allocator/mesos/hierarchical.cpp (line 1058)
<https://reviews.apache.org/r/39401/#comment162132>

    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 "+="?



src/master/allocator/mesos/hierarchical.cpp (line 1097)
<https://reviews.apache.org/r/39401/#comment162129>

    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;
    }
    ```


- Qian Zhang


On Oct. 24, 2015, 12:38 a.m., Alexander Rukletsov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39401/
> -----------------------------------------------------------
> 
> (Updated Oct. 24, 2015, 12:38 a.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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message