Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-11-22 Thread Alexander Rukletsov 

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

(Updated Nov. 22, 2015, 11:55 p.m.)


Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
and Joseph Wu.


Bugs: MESOS-3718
https://issues.apache.org/jira/browse/MESOS-3718


Repository: mesos


Description
---

See summary.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
14fef63714721fcda7cea3c28704766efda6d007 

Diff: https://reviews.apache.org/r/39401/diff/


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-11-22 Thread Mesos ReviewBot 

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


Bad patch!

Reviews applied: [39401]

Failed command: ./support/apply-review.sh -n -r 39401

Error:
 2015-11-23 00:20:07 URL:https://reviews.apache.org/r/39401/diff/raw/ 
[8039/8039] -> "39401.patch" [1]
error: patch failed: src/master/allocator/mesos/hierarchical.cpp:988
error: src/master/allocator/mesos/hierarchical.cpp: patch does not apply
Failed to apply patch

- Mesos ReviewBot


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

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-11-14 Thread Alexander Rukletsov 


> On Oct. 26, 2015, 1:49 p.m., Qian Zhang wrote:
> > For this patch, it seems that we add the code related to quota support in 
> > the slave foreach loop in the HierarchicalAllocatorProcess::allocate(const 
> > hashset& slaveIds_) method, so that means for **each slave**, we 
> > handle quota first and then the existing DRF fair share. I think there 
> > might be an issue for this approach: let say for the first slave, its 
> > available unreserved non-revocable resources can not satisfy a role’s quota 
> > due to the framework in this role has a filter for this slave, and then we 
> > lay aside the filtered resources of this slave for this role immediately. I 
> > think it might be too early for doing this since the other slaves may have 
> > resources which can satisfy this role’s quota. But if we lay aside this 
> > slave's resource for this role at this point, then the result is the 
> > framework of this role will not use these resources (due to the filter) AND 
> > all other role’s frameworks can not be offered with these resources too, 
> > this is kind of wasting resource
 s.
> > 
> > I think maybe we can handle this quota support in this way: In 
> > HierarchicalAllocatorProcess::allocate(const hashset& slaveIds_), 
> > leave the existing 3 levels foreach loops (slave/role/framework) as they 
> > are, and add the quota related code separately before them in this way: 
> > traverse all quota’ed roles, for each of them, traverse all the slaves, and 
> > allocate each slave’s available unreserved non-revocable resources to the 
> > role’s framework (take filter and suppress into account) until the role’s 
> > quota is satisfied. After all the quota’ed role has been traversed, if 
> > there are still some role’s quotas are not satisfied, then lay aside 
> > resources (should be the resources filtered or suppressed) for them. In 
> > this way, before laying aside resources, we have tried our best to use all 
> > slave's the available resources to satisfy the quotas first, there should 
> > be less resources wasted.
> 
> Alexander Rukletsov wrote:
> I'm not sure I got your point. If my mental compiler is correct, if a 
> framework in quota'ed role opts out, we do not immediately lay aside 
> resources. We do that after we have checked all the frameworks in the role in 
> a separate loop.
> 
> Qian Zhang wrote:
> Let me clarify my point with an example:
> Say in the Mesos cluster, there are 2 agents, a1 and a2, each has 4GB 
> memory. And there are 2 roles, r1 and r2, r1 has a quota set (4GB) and r2 has 
> not quota set. r1 has a framework f1 which currenlty has no allocation but 
> has a filter (4GB memory on a1), r2 also has a framework f2 which currently 
> has no allocation too and has no filter. And there is no static/dynamic 
> reservation and no revocable resources. Now with the logic in this patch, for 
> a1, in the quotaRoleSorter foreach loop, when we handle the quota for r1, we 
> will not allocate a1's resouces to f1 because f1 has a filter, so a1's 4GB 
> memory will be laid aside to satisfy r1's quota. And then in the roleSorter 
> foreach loop, we will NOT allocate a1's resources to f2 too since currently 
> a1 has no available resources due to all its 4GB memory has been laid aside 
> for r1. And then when we handle a2, its 4GB memory will be allocated to f1, 
> so f2 will not get anything in the end. So the result is, a1's 4GB memory is 
> laid aside
  to satisfy r1's quota, a2's 4GB is allocated to f1, and f2 gets nothing. But 
I think for this example, the expected result should be, f1 gets a2's 4GB (r1's 
quota is also satisfied) and f2 gets a1's 4GB.
> 
> Alexander Rukletsov wrote:
> This can happen during the allocation cycle, but we do not persist laid 
> aside resources between allocation cycles. Without refactoring `allocate()` 
> we do not know whether we get a suitable agent, hence we have to lay aside. 
> But at the next allocation cycle, `r1`'s quota is satisfied and `f2` will get 
> `a1`'s 4GB, which is OK in my opinion.
> 
> Qian Zhang wrote:
> Yes, I understand ```f2``` will get 4GB at the ***next*** allocation 
> cycle. But with the proposal in my first post, in a ***single*** allocation 
> cycle, both ```f1``` and ```f2``` can get 4GB respectively because when we 
> find we can not allocate ```a1```'s 4GB to f1 due to the filter, we will NOT 
> lay aside resources at this point, instead we will try ```a2``` and allocate 
> ```a2```'s 4GB to f1, and then when we handle the fair share, we will 
> allocate ```a2```'s 4GB to ```f2```. I think this proposal is also aligned 
> with the principle mentioned in the design doc: quota first, fair share 
> second. My understanding to this design principle is, we handle all role's 
> quota for all slaves first, and then handle all role's fair share for all 
> slaves (my proposal), rather than for ***each slave*** handle all role's 
> quota and then all role's fair share (this patch).
> 
> Alexander Rukletsov

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-11-10 Thread Qian Zhang 


