Re: Review Request 42738: Included reserved resources in the quota role sorter for DRF.

2016-01-26 Thread Michael Park


> On Jan. 26, 2016, 7:34 a.m., Klaus Ma wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1195
> > 
> >
> > And another question: which resources should be used firstly, 
> > `reserved` or `unreserved`? If `reserved` > `Quota`, should be also assign 
> > `unreserved` resources to it?
> > 
> > My suggestion is to allocate `reserved` resource as `Quota` firstly, 
> > then assign `Quota` for `unreserved`, the latest action is DRF for other 
> > `unreserved` resources. Any comments?
> 
> Joris Van Remoortere wrote:
> Why do we need to add an extra level here?
> If we account `reserved` resources towards quota (see subsequent 
> patches), then we would only allocate `unreserved` resources if `quota` > 
> `reserved`.
> 
> Klaus Ma wrote:
> I'm OK if we have handled such cases :). Let me go through all patches.

I think that the following TODO will address this issue.

```
  // TODO(mpark): Reserved resources should be accounted for fairness
  // calculation whether they are allocated or not, since they model a long or
  // forever running task. That is, the effect of reserving resources is
  // equivalent to launching a task in that the resources that make up the
  // reservation are not available to other roles as non-revocable.
```

However, let's continue the discussion outside of this review.


- Michael


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


On Jan. 26, 2016, 11:04 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42738/
> ---
> 
> (Updated Jan. 26, 2016, 11:04 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 2d01034f43c3653f6233792ee6614fa311249e5c 
>   src/master/allocator/mesos/hierarchical.cpp 
> fa9939700ff44911b9d149a391677b3eb07577ae 
> 
> Diff: https://reviews.apache.org/r/42738/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>



Re: Review Request 42738: Included reserved resources in the quota role sorter for DRF.

2016-01-26 Thread Michael Park

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

(Updated Jan. 26, 2016, 11:04 p.m.)


Review request for mesos and Joris Van Remoortere.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
2d01034f43c3653f6233792ee6614fa311249e5c 
  src/master/allocator/mesos/hierarchical.cpp 
fa9939700ff44911b9d149a391677b3eb07577ae 

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


Testing
---


Thanks,

Michael Park



Re: Review Request 42738: Included reserved resources in the quota role sorter for DRF.

2016-01-26 Thread Joris Van Remoortere

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


Fix it, then Ship it!





src/master/allocator/mesos/hierarchical.cpp (lines 1353 - 1358)


You moved this to the header so we can take it out here.


- Joris Van Remoortere


On Jan. 26, 2016, 10:29 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42738/
> ---
> 
> (Updated Jan. 26, 2016, 10:29 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 2d01034f43c3653f6233792ee6614fa311249e5c 
>   src/master/allocator/mesos/hierarchical.cpp 
> fa9939700ff44911b9d149a391677b3eb07577ae 
> 
> Diff: https://reviews.apache.org/r/42738/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Michael Park
> 
>



Re: Review Request 42738: Included reserved resources in the quota role sorter for DRF.

2016-01-26 Thread Michael Park

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

(Updated Jan. 26, 2016, 10:29 p.m.)


Review request for mesos and Joris Van Remoortere.


Changes
---

Addressed Joris' comments.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
2d01034f43c3653f6233792ee6614fa311249e5c 
  src/master/allocator/mesos/hierarchical.cpp 
fa9939700ff44911b9d149a391677b3eb07577ae 

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


Testing
---


Thanks,

Michael Park



Re: Review Request 42738: Included reserved resources in the quota role sorter for DRF.

2016-01-26 Thread Michael Park

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

(Updated Jan. 26, 2016, 10:02 p.m.)


Review request for mesos and Joris Van Remoortere.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
2d01034f43c3653f6233792ee6614fa311249e5c 
  src/master/allocator/mesos/hierarchical.cpp 
fa9939700ff44911b9d149a391677b3eb07577ae 

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


Testing
---


Thanks,

Michael Park