Re: Review Request 39400: Quota: Implemented quota API.

[Qian Zhang]

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



src/master/allocator/mesos/hierarchical.hpp (line 391)


For this newly added member `quotaRoleSorter`, I think we also need to 
initialize it in the constructor like what we did for `roleSorter`.
```
HierarchicalAllocatorProcess(
  const std::function& _roleSorterFactory,
  const std::function& _frameworkSorterFactory)
: ProcessBase(process::ID::generate("hierarchical-allocator")),
  initialized(false),
  metrics(*this),
  roleSorterFactory(_roleSorterFactory),
  frameworkSorterFactory(_frameworkSorterFactory),
  roleSorter(NULL) {}<--- we initialized roleSorter here.
```


- Qian Zhang


On Nov. 11, 2015, 6:29 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39400/
> ---
> 
> (Updated Nov. 11, 2015, 6:29 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.hpp 
> cd0d3e0bd234fcf4db9af2e376ea311937204f75 
>   src/master/allocator/mesos/hierarchical.cpp 
> 14fef63714721fcda7cea3c28704766efda6d007 
> 
> Diff: https://reviews.apache.org/r/39400/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39400: Quota: Implemented quota API.

[Alexander Rukletsov]

> On Nov. 23, 2015, 8:07 a.m., Qian Zhang wrote:
> > src/master/allocator/mesos/hierarchical.hpp, line 391
> > 
> >
> > For this newly added member `quotaRoleSorter`, I think we also need to 
> > initialize it in the constructor like what we did for `roleSorter`.
> > ```
> > HierarchicalAllocatorProcess(
> >   const std::function& _roleSorterFactory,
> >   const std::function& _frameworkSorterFactory)
> > : ProcessBase(process::ID::generate("hierarchical-allocator")),
> >   initialized(false),
> >   metrics(*this),
> >   roleSorterFactory(_roleSorterFactory),
> >   frameworkSorterFactory(_frameworkSorterFactory),
> >   roleSorter(NULL) {}<--- we initialized roleSorter here.
> > ```

Let's fix it in https://reviews.apache.org/r/40586/


- Alexander


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


On Nov. 11, 2015, 10:29 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39400/
> ---
> 
> (Updated Nov. 11, 2015, 10:29 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.hpp 
> cd0d3e0bd234fcf4db9af2e376ea311937204f75 
>   src/master/allocator/mesos/hierarchical.cpp 
> 14fef63714721fcda7cea3c28704766efda6d007 
> 
> Diff: https://reviews.apache.org/r/39400/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39400: Quota: Implemented quota API.

[Joris Van Remoortere]

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

Ship it!



src/master/allocator/mesos/hierarchical.cpp (lines 152 - 153)


Let's add a comment as to why we don't update the quotaRoleSorter here yet 
(i.e. we do it during recovery).



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


Let's account for this in the quota role sorter as well (moving it from the 
next review into this one):
```
if (roles[role].quota.isSome()) {
  quotaRoleSorter->allocated(role, slaveId, resources.unreserved());
}
```


- Joris Van Remoortere


On Nov. 11, 2015, 10:29 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39400/
> ---
> 
> (Updated Nov. 11, 2015, 10:29 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.hpp 
> cd0d3e0bd234fcf4db9af2e376ea311937204f75 
>   src/master/allocator/mesos/hierarchical.cpp 
> 14fef63714721fcda7cea3c28704766efda6d007 
> 
> Diff: https://reviews.apache.org/r/39400/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39400: Quota: Implemented quota API.

[Joerg Schad]

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



src/master/allocator/mesos/hierarchical.hpp (line 390)


s/separately/ in addition 

Not really sure about the best formulation here but for me 'separately' 
implies that it is not part of the normal sorter anymore.



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


s/of/for 

similar issue with separately as above.



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


s/dedicated/additional



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


s/general/all ?



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


s/this/these (or all)?



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


s/is not set/has not been set before



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


s/an/the



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


s/an/the


- Joerg Schad


On Nov. 11, 2015, 10:29 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39400/
> ---
> 
> (Updated Nov. 11, 2015, 10:29 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.hpp 
> cd0d3e0bd234fcf4db9af2e376ea311937204f75 
>   src/master/allocator/mesos/hierarchical.cpp 
> 14fef63714721fcda7cea3c28704766efda6d007 
> 
> Diff: https://reviews.apache.org/r/39400/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39400: Quota: Implemented quota API.

[Alexander Rukletsov]

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

(Updated Nov. 11, 2015, 10:29 a.m.)


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


Changes
---

Rebased.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

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

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 39400: Quota: Implemented quota API.

[Alexander Rukletsov]

> On Nov. 10, 2015, 9:21 a.m., Joerg Schad wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 925
> > 
> >
> > Do you want to -symetrically to your todu when setting quota- print the 
> > actual quota removed?

I was thinking about it, do you think it adds extra value?


- Alexander


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


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/39400/
> ---
> 
> (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.hpp 
> cfd937ba306273c24fb5337dfeb1a15e1545169b 
>   src/master/allocator/mesos/hierarchical.cpp 
> f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 
> 
> Diff: https://reviews.apache.org/r/39400/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39400: Quota: Implemented quota API.

[Alexander Rukletsov]

> On Nov. 10, 2015, 10:09 a.m., Joris Van Remoortere wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 140
> > 
> >
> > Maybe s/Introduce/Consider introducing/
> > Are we sure we want to do this?

A very good point. I actually think each `TODO` which states something should 
have been converted to code. The only reason to leave a `TODO` is because there 
is a certain level of uncertainty.


> On Nov. 10, 2015, 10:09 a.m., Joris Van Remoortere wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 894
> > 
> >
> > let's clarify `just updates the numbers`. Also missing a period.

Will do, please check if this helps.


> On Nov. 10, 2015, 10:09 a.m., Joris Van Remoortere wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 906
> > 
> >
> > A comment as to why we want to `allocate()` here would be useful.

Will do, please check if this helps.


> On Nov. 10, 2015, 10:09 a.m., Joris Van Remoortere wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 927
> > 
> >
> > A comment here would be helpful as well.

Will do, please check if this helps.


> On Nov. 10, 2015, 10:09 a.m., Joris Van Remoortere wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 225
> > 
> >
> > I think we can add an equivalent comment as above this for loop (not 
> > your fault):
> > `// Update the allocation to this framework.`

Will do, please verify if this is what you would like to see here.


> On Nov. 10, 2015, 10:09 a.m., Joris Van Remoortere wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 330
> > 
> >
> > same as above. let's add a comment here.

Will do, please verify if this is what you would like to see here.


- Alexander


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


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/39400/
> ---
> 
> (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.hpp 
> cfd937ba306273c24fb5337dfeb1a15e1545169b 
>   src/master/allocator/mesos/hierarchical.cpp 
> f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 
> 
> Diff: https://reviews.apache.org/r/39400/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39400: Quota: Implemented quota API.

[Alexander Rukletsov]

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

(Updated Nov. 10, 2015, 11:06 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 (updated)
-

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

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 39400: Quota: Implemented quota API.

[Alexander Rukletsov]

> On Nov. 10, 2015, 9:21 a.m., Joerg Schad wrote:
> > src/master/allocator/mesos/hierarchical.hpp, line 359
> > 
> >
> > s/the/this

Re-phrasing it a bit deeper.


> On Nov. 10, 2015, 9:21 a.m., Joerg Schad wrote:
> > src/master/allocator/mesos/hierarchical.hpp, line 360
> > 
> >
> > Quota is satisfied before fair share, ...

Chose different phrasing.


> On Nov. 10, 2015, 9:21 a.m., Joerg Schad wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 137
> > 
> >
> > `Resources`

This is neither a type, nor an instance, but a proper english word.


> On Nov. 10, 2015, 9:21 a.m., Joerg Schad wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 416
> > 
> >
> > `resources`

This is neither a type, nor an instance, but a proper english word.


> On Nov. 10, 2015, 9:21 a.m., Joerg Schad wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 582
> > 
> >
> > `resources`

This is neither a type, nor an instance, but a proper english word.


- Alexander


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


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/39400/
> ---
> 
> (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.hpp 
> cfd937ba306273c24fb5337dfeb1a15e1545169b 
>   src/master/allocator/mesos/hierarchical.cpp 
> f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 
> 
> Diff: https://reviews.apache.org/r/39400/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39400: Quota: Implemented quota API.

[Joris Van Remoortere]

> On Oct. 25, 2015, 9:06 a.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 903
> > 
> >
> > I see that in the code, some are using 
> > 
> > '" << xxx << "'";
> > 
> > while some others are using
> > 
> > \"" << xxx << "\"";
> > 
> > What is the standard for different style?
> > 
> > Also when shall we need to add "" for one parameter in log message?
> 
> Alexander Rukletsov wrote:
> I think we are inconsistent. AFAIK, @neilc is doing some cleanup around.
> 
> Guangya Liu wrote:
> https://reviews.apache.org/r/39497 has been merged, it is using {{'" << 
> xxx << "'";}}, do we need to update this patch accordingly?

That review only sets the precedent for executor ids, which now have a 
streaming operator to take care of the formatting.


- Joris


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


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/39400/
> ---
> 
> (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.hpp 
> cfd937ba306273c24fb5337dfeb1a15e1545169b 
>   src/master/allocator/mesos/hierarchical.cpp 
> f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 
> 
> Diff: https://reviews.apache.org/r/39400/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39400: Quota: Implemented quota API.

[Joerg Schad]

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



src/master/allocator/mesos/hierarchical.hpp (line 358)


s/the/this



src/master/allocator/mesos/hierarchical.hpp (line 359)


Quota is satisfied before fair share, ...



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


`Resources`



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


`resources`



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


`resources`



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


s/numbers/numbers.



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


Do you want to -symetrically to your todu when setting quota- print the 
actual quota removed?


- Joerg Schad


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/39400/
> ---
> 
> (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.hpp 
> cfd937ba306273c24fb5337dfeb1a15e1545169b 
>   src/master/allocator/mesos/hierarchical.cpp 
> f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 
> 
> Diff: https://reviews.apache.org/r/39400/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39400: Quota: Implemented quota API.

[Joris Van Remoortere]

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



src/master/allocator/mesos/hierarchical.hpp (line 358)


s/the order the role's/the order that the role's/
would make this more clear for me.



src/master/allocator/mesos/hierarchical.hpp (line 359)


s/Quota goes before fair share/Quota comes before fair share/



src/master/allocator/mesos/hierarchical.hpp (line 360)


s/towards the allocation queue's front./towards the front of the allocation 
queue./



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


Maybe s/Introduce/Consider introducing/
Are we sure we want to do this?



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


I think we can add an equivalent comment as above this for loop (not your 
fault):
`// Update the allocation to this framework.`



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


same as above. let's add a comment here.



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


let's clarify `just updates the numbers`. Also missing a period.



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


A comment as to why we want to `allocate()` here would be useful.



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


A comment here would be helpful as well.


- Joris Van Remoortere


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/39400/
> ---
> 
> (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.hpp 
> cfd937ba306273c24fb5337dfeb1a15e1545169b 
>   src/master/allocator/mesos/hierarchical.cpp 
> f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 
> 
> Diff: https://reviews.apache.org/r/39400/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39400: Quota: Implemented quota API.

[Klaus Ma]

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

Ship it!


Ship It!

- Klaus Ma


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/39400/
> ---
> 
> (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.hpp 
> cfd937ba306273c24fb5337dfeb1a15e1545169b 
>   src/master/allocator/mesos/hierarchical.cpp 
> f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 
> 
> Diff: https://reviews.apache.org/r/39400/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39400: Quota: Implemented quota API.

[Alexander Rukletsov]

> On Oct. 28, 2015, 8:54 a.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 903
> > 
> >
> > Alex, just want to get more for why do we need to add '' for role? I 
> > know that role is user supplied, but does role will  contain whitespace? If 
> > not, I think that there is no need to add '' for role output, comments? 
> > Thanks!

If I read 
[this](https://github.com/apache/mesos/blob/master/src/master/master.cpp#L561) 
correctly, nothing prevents a role from having spaces in its name.


- Alexander


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


On Oct. 27, 2015, 7:17 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39400/
> ---
> 
> (Updated Oct. 27, 2015, 7:17 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.hpp 
> cfd937ba306273c24fb5337dfeb1a15e1545169b 
>   src/master/allocator/mesos/hierarchical.cpp 
> f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 
> 
> Diff: https://reviews.apache.org/r/39400/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39400: Quota: Implemented quota API.

[Guangya Liu]

> On Oct. 28, 2015, 8:54 a.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 903
> > 
> >
> > Alex, just want to get more for why do we need to add '' for role? I 
> > know that role is user supplied, but does role will  contain whitespace? If 
> > not, I think that there is no need to add '' for role output, comments? 
> > Thanks!
> 
> Alexander Rukletsov wrote:
> If I read 
> [this](https://github.com/apache/mesos/blob/master/src/master/master.cpp#L561)
>  correctly, nothing prevents a role from having spaces in its name.

Thanks Alex, yes, the role can include whitespace.


- Guangya


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


On Oct. 27, 2015, 7:17 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39400/
> ---
> 
> (Updated Oct. 27, 2015, 7:17 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.hpp 
> cfd937ba306273c24fb5337dfeb1a15e1545169b 
>   src/master/allocator/mesos/hierarchical.cpp 
> f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 
> 
> Diff: https://reviews.apache.org/r/39400/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39400: Quota: Implemented quota API.

[Guangya Liu]

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

Ship it!


Ship It!

- Guangya Liu


On Oct. 27, 2015, 7:17 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39400/
> ---
> 
> (Updated Oct. 27, 2015, 7:17 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.hpp 
> cfd937ba306273c24fb5337dfeb1a15e1545169b 
>   src/master/allocator/mesos/hierarchical.cpp 
> f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 
> 
> Diff: https://reviews.apache.org/r/39400/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39400: Quota: Implemented quota API.

[Guangya Liu]

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



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


Alex, just want to get more for why do we need to add '' for role? I know 
that role is user supplied, but does role will  contain whitespace? If not, I 
think that there is no need to add '' for role output, comments? Thanks!


- Guangya Liu


On 十月 27, 2015, 7:17 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39400/
> ---
> 
> (Updated 十月 27, 2015, 7:17 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.hpp 
> cfd937ba306273c24fb5337dfeb1a15e1545169b 
>   src/master/allocator/mesos/hierarchical.cpp 
> f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 
> 
> Diff: https://reviews.apache.org/r/39400/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39400: Quota: Implemented quota API.

[Alexander Rukletsov]

> On Oct. 26, 2015, 6:52 a.m., Qian Zhang wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 897
> > 
> >
> > I think we are not *moving" the role into the quota'ed role sorter, 
> > instead, we are adding it into the quota'ed role sorter, i.e., quota'ed 
> > role will be placed in both quota'd role sorter and the non-quota'ed role 
> > sorter.

My thinking was to stress that by doing that a different allocation algorithm 
applies. But you're right, it's misleading.


- Alexander


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


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/39400/
> ---
> 
> (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.hpp 
> cfd937ba306273c24fb5337dfeb1a15e1545169b 
>   src/master/allocator/mesos/hierarchical.cpp 
> f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 
> 
> Diff: https://reviews.apache.org/r/39400/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39400: Quota: Implemented quota API.

[Alexander Rukletsov]

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

(Updated Oct. 27, 2015, 7:17 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.hpp 
cfd937ba306273c24fb5337dfeb1a15e1545169b 
  src/master/allocator/mesos/hierarchical.cpp 
f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov

Re: Review Request 39400: Quota: Implemented quota API.

[Guangya Liu]

> On 十月 25, 2015, 9:06 a.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 903
> > 
> >
> > I see that in the code, some are using 
> > 
> > '" << xxx << "'";
> > 
> > while some others are using
> > 
> > \"" << xxx << "\"";
> > 
> > What is the standard for different style?
> > 
> > Also when shall we need to add "" for one parameter in log message?
> 
> Alexander Rukletsov wrote:
> I think we are inconsistent. AFAIK, @neilc is doing some cleanup around.

https://reviews.apache.org/r/39497 has been merged, it is using {{'" << xxx << 
"'";}}, do we need to update this patch accordingly?


- Guangya


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


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/39400/
> ---
> 
> (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.hpp 
> cfd937ba306273c24fb5337dfeb1a15e1545169b 
>   src/master/allocator/mesos/hierarchical.cpp 
> f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 
> 
> Diff: https://reviews.apache.org/r/39400/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39400: Quota: Implemented quota API.

[Alexander Rukletsov]

> On Oct. 25, 2015, 9:06 a.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 903
> > 
> >
> > I see that in the code, some are using 
> > 
> > '" << xxx << "'";
> > 
> > while some others are using
> > 
> > \"" << xxx << "\"";
> > 
> > What is the standard for different style?
> > 
> > Also when shall we need to add "" for one parameter in log message?

I think we are inconsistent. AFAIK, @neilc is doing some cleanup around.


- Alexander


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


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/39400/
> ---
> 
> (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.hpp 
> cfd937ba306273c24fb5337dfeb1a15e1545169b 
>   src/master/allocator/mesos/hierarchical.cpp 
> f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 
> 
> Diff: https://reviews.apache.org/r/39400/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39400: Quota: Implemented quota API.

[Guangya Liu]

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



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


I see that in the code, some are using 

'" << xxx << "'";

while some others are using

\"" << xxx << "\"";

What is the standard for different style?

Also when shall we need to add "" for one parameter in log message?


- 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/39400/
> ---
> 
> (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.hpp 
> cfd937ba306273c24fb5337dfeb1a15e1545169b 
>   src/master/allocator/mesos/hierarchical.cpp 
> f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 
> 
> Diff: https://reviews.apache.org/r/39400/diff/
> 
> 
> Testing
> ---
> 
> make check (Mac OS X 10.10.4)
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 39400: Quota: Implemented quota API.

[Alexander Rukletsov]

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

(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.hpp 
cfd937ba306273c24fb5337dfeb1a15e1545169b 
  src/master/allocator/mesos/hierarchical.cpp 
f4e4a123d3da0442e8b0b0ad14d1ee760752ba36 

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


Testing
---

make check (Mac OS X 10.10.4)


Thanks,

Alexander Rukletsov