> On Nov. 1, 2015, 8:14 p.m., Qian Zhang wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1005
> > 
> >
> > For this TODO, what do we plan to do in future? Include the dynamic 
> > reserved resources for this role on this agent in 
> > ```roleConsumedResources```? And what about the static reserved resources?
> 
> Alexander Rukletsov wrote:
> Dynamic reservations should account towards role's quota. [Note about 
> static 
> reservations](https://docs.google.com/document/d/16iRNmziasEjVOblYp5bbkeBZ7pnjNlaIzPQqMTHQ-9I/edit?pli=1#heading=h.xumvch9xiky2)
> 
> Qian Zhang wrote:
> So when we check if a role's quota is satisfied or not, we will add the 
> role's allocated resources with the role's dynamically reserved resources, 
> and check if the sum contains the role's quota. But for role's statically 
> reserved resources, we will consider they are part of quota. So in future 
> (after these TODOs are handled) when we check if a role's quota is satified 
> or not, the formula should be ```(role's allocated resource + role's 
> dynamically reserved resources) > (role's quota + role's statically reserved 
> resources)```, right?

Alex, any comments for my question above? :-)


- Qian


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


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

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-11-10 Thread Joerg Schad 

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



src/master/allocator/mesos/hierarchical.cpp (line 984)


laidAside are rejected resources? In that case I would prefer 
rejectedResources.



src/master/allocator/mesos/hierarchical.cpp (line 997)


s/comes first/is satisfied first



src/master/allocator/mesos/hierarchical.cpp (line 1003)


We can sum up resources as quota is only applicable to scalar resources.



src/master/allocator/mesos/hierarchical.cpp (line 1006)


Wouldn't it make sense to have a flag indicating when quota ia satisfied 
for the first time in this loop (i.e. foreach slave)? Otherwise we have to 
recheck this for every agent in the cluster


- Joerg Schad


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

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-11-10 Thread Joris Van Remoortere 


> On Oct. 26, 2015, 8:27 a.m., Qian Zhang wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 1055-1057
> > 
> >
> > 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.
> 
> Alexander Rukletsov wrote:
> 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.

We also discussed this during the Community Sync on Nov 5th. Coarse-grained is 
sufficient for the MVP, and we can look at the fine-grained implications as 
parth of the next design doc.


- Joris


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


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

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-11-09 Thread Alexander Rukletsov 


