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

2015-11-23 Thread 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.

2015-11-23 Thread 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.

2015-11-22 Thread 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.

2015-11-19 Thread 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.

2015-11-11 Thread 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.

2015-11-10 Thread 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.

2015-11-10 Thread 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.

2015-11-10 Thread 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.

2015-11-10 Thread 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.

2015-11-10 Thread 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.

2015-11-10 Thread 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.

2015-11-10 Thread 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.

2015-11-06 Thread 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.

2015-10-29 Thread 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.

2015-10-29 Thread 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.

2015-10-29 Thread 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.

2015-10-28 Thread 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.

2015-10-27 Thread Qian Zhang 

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

Ship it!


Ship It!

- Qian Zhang


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

2015-10-27 Thread 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.

2015-10-27 Thread 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.

2015-10-26 Thread 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.

2015-10-26 Thread 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.

2015-10-25 Thread Qian Zhang 

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



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


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.


- 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/39400/
> ---
> 
> (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.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.

2015-10-25 Thread 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.

2015-10-23 Thread 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