> On Oct. 25, 2015, 1:39 p.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1035
> > 
> >
> > I know that we have design to exclue the reserved resource from quota, 
> > but why not include the current role's reserved as available?
> 
> Alexander Rukletsov wrote:
> If you mean dynamically reserved resources than yes, we can do that. If 
> you mean statically reserved than nope, we decided not to conflate them with 
> quota.
> 
> Guangya Liu wrote:
> I mean both static and dynamic reservation, I think that both of the 
> reservations for one role should be included in the role's quota, can you 
> pleaes clairify why not include static reservation in one role's quota?
> 
> Alexander Rukletsov wrote:
> Does 
> [this](https://docs.google.com/document/d/16iRNmziasEjVOblYp5bbkeBZ7pnjNlaIzPQqMTHQ-9I/edit?pli=1#heading=h.xumvch9xiky2)
>  answer your question?
> 
> Guangya Liu wrote:
> I see that the design document want to use the last one "Static 
> reservations are static (implicit) part of quota, operators cannot change 
> them via quota mechanisms.", my understanding is that the static reservations 
> will be treated as the implicit part of quota, right?

Yes, which means we do *not* take them into account in the quota part of the 
allocator, but we plan to show them in the quota `get` endpoint in order to 
help operators understand reservations.


- Alexander


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


On Nov. 5, 2015, 6:25 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39401/
> ---
> 
> (Updated Nov. 5, 2015, 6:25 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> and Joseph Wu.
> 
> 
> 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
> 
>

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-11-09 Thread Alexander Rukletsov 

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

(Updated Nov. 9, 2015, 10:15 a.m.)


Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
and Joseph Wu.


Changes
---

Improved naming.


Bugs: MESOS-3718
https://issues.apache.org/jira/browse/MESOS-3718


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  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

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-11-07 Thread Klaus Ma 


> On Oct. 26, 2015, 10:07 p.m., Klaus Ma wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1004
> > 
> >
> > Just check the code, dynamically reserved resource are included in 
> > allocation.
> 
> Qian Zhang wrote:
> Really?:-) I think once a resource is dynamically reserved, it will be 
> removed from the framework's allocation and marked as reserved in slave's 
> total resources, so that in next allocation cycle it can be offered to the 
> framework of the role which reserves it.
> 
> Alexander Rukletsov wrote:
> A reserved resource can be allocated or not. For example, if a framework 
> declines a reserved resource, it will be removed from allocated.

Thanks, @AlexR; got your point.


- Klaus


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


On Nov. 6, 2015, 2:25 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39401/
> ---
> 
> (Updated Nov. 6, 2015, 2:25 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Joerg Schad, Joris Van Remoortere, 
> and Joseph Wu.
> 
> 
> 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
> 
>

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-11-03 Thread Qian Zhang 


> On Nov. 1, 2015, 8:14 p.m., Qian Zhang wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1005
> > 
> >
> > For this TODO, what do we plan to do in future? Include the dynamic 
> > reserved resources for this role on this agent in 
> > ```roleConsumedResources```? And what about the static reserved resources?
> 
> Alexander Rukletsov wrote:
> Dynamic reservations should account towards role's quota. [Note about 
> static 
> reservations](https://docs.google.com/document/d/16iRNmziasEjVOblYp5bbkeBZ7pnjNlaIzPQqMTHQ-9I/edit?pli=1#heading=h.xumvch9xiky2)

So when we check if a role's quota is satisfied or not, we will add the role's 
allocated resources with the role's dynamically reserved resources, and check 
if the sum contains the role's quota. But for role's statically reserved 
resources, we will consider they are part of quota. So in future (after these 
TODOs are handled) when we check if a role's quota is satified or not, the 
formula should be ```(role's allocated resource + role's dynamically reserved 
resources) > (role's quota + role's statically reserved resources)```, right?


- Qian


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


On Nov. 2, 2015, 11:16 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39401/
> ---
> 
> (Updated Nov. 2, 2015, 11:16 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
> 
>

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-11-02 Thread Alexander Rukletsov 


> On Oct. 25, 2015, 1:39 p.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1035
> > 
> >
> > I know that we have design to exclue the reserved resource from quota, 
> > but why not include the current role's reserved as available?
> 
> Alexander Rukletsov wrote:
> If you mean dynamically reserved resources than yes, we can do that. If 
> you mean statically reserved than nope, we decided not to conflate them with 
> quota.
> 
> Guangya Liu wrote:
> I mean both static and dynamic reservation, I think that both of the 
> reservations for one role should be included in the role's quota, can you 
> pleaes clairify why not include static reservation in one role's quota?

Does 
[this](https://docs.google.com/document/d/16iRNmziasEjVOblYp5bbkeBZ7pnjNlaIzPQqMTHQ-9I/edit?pli=1#heading=h.xumvch9xiky2)
 answer your question?


- Alexander


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


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

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-11-02 Thread Alexander Rukletsov 


> On Nov. 1, 2015, 12:14 p.m., Qian Zhang wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1005
> > 
> >
> > For this TODO, what do we plan to do in future? Include the dynamic 
> > reserved resources for this role on this agent in 
> > ```roleConsumedResources```? And what about the static reserved resources?

Dynamic reservations should account towards role's quota. [Note about static 
reservations](https://docs.google.com/document/d/16iRNmziasEjVOblYp5bbkeBZ7pnjNlaIzPQqMTHQ-9I/edit?pli=1#heading=h.xumvch9xiky2)


> On Nov. 1, 2015, 12:14 p.m., Qian Zhang wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1035
> > 
> >
> > Can you please clarify a bit about this TODO? What do we plan to do 
> > about it in future?

It may make sense to allocate reserved but yet unallocated resources during as 
part of the role's quota.


- Alexander


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


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

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-11-02 Thread Guangya Liu 


> On Oct. 25, 2015, 1:39 p.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1035
> > 
> >
> > I know that we have design to exclue the reserved resource from quota, 
> > but why not include the current role's reserved as available?
> 
> Alexander Rukletsov wrote:
> If you mean dynamically reserved resources than yes, we can do that. If 
> you mean statically reserved than nope, we decided not to conflate them with 
> quota.
> 
> Guangya Liu wrote:
> I mean both static and dynamic reservation, I think that both of the 
> reservations for one role should be included in the role's quota, can you 
> pleaes clairify why not include static reservation in one role's quota?
> 
> Alexander Rukletsov wrote:
> Does 
> [this](https://docs.google.com/document/d/16iRNmziasEjVOblYp5bbkeBZ7pnjNlaIzPQqMTHQ-9I/edit?pli=1#heading=h.xumvch9xiky2)
>  answer your question?

I see that the design document want to use the last one "Static reservations 
are static (implicit) part of quota, operators cannot change them via quota 
mechanisms.", my understanding is that the static reservations will be treated 
as the implicit part of quota, right?


- Guangya


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


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

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-11-01 Thread Qian Zhang 

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



src/master/allocator/mesos/hierarchical.cpp (line 1005)


For this TODO, what do we plan to do in future? Include the dynamic 
reserved resources for this role on this agent in ```roleConsumedResources```? 
And what about the static reserved resources?



src/master/allocator/mesos/hierarchical.cpp (line 1035)


Can you please clarify a bit about this TODO? What do we plan to do about 
it in future?


- Qian Zhang


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

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-11-01 Thread Guangya Liu 


> On 十月 25, 2015, 1:39 p.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1035
> > 
> >
> > I know that we have design to exclue the reserved resource from quota, 
> > but why not include the current role's reserved as available?
> 
> Alexander Rukletsov wrote:
> If you mean dynamically reserved resources than yes, we can do that. If 
> you mean statically reserved than nope, we decided not to conflate them with 
> quota.

I mean both static and dynamic reservation, I think that both of the 
reservations for one role should be included in the role's quota, can you 
pleaes clairify why not include static reservation in one role's quota?


> On 十月 25, 2015, 1:39 p.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1058
> > 
> >
> > Where does those offerable resource offered to master?
> 
> Alexander Rukletsov wrote:
> 
> https://github.com/apache/mesos/blob/master/src/master/allocator/mesos/hierarchical.cpp#L956
> 
> Dropping the issue, feel free to reopen if I haven't answered your 
> question.

I did not see that those resources are offered to master via the offer 
callback, but I see that you already updated code diff to resolve this. ;-)


> On 十月 25, 2015, 1:39 p.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 982
> > 
> >
> > s/unsatisfiedRoleQuotas/unAllocatedRoleQuotas?
> 
> Alexander Rukletsov wrote:
> Dynamic reservations may not be allocated, but should still count towards 
> quota.

But this is actually not allocated role quota, can we use a more meaningful 
name? The name of `unsatisfiedRoleQuotas` is more likely to be the value that 
the role quota needed to fill in its quota guarantee value. Comments?


- Guangya


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


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

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-10-29 Thread Alexander Rukletsov 


> On Oct. 26, 2015, 8:27 a.m., Qian Zhang wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 1017-1020
> > 
> >
> > 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.
> 
> Alexander Rukletsov wrote:
> 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?
> 
> Qian Zhang wrote:
> I do not see we recalculate roleAllocatedResources in the 
> frameworkSorters foreach loop, actually we do that outside of that loop (at 
> the end of the quotaRoleSorter foreach loop), that means, even we allocate 
> resources to some frameworks in the frameworkSorters foreach loop, the 
> variable roleAllocatedResources will NOT be recalculated. That's why I 
> suggested to recalculate roleAllocatedResources every time when we allocate 
> some resources to a framework :-)

I see your point. I'll change the behaviour.


- Alexander


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


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

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-10-29 Thread Alexander Rukletsov 

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

(Updated Oct. 29, 2015, 7:29 p.m.)


Review request for mesos, Bernd Mathiske, Joerg Schad, and Joris Van Remoortere.


Changes
---

Addressed some comments.


Bugs: MESOS-3718
https://issues.apache.org/jira/browse/MESOS-3718


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  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

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-10-29 Thread Alexander Rukletsov 


> On Oct. 26, 2015, 1:49 p.m., Qian Zhang wrote:
> > For this patch, it seems that we add the code related to quota support in 
> > the slave foreach loop in the HierarchicalAllocatorProcess::allocate(const 
> > hashset& slaveIds_) method, so that means for **each slave**, we 
> > handle quota first and then the existing DRF fair share. I think there 
> > might be an issue for this approach: let say for the first slave, its 
> > available unreserved non-revocable resources can not satisfy a role’s quota 
> > due to the framework in this role has a filter for this slave, and then we 
> > lay aside the filtered resources of this slave for this role immediately. I 
> > think it might be too early for doing this since the other slaves may have 
> > resources which can satisfy this role’s quota. But if we lay aside this 
> > slave's resource for this role at this point, then the result is the 
> > framework of this role will not use these resources (due to the filter) AND 
> > all other role’s frameworks can not be offered with these resources too, 
> > this is kind of wasting resource
 s.
> > 
> > I think maybe we can handle this quota support in this way: In 
> > HierarchicalAllocatorProcess::allocate(const hashset& slaveIds_), 
> > leave the existing 3 levels foreach loops (slave/role/framework) as they 
> > are, and add the quota related code separately before them in this way: 
> > traverse all quota’ed roles, for each of them, traverse all the slaves, and 
> > allocate each slave’s available unreserved non-revocable resources to the 
> > role’s framework (take filter and suppress into account) until the role’s 
> > quota is satisfied. After all the quota’ed role has been traversed, if 
> > there are still some role’s quotas are not satisfied, then lay aside 
> > resources (should be the resources filtered or suppressed) for them. In 
> > this way, before laying aside resources, we have tried our best to use all 
> > slave's the available resources to satisfy the quotas first, there should 
> > be less resources wasted.
> 
> Alexander Rukletsov wrote:
> I'm not sure I got your point. If my mental compiler is correct, if a 
> framework in quota'ed role opts out, we do not immediately lay aside 
> resources. We do that after we have checked all the frameworks in the role in 
> a separate loop.
> 
> Qian Zhang wrote:
> Let me clarify my point with an example:
> Say in the Mesos cluster, there are 2 agents, a1 and a2, each has 4GB 
> memory. And there are 2 roles, r1 and r2, r1 has a quota set (4GB) and r2 has 
> not quota set. r1 has a framework f1 which currenlty has no allocation but 
> has a filter (4GB memory on a1), r2 also has a framework f2 which currently 
> has no allocation too and has no filter. And there is no static/dynamic 
> reservation and no revocable resources. Now with the logic in this patch, for 
> a1, in the quotaRoleSorter foreach loop, when we handle the quota for r1, we 
> will not allocate a1's resouces to f1 because f1 has a filter, so a1's 4GB 
> memory will be laid aside to satisfy r1's quota. And then in the roleSorter 
> foreach loop, we will NOT allocate a1's resources to f2 too since currently 
> a1 has no available resources due to all its 4GB memory has been laid aside 
> for r1. And then when we handle a2, its 4GB memory will be allocated to f1, 
> so f2 will not get anything in the end. So the result is, a1's 4GB memory is 
> laid aside
  to satisfy r1's quota, a2's 4GB is allocated to f1, and f2 gets nothing. But 
I think for this example, the expected result should be, f1 gets a2's 4GB (r1's 
quota is also satisfied) and f2 gets a1's 4GB.
> 
> Alexander Rukletsov wrote:
> This can happen during the allocation cycle, but we do not persist laid 
> aside resources between allocation cycles. Without refactoring `allocate()` 
> we do not know whether we get a suitable agent, hence we have to lay aside. 
> But at the next allocation cycle, `r1`'s quota is satisfied and `f2` will get 
> `a1`'s 4GB, which is OK in my opinion.
> 
> Qian Zhang wrote:
> Yes, I understand ```f2``` will get 4GB at the ***next*** allocation 
> cycle. But with the proposal in my first post, in a ***single*** allocation 
> cycle, both ```f1``` and ```f2``` can get 4GB respectively because when we 
> find we can not allocate ```a1```'s 4GB to f1 due to the filter, we will NOT 
> lay aside resources at this point, instead we will try ```a2``` and allocate 
> ```a2```'s 4GB to f1, and then when we handle the fair share, we will 
> allocate ```a2```'s 4GB to ```f2```. I think this proposal is also aligned 
> with the principle mentioned in the design doc: quota first, fair share 
> second. My understanding to this design principle is, we handle all role's 
> quota for all slaves first, and then handle all role's fair share for all 
> slaves (my proposal), rather than for ***each slave*** handle all role's 
> quota and then all role's fair share (this patch).

Design doc addresses the

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-10-27 Thread Qian Zhang 


> On Oct. 26, 2015, 4:27 p.m., Qian Zhang wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 1017-1020
> > 
> >
> > 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.
> 
> Alexander Rukletsov wrote:
> 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?

I do not see we recalculate roleAllocatedResources in the frameworkSorters 
foreach loop, actually we do that outside of that loop (at the end of the 
quotaRoleSorter foreach loop), that means, even we allocate resources to some 
frameworks in the frameworkSorters foreach loop, the variable 
roleAllocatedResources will NOT be recalculated. That's why I suggested to 
recalculate roleAllocatedResources every time when we allocate some resources 
to a framework :-)


> On Oct. 26, 2015, 4:27 p.m., Qian Zhang wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 996
> > 
> >
> > These newly added code makes allocate() a huge method (more than 200 
> > lines), maybe move these codes into a separate method?
> 
> Alexander Rukletsov wrote:
> 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 ; ).

Agree :-)


- Qian


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


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
> 
>

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-10-27 Thread Qian Zhang 


> On Oct. 26, 2015, 10:07 p.m., Klaus Ma wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1004
> > 
> >
> > Just check the code, dynamically reserved resource are included in 
> > allocation.

Really?:-) I think once a resource is dynamically reserved, it will be removed 
from the framework's allocation and marked as reserved in slave's total 
resources, so that in next allocation cycle it can be offered to the framework 
of the role which reserves it.


- Qian


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


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
> 
>

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-10-27 Thread Qian Zhang 


> On Oct. 26, 2015, 9:49 p.m., Qian Zhang wrote:
> > For this patch, it seems that we add the code related to quota support in 
> > the slave foreach loop in the HierarchicalAllocatorProcess::allocate(const 
> > hashset& slaveIds_) method, so that means for **each slave**, we 
> > handle quota first and then the existing DRF fair share. I think there 
> > might be an issue for this approach: let say for the first slave, its 
> > available unreserved non-revocable resources can not satisfy a role’s quota 
> > due to the framework in this role has a filter for this slave, and then we 
> > lay aside the filtered resources of this slave for this role immediately. I 
> > think it might be too early for doing this since the other slaves may have 
> > resources which can satisfy this role’s quota. But if we lay aside this 
> > slave's resource for this role at this point, then the result is the 
> > framework of this role will not use these resources (due to the filter) AND 
> > all other role’s frameworks can not be offered with these resources too, 
> > this is kind of wasting resource
 s.
> > 
> > I think maybe we can handle this quota support in this way: In 
> > HierarchicalAllocatorProcess::allocate(const hashset& slaveIds_), 
> > leave the existing 3 levels foreach loops (slave/role/framework) as they 
> > are, and add the quota related code separately before them in this way: 
> > traverse all quota’ed roles, for each of them, traverse all the slaves, and 
> > allocate each slave’s available unreserved non-revocable resources to the 
> > role’s framework (take filter and suppress into account) until the role’s 
> > quota is satisfied. After all the quota’ed role has been traversed, if 
> > there are still some role’s quotas are not satisfied, then lay aside 
> > resources (should be the resources filtered or suppressed) for them. In 
> > this way, before laying aside resources, we have tried our best to use all 
> > slave's the available resources to satisfy the quotas first, there should 
> > be less resources wasted.
> 
> Alexander Rukletsov wrote:
> I'm not sure I got your point. If my mental compiler is correct, if a 
> framework in quota'ed role opts out, we do not immediately lay aside 
> resources. We do that after we have checked all the frameworks in the role in 
> a separate loop.

Let me clarify my point with an example:
Say in the Mesos cluster, there are 2 agents, a1 and a2, each has 4GB memory. 
And there are 2 roles, r1 and r2, r1 has a quota set (4GB) and r2 has not quota 
set. r1 has a framework f1 which currenlty has no allocation but has a filter 
(4GB memory on a1), r2 also has a framework f2 which currently has no 
allocation too and has no filter. And there is no static/dynamic reservation 
and no revocable resources. Now with the logic in this patch, for a1, in the 
quotaRoleSorter foreach loop, when we handle the quota for r1, we will not 
allocate a1's resouces to f1 because f1 has a filter, so a1's 4GB memory will 
be laid aside to satisfy r1's quota. And then in the roleSorter foreach loop, 
we will NOT allocate a1's resources to f2 too since currently a1 has no 
available resources due to all its 4GB memory has been laid aside for r1. And 
then when we handle a2, its 4GB memory will be allocated to f1, so f2 will not 
get anything in the end. So the result is, a1's 4GB memory is laid aside to sa
 tisfy r1's quota, a2's 4GB is allocated to f1, and f2 gets nothing. But I 
think for this example, the expected result should be, f1 gets a2's 4GB (r1's 
quota is also satisfied) and f2 gets a1's 4GB.


- Qian


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


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
> 
>

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-10-27 Thread Alexander Rukletsov 

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

(Updated Oct. 27, 2015, 7:27 p.m.)


Review request for mesos, Bernd Mathiske, Joerg Schad, and Joris Van Remoortere.


Changes
---

Addressed comments.


Bugs: MESOS-3718
https://issues.apache.org/jira/browse/MESOS-3718


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  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

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-10-27 Thread Alexander Rukletsov 


> On Oct. 26, 2015, 1:49 p.m., Qian Zhang wrote:
> > For this patch, it seems that we add the code related to quota support in 
> > the slave foreach loop in the HierarchicalAllocatorProcess::allocate(const 
> > hashset& slaveIds_) method, so that means for **each slave**, we 
> > handle quota first and then the existing DRF fair share. I think there 
> > might be an issue for this approach: let say for the first slave, its 
> > available unreserved non-revocable resources can not satisfy a role’s quota 
> > due to the framework in this role has a filter for this slave, and then we 
> > lay aside the filtered resources of this slave for this role immediately. I 
> > think it might be too early for doing this since the other slaves may have 
> > resources which can satisfy this role’s quota. But if we lay aside this 
> > slave's resource for this role at this point, then the result is the 
> > framework of this role will not use these resources (due to the filter) AND 
> > all other role’s frameworks can not be offered with these resources too, 
> > this is kind of wasting resource
 s.
> > 
> > I think maybe we can handle this quota support in this way: In 
> > HierarchicalAllocatorProcess::allocate(const hashset& slaveIds_), 
> > leave the existing 3 levels foreach loops (slave/role/framework) as they 
> > are, and add the quota related code separately before them in this way: 
> > traverse all quota’ed roles, for each of them, traverse all the slaves, and 
> > allocate each slave’s available unreserved non-revocable resources to the 
> > role’s framework (take filter and suppress into account) until the role’s 
> > quota is satisfied. After all the quota’ed role has been traversed, if 
> > there are still some role’s quotas are not satisfied, then lay aside 
> > resources (should be the resources filtered or suppressed) for them. In 
> > this way, before laying aside resources, we have tried our best to use all 
> > slave's the available resources to satisfy the quotas first, there should 
> > be less resources wasted.
> 
> Alexander Rukletsov wrote:
> I'm not sure I got your point. If my mental compiler is correct, if a 
> framework in quota'ed role opts out, we do not immediately lay aside 
> resources. We do that after we have checked all the frameworks in the role in 
> a separate loop.
> 
> Qian Zhang wrote:
> Let me clarify my point with an example:
> Say in the Mesos cluster, there are 2 agents, a1 and a2, each has 4GB 
> memory. And there are 2 roles, r1 and r2, r1 has a quota set (4GB) and r2 has 
> not quota set. r1 has a framework f1 which currenlty has no allocation but 
> has a filter (4GB memory on a1), r2 also has a framework f2 which currently 
> has no allocation too and has no filter. And there is no static/dynamic 
> reservation and no revocable resources. Now with the logic in this patch, for 
> a1, in the quotaRoleSorter foreach loop, when we handle the quota for r1, we 
> will not allocate a1's resouces to f1 because f1 has a filter, so a1's 4GB 
> memory will be laid aside to satisfy r1's quota. And then in the roleSorter 
> foreach loop, we will NOT allocate a1's resources to f2 too since currently 
> a1 has no available resources due to all its 4GB memory has been laid aside 
> for r1. And then when we handle a2, its 4GB memory will be allocated to f1, 
> so f2 will not get anything in the end. So the result is, a1's 4GB memory is 
> laid aside
  to satisfy r1's quota, a2's 4GB is allocated to f1, and f2 gets nothing. But 
I think for this example, the expected result should be, f1 gets a2's 4GB (r1's 
quota is also satisfied) and f2 gets a1's 4GB.

This can happen during the allocation cycle, but we do not persist laid aside 
resources between allocation cycles. Without refactoring `allocate()` we do not 
know whether we get a suitable agent, hence we have to lay aside. But at the 
next allocation cycle, `r1`'s quota is satisfied and `f2` will get `a1`'s 4GB, 
which is OK in my opinion.


- Alexander


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


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

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-10-27 Thread Alexander Rukletsov 


> On Oct. 26, 2015, 2:07 p.m., Klaus Ma wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1004
> > 
> >
> > Just check the code, dynamically reserved resource are included in 
> > allocation.
> 
> Qian Zhang wrote:
> Really?:-) I think once a resource is dynamically reserved, it will be 
> removed from the framework's allocation and marked as reserved in slave's 
> total resources, so that in next allocation cycle it can be offered to the 
> framework of the role which reserves it.

A reserved resource can be allocated or not. For example, if a framework 
declines a reserved resource, it will be removed from allocated.


- Alexander


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


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

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-10-26 Thread Qian Zhang 

---
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)


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)


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)


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)


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)


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
> 
>

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-10-26 Thread Klaus Ma 

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



src/master/allocator/mesos/hierarchical.cpp (line 1004)


Just check the code, dynamically reserved resource are included in 
allocation.


- Klaus Ma


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
> 
>

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-10-26 Thread Qian Zhang 

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


For this patch, it seems that we add the code related to quota support in the 
slave foreach loop in the HierarchicalAllocatorProcess::allocate(const 
hashset& slaveIds_) method, so that means for **each slave**, we 
handle quota first and then the existing DRF fair share. I think there might be 
an issue for this approach: let say for the first slave, its available 
unreserved non-revocable resources can not satisfy a role’s quota due to the 
framework in this role has a filter for this slave, and then we lay aside the 
filtered resources of this slave for this role immediately. I think it might be 
too early for doing this since the other slaves may have resources which can 
satisfy this role’s quota. But if we lay aside this slave's resource for this 
role at this point, then the result is the framework of this role will not use 
these resources (due to the filter) AND all other role’s frameworks can not be 
offered with these resources too, this is kind of wasting resources.

I think maybe we can handle this quota support in this way: In 
HierarchicalAllocatorProcess::allocate(const hashset& slaveIds_), 
leave the existing 3 levels foreach loops (slave/role/framework) as they are, 
and add the quota related code separately before them in this way: traverse all 
quota’ed roles, for each of them, traverse all the slaves, and allocate each 
slave’s available unreserved non-revocable resources to the role’s framework 
(take filter and suppress into account) until the role’s quota is satisfied. 
After all the quota’ed role has been traversed, if there are still some role’s 
quotas are not satisfied, then lay aside resources (should be the resources 
filtered or suppressed) for them. In this way, before laying aside resources, 
we have tried our best to use all slave's the available resources to satisfy 
the quotas first, there should be less resources wasted.

- 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
> 
>

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-10-26 Thread Qian Zhang 

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



src/master/allocator/mesos/hierarchical.cpp (line 1156)


I think we need to check if this is a quota'ed role first.


- 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
> 
>

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-10-26 Thread Klaus Ma 

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



src/master/allocator/mesos/hierarchical.cpp (line 1018)


Can we move this out of the for loop? if the role is satisfied, framewor 
sort is not necessary.



src/master/allocator/mesos/hierarchical.cpp (line 1056)


s/overcimmittment/overcommittment



src/master/allocator/mesos/hierarchical.cpp (line 1094)


`allocable(...)` checked whether resource is enough to allocate (cpu > 
MIN_CPU && men > MIN_MEN). So if the resources can not allocate here, is also 
can not allocate in DRF stage.



src/master/allocator/mesos/hierarchical.cpp (line 1101)


My understading of this code is to lay aside resources for unsatisfaied 
Quota in next allocation cycle, right?
If so, I think we can move this out to another `foreach slaveIds` loop, so 
we did not need to lay aside resources.


- Klaus Ma


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
> 
>

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-10-26 Thread Artem Harutyunyan 

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



src/master/allocator/mesos/hierarchical.cpp (line 998)


s/dedicatedsorter/dedicated sorter/



src/master/allocator/mesos/hierarchical.cpp (line 1015)


s/sort/sorts/?



src/master/allocator/mesos/hierarchical.cpp (line 1026)


s/the/that/?



src/master/allocator/mesos/hierarchical.cpp (lines 1044 - 1046)


s/the/that/

Seems like a copy of the comment above, is this intentional?



src/master/allocator/mesos/hierarchical.cpp (line 1056)


s/overcimmittment/overcommitment of/

or 

s/overcimmittment/overcommitting/



src/master/allocator/mesos/hierarchical.cpp (line 1057)


s/guarantee/a guarantee/

It'd be great to expand this comment a bit and describe what the gurantee 
represents.


- Artem Harutyunyan


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

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-10-26 Thread Alexander Rukletsov 


> On Oct. 26, 2015, 1:51 p.m., Artem Harutyunyan wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 1044-1046
> > 
> >
> > s/the/that/
> > 
> > Seems like a copy of the comment above, is this intentional?

Yep, in both cases we have to laid aside resources.


> On Oct. 26, 2015, 1:51 p.m., Artem Harutyunyan wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1057
> > 
> >
> > s/guarantee/a guarantee/
> > 
> > It'd be great to expand this comment a bit and describe what the 
> > gurantee represents.

Do you think this is a good place for that?


- Alexander


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


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
> 
>

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-10-26 Thread Alexander Rukletsov 


> On Oct. 26, 2015, 8:27 a.m., Qian Zhang wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 996
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> > 
> >
> > 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
> 
>

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-10-26 Thread Alexander Rukletsov 


> On Oct. 26, 2015, 2 p.m., Klaus Ma wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1018
> > 
> >
> > Can we move this out of the for loop? if the role is satisfied, 
> > framewor sort is not necessary.

And what if it is not but it got satisfied after we allocated resources to 
some—but not all!—frameworks in this role?


> On Oct. 26, 2015, 2 p.m., Klaus Ma wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1094
> > 
> >
> > `allocable(...)` checked whether resource is enough to allocate (cpu > 
> > MIN_CPU && men > MIN_MEN). So if the resources can not allocate here, is 
> > also can not allocate in DRF stage.

But `resources` may differ: during DRF a framework may get some revocable 
resources which may render the whole offer allocatable.


> On Oct. 26, 2015, 2 p.m., Klaus Ma wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1101
> > 
> >
> > My understading of this code is to lay aside resources for unsatisfaied 
> > Quota in next allocation cycle, right?
> > If so, I think we can move this out to another `foreach slaveIds` loop, 
> > so we did not need to lay aside resources.

Im not sure I got your proposal. We should bookkeep resources that are 
"reserved" or "laid aside" as part of role's quota in order not to offer them 
to anybody else as non-revocable. However, we may want to offer them as 
revocable offers.


- Alexander


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


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
> 
>

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-10-26 Thread Alexander Rukletsov 


> On Oct. 26, 2015, 1:13 p.m., Qian Zhang wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1156
> > 
> >
> > I think we need to check if this is a quota'ed role first.

Correct, thanks!


- Alexander


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


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
> 
>

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-10-26 Thread Alexander Rukletsov 


> On Oct. 26, 2015, 1:49 p.m., Qian Zhang wrote:
> > For this patch, it seems that we add the code related to quota support in 
> > the slave foreach loop in the HierarchicalAllocatorProcess::allocate(const 
> > hashset& slaveIds_) method, so that means for **each slave**, we 
> > handle quota first and then the existing DRF fair share. I think there 
> > might be an issue for this approach: let say for the first slave, its 
> > available unreserved non-revocable resources can not satisfy a role’s quota 
> > due to the framework in this role has a filter for this slave, and then we 
> > lay aside the filtered resources of this slave for this role immediately. I 
> > think it might be too early for doing this since the other slaves may have 
> > resources which can satisfy this role’s quota. But if we lay aside this 
> > slave's resource for this role at this point, then the result is the 
> > framework of this role will not use these resources (due to the filter) AND 
> > all other role’s frameworks can not be offered with these resources too, 
> > this is kind of wasting resource
 s.
> > 
> > I think maybe we can handle this quota support in this way: In 
> > HierarchicalAllocatorProcess::allocate(const hashset& slaveIds_), 
> > leave the existing 3 levels foreach loops (slave/role/framework) as they 
> > are, and add the quota related code separately before them in this way: 
> > traverse all quota’ed roles, for each of them, traverse all the slaves, and 
> > allocate each slave’s available unreserved non-revocable resources to the 
> > role’s framework (take filter and suppress into account) until the role’s 
> > quota is satisfied. After all the quota’ed role has been traversed, if 
> > there are still some role’s quotas are not satisfied, then lay aside 
> > resources (should be the resources filtered or suppressed) for them. In 
> > this way, before laying aside resources, we have tried our best to use all 
> > slave's the available resources to satisfy the quotas first, there should 
> > be less resources wasted.

I'm not sure I got your point. If my mental compiler is correct, if a framework 
in quota'ed role opts out, we do not immediately lay aside resources. We do 
that after we have checked all the frameworks in the role in a separate loop.


- Alexander


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


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
> 
>

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-10-26 Thread Alexander Rukletsov 


> On Oct. 25, 2015, 1:39 p.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 982
> > 
> >
> > s/unsatisfiedRoleQuotas/unAllocatedRoleQuotas?

Dynamic reservations may not be allocated, but should still count towards quota.


> On Oct. 25, 2015, 1:39 p.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1035
> > 
> >
> > I know that we have design to exclue the reserved resource from quota, 
> > but why not include the current role's reserved as available?

If you mean dynamically reserved resources than yes, we can do that. If you 
mean statically reserved than nope, we decided not to conflate them with quota.


> On Oct. 25, 2015, 1:39 p.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1058
> > 
> >
> > Where does those offerable resource offered to master?

https://github.com/apache/mesos/blob/master/src/master/allocator/mesos/hierarchical.cpp#L956

Dropping the issue, feel free to reopen if I haven't answered your question.


> On Oct. 25, 2015, 1:39 p.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1074
> > 
> >
> > s/unsatisfiedRoleQuota/unAllocatedQuota?

See above.


> On Oct. 25, 2015, 1:39 p.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1102
> > 
> >
> > Why adding those resource as allocated? The `resource` here is 
> > unAllocated resource, why adding it to allocated?

Later on, during the DRF stage we do some resource math, which should be 
correct: 
https://github.com/apache/mesos/blob/master/src/master/allocator/mesos/hierarchical.cpp#L910


- Alexander


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


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
> 
>

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-10-25 Thread Guangya Liu 

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



src/master/allocator/mesos/hierarchical.cpp (line 982)


s/unsatisfiedRoleQuotas/unAllocatedRoleQuotas?



src/master/allocator/mesos/hierarchical.cpp (line 1035)


I know that we have design to exclue the reserved resource from quota, but 
why not include the current role's reserved as available?



src/master/allocator/mesos/hierarchical.cpp (line 1058)


Where does those offerable resource offered to master?



src/master/allocator/mesos/hierarchical.cpp (line 1074)


s/unsatisfiedRoleQuota/unAllocatedQuota?



src/master/allocator/mesos/hierarchical.cpp (line 1102)


Why adding those resource as allocated? The `resource` here is unAllocated 
resource, why adding it to allocated?


- Guangya Liu


On 十月 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 十月 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
> 
>

Re: Review Request 39401: Quota: Updated allocate() in the hierarchical allocator to support quota.

2015-10-23 Thread Alexander Rukletsov 

---
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