Re: Review Request 43561: Improve Ranges parsing to handle single values.

[Klaus Ma]

> On June 8, 2016, 5:32 a.m., Benjamin Mahler wrote:
> > src/tests/values_tests.cpp, lines 39-72
> > <https://reviews.apache.org/r/43561/diff/5/?file=1310343#file1310343line39>
> >
> > It would be great to split apart this test into the respective value 
> > types, e.g.
> > 
> > ```
> > ValuesTest.ParseScalar
> > ValuesTest.ParseRanges
> > ValuesTest.ParseSet
> > ```
> > 
> > With more test cases that show what is currently accepted and what is 
> > not. There are a lot of other formats currently accepted from what I can 
> > tell (see my comment below).

Logged MESOS-5603 to trace those test refactor; I'll continue to work on this.

https://issues.apache.org/jira/browse/MESOS-5603


- Klaus


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


On June 11, 2016, 10:29 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43561/
> ---
> 
> (Updated June 11, 2016, 10:29 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4627
> https://issues.apache.org/jira/browse/MESOS-4627
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improve Ranges parsing to handle single values.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/43561/diff/
> 
> 
> Testing
> ---
> 
> make
> make check GTEST_FILTER=~"*"
> ./src/mesos-test
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 43561: Improve Ranges parsing to handle single values.

[Klaus Ma]

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

(Updated June 11, 2016, 10:29 p.m.)


Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Joris Van 
Remoortere.


Changes
---

rebase


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


Repository: mesos


Description
---

Improve Ranges parsing to handle single values.


Diffs (updated)
-

  src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
  src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 

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


Testing
---

make
make check GTEST_FILTER=~"*"
./src/mesos-test


Thanks,

Klaus Ma

Re: Review Request 43561: Improve Ranges parsing to handle single values.

[Klaus Ma]

> On June 8, 2016, 5:32 a.m., Benjamin Mahler wrote:
> > src/tests/values_tests.cpp, line 210
> > <https://reviews.apache.org/r/43561/diff/5/?file=1310343#file1310343line210>
> >
> > If I understand correctly, the following are currently accepted way to 
> > specify 1-4:
> > 
> > `[[1-4]]`
> > `[1-2]\n[3-4]`
> > `[1-2],[3-4]`
> > 
> > I'm ok with rejecting these in favor of the following canonical formats:
> > 
> > `[1-2,3-4]`
> > 
> > Although it doesn't have to be coalesced. However, we should notify the 
> > user mailing list that we're moving towards more strict parsing of ranges 
> > if you're going to change what we accept.
> > 
> > Ideally, we could pull in a regex or parsing expression grammar library 
> > to more formally define our formats. The current parsing code is a mess.

Logged MESOS-5602 to trace "expression grammar library", 
https://issues.apache.org/jira/browse/MESOS-5602

And i'll send an email to dev@ and user@ list for this notification.


- Klaus


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


On June 11, 2016, 10:29 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43561/
> ---
> 
> (Updated June 11, 2016, 10:29 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4627
> https://issues.apache.org/jira/browse/MESOS-4627
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improve Ranges parsing to handle single values.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/43561/diff/
> 
> 
> Testing
> ---
> 
> make
> make check GTEST_FILTER=~"*"
> ./src/mesos-test
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 48914: Added GPU_RESOURCES capability to FrameworkInfo.

[Klaus Ma]

> On June 19, 2016, 10:18 a.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 1277-1279
> > 
> >
> > This logic seems also allow agents without gpu resources offered to the 
> > framework with gpuCapble, is it expected behavior?
> 
> Kevin Klues wrote:
> Yes, that was my intended behaviour as I wrote it. If we think there is 
> some problem with this, we should discuss it further.
> 
> Guangya Liu wrote:
> So what is the use of the agent without GPU for the framework which 
> request gpu resources?

As far as I known, some GPU application can also run on CPU.


- Klaus


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


On June 19, 2016, 6:07 a.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48914/
> ---
> 
> (Updated June 19, 2016, 6:07 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5634
> https://issues.apache.org/jira/browse/MESOS-5634
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Due to the scarce resource problem described in MESOS-5377, we are
> introducing a GPU_RESOURCES Framework capability. This capability
> allows the Mesos allocator to make better decisions about which
> frameworks should receive resources from GPU capable machines. In
> essence, the allocator ONLY allocate resources from GPU capable
> machines to frameworks that have this capability. This is necessary to
> prevent non-GPU workloads from filling up the GPU machines and
> preventing GPU workloads to run.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto e4c5bd31cf035707036eb509336fe051119b4e78 
>   include/mesos/v1/mesos.proto 9be22f02861f1eb89ab547d88530faf90ebee7ab 
>   src/master/allocator/mesos/hierarchical.hpp 
> 9c6b23abe2b0cb16412f1ed90165f8d0c14552fa 
>   src/master/allocator/mesos/hierarchical.cpp 
> 8b7b3afb5770c617918ec4864faaf8d8a7a81ef2 
>   src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
> e06d107f2dcdb9b470e330c8ceee66a54220d41b 
> 
> Diff: https://reviews.apache.org/r/48914/diff/
> 
> 
> Testing
> ---
> 
> $ make -j check; sudo GTEST_FILTER="*NVIDIA*" src/mesos-tests
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Review Request 49140: Added startsWith/endsWith to support char.

[Klaus Ma]

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

Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

Added startsWith/endsWith to support char.


Diffs
-

  3rdparty/stout/include/stout/strings.hpp 
162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 

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


Testing
---

make


Thanks,

Klaus Ma

Re: Review Request 41857: Got evictable executors.

[Klaus Ma]

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

(Updated Jan. 11, 2016, 8:18 p.m.)


Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Joris Van 
Remoortere, Joseph Wu, and Jian Qiu.


Changes
---

Return empty list instead of None() when no executors to evict.


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


Repository: mesos


Description
---

get evictable executors


Diffs (updated)
-

  src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
  src/slave/slave.cpp 90d0fecd2d83fd174134870a577ac59d79c0006f 

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


Testing
---


Thanks,

Klaus Ma

Re: Review Request 35711: Disallow special characters in role name.

[Klaus Ma]

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



include/mesos/roles.hpp (line 51)
<https://reviews.apache.org/r/35711/#comment174396>

Would you highlight which case is considered not valid? anyone of them is 
not valid or all of them are not valid?



src/common/roles.cpp (line 32)
<https://reviews.apache.org/r/35711/#comment174400>

Do we need to define a const value `REOLE_SPLITOR` to replace ','?



src/common/roles.cpp (line 34)
<https://reviews.apache.org/r/35711/#comment174397>

Do we need to check whether duplicated roles here?



src/common/roles.cpp (lines 56 - 63)
<https://reviews.apache.org/r/35711/#comment174399>

Just another question about the role: do we support other language, e.g. 
Chinese? If not, I'd suggest also to highlight in document.



src/common/roles.cpp (line 64)
<https://reviews.apache.org/r/35711/#comment174398>

I'm not sure whether this will introduce porting issue: for the whitespace, 
it's better to use C std (isspace) lib to check.


- Klaus Ma


On Jan. 9, 2016, 7:15 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35711/
> ---
> 
> (Updated Jan. 9, 2016, 7:15 p.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, and Michael Park.
> 
> 
> Bugs: MESOS-2210
> https://issues.apache.org/jira/browse/MESOS-2210
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Disallow special characters in role name.
> 
> 
> Diffs
> -
> 
>   docs/roles.md af1adad7ec8122fd10f7de44848014b850416bcd 
>   include/mesos/roles.hpp PRE-CREATION 
>   src/Makefile.am bbd0c119321fa9d11fea61b140428dd92d1258c8 
>   src/common/resources.cpp 6afd2dfd81adecf7bdb3e523bc840efb62b79ef4 
>   src/common/roles.cpp PRE-CREATION 
>   src/master/master.cpp 2d9b7f9540574aa3ef9e5af3b2b8922dffeebac8 
>   src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 
>   src/tests/role_tests.cpp 373ae267b85588fe491ab0a0ce8aa195f971aac3 
> 
> Diff: https://reviews.apache.org/r/35711/diff/
> 
> 
> Testing
> ---
> 
> make -j8 check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 42113: Handle unreserve logic for dynamic reservation (2/3).

[Klaus Ma]

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



src/master/allocator/mesos/hierarchical.cpp (line 692)
<https://reviews.apache.org/r/42113/#comment175254>

`freeAllocationSlack` is unused in the following codes? Why it's here.



src/master/allocator/mesos/hierarchical.cpp (lines 707 - 710)
<https://reviews.apache.org/r/42113/#comment175259>

Should be

if (freeAllocationSlack.empty())
  break;

Resources freeResources = freeAllocationSlack.get(name);

if (freeResources.contains(unreservedResources)) {
  slaves[slaveId].total -= unreservedResources;
  freeAllocationSlack -= unreservedResources;
} else {
  slaves[slaveId].total -= freeResources;
  freeAllocationSlack -= freeResources;
}



src/master/allocator/mesos/hierarchical.cpp (line 708)
<https://reviews.apache.org/r/42113/#comment175255>

There should be `freeAllocationSlack` which is not allocated for now.


- Klaus Ma


On Jan. 13, 2016, 9:01 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42113/
> ---
> 
> (Updated Jan. 13, 2016, 9:01 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
> 
> 
> Bugs: MESOS-4145
> https://issues.apache.org/jira/browse/MESOS-4145
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Handle unreserve logic for dynamic reservation with allocation slack.
> 
> This patch is halding the case when using updateAllocation to unreserve
> some resources.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> d541bfa3f4190865c65d35c9d1ffdb8a3f194056 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
> 
> Diff: https://reviews.apache.org/r/42113/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Review Request 42289: Calcuated 'remainingClusterResources' by all activated slaves.

[Klaus Ma]

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

Review request for mesos, Alexander Rukletsov and Neil Conway.


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


Repository: mesos


Description
---

Calcuated 'remainingClusterResources' by all activated slaves.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
d541bfa3f4190865c65d35c9d1ffdb8a3f194056 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 41791: Updated allocation slack for dynamic reserve (1/3).

[Klaus Ma]

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



src/master/allocator/mesos/hierarchical.cpp (line 677)
<https://reviews.apache.org/r/41791/#comment175231>

It should be `newStatelessReserved.contains(total)`; in `contains`, it'll 
return false if any resource did not contain.



src/master/allocator/mesos/hierarchical.cpp (lines 732 - 737)
<https://reviews.apache.org/r/41791/#comment175252>

Update the comments to list cases, for example:
1. if new `reserved.stateless` > old, pala...
2. if new ... <, pala...
  - if xxx
  - if xxx


- Klaus Ma


On Jan. 13, 2016, 8:58 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41791/
> ---
> 
> (Updated Jan. 13, 2016, 8:58 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
> 
> 
> Bugs: MESOS-4145
> https://issues.apache.org/jira/browse/MESOS-4145
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update allocation slack resources if reserve some
> new stateless reserved resources. For such case,
> the allocation slack resources will be increased.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> d541bfa3f4190865c65d35c9d1ffdb8a3f194056 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
> 
> Diff: https://reviews.apache.org/r/41791/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 42194: Handle unreserve logic for dynamic reservation (3/3).

[Klaus Ma]

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


If there is no much different with `updateAllocation`, I'd suggest to merge 
this patch with `updateAllocation`.

- Klaus Ma


On Jan. 13, 2016, 9:10 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42194/
> ---
> 
> (Updated Jan. 13, 2016, 9:10 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
> 
> 
> Bugs: MESOS-4145
> https://issues.apache.org/jira/browse/MESOS-4145
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Handle unreserve logic for dynamic reservation with allocation slack.
> 
> This patch is halding the case when using updateAvailable to unreserve
> some resources.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> d541bfa3f4190865c65d35c9d1ffdb8a3f194056 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
> 
> Diff: https://reviews.apache.org/r/42194/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 41772: Added helper function to flatten resources.

[Klaus Ma]

> On Jan. 7, 2016, 8:31 a.m., Joseph Wu wrote:
> > src/common/resources.cpp, lines 894-904
> > <https://reviews.apache.org/r/41772/diff/2/?file=1180306#file1180306line894>
> >
> > This seems more appropriate as an optional parameter for 
> > `Resources::flatten`, just like `Option`.
> 
> Guangya Liu wrote:
> My original thinking was adding an optional parameter to flatten(), the 
> reason that I did not do that is because every time I want to set 
> ALLOCATION_SLACK or USAGE_SLACK for a resource, I need to put all paramters, 
> it is a long sentence, so I just add this new helper.
> 
> The caller need to call {flatten("*", None(), 
> Resource::RevocableInfo::ALLOCATION_SLACK);} if want to set a resources as 
> allocation slack.
> 
> Joseph Wu wrote:
> It's fine to have somewhat long expressions.  I think it's preferable 
> over having too many helpers.
> 
> Guangya Liu wrote:
> @Joseph, seems we need to reach an agreement for this ;-) Can you please 
> take a look at the following patches in the patch chain, you may notice that 
> other patches highly depend on those helper functions.
> 
> So what about the following:
> 1) Rename flattenReserved() to reserved_() which will return all reserved 
> resources with different roles in a flatten mode.
> 2) Remove flattenSlack() and merge it to flatten().
> 
> Comments?
> 
> Klaus Ma wrote:
> regarding 1), I'd like to add `allocationSlackable` which mean the 
> reosurces can be offered as `ALLOCATION_SLACK`, it's peer of 
> `allocationSlack`.
> 
> Guangya Liu wrote:
> `allcationSlackable` is more like a `bool` function, but here I want to 
> return a `flatten reserved` resources.

It's not about `bool` :). I'd like to avoid the backgroud of 
`ALLOCATION_SLACK-able` == `stateless.reserved`: the general point is 
`ALLOCATION_SLACK-able` resources can be lend to other Tenant framework, and 
those resource will be preempted when Lender framework launch tasks. That's 
major reason that I'd suggest to introduce helper fuction to avoid 
`stateless.reserved` in Master/Agent source code.

En, maybe this can be treat as new MVP named borrow/lend policy worked with 
Quota; it's different with Optimistic Offer Phase 2, the revocalbe resources 
will not send to multiple framework; the revocable resources will be shared 
between frameworks by ratio. @Joseph/Ben, any comments.


- Klaus


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


On Jan. 13, 2016, 8:16 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41772/
> -------
> 
> (Updated Jan. 13, 2016, 8:16 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, and Klaus Ma.
> 
> 
> Bugs: MESOS-4267
> https://issues.apache.org/jira/browse/MESOS-4267
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added two new helper functions to flatten resources.
> 1) Flatten reserved resources.
> 2) Flatten allocation slack revocable resources.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp cc8fef9470d779078aa408ed03e747e5a492deaa 
>   include/mesos/v1/resources.hpp f4892977f8d7b0439db6e9cf7921334f606a496c 
>   src/common/resources.cpp 6afd2dfd81adecf7bdb3e523bc840efb62b79ef4 
>   src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 
>   src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 
> 
> Diff: https://reviews.apache.org/r/41772/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> GLOG_v=2 ./bin/mesos-tests.sh  --gtest_filter="ResourcesOperationTest.*" 
> --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 35711: Disallow special characters in role name.

[Klaus Ma]

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

Ship it!


Ship It!

- Klaus Ma


On Jan. 14, 2016, 1:12 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35711/
> ---
> 
> (Updated Jan. 14, 2016, 1:12 a.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, and Michael Park.
> 
> 
> Bugs: MESOS-2210
> https://issues.apache.org/jira/browse/MESOS-2210
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Disallow special characters in role name.
> 
> 
> Diffs
> -
> 
>   docs/roles.md af1adad7ec8122fd10f7de44848014b850416bcd 
>   include/mesos/roles.hpp PRE-CREATION 
>   src/Makefile.am 8cbfb1ba5fa49f2d3cc26ea325838a1c68a79660 
>   src/common/resources.cpp 6afd2dfd81adecf7bdb3e523bc840efb62b79ef4 
>   src/common/roles.cpp PRE-CREATION 
>   src/master/master.cpp c122c30d943813fc3ce9e7025783c7231809b022 
>   src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 
>   src/tests/role_tests.cpp 373ae267b85588fe491ab0a0ce8aa195f971aac3 
> 
> Diff: https://reviews.apache.org/r/35711/diff/
> 
> 
> Testing
> ---
> 
> make -j8 check
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 42254: Checked whether the remaining cluster resources is allocatable.

[Klaus Ma]

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

Review request for mesos, Alexander Rukletsov and Guangya Liu.


Repository: mesos


Description
---

Checked whether the remaining cluster resources is allocatable.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
d541bfa3f4190865c65d35c9d1ffdb8a3f194056 

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


Testing
---


Thanks,

Klaus Ma

Re: Review Request 42221: Removed references to wDRF from allocator.

[Klaus Ma]

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



src/master/allocator/mesos/hierarchical.cpp (line 1256)
<https://reviews.apache.org/r/42221/#comment175029>

Not related to this patch, But I think `allocable()` is better than 
`empty()`.


- Klaus Ma


On Jan. 13, 2016, 7:06 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42221/
> ---
> 
> (Updated Jan. 13, 2016, 7:06 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since wDRF is encapsulated in the choice of sorter, it should not be
> mentioned in the allocator code.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> df8bccaf2b8cfc0cb5ca18d4867371ae7a84c12f 
> 
> Diff: https://reviews.apache.org/r/42221/diff/
> 
> 
> Testing
> ---
> 
> `make check` on Mac OS 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 42222: Added a comment on allocator recovery.

[Klaus Ma]

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

Ship it!


Ship It!

- Klaus Ma


On Jan. 13, 2016, 7:06 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4/
> ---
> 
> (Updated Jan. 13, 2016, 7:06 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> df8bccaf2b8cfc0cb5ca18d4867371ae7a84c12f 
> 
> Diff: https://reviews.apache.org/r/4/diff/
> 
> 
> Testing
> ---
> 
> None: not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 41772: Added helper function to flatten resources.

[Klaus Ma]

> On Jan. 7, 2016, 8:31 a.m., Joseph Wu wrote:
> > src/common/resources.cpp, lines 894-904
> > <https://reviews.apache.org/r/41772/diff/2/?file=1180306#file1180306line894>
> >
> > This seems more appropriate as an optional parameter for 
> > `Resources::flatten`, just like `Option`.
> 
> Guangya Liu wrote:
> My original thinking was adding an optional parameter to flatten(), the 
> reason that I did not do that is because every time I want to set 
> ALLOCATION_SLACK or USAGE_SLACK for a resource, I need to put all paramters, 
> it is a long sentence, so I just add this new helper.
> 
> The caller need to call {flatten("*", None(), 
> Resource::RevocableInfo::ALLOCATION_SLACK);} if want to set a resources as 
> allocation slack.
> 
> Joseph Wu wrote:
> It's fine to have somewhat long expressions.  I think it's preferable 
> over having too many helpers.
> 
> Guangya Liu wrote:
> @Joseph, seems we need to reach an agreement for this ;-) Can you please 
> take a look at the following patches in the patch chain, you may notice that 
> other patches highly depend on those helper functions.
> 
> So what about the following:
> 1) Rename flattenReserved() to reserved_() which will return all reserved 
> resources with different roles in a flatten mode.
> 2) Remove flattenSlack() and merge it to flatten().
> 
> Comments?

regarding 1), I'd like to add `allocationSlackable` which mean the reosurces 
can be offered as `ALLOCATION_SLACK`, it's peer of `allocationSlack`.


- Klaus


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


On Jan. 13, 2016, 8:16 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41772/
> ---
> 
> (Updated Jan. 13, 2016, 8:16 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, and Klaus Ma.
> 
> 
> Bugs: MESOS-4267
> https://issues.apache.org/jira/browse/MESOS-4267
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added two new helper functions to flatten resources.
> 1) Flatten reserved resources.
> 2) Flatten allocation slack revocable resources.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp cc8fef9470d779078aa408ed03e747e5a492deaa 
>   include/mesos/v1/resources.hpp f4892977f8d7b0439db6e9cf7921334f606a496c 
>   src/common/resources.cpp 6afd2dfd81adecf7bdb3e523bc840efb62b79ef4 
>   src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 
>   src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 
> 
> Diff: https://reviews.apache.org/r/41772/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> GLOG_v=2 ./bin/mesos-tests.sh  --gtest_filter="ResourcesOperationTest.*" 
> --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 42113: Handle unreserve logic for dynamic reservation (2/3).

[Klaus Ma]

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



src/master/allocator/mesos/hierarchical.cpp (line 724)
<https://reviews.apache.org/r/42113/#comment175324>

Should be `freeAllocationSlack.get(name)`


- Klaus Ma


On Jan. 14, 2016, 8:46 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42113/
> ---
> 
> (Updated Jan. 14, 2016, 8:46 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
> 
> 
> Bugs: MESOS-4145
> https://issues.apache.org/jira/browse/MESOS-4145
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Handle unreserve logic for dynamic reservation with allocation slack.
> 
> This patch is halding the case when using updateAllocation to unreserve
> some resources.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> d541bfa3f4190865c65d35c9d1ffdb8a3f194056 
>   src/tests/hierarchical_allocator_tests.cpp 
> e044f832c2c16e53e663c6ced5452649bb0dcb59 
> 
> Diff: https://reviews.apache.org/r/42113/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 41308: MESOS-1718: Unit Test for moving getExecutorInfo from slave to master.

[Klaus Ma]

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

(Updated Jan. 18, 2016, 10:19 p.m.)


Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.


Changes
---

rebase: add executorLost handler for command executor.


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


Repository: mesos


Description
---

MESOS-1718: Unit Test for moving getExecutorInfo from slave to master


Diffs (updated)
-

  src/master/master.cpp 9ee56277c8a472be9e683d5db505becfb5f7c422 
  src/tests/containerizer/docker_containerizer_tests.cpp 
a1a44daae68e5a4c6961e13ee9e16207c5d54768 
  src/tests/disk_quota_tests.cpp 1577cf71c4d1fbad11e2ec939c4ceae2d5b25f97 
  src/tests/fetcher_cache_tests.cpp 1fb1e213d3c35479789688d1a3a49a3c6058b198 
  src/tests/master_tests.cpp aa0e0d8087175e3a3ed5b63a23d31aa6fe00d1b3 
  src/tests/master_validation_tests.cpp 
6fae01fa1833ae05ec82618a4ae28ac5bd275bd5 
  src/tests/monitor_tests.cpp 2226458b59b4a279a92e1353bd61457a0018d2a9 
  src/tests/oversubscription_tests.cpp 7a75fb38e0177e33cf0e7cb82b4b9ebf8f05fe0a 
  src/tests/reservation_endpoints_tests.cpp 
b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 
  src/tests/slave_recovery_tests.cpp 6683a081ac231f1e275a3cdb8ee841da430a9f66 
  src/tests/slave_tests.cpp 7fe566770bbd802111885de061a53a3edf914840 

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


Testing
---


Thanks,

Klaus Ma

Re: Review Request 41308: MESOS-1718: Unit Test for moving getExecutorInfo from slave to master.

[Klaus Ma]

> On Jan. 13, 2016, 8:39 p.m., Klaus Ma wrote:
> > @Joseph/Vinod, would you help to review those patches move executor from 
> > slave to master?
> 
> Joseph Wu wrote:
> There are still several open issues open on each prior review.  Can you 
> go through and make sure those are addressed?
> 
> Klaus Ma wrote:
> Honestly, I'd like you to help review the code diff about executor's 
> resources: where should get command line executor's resources? cut from tasks 
> or add additional resources to it.

@Joseph, the patches are rebased and commnets are addressed :). Would you help 
to reivew it?


- Klaus


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


On Jan. 18, 2016, 10:19 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41308/
> ---
> 
> (Updated Jan. 18, 2016, 10:19 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
> Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-1718
> https://issues.apache.org/jira/browse/MESOS-1718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-1718: Unit Test for moving getExecutorInfo from slave to master
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 9ee56277c8a472be9e683d5db505becfb5f7c422 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> a1a44daae68e5a4c6961e13ee9e16207c5d54768 
>   src/tests/disk_quota_tests.cpp 1577cf71c4d1fbad11e2ec939c4ceae2d5b25f97 
>   src/tests/fetcher_cache_tests.cpp 1fb1e213d3c35479789688d1a3a49a3c6058b198 
>   src/tests/master_tests.cpp aa0e0d8087175e3a3ed5b63a23d31aa6fe00d1b3 
>   src/tests/master_validation_tests.cpp 
> 6fae01fa1833ae05ec82618a4ae28ac5bd275bd5 
>   src/tests/monitor_tests.cpp 2226458b59b4a279a92e1353bd61457a0018d2a9 
>   src/tests/oversubscription_tests.cpp 
> 7a75fb38e0177e33cf0e7cb82b4b9ebf8f05fe0a 
>   src/tests/reservation_endpoints_tests.cpp 
> b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 
>   src/tests/slave_recovery_tests.cpp 6683a081ac231f1e275a3cdb8ee841da430a9f66 
>   src/tests/slave_tests.cpp 7fe566770bbd802111885de061a53a3edf914840 
> 
> Diff: https://reviews.apache.org/r/41308/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 41306: MESOS-1718: use command line executor to launch tasks.

[Klaus Ma]

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

(Updated Jan. 16, 2016, 1:38 p.m.)


Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.


Changes
---

Add period to the summary.


Summary (updated)
-

MESOS-1718: use command line executor to launch tasks.


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


Repository: mesos


Description
---

MESOS-1718: use command line executor to launch tasks


Diffs
-

  src/master/validation.cpp c7cf56815fc743ff52ef423b23d78398ad1b35a3 
  src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
  src/slave/slave.cpp 759c8d5b1bfb5ad723aa423e1487998ed62bbc3a 

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


Testing
---


Thanks,

Klaus Ma

Re: Review Request 41308: MESOS-1718: Unit Test for moving getExecutorInfo from slave to master.

[Klaus Ma]

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

(Updated Jan. 16, 2016, noon)


Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.


Changes
---

Add period to old summary.


Summary (updated)
-

MESOS-1718: Unit Test for moving getExecutorInfo from slave to master.


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


Repository: mesos


Description
---

MESOS-1718: Unit Test for moving getExecutorInfo from slave to master


Diffs
-

  src/master/master.cpp 863a11c82d322f56db1ccf25b73a41304a46 
  src/tests/containerizer/docker_containerizer_tests.cpp 
cb58b7183c36d96b9ac4803c63980c278a50c97b 
  src/tests/master_tests.cpp 9638fb867b07f85a02d3b78f8282843d8871cabe 
  src/tests/master_validation_tests.cpp 
fbf8fadbc04a7cbc60ee6091e0224339389b400f 
  src/tests/monitor_tests.cpp 2226458b59b4a279a92e1353bd61457a0018d2a9 
  src/tests/reservation_endpoints_tests.cpp 
b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 
  src/tests/slave_recovery_tests.cpp 6683a081ac231f1e275a3cdb8ee841da430a9f66 
  src/tests/slave_tests.cpp 076660daf025d6fd5065cd0c1930f17ecc5ca5aa 

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


Testing
---


Thanks,

Klaus Ma

Re: Review Request 41302: MESOS-1718: add slave's configuration into SlaveInfo.

[Klaus Ma]

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

(Updated Jan. 16, 2016, 1:46 p.m.)


Review request for mesos, Ben Mahler, Ian Downes, Joseph Wu, and Vinod Kone.


Changes
---

Add period to the end of Summary.


Summary (updated)
-

MESOS-1718: add slave's configuration into SlaveInfo.


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


Repository: mesos


Description
---

MESOS-1718: add slave's configuration into SlaveInfo


Diffs
-

  include/mesos/mesos.proto b12e0f3eff44d90ec01360fc08bf9e597d7ed9dd 
  include/mesos/v1/mesos.proto fa7e82e03b11cf6619a4f16e8e0fbbf755bf210c 

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


Testing
---


Thanks,

Klaus Ma

Re: Review Request 41305: MESOS-1718: move getExecutorInfo from slave to master.

[Klaus Ma]

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

(Updated Jan. 16, 2016, 1:46 p.m.)


Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.


Changes
---

Add period to Summary.


Summary (updated)
-

MESOS-1718: move getExecutorInfo from slave to master.


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


Repository: mesos


Description
---

MESOS-1718: move getExecutorInfo from slave to master


Diffs
-

  src/master/constants.hpp ebab341e58035d4b579828add752c1ee37efeb95 
  src/master/master.hpp f02d165874fa8023675e545793de699aeecae29b 
  src/master/master.cpp 863a11c82d322f56db1ccf25b73a41304a46 
  src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
  src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
  src/slave/slave.cpp 759c8d5b1bfb5ad723aa423e1487998ed62bbc3a 

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


Testing
---


Thanks,

Klaus Ma

Re: Review Request 41306: MESOS-1718: use command line executor to launch tasks.

[Klaus Ma]

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

(Updated Jan. 18, 2016, 3:19 p.m.)


Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.


Changes
---

Address comments


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


Repository: mesos


Description
---

MESOS-1718: use command line executor to launch tasks


Diffs (updated)
-

  src/master/validation.cpp c7cf56815fc743ff52ef423b23d78398ad1b35a3 
  src/slave/slave.cpp 759c8d5b1bfb5ad723aa423e1487998ed62bbc3a 

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


Testing
---


Thanks,

Klaus Ma

Re: Review Request 41772: Added helper function to flatten resources.

[Klaus Ma]

> On Jan. 7, 2016, 8:31 a.m., Joseph Wu wrote:
> > src/common/resources.cpp, lines 894-904
> > <https://reviews.apache.org/r/41772/diff/2/?file=1180306#file1180306line894>
> >
> > This seems more appropriate as an optional parameter for 
> > `Resources::flatten`, just like `Option`.
> 
> Guangya Liu wrote:
> My original thinking was adding an optional parameter to flatten(), the 
> reason that I did not do that is because every time I want to set 
> ALLOCATION_SLACK or USAGE_SLACK for a resource, I need to put all paramters, 
> it is a long sentence, so I just add this new helper.
> 
> The caller need to call {flatten("*", None(), 
> Resource::RevocableInfo::ALLOCATION_SLACK);} if want to set a resources as 
> allocation slack.
> 
> Joseph Wu wrote:
> It's fine to have somewhat long expressions.  I think it's preferable 
> over having too many helpers.
> 
> Guangya Liu wrote:
> @Joseph, seems we need to reach an agreement for this ;-) Can you please 
> take a look at the following patches in the patch chain, you may notice that 
> other patches highly depend on those helper functions.
> 
> So what about the following:
> 1) Rename flattenReserved() to reserved_() which will return all reserved 
> resources with different roles in a flatten mode.
> 2) Remove flattenSlack() and merge it to flatten().
> 
> Comments?
> 
> Klaus Ma wrote:
> regarding 1), I'd like to add `allocationSlackable` which mean the 
> reosurces can be offered as `ALLOCATION_SLACK`, it's peer of 
> `allocationSlack`.
> 
> Guangya Liu wrote:
> `allcationSlackable` is more like a `bool` function, but here I want to 
> return a `flatten reserved` resources.
> 
> Klaus Ma wrote:
> It's not about `bool` :). I'd like to avoid the backgroud of 
> `ALLOCATION_SLACK-able` == `stateless.reserved`: the general point is 
> `ALLOCATION_SLACK-able` resources can be lend to other Tenant framework, and 
> those resource will be preempted when Lender framework launch tasks. That's 
> major reason that I'd suggest to introduce helper fuction to avoid 
> `stateless.reserved` in Master/Agent source code.
> 
> En, maybe this can be treat as new MVP named borrow/lend policy worked 
> with Quota; it's different with Optimistic Offer Phase 2, the revocalbe 
> resources will not send to multiple framework; the revocable resources will 
> be shared between frameworks by ratio. @Joseph/Ben, any comments.
> 
> Guangya Liu wrote:
> I think we did discuss this problem when start coding for phase 1, the 
> phase 1 only consider the allocation slack which is get from unused reserved 
> resources, but Quota is not using reserved resources but only unreserved non 
> revocable resources. I think that we can put this to post MVP and write a new 
> document for this and  clarify the behavior there. Comments?

Yes, that's why I call it "new MVP" :). It seems similar proposal with 
MESOS-4392, I'll append my comments to MESOS-4392.


- Klaus


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


On Jan. 16, 2016, 12:23 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41772/
> ---
> 
> (Updated Jan. 16, 2016, 12:23 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, and Klaus Ma.
> 
> 
> Bugs: MESOS-4267
> https://issues.apache.org/jira/browse/MESOS-4267
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added helper function to flatten resources.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp cc8fef9470d779078aa408ed03e747e5a492deaa 
>   include/mesos/v1/resources.hpp f4892977f8d7b0439db6e9cf7921334f606a496c 
>   src/common/resources.cpp 575d6651185d8431f01d589f4afc255cb751181a 
>   src/tests/resources_tests.cpp b42610f1bf8eacfd7bf388d351f8745f1d96f666 
>   src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 
> 
> Diff: https://reviews.apache.org/r/41772/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> GLOG_v=2 ./bin/mesos-tests.sh  --gtest_filter="ResourcesOperationTest.*" 
> --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 42289: Calcuated 'remainingClusterResources' by all activated slaves.

[Klaus Ma]

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


Ping @alex-mesos/@jvanremoortere, any comments.

- Klaus Ma


On Jan. 14, 2016, 4:58 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42289/
> ---
> 
> (Updated Jan. 14, 2016, 4:58 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, Joris Van 
> Remoortere, and Neil Conway.
> 
> 
> Bugs: MESOS-4102
> https://issues.apache.org/jira/browse/MESOS-4102
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Calcuated 'remainingClusterResources' by all activated slaves.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> d541bfa3f4190865c65d35c9d1ffdb8a3f194056 
> 
> Diff: https://reviews.apache.org/r/42289/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 41308: MESOS-1718: Unit Test for moving getExecutorInfo from slave to master

[Klaus Ma]

> On Jan. 13, 2016, 8:39 p.m., Klaus Ma wrote:
> > @Joseph/Vinod, would you help to review those patches move executor from 
> > slave to master?
> 
> Joseph Wu wrote:
> There are still several open issues open on each prior review.  Can you 
> go through and make sure those are addressed?

Honestly, I'd like you to help review the code diff about executor's resources: 
where should get command line executor's resources? cut from tasks or add 
additional resources to it.


- Klaus


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


On Dec. 12, 2015, 5:58 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41308/
> ---
> 
> (Updated Dec. 12, 2015, 5:58 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
> Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-1718
> https://issues.apache.org/jira/browse/MESOS-1718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-1718: Unit Test for moving getExecutorInfo from slave to master
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 3f199e6 
>   src/tests/master_tests.cpp 865fa4a 
>   src/tests/master_validation_tests.cpp fbf8fad 
>   src/tests/monitor_tests.cpp a848d14 
>   src/tests/reservation_endpoints_tests.cpp d5d2aa7 
>   src/tests/slave_recovery_tests.cpp c0e4ff7 
>   src/tests/slave_tests.cpp 4975bea 
> 
> Diff: https://reviews.apache.org/r/41308/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 41302: MESOS-1718: add slave's configuration into SlaveInfo

[Klaus Ma]

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

(Updated Jan. 15, 2016, 1:11 p.m.)


Review request for mesos, Ben Mahler, Ian Downes, Joseph Wu, and Vinod Kone.


Changes
---

rebase


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


Repository: mesos


Description
---

MESOS-1718: add slave's configuration into SlaveInfo


Diffs (updated)
-

  include/mesos/mesos.proto b12e0f3eff44d90ec01360fc08bf9e597d7ed9dd 
  include/mesos/v1/mesos.proto fa7e82e03b11cf6619a4f16e8e0fbbf755bf210c 

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


Testing
---


Thanks,

Klaus Ma

Re: Review Request 41305: MESOS-1718: move getExecutorInfo from slave to master

[Klaus Ma]

> On Dec. 14, 2015, 2:31 p.m., Qian Zhang wrote:
> > src/slave/slave.cpp, line 1370
> > <https://reviews.apache.org/r/41305/diff/1/?file=1161529#file1161529line1370>
> >
> > Before trying to get task's executor via `task.executor()`, suggest to 
> > add a CHECK to ensure the task always has an executor.

It's not necessary, executor is set in master.


- Klaus


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


On Jan. 15, 2016, 1:17 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41305/
> ---
> 
> (Updated Jan. 15, 2016, 1:17 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
> Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-1718
> https://issues.apache.org/jira/browse/MESOS-1718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-1718: move getExecutorInfo from slave to master
> 
> 
> Diffs
> -
> 
>   src/master/constants.hpp ebab341e58035d4b579828add752c1ee37efeb95 
>   src/master/master.hpp f02d165874fa8023675e545793de699aeecae29b 
>   src/master/master.cpp 863a11c82d322f56db1ccf25b73a41304a46 
>   src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
>   src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
>   src/slave/slave.cpp 759c8d5b1bfb5ad723aa423e1487998ed62bbc3a 
> 
> Diff: https://reviews.apache.org/r/41305/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 41308: MESOS-1718: Unit Test for moving getExecutorInfo from slave to master

[Klaus Ma]

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

(Updated Jan. 15, 2016, 1:19 p.m.)


Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.


Changes
---

rebase


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


Repository: mesos


Description
---

MESOS-1718: Unit Test for moving getExecutorInfo from slave to master


Diffs (updated)
-

  src/master/master.cpp 863a11c82d322f56db1ccf25b73a41304a46 
  src/tests/containerizer/docker_containerizer_tests.cpp 
cb58b7183c36d96b9ac4803c63980c278a50c97b 
  src/tests/master_tests.cpp 9638fb867b07f85a02d3b78f8282843d8871cabe 
  src/tests/master_validation_tests.cpp 
fbf8fadbc04a7cbc60ee6091e0224339389b400f 
  src/tests/monitor_tests.cpp 2226458b59b4a279a92e1353bd61457a0018d2a9 
  src/tests/reservation_endpoints_tests.cpp 
b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 
  src/tests/slave_recovery_tests.cpp 6683a081ac231f1e275a3cdb8ee841da430a9f66 
  src/tests/slave_tests.cpp 076660daf025d6fd5065cd0c1930f17ecc5ca5aa 

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


Testing
---


Thanks,

Klaus Ma

Re: Review Request 41306: MESOS-1718: use command line executor to launch tasks

[Klaus Ma]

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

(Updated Jan. 15, 2016, 1:18 p.m.)


Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.


Changes
---

rebase


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


Repository: mesos


Description
---

MESOS-1718: use command line executor to launch tasks


Diffs (updated)
-

  src/master/validation.cpp c7cf56815fc743ff52ef423b23d78398ad1b35a3 
  src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
  src/slave/slave.cpp 759c8d5b1bfb5ad723aa423e1487998ed62bbc3a 

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


Testing
---


Thanks,

Klaus Ma

Re: Review Request 41305: MESOS-1718: move getExecutorInfo from slave to master

[Klaus Ma]

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

(Updated Jan. 15, 2016, 1:17 p.m.)


Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.


Changes
---

rebase


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


Repository: mesos


Description
---

MESOS-1718: move getExecutorInfo from slave to master


Diffs (updated)
-

  src/master/constants.hpp ebab341e58035d4b579828add752c1ee37efeb95 
  src/master/master.hpp f02d165874fa8023675e545793de699aeecae29b 
  src/master/master.cpp 863a11c82d322f56db1ccf25b73a41304a46 
  src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
  src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
  src/slave/slave.cpp 759c8d5b1bfb5ad723aa423e1487998ed62bbc3a 

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


Testing
---


Thanks,

Klaus Ma

Re: Review Request 42289: Calcuated 'remainingClusterResources' by all activated slaves.

[Klaus Ma]

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

(Updated Jan. 19, 2016, 4:40 p.m.)


Review request for mesos, Alexander Rukletsov, Ben Mahler, Joris Van 
Remoortere, and Neil Conway.


Changes
---

Also handle slave in whitelist.


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


Repository: mesos


Description
---

Calcuated 'remainingClusterResources' by all activated slaves.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
48acde69b1a2f305b568a7e322a58708063dd30a 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 42289: Calcuated 'remainingClusterResources' by all activated slaves.

[Klaus Ma]

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

(Updated Jan. 19, 2016, 6:55 p.m.)


Review request for mesos, Alexander Rukletsov, Ben Mahler, Joris Van 
Remoortere, and Neil Conway.


Changes
---

Address comments.


Summary (updated)
-

Calcuated 'remainingClusterResources' by all activated slaves.


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


Repository: mesos


Description (updated)
---

__Phenomenon__:
Quota doesn't allocate resources on slave joining.

__Root Cause__:
Event-triggered allocations do not include all available agents. If we
calculate remaining resources in the cluster using the partial view,
we may overlook already laid away resources for quota'ed roles and lay
away more. Hence we may unnecessarily deprive non-quota'ed frameworks
of resources.

Refer to AlexR's comments for more detail:
https://issues.apache.org/jira/browse/MESOS-4102?focusedCommentId=15048495=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15048495

__Solution/Fix__:
Calcuated 'remainingClusterResources' by all activated slaves.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
48acde69b1a2f305b568a7e322a58708063dd30a 
  src/tests/hierarchical_allocator_tests.cpp 
9362dd306497ba01e0f387c3862456cdcac6f863 

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


Testing
---

make
make check


Thanks,

Klaus Ma

Re: Review Request 42289: Calcuated 'remainingClusterResources' by all activated slaves.

[Klaus Ma]

> On Jan. 19, 2016, 5:23 p.m., Alexander Rukletsov wrote:
> > Let's tweak some wording and testing and we are good to go!
> > 
> > I liked the initial summary more. IMO a patch should describe the solution, 
> > and not the problem. It's quite opposite for JIRA tickets, hence I'm 
> > convinced patches and tickets should not share the same title. How about 
> > "Calcuated 'remainingClusterResources' using all available agents."
> > 
> > I also think it won't hurt to extend the description and explain why we 
> > need this change. How about
> > "Event-triggered allocations do not include all available agents. If we
> > calculate remaining resources in the cluster using the partial view,
> > we may overlook already laid away resources for quota'ed roles and lay
> > away more. Hence we may unnecessarily deprive non-quota'ed frameworks
> > of resources."
> > 
> > Changes touching allocator are vulnerable to races, especially in tests. 
> > Please extend the testing (and mention this in the "Testing Done" section) 
> > with goodies like `GTEST_FILTER="*HierarchicalAllocatorTest*" 
> > ./bin/mesos-tests.sh --gtest_shuffle --gtest-repeat`.
> > 
> > Let's add a test for this change. Ideally the test should fail without the 
> > change and pass it with the change. I think Neil has already provided the 
> > test in the ticket, could you please include it?
> 
> Alexander Rukletsov wrote:
> Maybe even better, you can modify existing `QuotaAbsentFramework` test.

Yes, that's what I'm thinking :). Current patch passed Neil's test in Mac OS, 
I'll re-check it in Ubuntu for `./bin/mesos-tests.sh`; it seems perf did not 
support no-Linux system.


- Klaus


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


On Jan. 19, 2016, 6:55 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42289/
> ---
> 
> (Updated Jan. 19, 2016, 6:55 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, Joris Van 
> Remoortere, and Neil Conway.
> 
> 
> Bugs: MESOS-4102
> https://issues.apache.org/jira/browse/MESOS-4102
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> __Phenomenon__:
> Quota doesn't allocate resources on slave joining.
> 
> __Root Cause__:
> Event-triggered allocations do not include all available agents. If we
> calculate remaining resources in the cluster using the partial view,
> we may overlook already laid away resources for quota'ed roles and lay
> away more. Hence we may unnecessarily deprive non-quota'ed frameworks
> of resources.
> 
> Refer to AlexR's comments for more detail:
> https://issues.apache.org/jira/browse/MESOS-4102?focusedCommentId=15048495=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15048495
> 
> __Solution/Fix__:
> Calcuated 'remainingClusterResources' by all activated slaves.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 48acde69b1a2f305b568a7e322a58708063dd30a 
>   src/tests/hierarchical_allocator_tests.cpp 
> 9362dd306497ba01e0f387c3862456cdcac6f863 
> 
> Diff: https://reviews.apache.org/r/42289/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 41302: MESOS-1718: add slave's configuration into SlaveInfo.

[Klaus Ma]

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

(Updated Jan. 19, 2016, 10:56 a.m.)


Review request for mesos, Ben Mahler, Ian Downes, Joseph Wu, and Vinod Kone.


Changes
---

rebase for apply conflict


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


Repository: mesos


Description
---

MESOS-1718: add slave's configuration into SlaveInfo


Diffs (updated)
-

  include/mesos/mesos.proto 0be4bed336e86a5c377e87ac6212c70ac3b4c66b 
  include/mesos/v1/mesos.proto c3244e87f9351c71312d2eace7a49bcac926fafd 

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


Testing
---


Thanks,

Klaus Ma

Re: Review Request 41305: MESOS-1718: move getExecutorInfo from slave to master.

[Klaus Ma]

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

(Updated Jan. 19, 2016, 10:55 a.m.)


Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.


Changes
---

rebase for apply conflict


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


Repository: mesos


Description
---

MESOS-1718: move getExecutorInfo from slave to master


Diffs (updated)
-

  src/master/constants.hpp 2c3299bc1aaa0888f7e47a71965c56ada8ecc21f 
  src/master/master.hpp 3a7e18232323a1c051bcc97915484b1195fffe58 
  src/master/master.cpp 9ee56277c8a472be9e683d5db505becfb5f7c422 
  src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
  src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
  src/slave/slave.cpp bb501810d7bb1261ebbbdd147c49948e5a2f8665 

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


Testing
---


Thanks,

Klaus Ma

Re: Review Request 41306: MESOS-1718: use command line executor to launch tasks.

[Klaus Ma]

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

(Updated Jan. 19, 2016, 10:55 a.m.)


Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.


Changes
---

rebase for apply conflict


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


Repository: mesos


Description
---

MESOS-1718: use command line executor to launch tasks


Diffs (updated)
-

  src/master/validation.cpp 98360b690382ed1912a868ac93b058cb28003a12 
  src/slave/slave.cpp bb501810d7bb1261ebbbdd147c49948e5a2f8665 

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


Testing
---


Thanks,

Klaus Ma

Re: Review Request 41308: MESOS-1718: Unit Test for moving getExecutorInfo from slave to master.

[Klaus Ma]

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

(Updated Jan. 19, 2016, 10:55 a.m.)


Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.


Changes
---

rebase for apply conflict


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


Repository: mesos


Description
---

MESOS-1718: Unit Test for moving getExecutorInfo from slave to master


Diffs (updated)
-

  src/master/master.cpp 9ee56277c8a472be9e683d5db505becfb5f7c422 
  src/tests/containerizer/docker_containerizer_tests.cpp 
81ab3625a69644d9659d484f3c605aaa5a10b397 
  src/tests/disk_quota_tests.cpp 1577cf71c4d1fbad11e2ec939c4ceae2d5b25f97 
  src/tests/fetcher_cache_tests.cpp 2747b72ba49c9fde04e556b649601b037517283e 
  src/tests/master_tests.cpp aa0e0d8087175e3a3ed5b63a23d31aa6fe00d1b3 
  src/tests/master_validation_tests.cpp 
6fae01fa1833ae05ec82618a4ae28ac5bd275bd5 
  src/tests/monitor_tests.cpp 2226458b59b4a279a92e1353bd61457a0018d2a9 
  src/tests/oversubscription_tests.cpp 7a75fb38e0177e33cf0e7cb82b4b9ebf8f05fe0a 
  src/tests/reservation_endpoints_tests.cpp 
b8edd6fafedd4c2221a8d19c1ebc71254071a8c7 
  src/tests/slave_recovery_tests.cpp 6683a081ac231f1e275a3cdb8ee841da430a9f66 
  src/tests/slave_tests.cpp 7fe566770bbd802111885de061a53a3edf914840 

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


Testing
---


Thanks,

Klaus Ma

Re: Review Request 42289: Calcuated 'remainingClusterResources' by all activated slaves.

[Klaus Ma]

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

(Updated Jan. 19, 2016, 7:10 p.m.)


Review request for mesos, Alexander Rukletsov, Ben Mahler, Joris Van 
Remoortere, and Neil Conway.


Changes
---

Repeated test done for allocator.


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


Repository: mesos


Description
---

__Phenomenon__:
Quota doesn't allocate resources on slave joining.

__Root Cause__:
Event-triggered allocations do not include all available agents. If we
calculate remaining resources in the cluster using the partial view,
we may overlook already laid away resources for quota'ed roles and lay
away more. Hence we may unnecessarily deprive non-quota'ed frameworks
of resources.

Refer to AlexR's comments for more detail:
https://issues.apache.org/jira/browse/MESOS-4102?focusedCommentId=15048495=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15048495

__Solution/Fix__:
Calcuated 'remainingClusterResources' by all activated slaves.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
48acde69b1a2f305b568a7e322a58708063dd30a 
  src/tests/hierarchical_allocator_tests.cpp 
9362dd306497ba01e0f387c3862456cdcac6f863 

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


Testing (updated)
---

make
make check
./bin/mesos-tests.sh --gtest_filter="HierarchicalAllocatorTest.*" --verbose 
--gtest_repeat=100 --gtest_shuffle


Thanks,

Klaus Ma

Re: Review Request 42289: Calcuated 'remainingClusterResources' by all activated slaves.

[Klaus Ma]

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

(Updated Jan. 19, 2016, 8:03 p.m.)


Review request for mesos, Alexander Rukletsov, Ben Mahler, Joris Van 
Remoortere, and Neil Conway.


Changes
---

Address comments


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


Repository: mesos


Description
---

__Phenomenon__:
Quota doesn't allocate resources on slave joining.

__Root Cause__:
Event-triggered allocations do not include all available agents. If we
calculate remaining resources in the cluster using the partial view,
we may overlook already laid away resources for quota'ed roles and lay
away more. Hence we may unnecessarily deprive non-quota'ed frameworks
of resources.

Refer to AlexR's comments for more detail:
https://issues.apache.org/jira/browse/MESOS-4102?focusedCommentId=15048495=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15048495

__Solution/Fix__:
Calcuated 'remainingClusterResources' by all activated slaves.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
48acde69b1a2f305b568a7e322a58708063dd30a 
  src/tests/hierarchical_allocator_tests.cpp 
9362dd306497ba01e0f387c3862456cdcac6f863 

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


Testing
---

make
make check
./bin/mesos-tests.sh --gtest_filter="HierarchicalAllocatorTest.*" --verbose 
--gtest_repeat=100 --gtest_shuffle


Thanks,

Klaus Ma

Re: Review Request 49140: Added startsWith/endsWith to support char.

[Klaus Ma]

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

(Updated June 26, 2016, 5:40 p.m.)


Review request for mesos and Michael Park.


Changes
---

Address mycpark's comments.


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


Repository: mesos


Description
---

Added startsWith/endsWith to support char.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp 9bae71246e751e491be5a989eea8aca29c9aa751 
  3rdparty/stout/include/stout/strings.hpp 
162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 
  docs/contributors.yaml 083715f23bb2d0610d94dd8995a026254125 
  docs/home.md 2d728333a3c1421b310d861b71c92691fd41a482 
  docs/networking.md f6652f58b02edee08e0b2410c23b2beb4d25e83b 
  docs/port-mapping-isolator.md 6a6bab9df3f6f3960ceeab38b6302823dceae9c2 
  docs/submitting-a-patch.md ffc6e561b8721b8849ef6025c15936ea712d3bfa 
  src/Makefile.am 61789a7603dac08a5f8ac4fe3d63b43e123ed98a 
  src/common/http.hpp 55bd0ac81af80c656a4a80766a3e4b21db9cf0cf 
  src/common/http.cpp 5cc9e86bc7966051507ce6620651d1f5d0914994 
  src/files/files.cpp a5a1b86e14f63e5e3834a2900270252fbe16f586 
  src/master/http.cpp ad3f723ec21dd0e9f3a3f015d4de0702625cd603 
  src/master/master.cpp d89c049326358bad509bb1455c81eb11610eeeb8 
  src/slave/containerizer/mesos/isolators/gpu/allocator.hpp 
b2eabfebef99ccebef427d144bb816adc0175ecf 
  src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
769dfa242ed5322a72b65fbb422894e70af2ad0c 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
92b33111799cb4e1c8bc2051381e1254d701d95c 
  src/slave/containerizer/mesos/isolators/network/cni/paths.hpp 
7678a7c81c3cdb27410c1f066021eb34bd02a83f 
  src/slave/containerizer/mesos/isolators/network/cni/paths.cpp 
f9056c90f1075cb19c4f554e7d5b629561d06035 
  src/slave/http.cpp c038bf0c9680ec86f77f1a27efeb7354a9e67627 
  src/slave/slave.cpp da643e6e50b2f313705d2f862c961291aa5d2f22 
  src/tests/containerizer/cni_isolator_tests.cpp 
991823b96f8eac7539625ef0f1e045e6a10464ac 
  src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
28ec3f9954576d78153e9d0f57e22a240e950639 
  src/tests/files_tests.cpp 31337e280c6224a8c949c8868a53e5a785b4573f 
  src/tests/mesos.hpp bcd060ec07da27db8738950bd81ead37da11ee70 
  src/tests/mesos.cpp 6918343b13a735aec243b54a9fcbced056894f53 
  src/tests/persistent_volume_endpoints_tests.cpp 
6c85e19eeaa69bf3a4e3077261331191db6eec06 
  src/tests/reservation_endpoints_tests.cpp 
3ee59d5db0089dd59acfe48a77910d069ffc377b 
  src/tests/slave_authorization_tests.cpp 
18bcb0e499a9d2d84113b5b9e609e5e40913ebcc 
  support/docker_build.sh 8ae1aadbc12b12e44984d34ccfbcb8a97bf05bcf 

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


Testing
---

make


Thanks,

Klaus Ma

Re: Review Request 49140: Added startsWith/endsWith to support char.

[Klaus Ma]

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

(Updated June 26, 2016, 5:46 p.m.)


Review request for mesos and Michael Park.


Changes
---

rebase


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


Repository: mesos


Description
---

Added startsWith/endsWith to support char.


Diffs (updated)
-

  3rdparty/stout/include/stout/strings.hpp 
162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 

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


Testing
---

make


Thanks,

Klaus Ma

Review Request 49223: WIP: enhance value parsing.

[Klaus Ma]

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

Review request for mesos.


Repository: mesos


Description
---

WIP: enhance value parsing.


Diffs
-

  src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 

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


Testing
---

make


Thanks,

Klaus Ma

Re: Review Request 49223: Enhanced Value parsing.

[Klaus Ma]

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

(Updated June 27, 2016, 11:18 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Enhance UT cases.


Summary (updated)
-

Enhanced Value parsing.


Repository: mesos


Description (updated)
---

Enhanced Value parsing:

1. Did not support `[1-2, [3-4]]` as Ranges; it should be `[1-2, 3-4]`.
2. Did not support `{a{b, c}d}` as Set; it should be `{ab, cd}`
3. Add check for Test agains `[a-zA-Z0-9_/.-]`


Diffs (updated)
-

  src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
  src/tests/attributes_tests.cpp cb71be5ecead322d90943146f54f8a0c915eba1c 
  src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 

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


Testing (updated)
---

make && make check


Thanks,

Klaus Ma

Re: Review Request 49223: Enhanced Value parsing.

[Klaus Ma]

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

(Updated June 27, 2016, 11:22 a.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

typo


Repository: mesos


Description (updated)
---

Enhanced Value parsing:

1. Did not support `[1-2, [3-4]]` as Ranges; it should be `[1-2, 3-4]`.
2. Did not support `{a{b, c}d}` as Set; it should be `{ab, cd}`
3. Add check for Text against `[a-zA-Z0-9_/.-]`


Diffs
-

  src/common/values.cpp 587cb68551d438621e215953e89818b623b7f71b 
  src/tests/attributes_tests.cpp cb71be5ecead322d90943146f54f8a0c915eba1c 
  src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 

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


Testing
---

make && make check


Thanks,

Klaus Ma

Review Request 49246: Enhanced startsWith/endsWith's performance.

[Klaus Ma]

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

Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

Enhanced startsWith/endsWith's performance.


Diffs
-

  3rdparty/stout/include/stout/strings.hpp 
162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 

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


Testing
---

make && make check


Thanks,

Klaus Ma

Re: Review Request 49246: Enhanced startsWith/endsWith's performance.

[Klaus Ma]

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

(Updated June 27, 2016, 2:16 p.m.)


Review request for mesos and Michael Park.


Changes
---

Update JIRA


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


Repository: mesos


Description
---

Enhanced startsWith/endsWith's performance.


Diffs
-

  3rdparty/stout/include/stout/strings.hpp 
162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 

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


Testing
---

make && make check


Thanks,

Klaus Ma

Re: Review Request 49140: Added startsWith/endsWith to support char.

[Klaus Ma]

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



Sync up with mycpark offline, I'll also update the string version of 
`startsWith/endsWith` to improve the performance.

- Klaus Ma


On June 26, 2016, 5:46 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49140/
> ---
> 
> (Updated June 26, 2016, 5:46 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-5692
> https://issues.apache.org/jira/browse/MESOS-5692
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added startsWith/endsWith to support char.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/strings.hpp 
> 162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 
> 
> Diff: https://reviews.apache.org/r/49140/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 43328: Title for documentation webpages.

[Klaus Ma]

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


Ship it!




Ship It!

- Klaus Ma


On Feb. 8, 2016, 11:28 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43328/
> ---
> 
> (Updated Feb. 8, 2016, 11:28 p.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Bugs: MESOS-4588
> https://issues.apache.org/jira/browse/MESOS-4588
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Title for documentation webpages.
> 
> 
> Diffs
> -
> 
>   docs/allocation-module.md 3bbb9045dea2c0e50c4791cc3e220cfe383e0b7b 
>   docs/app-framework-development-guide.md 
> e0f40adacf96bdf0c510b3400eb0ed0cd964ab9d 
>   docs/architecture.md d1b3c0e7ffab1b9e70fc51282a46cf97b7718a48 
>   docs/attributes-resources.md 53a27f48c94baa9b25a072e0fc503bde0b240ba2 
>   docs/authentication.md b40b09eaceddc8e5498b54b3e8ef7a5dbd7b9dc2 
>   docs/authorization.md dbbfd60cb35cbb67e47b6a468d4f4ab824981e5d 
>   docs/c++-style-guide.md 5e2d88d5a7ae06191346bf672f58dbe27b2bccee 
>   docs/clang-format.md 5e720e016ae2635c6a3e1825c0a7fc36f1ef07b7 
>   docs/committer-candidate-checklist.md 
> fd55a398c836a30ba80203a64ba10fefe2cb9108 
>   docs/committers.md 94ee9a6553ad5cd894db6f71825f46a7a3239eb8 
>   docs/committing.md eabc0aad9c4fb96586fd1ae3bd2935e1a1a9c3a7 
>   docs/configuration.md b1ef1314eb9482a55015baa42b51d59b9f464a29 
>   docs/containerizer-internals.md 20bf2d16d4f06994ee766a0c15f3528513751efe 
>   docs/containerizer.md cd23cf9c006b862e6e2cf4e215706eff03cd07f8 
>   docs/deploy-scripts.md 87f8eb6254dbf00dc537a180b8d3e0084165d17c 
>   docs/docker-containerizer.md 89dfa0ba4b8f7b9761ce48340b30daa5a30bd8cc 
>   docs/effective-code-reviewing.md 5a633bc324290bc1f57884bab550ee9f246f9d0f 
>   docs/engineering-principles-and-practices.md 
> 6babb929ead758ee5187d5fc5760d084876c3298 
>   docs/executor-http-api.md fd80005002694bcc72abd04ceea35a2d00814401 
>   docs/external-containerizer.md 8a16e5a75e374fc2848e57cb108dc5b4df7e5be1 
>   docs/fetcher-cache-internals.md 1ccb1c2cbb0a80771f79e98c0bdcc8ee8464ea30 
>   docs/fetcher.md cb4f3c33fb67f7ac65dbe7ebe1347d2599c43f37 
>   docs/framework-rate-limiting.md af33c5e9ed7c5519dc58c7c5657f3c999de3dd1f 
>   docs/frameworks.md e3d50f608ca1e5752f65c411eb09483074146562 
>   docs/getting-started.md 7177f9d5ae33dbabf0ed1bb1b9870873906e1a56 
>   docs/high-availability-framework-guide.md 
> 2576fa2a6e3021c34d643d3da7585fc6d95913c7 
>   docs/high-availability.md 3e9802bd3a2a6f75fff3aea3cfb319adce3271cf 
>   docs/home.md dea6ec2605662dbd4b10d69b2bf8f35af50389ec 
>   docs/logging.md d79a74708775e5dc01df559042cdf790dfd20bd6 
>   docs/maintenance.md e6bfe0f655581a6a72de4579bd7e5753625c0e51 
>   docs/mesos-containerizer.md 87f145cd957dcb8fd3188c866212b417f0ab6296 
>   docs/mesos-provisioner.md 3597cfc1d484a25f885ff95ab6ce05d615fc2c74 
>   docs/modules.md 8e0249d9829a06b3245fc193f0ee383956d33375 
>   docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
>   docs/network-monitoring.md 08fcaa8ebc7313a4cf9a719ff1a9b11fcd926c7f 
>   docs/networking-for-mesos-managed-containers.md 
> f2fbea54cc3a7f1727c53629c86feb25d7f5fd68 
>   docs/newbie-guide.md f6e61210ed834d245fd4922c1dbbf1e7d05ff64e 
>   docs/operational-guide.md 4680ee3397d081acd6f82499703de4456e7ca4f4 
>   docs/oversubscription.md 0b1c20bf9bae9c179d82f5f611638faf91f91431 
>   docs/persistent-volume.md 3a3e370695f6e5c0cb5be24d64eb885050296c0b 
>   docs/powered-by-mesos.md 6e113ee6923d88a4d8188fa55c6c23fd8aaa35b4 
>   docs/presentations.md 2ce4b12b2a1eceb927206d7e7828bb3bbee49b52 
>   docs/quota.md 208bfa06a9fc50843439bab9a041ccb557657b5d 
>   docs/reconciliation.md ea11905720a3cd60e88b18e64e5b882f5e250166 
>   docs/release-guide.md 7c9ee140b394d61581f736f2413969d650f18fa4 
>   docs/reporting-a-bug.md b980159a7988ba952d908034317b39007c12cf2d 
>   docs/reservation.md 8d2d33a6518c73542cbfb3a5ee36da1c00c6ff1a 
>   docs/roadmap.md 1f68ba0f63325c2f4cf5718c7afc67d39622ae9c 
>   docs/roles.md 609a63cbff2d9c652af45ba16152ce3caf48 
>   docs/sandbox.md 276e1126f495e7af15afd4f4ad8c63beb40db739 
>   docs/scheduler-http-api.md 3c0b8ff8b831dfe595652229111b7f2ca1a2b933 
>   docs/slave-recovery.md acf36b9693972641f38a96e8d6b682e02b6cdbb3 
>   docs/ssl.md be30b696102b2a5b16d88bc10894f13e67d27f4b 
>   docs/submitting-a-patch.md 50361a9c9c4a246d6ee939f057eeabf4c88eb297 
>   docs/testing-patterns.md cc150d6204992b551fe820d5aab54d6a6

Re: Review Request 43159: Removed the duplicate "active" field in json schema of `Framework`.

[Klaus Ma]

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


Ship it!




Ship It!

- Klaus Ma


On Feb. 4, 2016, 7:11 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43159/
> ---
> 
> (Updated Feb. 4, 2016, 7:11 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4582
> https://issues.apache.org/jira/browse/MESOS-4582
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new `jsonify` library is a writer-based approach, and does not keep track 
> of the fields that have been written out so far.
> The previous version of `summarize(framework)` and `model(framework)` had a 
> duplicate `"active"` field which was de-duplicated since they simply get 
> inserted to a `std::map`, overriding the previous value.
> 
> In the `jsonify` case, this pattern results in duplicate key in the JSON 
> output. Although the presence of duplicate keys is technically not 
> __invalid__ according to the JSON specification, some JSON libraries disallow 
> them.
> As such, we should generate JSON outputs without duplicate keys.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 3d7a624b78fd85a8d99bce609e37411ed660678c 
> 
> Diff: https://reviews.apache.org/r/43159/diff/
> 
> 
> Testing
> ---
> 
> Verified that `make check` __with__ https://reviews.apache.org/r/43160/ + 
> https://reviews.apache.org/r/43161/ and __without__ this patch breaks.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 43159: Removed the duplicate "active" field in json schema of `Framework`.

[Klaus Ma]

> On Feb. 7, 2016, 11:29 a.m., Klaus Ma wrote:
> > Ship It!

Add test case for duplicated keys in our output: did not prase json, just 
`std::count` should be OK.


- Klaus


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


On Feb. 4, 2016, 7:11 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43159/
> ---
> 
> (Updated Feb. 4, 2016, 7:11 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4582
> https://issues.apache.org/jira/browse/MESOS-4582
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new `jsonify` library is a writer-based approach, and does not keep track 
> of the fields that have been written out so far.
> The previous version of `summarize(framework)` and `model(framework)` had a 
> duplicate `"active"` field which was de-duplicated since they simply get 
> inserted to a `std::map`, overriding the previous value.
> 
> In the `jsonify` case, this pattern results in duplicate key in the JSON 
> output. Although the presence of duplicate keys is technically not 
> __invalid__ according to the JSON specification, some JSON libraries disallow 
> them.
> As such, we should generate JSON outputs without duplicate keys.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 3d7a624b78fd85a8d99bce609e37411ed660678c 
> 
> Diff: https://reviews.apache.org/r/43159/diff/
> 
> 
> Testing
> ---
> 
> Verified that `make check` __with__ https://reviews.apache.org/r/43160/ + 
> https://reviews.apache.org/r/43161/ and __without__ this patch breaks.
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 42946: Replaced tabs with spaces in configure.ac.

[Klaus Ma]

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



ping @Joris, any comments on this?

- Klaus Ma


On Jan. 29, 2016, 4:19 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42946/
> ---
> 
> (Updated Jan. 29, 2016, 4:19 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4519
> https://issues.apache.org/jira/browse/MESOS-4519
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replace tabs with spaces in configure.ac.
> 
> 
> Diffs
> -
> 
>   configure.ac cb39c7f 
> 
> Diff: https://reviews.apache.org/r/42946/diff/
> 
> 
> Testing
> ---
> 
> ../configure
> make
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 43267: Returned "ServiceUnavailable" for slave's /state during recovery.

[Klaus Ma]

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




src/slave/http.cpp (line 402)
<https://reviews.apache.org/r/43267/#comment179393>

For GUI developer, it's better to export more info when it's recovering, 
e.g. build_time, build_date. So I'd like to export agent state in `/state`.



src/tests/slave_tests.cpp (line 1405)
<https://reviews.apache.org/r/43267/#comment179394>

```
driver.stop()
driver.join()
```


- Klaus Ma


On Feb. 6, 2016, 6:46 a.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43267/
> ---
> 
> (Updated Feb. 6, 2016, 6:46 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Ben Mahler.
> 
> 
> Bugs: MESOS-4066
> https://issues.apache.org/jira/browse/MESOS-4066
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will help operators hitting the /state endpoint by not returning 
> incomplete
> data when slave is still recoverying.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 9167030e77efc7e1d0dc0c6bd800d20ba9c55e3c 
>   src/tests/slave_tests.cpp b2b1fd4be933512c3dffa8c1c579b59782a37d77 
> 
> Diff: https://reviews.apache.org/r/43267/diff/
> 
> 
> Testing
> ---
> 
> make -j3 check GTEST_FILTER="*StateEndpointUnavailableDuringRecovery*"
> 
> ./bin/mesos-tests.sh 
> --gtest_filter="*StateEndpointUnavailableDuringRecovery*" --gtest_repeat=1000 
> --gtest_break_on_failure
> 
> ./bin/mesos-tests.sh
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 42589: Added test case for allocator recover with Quota.

[Klaus Ma]

> On Feb. 1, 2016, 7:57 p.m., Alexander Rukletsov wrote:
> > What is the use case you want to test here? Specifically, why do you test 
> > *both* resume paths simulataneously? Will the outcome of the test change if 
> > you remove one of those?

This is the test case for https://reviews.apache.org/r/42535 ; without r42535, 
this test will crash.


- Klaus


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


On Jan. 21, 2016, 2:27 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42589/
> ---
> 
> (Updated Jan. 21, 2016, 2:27 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, and Joris Van 
> Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test case for allocator recover with Quota.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 953712149bd951789beb29c72779c4ac65aa48dc 
> 
> Diff: https://reviews.apache.org/r/42589/diff/
> 
> 
> Testing
> ---
> 
> make
> make check GTEST_FILTER="HierarchicalAllocatorTest.RecoverWithQuota" (CHECK() 
> failed without fix)
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 43144: Added env var to set upper bound on number of libprocess worker threads.

[Klaus Ma]

> On Feb. 4, 2016, 9:38 p.m., Klaus Ma wrote:
> > 3rdparty/libprocess/src/process.cpp, line 2211
> > <https://reviews.apache.org/r/43144/diff/1/?file=1231709#file1231709line2211>
> >
> > Just `os::getenv("LIB...")` is OK.
> 
> Maged Michael wrote:
> The string is used in two places: getenv and VLOG(WARNING). I thought to 
> avoid mismatch between the two literals.
> How about 
>constexpr auto env_var = "LIBPROCESS_MAX_WORKER_THREADS";
> ?

That's OK to me.


- Klaus


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


On Feb. 5, 2016, 3:55 a.m., Maged Michael wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43144/
> -------
> 
> (Updated Feb. 5, 2016, 3:55 a.m.)
> 
> 
> Review request for mesos and Klaus Ma.
> 
> 
> Bugs: MESOS-4353
> https://issues.apache.org/jira/browse/MESOS-4353
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added env var to set upper bound on number of libprocess worker threads.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp d8a74d7 
> 
> Diff: https://reviews.apache.org/r/43144/diff/
> 
> 
> Testing
> ---
> 
> Passed make check on x86_64 Ubuntu 14.04.
> 
> Invocations of mesos-master, mesos-slave, and mesos-tests with the 
> LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
> "-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system. The results were as 
> expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were created, respectively. 
> The last 3 tests generated warnings.
> 
> 
> Thanks,
> 
> Maged Michael
> 
>

Re: Review Request 40375: Support distinguishing revocable resources in the Resource protobuf.

[Klaus Ma]

> On Feb. 4, 2016, 7:43 p.m., Alexander Rukletsov wrote:
> > include/mesos/mesos.proto, lines 643-649
> > <https://reviews.apache.org/r/40375/diff/10/?file=1185761#file1185761line643>
> >
> > This looks like an internal information, the *source* of a revocable 
> > resource. While we definitely need this is the allocator, I'm not sure we 
> > should expose it in the public protobuf. I think frameworks are more 
> > interested in the *quality* of a resource. Can we keep the source private 
> > somehow?

Framework need to know revocable type as master did not allow to use both 
USAGE_SLACK and ALLOCATION_SLACK for one task; it's because the controller 
(QoSController & evicting modules) is different for now.


- Klaus


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


On Jan. 7, 2016, 11:43 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40375/
> ---
> 
> (Updated Jan. 7, 2016, 11:43 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-3888
> https://issues.apache.org/jira/browse/MESOS-3888
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3888: We need to distinguish revocable resource for usage slack and 
> allocation slack.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 2431fdd6b84625c6140a2b3913736bffada4e7f6 
>   include/mesos/v1/mesos.proto 4aed0980b28dc1000aa2821f35303b736bc5bff8 
>   src/common/resources.cpp 6afd2dfd81adecf7bdb3e523bc840efb62b79ef4 
>   src/tests/resources_tests.cpp e4a3435adc14f3b6b278b32348a6991543d5a320 
>   src/v1/resources.cpp 8de6672ba9b34947db81c74b8e03e8965e8af5fc 
> 
> Diff: https://reviews.apache.org/r/40375/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 43149: Add LIBPROCESS_THREAD_COUNT to override the thread pool size.

[Klaus Ma]

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




3rdparty/libprocess/src/process.cpp (line 2204)
<https://reviews.apache.org/r/43149/#comment179067>

Check negative and zero value.



3rdparty/libprocess/src/process.cpp (line 2205)
<https://reviews.apache.org/r/43149/#comment179068>

Add test for this patch.


- Klaus Ma


On Feb. 4, 2016, 8:40 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43149/
> ---
> 
> (Updated Feb. 4, 2016, 8:40 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4592
> https://issues.apache.org/jira/browse/MESOS-4592
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check the LIBPROCESS_THREAD_COUNT environment variable to determine
> the number of threads in the libprocess thread pool.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 65a247a5bc7cf0bd6f23f6fc5c349ecce60f5ec0 
> 
> Diff: https://reviews.apache.org/r/43149/diff/
> 
> 
> Testing
> ---
> 
> make check on OS X. Running in production for many months.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 43144: Added env var to set upper bound on number of worker threads.

[Klaus Ma]

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




3rdparty/libprocess/src/process.cpp (line 2202)
<https://reviews.apache.org/r/43144/#comment179070>

blank line before.



3rdparty/libprocess/src/process.cpp (line 2211)
<https://reviews.apache.org/r/43144/#comment179069>

Just `os::getenv("LIB...")` is OK.


- Klaus Ma


On Feb. 4, 2016, 4:57 a.m., Maged Michael wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43144/
> ---
> 
> (Updated Feb. 4, 2016, 4:57 a.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added env var to set upper bound on number of worker threads.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> d8a74d7637d20c81f384e974e4fdeba22effb437 
> 
> Diff: https://reviews.apache.org/r/43144/diff/
> 
> 
> Testing
> ---
> 
> Invocations of mesos-master, mesos-slave, and mesos-tests with the 
> LIBPROCESS_MAX_WORKER_THREADS set to the values "1", "7", "9", "1000", "0", 
> "-1", and "abc" on a 4-core x86_64 Ubuntu 14.04 system.
> 
> The results were as expected. 1, 7, 8, 8, 8, 8, and 8 worker threads were 
> created, respectively. The last 3 tests generated warnings, e.g., 
> "LIBPROCESS_MAX_WORKER_THREADS=abc is invalid".
> 
> 
> Thanks,
> 
> Maged Michael
> 
>

Re: Review Request 42516: Add support for user-defined networks.

[Klaus Ma]

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




src/docker/docker.cpp (line 529)
<https://reviews.apache.org/r/42516/#comment180148>

Also check whether `network_name()` is empty string `""`.


please add unit test cases for this patches; refer to `bridge` test cases.

- Klaus Ma


On Feb. 10, 2016, 4:34 p.m., Ezra Silvera wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42516/
> ---
> 
> (Updated Feb. 10, 2016, 4:34 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4369
> https://issues.apache.org/jira/browse/MESOS-4369
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Signed-off-by: Ezra Silvera <e...@il.ibm.com>
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5d1a20b4ff647f43701a353472b56a4b74b6bbc3 
>   include/mesos/v1/mesos.proto 415a20d9f3385e9627635dd89d78c58d03e3c116 
>   src/docker/docker.cpp 52728707d985517e57525af7e470ccb468039373 
> 
> Diff: https://reviews.apache.org/r/42516/diff/
> 
> 
> Testing
> ---
> 
> Using Swarm running on Mesos create a network with "docker  network create 
> --driver=bridge myNetwork"   and then create a container on that network:  
> "docker run --net=myNetwork"
> 
> 
> Thanks,
> 
> Ezra Silvera
> 
>

Re: Review Request 41305: MESOS-1718: move getExecutorInfo from slave to master.

[Klaus Ma]

> On Jan. 22, 2016, 5:37 a.m., Joseph Wu wrote:
> > src/master/master.cpp, lines 3770-3779
> > <https://reviews.apache.org/r/41305/diff/3/?file=1200898#file1200898line3770>
> >
> > It should still be invalid to supply both a CommandInfo and 
> > ExecutorInfo in the same TaskInfo.

The check was removed at https://reviews.apache.org/r/41306


- Klaus


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


On Jan. 19, 2016, 10:55 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41305/
> ---
> 
> (Updated Jan. 19, 2016, 10:55 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Ian 
> Downes, Joris Van Remoortere, Joseph Wu, and Vinod Kone.
> 
> 
> Bugs: MESOS-1718
> https://issues.apache.org/jira/browse/MESOS-1718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-1718: move getExecutorInfo from slave to master
> 
> 
> Diffs
> -
> 
>   src/master/constants.hpp 2c3299bc1aaa0888f7e47a71965c56ada8ecc21f 
>   src/master/master.hpp 3a7e18232323a1c051bcc97915484b1195fffe58 
>   src/master/master.cpp 9ee56277c8a472be9e683d5db505becfb5f7c422 
>   src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
>   src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
>   src/slave/slave.cpp bb501810d7bb1261ebbbdd147c49948e5a2f8665 
> 
> Diff: https://reviews.apache.org/r/41305/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 41305: MESOS-1718: move getExecutorInfo from slave to master.

[Klaus Ma]

> On Jan. 22, 2016, 5:37 a.m., Joseph Wu wrote:
> > src/master/master.cpp, line 2860
> > <https://reviews.apache.org/r/41305/diff/3/?file=1200898#file1200898line2860>
> >
> > `launcher_dir` is an optional field.
> > 
> > Same for the other fields you added (`sandbox_dir`, `switch_user`, 
> > `executor_rootfs).

@Joseph, do you mean we need to check whether the value is set? I think it's 
also related with compatibility with old slave, we should also handle it when 
old slave did not report `slaveInfo`, right?


- Klaus


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


On Feb. 12, 2016, 11:02 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41305/
> ---
> 
> (Updated Feb. 12, 2016, 11:02 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Joseph Wu.
> 
> 
> Bugs: MESOS-1718
> https://issues.apache.org/jira/browse/MESOS-1718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-1718: move getExecutorInfo from slave to master
> 
> 
> Diffs
> -
> 
>   src/master/constants.hpp 2c3299bc1aaa0888f7e47a71965c56ada8ecc21f 
>   src/master/master.hpp 3a7e18232323a1c051bcc97915484b1195fffe58 
>   src/master/master.cpp 9ee56277c8a472be9e683d5db505becfb5f7c422 
>   src/slave/constants.hpp bcbb1401aa8f9f04c4f9256bb4f560e18d8994e0 
>   src/slave/slave.hpp b7586ce42bfac9d9885a3eb8d82deb94680c236c 
>   src/slave/slave.cpp bb501810d7bb1261ebbbdd147c49948e5a2f8665 
> 
> Diff: https://reviews.apache.org/r/41305/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 42908: Fixed a flaky test in quota tests.

[Klaus Ma]

> On Jan. 29, 2016, 4:42 p.m., Klaus Ma wrote:
> > src/tests/master_quota_tests.cpp, line 957
> > <https://reviews.apache.org/r/42908/diff/1/?file=1224834#file1224834line957>
> >
> > Just check the code on `setQuota-->rescindOffer`, why not let allocator 
> > to do the `rescindOffer`? It makes sense to let allocator to descide how 
> > many resources/offer should be rescind.
> 
> Alexander Rukletsov wrote:
> If I understand you correctly, then it's because the allocator should 
> manage offers. Currently, the allocator allocates resources which are packed 
> into offers by the master. Moving this to allocator is quite a bit of 
> refactoring, agreed? Do you propose to do this refactoring as part of the 
> patch? I'm not sure how schould I "fix" the issue.

Yes, I agree it's quite a bit of refactoring; we definely not "fix" it in this 
path :). I thinke we can log a JIRA to discuss it: whether it worth to do; if 
worth, how. Is that OK?


- Klaus


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


On Jan. 29, 2016, 6:34 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42908/
> ---
> 
> (Updated Jan. 29, 2016, 6:34 p.m.)
> 
> 
> Review request for mesos, Michael Park and Qian Zhang.
> 
> 
> Bugs: MESOS-4542
> https://issues.apache.org/jira/browse/MESOS-4542
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `AvailableResourcesAfterRescinding` test became flaky after we
> stopped offering unreserved resources beyond quota in
> https://reviews.apache.org/r/42835. Hence the allocator offers
> rescinded resources to `framework1` if an allocation happens before
> the test finishes, which violates the expectation that `framework1`
> receives resources only once. Since we do not really care about
> allocations in this test but rather about rescinded resources, the
> fix is just to ignore subsequent offers to `framework1`.
> 
> 
> Diffs
> -
> 
>   src/tests/master_quota_tests.cpp 04efcf3362d3594e0ad8077793fa1f32536dd658 
> 
> Diff: https://reviews.apache.org/r/42908/diff/
> 
> 
> Testing
> ---
> 
> On Mac OS 10.10.4:
> `GTEST_FILTER="MasterQuotaTest.AvailableResourcesAfterRescinding" 
> ./bin/mesos-tests.sh --gtest_shuffle --gtest_break_on_failure 
> --gtest_repeat=5000`
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 42908: Fixed a flaky test in quota tests.

[Klaus Ma]

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




src/tests/master_quota_tests.cpp (line 957)
<https://reviews.apache.org/r/42908/#comment178019>

Just check the code on `setQuota-->rescindOffer`, why not let allocator to 
do the `rescindOffer`? It makes sense to let allocator to descide how many 
resources/offer should be rescind.



src/tests/master_quota_tests.cpp (line 992)
<https://reviews.apache.org/r/42908/#comment178000>

```
driver.stop();
driver.join();
    ```


- Klaus Ma


On Jan. 28, 2016, 8:20 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42908/
> ---
> 
> (Updated Jan. 28, 2016, 8:20 p.m.)
> 
> 
> Review request for mesos, Michael Park and Qian Zhang.
> 
> 
> Bugs: MESOS-4542
> https://issues.apache.org/jira/browse/MESOS-4542
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `AvailableResourcesAfterRescinding` test became flaky after we
> stopped offering unreserved resources beyond quota in
> https://reviews.apache.org/r/42835. Hence the allocator offers
> rescinded resources to `framework1` if an allocation happens before
> the test finishes, which violates the expectation that `framework1`
> receives resources only once. Since we do not really care about
> allocations in this test but rather about rescinded resources, the
> fix is just to ignore subsequent offers to `framework1`.
> 
> 
> Diffs
> -
> 
>   src/tests/master_quota_tests.cpp 04efcf3362d3594e0ad8077793fa1f32536dd658 
> 
> Diff: https://reviews.apache.org/r/42908/diff/
> 
> 
> Testing
> ---
> 
> On Mac OS 10.10.4:
> `GTEST_FILTER="MasterQuotaTest.AvailableResourcesAfterRescinding" 
> ./bin/mesos-tests.sh --gtest_shuffle --gtest_break_on_failure 
> --gtest_repeat=5000`
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 42946: Replaced tabs with spaces in configure.ac.

[Klaus Ma]

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

(Updated Jan. 29, 2016, 4:19 p.m.)


Review request for mesos, Benjamin Bannier and Joris Van Remoortere.


Changes
---

Update summary :).


Summary (updated)
-

Replaced tabs with spaces in configure.ac.


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


Repository: mesos


Description
---

Replace tabs with spaces in configure.ac.


Diffs
-

  configure.ac cb39c7f 

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


Testing
---

../configure
make


Thanks,

Klaus Ma

Re: Review Request 42908: Fixed a flaky test in quota tests.

[Klaus Ma]

> On Jan. 29, 2016, 4:42 p.m., Klaus Ma wrote:
> > src/tests/master_quota_tests.cpp, line 957
> > <https://reviews.apache.org/r/42908/diff/1/?file=1224834#file1224834line957>
> >
> > Just check the code on `setQuota-->rescindOffer`, why not let allocator 
> > to do the `rescindOffer`? It makes sense to let allocator to descide how 
> > many resources/offer should be rescind.
> 
> Alexander Rukletsov wrote:
> If I understand you correctly, then it's because the allocator should 
> manage offers. Currently, the allocator allocates resources which are packed 
> into offers by the master. Moving this to allocator is quite a bit of 
> refactoring, agreed? Do you propose to do this refactoring as part of the 
> patch? I'm not sure how schould I "fix" the issue.
> 
> Klaus Ma wrote:
> Yes, I agree it's quite a bit of refactoring; we definely not "fix" it in 
> this path :). I thinke we can log a JIRA to discuss it: whether it worth to 
> do; if worth, how. Is that OK?

I log MESOS-4553 for our discussion, I'd like to drop this comments :).


- Klaus


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


On Jan. 29, 2016, 6:34 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42908/
> ---
> 
> (Updated Jan. 29, 2016, 6:34 p.m.)
> 
> 
> Review request for mesos, Michael Park and Qian Zhang.
> 
> 
> Bugs: MESOS-4542
> https://issues.apache.org/jira/browse/MESOS-4542
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `AvailableResourcesAfterRescinding` test became flaky after we
> stopped offering unreserved resources beyond quota in
> https://reviews.apache.org/r/42835. Hence the allocator offers
> rescinded resources to `framework1` if an allocation happens before
> the test finishes, which violates the expectation that `framework1`
> receives resources only once. Since we do not really care about
> allocations in this test but rather about rescinded resources, the
> fix is just to ignore subsequent offers to `framework1`.
> 
> 
> Diffs
> -
> 
>   src/tests/master_quota_tests.cpp 04efcf3362d3594e0ad8077793fa1f32536dd658 
> 
> Diff: https://reviews.apache.org/r/42908/diff/
> 
> 
> Testing
> ---
> 
> On Mac OS 10.10.4:
> `GTEST_FILTER="MasterQuotaTest.AvailableResourcesAfterRescinding" 
> ./bin/mesos-tests.sh --gtest_shuffle --gtest_break_on_failure 
> --gtest_repeat=5000`
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 42876: Disallowed non-`const` iteration over `Resources`.

[Klaus Ma]

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


Ship it!




Ship It!

- Klaus Ma


On Jan. 28, 2016, 6:50 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42876/
> ---
> 
> (Updated Jan. 28, 2016, 6:50 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-4534
> https://issues.apache.org/jira/browse/MESOS-4534
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp cc8fef9470d779078aa408ed03e747e5a492deaa 
>   include/mesos/v1/resources.hpp f4892977f8d7b0439db6e9cf7921334f606a496c 
>   src/master/allocator/mesos/hierarchical.cpp 
> dbc77a2d0ce797eea2287b0242f5d2da81c16455 
>   src/master/http.cpp 12c1fe5a514903f657911302e8770e9b245fdbb7 
>   src/slave/resource_estimators/fixed.cpp 
> aedd08df8db59be49b374e61eb268a947d35acea 
>   src/tests/resources_tests.cpp 54a4fa88bfdcff3c0a7e89cbf3a1674c954b7f23 
> 
> Diff: https://reviews.apache.org/r/42876/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 42877: Cleaned up MesosSchedulerDriver shutdown in unit tests.

[Klaus Ma]

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


Ship it!




Ship It!

- Klaus Ma


On Jan. 28, 2016, 6:56 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42877/
> ---
> 
> (Updated Jan. 28, 2016, 6:56 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For consistency, we should do `driver.stop(); driver.join();` if a
> `MesosSchedulerDriver` has been started by the unit test. The destructor for
> `MesosSchedulerDriver` will do something _similar_, so the consequence of
> omitting these calls is not dire, but it seems safer to be consistent between
> all the test cases.
> 
> 
> Diffs
> -
> 
>   src/tests/executor_http_api_tests.cpp 
> 36a042ed103271ca873450236f39a8152fbbf07e 
>   src/tests/oversubscription_tests.cpp 
> 6f43103e81303015fb614653e3bfece55009d1bf 
>   src/tests/reservation_endpoints_tests.cpp 
> 35c093567b07a11ca6eee85e2ff91894a460a7af 
>   src/tests/scheduler_event_call_tests.cpp 
> bd8920fa9d5475e5f6533c8424ebff1588bfe645 
>   src/tests/scheduler_http_api_tests.cpp 
> 9eb1de7d9541395b92b951f0fe0ddbb2f219fe30 
>   src/tests/slave_tests.cpp e943ab99baf3d74679a5da89a89f6a4b7ead 
> 
> Diff: https://reviews.apache.org/r/42877/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Note that adding the `driver.stop(); driver.join();` calls to two of the 
> slave tests requires adding an extra shutdown expectation to avoid a GMock 
> warning.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 42662: Added common command utils file.

[Klaus Ma]

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




src/common/command_utils.cpp (line 17)
<https://reviews.apache.org/r/42662/#comment177739>

Move to line 28.



src/common/command_utils.cpp (lines 91 - 92)
<https://reviews.apache.org/r/42662/#comment177741>

Move to one line if less than 80.



src/tests/common/command_utils_tests.cpp (line 71)
<https://reviews.apache.org/r/42662/#comment177757>

Add a case for `tar/untar` failure.


- Klaus Ma


On Jan. 28, 2016, 4:29 a.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42662/
> ---
> 
> (Updated Jan. 28, 2016, 4:29 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This common file is a good place to add common command line utilities like 
> tar,
> digests(sha256, sha512, etc). Currently this functionality is spread in the 
> code
> base and this change would enable all those call sites to be replaced with a
> common code.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 47d0a7c0fe73b9297cd7dde6086b5e6e9e1f9e4e 
>   src/Makefile.am 19bf3a7c2e43ca04ed6e6d506e052de5537f7c2f 
>   src/common/command_utils.hpp PRE-CREATION 
>   src/common/command_utils.cpp PRE-CREATION 
>   src/tests/common/command_utils_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42662/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 42792: Fix compile error in container logger tests.

[Klaus Ma]

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


Ship it!




Ship It!

- Klaus Ma


On Jan. 26, 2016, 4:12 p.m., Jian Qiu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42792/
> ---
> 
> (Updated Jan. 26, 2016, 4:12 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-4508
> https://issues.apache.org/jira/browse/MESOS-4508
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use unsigned value when comparing with kilobytes.
> 
> 
> Diffs
> -
> 
>   src/tests/container_logger_tests.cpp 
> ce551cbdb89ffb1fead3a58f2d0a1bd27f20f305 
> 
> Diff: https://reviews.apache.org/r/42792/diff/
> 
> 
> Testing
> ---
> 
> make check on OSX and Ubuntu 15.04
> 
> 
> Thanks,
> 
> Jian Qiu
> 
>

Re: Review Request 42589: Added test case for allocator recover with Quota.

[Klaus Ma]

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



ping AlexR/Joris, can you help to review this test case for allocator recover 
with Quota?

- Klaus Ma


On Jan. 21, 2016, 2:27 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42589/
> ---
> 
> (Updated Jan. 21, 2016, 2:27 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, and Joris Van 
> Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test case for allocator recover with Quota.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 953712149bd951789beb29c72779c4ac65aa48dc 
> 
> Diff: https://reviews.apache.org/r/42589/diff/
> 
> 
> Testing
> ---
> 
> make
> make check GTEST_FILTER="HierarchicalAllocatorTest.RecoverWithQuota" (CHECK() 
> failed without fix)
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 41772: Added helper function to flatten resources.

[Klaus Ma]

> On Jan. 20, 2016, 6:18 a.m., Joseph Wu wrote:
> > src/common/resources.cpp, line 880
> > <https://reviews.apache.org/r/41772/diff/6/?file=1199181#file1199181line880>
> >
> > It should be fine to just name this `flatten`.
> > 
> > You should also consider changing the parameter type from 
> > `RevocableInfo::Type` to just `RevocableInfo`.  Supplying the whole 
> > `RevocableInfo` will save us another tweak to the helpers if we change the 
> > revocable protobufs again.
> 
> Guangya Liu wrote:
> I think that we cannot rename it to `flatten` as the compiler will not 
> know what does `flatten()` mean.
> 
> Klaus Ma wrote:
> Regarding from `RevocableInfo::Type` to `RevocableInfo`, do you mean 
> change `RevocableInfo` from `message` to `enum`? I think it's better to 
> define const in `Resources` instead of changing `RevocableInfo`'s type, e.g. 
> `static const Resource::RevocableInfo::Type Resources::ALLOCATION_SLACK = 
> Resource::RevocableInfo::Type::ALLOCATION_SLACK`.
> 
> Regarding `flatten`, there're two options to me:
> 
> * Option 1: add `RevocableInfo::Type` to as 3rd parameter in current 
> `flatten` with `None()` as default vaule, and a new helper function to accept 
> `RevocableInfo::Type` only without default value. The sample code is:
> 
> ```
> Resources flatten(string role = "*", Option 
> resveration = None(), Option revocable = None()) {
>   ...
>   if (revocable.isSome()) {
> flatten(revocable.get());
>   } else {
> // clear revocable info.
>   }
> }
> 
> Resources flatten(RevocableInfo::Type revocable) {
>   foreach resources {
> resource.mutable_revocable()->set_type(revocable);
>   }
> }
> ```
> 
> * Option 2: add new `flatten` functions but did not provide default 
> value; `None()` means clear the `RevocableInfo::Type`.
> 
> I perfer to Option 1 :). Any comments?
> 
> Guangya Liu wrote:
> option 1 will cause all caller who want to flatten allocation slack need 
> to input all of the four parameter, it is really not convenient for the 
> caller.
> Option 2 need to add a new `flatten` with a `requested` but not `option` 
> parameter, I think that maybe 2 is better.
> 
> Klaus Ma wrote:
> regarding 1, if want to clear the flags (role, reservation and 
> revocable), call `flatten()`; if want to set `ALLOCATION_SLACK`, call 
> `flatten(Resources::ALLOCATION_SLACK)`.
> 
> Guangya Liu wrote:
> Just clarify 1) here, I have some mis-understanding for it.
> 
> Option 1) has involved two APIs, one is update current `Resources 
> flatten(string role = "*", Option resveration = None(), , 
> Option revocable = None() )`, the other is add a new 
> `flatten(RevocableInfo::Type revocable)`. The `first flatten` is used to 
> clear `allocation slack` and the `second flatten` is used to set `allocation 
> slack` flag.
> 
> Seems 1) is better as it faciliates the caller.
> 
> Joseph Wu wrote:
> Given how you're using `flatten`, it would be better to have two calls:
> `Resources flatten(string role = "*", Option resveration 
> = None())`
> and 
> `Resources flatten(Option revocable)`  // No default so 
> there's no ambiguity.
> 
> ---
> 
> Note: If you wanted a single `flatten`, you'd need to have 
> `Option<Option>` so that you have the option of setting, 
> un-setting, or not-changing each field.
> 
> Guangya Liu wrote:
> I think that adding a new `flatten` without default value might be better 
> and clear.
> 
> Guangya Liu wrote:
> But the problem is that even with 
> 
> `Resources flatten(string role = "*", Option resveration 
> = None())`
> and 
> `Resources flatten(Option revocable)`
> 
> There are still ambiguity when calling `flatten(role)`, comments?
> 
> Guangya Liu wrote:
> In file included from ../../src/tests/fault_tolerance_tests.cpp:56:
> ../../src/tests/mesos.hpp:665:39: error: call to member function 
> 'flatten' is ambiguous
> remaining.find(TASK_RESOURCES.flatten(role));
>~^
> What about still use `flattenSlack`?
> 
> Klaus Ma wrote:
> Sorry, just checked this error; it seems we `make` passed but `make 
> check` failed, right?
> 
> Guangya Liu wrote:
> Yes, the `make tests/check` failed, any idea for this?
> 
> Joseph Wu wrote:
> It's possible to fix the ambiguity (thanks MPark!), although I'm not su

Review Request 42946: Replace tabs with spaces in configure.ac.

[Klaus Ma]

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

Review request for mesos.


Repository: mesos


Description
---

Replace tabs with spaces in configure.ac.


Diffs
-

  configure.ac cb39c7f 

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


Testing
---


Thanks,

Klaus Ma

Re: Review Request 42946: Replace tabs with spaces in configure.ac.

[Klaus Ma]

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

(Updated Jan. 29, 2016, 10:55 a.m.)


Review request for mesos, Benjamin Bannier and Joris Van Remoortere.


Changes
---

Add reviewers.


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


Repository: mesos


Description
---

Replace tabs with spaces in configure.ac.


Diffs
-

  configure.ac cb39c7f 

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


Testing (updated)
---

../configure
make


Thanks,

Klaus Ma

Re: Review Request 40632: Enabled oversubscribed resources for reservations in allocator.

[Klaus Ma]

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




src/master/allocator/mesos/hierarchical.cpp (lines 1333 - 1335)
<https://reviews.apache.org/r/40632/#comment177869>

The result is un-stable here: if framework accept reserved resource come 
firstly, the others can not get those revocable resources; but if tenant 
framework come fristly, it will get revocable resources. It all depedent on 
sorter, and the offer maybe wrong in about cases.


- Klaus Ma


On Jan. 23, 2016, 11:18 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40632/
> ---
> 
> (Updated Jan. 23, 2016, 11:18 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
> 
> 
> Bugs: MESOS-4145
> https://issues.apache.org/jira/browse/MESOS-4145
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled oversubscribed resources for reservations in allocator.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 65c7e6b15c5308c0910667e1b12f39b21293a316 
>   src/tests/hierarchical_allocator_tests.cpp 
> b1cb955b7eb1213c7ba4a9c5181545bb49154f06 
> 
> Diff: https://reviews.apache.org/r/40632/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> GLOG_v=2  ./bin/mesos-tests.sh  --gtest_filter="HierarchicalAllocatorTest.*" 
> --verbose --gtest_repeat=100 --gtest_shuffle
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 42936: Edited flag help strings for style.

[Klaus Ma]

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


Fix it, then Ship it!





src/master/flags.cpp (line 144)
<https://reviews.apache.org/r/42936/#comment177994>

Move "By" to the next line.


- Klaus Ma


On Jan. 29, 2016, 12:30 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42936/
> ---
> 
> (Updated Jan. 29, 2016, 12:30 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4298
> https://issues.apache.org/jira/browse/MESOS-4298
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Edited flag help strings for style.
> 
> Backticks were inserted where appropriate. This not only matches the 
> convention in the comments within our code, helps to enable the automatic 
> generation of markdown-formatted documentation from these help strings.
> 
> 
> Diffs
> -
> 
>   src/logging/flags.cpp 978d735c8c8e9f3c46669cc633773f1ec1e1725d 
>   src/master/flags.cpp 6e7e17650341bc17c3af6f92fe83f974d4ce1efd 
>   src/master/main.cpp 0d5ac49121491152d8426cd764a1f1a0f37483ae 
>   src/slave/flags.cpp 75d7429a4e3e1d6259296257c0ace1ade365ac2b 
>   src/slave/main.cpp 9d48a0823189ea6505073a2803f02d90dc382ab4 
> 
> Diff: https://reviews.apache.org/r/42936/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 43879: Added allocator metrics for number of allocations made.

[Klaus Ma]

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




src/tests/hierarchical_allocator_tests.cpp (line 2385)
<https://reviews.apache.org/r/43879/#comment181976>

Why a anonymous namespace? `static` should be fine.



src/tests/hierarchical_allocator_tests.cpp (line 2395)
<https://reviews.apache.org/r/43879/#comment181975>

What are you going to do for this comments? or just a notes?


- Klaus Ma


On Feb. 24, 2016, 9:25 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43879/
> ---
> 
> (Updated Feb. 24, 2016, 9:25 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4718
> https://issues.apache.org/jira/browse/MESOS-4718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator metrics for number of allocations made.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 3043888630b066505410d3b32c5b3f813cc458c1 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
>   src/tests/hierarchical_allocator_tests.cpp 
> 5f771f02db9bd098f3cd36730cd84bf2f5e87a33 
> 
> Diff: https://reviews.apache.org/r/43879/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X)
> 
> I confirmed that this does not lead to general performance regressions in the 
> allocator; this is partially expected since the added code only inserts 
> metrics in the allocator while the actual work is perform asynchronously. 
> These tests where performed with 
> `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an optimized build 
> under OS X using clang(trunk) as compiler.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 43880: Added allocator metrics for total and allocated scalar resources.

[Klaus Ma]

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




src/master/allocator/mesos/hierarchical.cpp (line 416)
<https://reviews.apache.org/r/43880/#comment181978>

I think we also need to update it in `updateSlave`; if normal resources of 
agent is "cpus:16" and Estimator report "mem:10", we'll miss mem in total 
metrics.



src/master/allocator/mesos/hierarchical.cpp (line 428)
<https://reviews.apache.org/r/43880/#comment181979>

Not yours, but I think we need to update roleSorter when slave 
active/deactive.

Filed https://issues.apache.org/jira/browse/MESOS-4755 to trace this.



src/tests/hierarchical_allocator_tests.cpp (line 2457)
<https://reviews.apache.org/r/43880/#comment181980>

Can we move the comments above the code and move `getMetric()` into one 
line?


- Klaus Ma


On Feb. 24, 2016, 6:58 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43880/
> ---
> 
> (Updated Feb. 24, 2016, 6:58 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4720
> https://issues.apache.org/jira/browse/MESOS-4720
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator metrics for total and allocated scalar resources.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 3043888630b066505410d3b32c5b3f813cc458c1 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
>   src/tests/hierarchical_allocator_tests.cpp 
> 5f771f02db9bd098f3cd36730cd84bf2f5e87a33 
> 
> Diff: https://reviews.apache.org/r/43880/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X)
> 
> I confirmed that this does not lead to general performance regressions in the 
> allocator; this is partially expected since the added code only inserts 
> metrics in the allocator while the actual work is perform asynchronously. 
> These tests where performed with 
> `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an optimized build 
> under OS X using clang(trunk) as compiler.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 43816: Updated `/frameworks` master endpoint to use jsonify.

[Klaus Ma]

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


Ship it!




Ship It!

- Klaus Ma


On Feb. 22, 2016, 3:11 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43816/
> ---
> 
> (Updated Feb. 22, 2016, 3:11 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-4731
> https://issues.apache.org/jira/browse/MESOS-4731
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `/frameworks` master endpoint to use jsonify.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp ae6bc7852202480e58f579a5b48ab5b5e5ff9317 
> 
> Diff: https://reviews.apache.org/r/43816/diff/
> 
> 
> Testing
> ---
> 
> 1. make check
> 2. Verified that after introducing a bug into the jsonify version of 
> `frameworks()`, `make check` fails (i.e., the test suite covers the 
> `/frameworks` endpoint).
> 3. Compared output of `/frameworks` with old and new implementation on a test 
> Mesos installation to try to gauge correctness visually.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43868: Add a getter for master::Flags.

[Klaus Ma]

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




src/master/master.hpp (line 545)
<https://reviews.apache.org/r/43868/#comment181728>

`getFlags() const`



src/tests/master_tests.cpp (line 975)
<https://reviews.apache.org/r/43868/#comment181729>

add blank line.



src/tests/master_tests.cpp (line 983)
<https://reviews.apache.org/r/43868/#comment181730>

Is this necessary? If so, please clear it up.


Can you also show which part of code will use this function?

- Klaus Ma


On Feb. 23, 2016, 1:23 p.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43868/
> ---
> 
> (Updated Feb. 23, 2016, 1:23 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Joris Van Remoortere, and Qian Zhang.
> 
> 
> Bugs: MESOS-3481
> https://issues.apache.org/jira/browse/MESOS-3481
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> MESOS-3481 Add const accessor to Master flags.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 13c6ff153e77c527822309e787942eb463d59e7d 
>   src/tests/master_tests.cpp 0bd8c0e42f335cad7ed858c6af5aa4f07bb37dbf 
> 
> Diff: https://reviews.apache.org/r/43868/diff/
> 
> 
> Testing
> ---
> 
> Added one new unit test MasterTest::Flags
> 
> `make check` passed
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 42390: Fixed fetching uris when slave is running inside a container.

[Klaus Ma]

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


Ship it!




Ship It!

- Klaus Ma


On Jan. 20, 2016, 10:12 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42390/
> ---
> 
> (Updated Jan. 20, 2016, 10:12 p.m.)
> 
> 
> Review request for mesos and Timothy Chen.
> 
> 
> Bugs: MESOS-4249
> https://issues.apache.org/jira/browse/MESOS-4249
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This still requires the slave work dir is a mounted volume, otherwise
> the slave has no way to fetch into the task container's sandbox.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.cpp da19975c1f1e59fab0a96ebd9aa815bba2a35ece 
> 
> Diff: https://reviews.apache.org/r/42390/diff/
> 
> 
> Testing
> ---
> 
> - Start a master, then start the slave inside a docker container, with the 
> work_dir being a mounted volume
> - Start a simple framework, which, on receiving the first offer, launches a 
> task with a uri in the command info
> - When the task container is launched, check the file is fetched into the 
> sandbox
> ```sh
> $ docker exec  ls /mnt/mesos/sandbox
> robots.txt
> stderr
> stdout
> ```
> 
> I have collected the scripts I use in testing in [this 
> gist](https://gist.github.com/lins05/14455e92f37e91fd46ff)
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 43823: Updated `/tasks` master endpoint to use jsonify.

[Klaus Ma]

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


Ship it!




Ship It!

- Klaus Ma


On Feb. 22, 2016, 3:13 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43823/
> ---
> 
> (Updated Feb. 22, 2016, 3:13 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `/tasks` master endpoint to use jsonify.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp ae6bc7852202480e58f579a5b48ab5b5e5ff9317 
> 
> Diff: https://reviews.apache.org/r/43823/diff/
> 
> 
> Testing
> ---
> 
> make check. Also verified that this endpoint is covered by the unit tests.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43848: Used `size_t` to track number of frameworks per role.

[Klaus Ma]

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


Ship it!




Ship It!

- Klaus Ma


On Feb. 23, 2016, 3:54 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43848/
> ---
> 
> (Updated Feb. 23, 2016, 3:54 a.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Per informal project style, we prefer using unsigned types to
> represent non-negative quantities.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 0d39d3f3b5f4ff7f62f9de7200d062845c71818a 
> 
> Diff: https://reviews.apache.org/r/43848/diff/
> 
> 
> Testing
> ---
> 
> make check on OSX
> 
> Compilation on Linux with GCC 5.3
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43822: Updated `/slaves` master endpoint to use jsonify.

[Klaus Ma]

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


Ship it!




Ship It!

- Klaus Ma


On Feb. 22, 2016, 3:12 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43822/
> ---
> 
> (Updated Feb. 22, 2016, 3:12 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated `/slaves` master endpoint to use jsonify.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp ae6bc7852202480e58f579a5b48ab5b5e5ff9317 
> 
> Diff: https://reviews.apache.org/r/43822/diff/
> 
> 
> Testing
> ---
> 
> make check. Also verified that this endpoint is covered by the unit tests.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43864: Fix typo of roles doc.

[Klaus Ma]

> On Feb. 24, 2016, 2:06 a.m., Neil Conway wrote:
> > The right place to make this fix is in `src/master/http.cpp`, and then 
> > rerun `support/generate-endpoint-help.py`. Seems like 
> > `Master::Http::ROLES_HELP` is missing commas in `DESCRIPTION`.

You're right, we missed commas in `Master::Http::ROLES_HELP` `DESCRIPTION`. 
Update the patches to address this comments :).


- Klaus


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


On Feb. 24, 2016, 11:34 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43864/
> ---
> 
> (Updated Feb. 24, 2016, 11:34 a.m.)
> 
> 
> Review request for mesos and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix typo of roles doc.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/master/roles.json.md 
> d67779c246cceae2209f2611f32ada4493ae6f83 
>   docs/endpoints/master/roles.md 976a9b7891a17652289126ec7e7ee73cea0c2e35 
>   src/master/http.cpp 939fab21a2240de7214ef809a194ffb3837a9f1b 
> 
> Diff: https://reviews.apache.org/r/43864/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 43864: Fix typo of roles doc.

[Klaus Ma]

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

(Updated Feb. 24, 2016, 11:34 a.m.)


Review request for mesos and Neil Conway.


Changes
---

Address comments.


Repository: mesos


Description
---

Fix typo of roles doc.


Diffs (updated)
-

  docs/endpoints/master/roles.json.md d67779c246cceae2209f2611f32ada4493ae6f83 
  docs/endpoints/master/roles.md 976a9b7891a17652289126ec7e7ee73cea0c2e35 
  src/master/http.cpp 939fab21a2240de7214ef809a194ffb3837a9f1b 

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


Testing
---


Thanks,

Klaus Ma

Re: Review Request 43884: Added allocator metrics for used quotas.

[Klaus Ma]

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




src/master/allocator/mesos/hierarchical.hpp (line 389)
<https://reviews.apache.org/r/43884/#comment182802>

Remove it when removeQuota.



src/master/allocator/mesos/hierarchical.cpp (line 1108)
<https://reviews.apache.org/r/43884/#comment182803>

This's not necessary; all resources in quotaSorter are `nonRevocable()`.


- Klaus Ma


On Feb. 27, 2016, 1 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43884/
> ---
> 
> (Updated Feb. 27, 2016, 1 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4723
> https://issues.apache.org/jira/browse/MESOS-4723
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator metrics for used quotas.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
>   src/master/allocator/mesos/hierarchical.hpp 
> 3043888630b066505410d3b32c5b3f813cc458c1 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
>   src/tests/hierarchical_allocator_tests.cpp 
> 5f771f02db9bd098f3cd36730cd84bf2f5e87a33 
> 
> Diff: https://reviews.apache.org/r/43884/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X)
> 
> I confirmed that this does not lead to general performance regressions in the 
> allocator; this is partially expected since the added code only inserts 
> metrics in the allocator while the actual work is perform asynchronously. 
> These tests where performed with 
> `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an optimized build 
> under OS X using clang(trunk) as compiler.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 43718: Added fs::supported() function.

[Klaus Ma]

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


Ship it!




Ship It!

- Klaus Ma


On Feb. 28, 2016, 9:08 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> ---
> 
> (Updated Feb. 28, 2016, 9:08 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added fs::supported() function.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595 
>   src/linux/fs.cpp 6bf87a19a795cf0a1970f160829b477a35cb789a 
>   src/tests/containerizer/fs_tests.cpp 
> 29e43877612fa151e6f6d79268a7411272a7bfeb 
> 
> Diff: https://reviews.apache.org/r/43718/diff/
> 
> 
> Testing
> ---
> 
> make check on ubuntu 14.04 64bit vm
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 44047: Added full reserved resource info to `/slaves` master endpoint.

[Klaus Ma]

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




docs/reservation.md (line 363)
<https://reviews.apache.org/r/44047/#comment182798>

s/slave/agent/



src/master/http.cpp (line 1047)
<https://reviews.apache.org/r/44047/#comment182799>

`` for /unreserve and /destroy-volumes.


- Klaus Ma


On Feb. 28, 2016, 8:17 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44047/
> ---
> 
> (Updated Feb. 28, 2016, 8:17 a.m.)
> 
> 
> Review request for mesos, Michael Park and Vinod Kone.
> 
> 
> Bugs: MESOS-4667
> https://issues.apache.org/jira/browse/MESOS-4667
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This allows operators to list all the dynamic reservations and persistent
> volumes in a cluster. This is important in itself; it also makes it easier to
> use the `/unreserve` and `/destroy-volumes` endpoints.
> 
> 
> Diffs
> -
> 
>   docs/persistent-volume.md 47ada98413f1670e9fc4ebd9d1ead6af9b120184 
>   docs/reservation.md 450f4eec49d957b096df1380c3e79d5f743cc829 
>   src/master/http.cpp f3ce1aa22f5f753fcb254e9ecaa8ba571e3d2829 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 08b9102318b826bab9d2c1d389fb80b86949218c 
> 
> Diff: https://reviews.apache.org/r/44047/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 44110: Updated flag examples to refer to /role instead of stats.json.

[Klaus Ma]

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


Ship it!




Ship It!

- Klaus Ma


On Feb. 27, 2016, 8:55 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44110/
> ---
> 
> (Updated Feb. 27, 2016, 8:55 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Vinod Kone.
> 
> 
> Bugs: MESOS-4509
> https://issues.apache.org/jira/browse/MESOS-4509
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated flag examples to refer to /role instead of stats.json.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 2353e78a80548b63f871c52e840ffe2fe869f4d7 
>   src/master/flags.cpp 60e085bd5c6689adb625a736edc76e814860ea7d 
>   src/slave/flags.cpp 1c6a87b670efde2deab4d6e3f24fd6eb3704a47d 
> 
> Diff: https://reviews.apache.org/r/44110/diff/
> 
> 
> Testing
> ---
> 
> make test + manually checked the flags example.
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 43881: Added allocator metric for the number of allocations to a framework.

[Klaus Ma]

> On Feb. 26, 2016, 12:30 a.m., Klaus Ma wrote:
> > src/master/allocator/mesos/hierarchical.hpp, line 376
> > <https://reviews.apache.org/r/43881/diff/7/?file=1270846#file1270846line376>
> >
> > We also need to remove counter in `removeFramework`; or we'll see 
> > metrics of removed framework.
> 
> Benjamin Bannier wrote:
> Right now this is intended behavior, any preference @bmahler?

For the cluster running for a long time, there'll be several un-available 
metrics after doing operation.
Just go through the other patches of metrics, similar comments on quota and 
filters.


- Klaus


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


On Feb. 27, 2016, 1:02 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43881/
> ---
> 
> (Updated Feb. 27, 2016, 1:02 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4719
> https://issues.apache.org/jira/browse/MESOS-4719
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator metric for the number of allocations to a framework.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
>   src/master/allocator/mesos/hierarchical.hpp 
> 3043888630b066505410d3b32c5b3f813cc458c1 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
>   src/tests/hierarchical_allocator_tests.cpp 
> 5f771f02db9bd098f3cd36730cd84bf2f5e87a33 
> 
> Diff: https://reviews.apache.org/r/43881/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X)
> 
> I confirmed that this does not lead to general performance regressions in the 
> allocator; this is partially expected since the added code only inserts 
> metrics in the allocator while the actual work is perform asynchronously. 
> These tests where performed with 
> `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an optimized build 
> under OS X using clang(trunk) as compiler.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 43718: Added fs::supported() function.

[Klaus Ma]

> On Feb. 28, 2016, 9:16 p.m., Guangya Liu wrote:
> > src/linux/fs.cpp, line 60
> > 
> >
> > I think that should `continue` here, if one line failed, other lines 
> > should still be tried to see if the fs is supportted.
> > 
> > But it would be good to log some WARNINIG message here before continue.
> > 
> > @Jie Yu, @Shuai Lin, what do you say?

According to the comments, only one or two columns is accepted. If the data 
format did not math our expectation, we should return error.

>  // Each line of /proc/filesystems is "nodev" + "\t" + "fsname", and the
>  // field "nodev" is optional. For the details, check the kernel src code:


- Klaus


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


On Feb. 28, 2016, 9:08 p.m., Shuai Lin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43718/
> ---
> 
> (Updated Feb. 28, 2016, 9:08 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4707
> https://issues.apache.org/jira/browse/MESOS-4707
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added fs::supported() function.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp fddee2441e008dc70d9cff5e63536ef21c865595 
>   src/linux/fs.cpp 6bf87a19a795cf0a1970f160829b477a35cb789a 
>   src/tests/containerizer/fs_tests.cpp 
> 29e43877612fa151e6f6d79268a7411272a7bfeb 
> 
> Diff: https://reviews.apache.org/r/43718/diff/
> 
> 
> Testing
> ---
> 
> make check on ubuntu 14.04 64bit vm
> 
> 
> Thanks,
> 
> Shuai Lin
> 
>

Re: Review Request 44063: Waited for status update to happen before proceeding in test.

[Klaus Ma]

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


Ship it!




Ship It!

- Klaus Ma


On Feb. 28, 2016, 6:27 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44063/
> ---
> 
> (Updated Feb. 28, 2016, 6:27 p.m.)
> 
> 
> Review request for mesos and Benjamin Hindman.
> 
> 
> Bugs: MESOS-4784
> https://issues.apache.org/jira/browse/MESOS-4784
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To observe a changed metric in \`slave/container_launch_errors\` the
> triggering action should have taken place. Currently, the test usually
> passes, since hitting the metrics endpoint via \`Metrics()\` could block
> for longer times due to the endpoint's implicit rate limiting. Once we
> disable that blocking with MESOS-4783 this implicit assumption becomes
> invalid and the test would be much more likely to fail.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 322f3ddaf11885d7e61e0e9232c0342e97d8bfa1 
> 
> Diff: https://reviews.apache.org/r/44063/diff/
> 
> 
> Testing
> ---
> 
> make check with unoptimized build OS X both with rate-limiting enabled and 
> disabled
> 
> With MESOS-4783 applied the test fails before this patch and passes after 
> applying it.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 43883: Added allocator metrics for number of offer filters per framework.

[Klaus Ma]

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




src/master/allocator/mesos/hierarchical.hpp (line 384)
<https://reviews.apache.org/r/43883/#comment182801>

Should we remove this `Gauge` when remove framework?


- Klaus Ma


On Feb. 27, 2016, 1:01 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43883/
> ---
> 
> (Updated Feb. 27, 2016, 1:01 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4722
> https://issues.apache.org/jira/browse/MESOS-4722
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator metrics for number of offer filters per framework.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
>   src/master/allocator/mesos/hierarchical.hpp 
> 3043888630b066505410d3b32c5b3f813cc458c1 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
>   src/tests/hierarchical_allocator_tests.cpp 
> 5f771f02db9bd098f3cd36730cd84bf2f5e87a33 
> 
> Diff: https://reviews.apache.org/r/43883/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X)
> 
> I confirmed that this does not lead to general performance regressions in the 
> allocator; this is partially expected since the added code only inserts 
> metrics in the allocator while the actual work is perform asynchronously. 
> These tests where performed with 
> `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an optimized build 
> under OS X using clang(trunk) as compiler.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 43881: Added allocator metric for the number of allocations to a framework.

[Klaus Ma]

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




src/master/allocator/mesos/hierarchical.hpp (line 376)
<https://reviews.apache.org/r/43881/#comment182180>

We also need to remove counter in `removeFramework`; or we'll see metrics 
of removed framework.



src/master/allocator/mesos/hierarchical.cpp (line 1496)
<https://reviews.apache.org/r/43881/#comment182179>

I'd suggest to do this in `addFramework` & `removeFramework`?



src/tests/hierarchical_allocator_tests.cpp (line 2447)
<https://reviews.apache.org/r/43881/#comment182181>

I think `1.0` is easier to read for other contributor.



src/tests/hierarchical_allocator_tests.cpp (line 2479)
<https://reviews.apache.org/r/43881/#comment182182>

ditto


- Klaus Ma


On Feb. 25, 2016, 7:37 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43881/
> ---
> 
> (Updated Feb. 25, 2016, 7:37 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4719
> https://issues.apache.org/jira/browse/MESOS-4719
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator metric for the number of allocations to a framework.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
>   src/master/allocator/mesos/hierarchical.hpp 
> 3043888630b066505410d3b32c5b3f813cc458c1 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
>   src/tests/hierarchical_allocator_tests.cpp 
> 5f771f02db9bd098f3cd36730cd84bf2f5e87a33 
> 
> Diff: https://reviews.apache.org/r/43881/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X)
> 
> I confirmed that this does not lead to general performance regressions in the 
> allocator; this is partially expected since the added code only inserts 
> metrics in the allocator while the actual work is perform asynchronously. 
> These tests where performed with 
> `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an optimized build 
> under OS X using clang(trunk) as compiler.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 43882: Added allocation metrics for allocation time.

[Klaus Ma]

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




src/master/allocator/mesos/hierarchical.cpp (line 1207)
<https://reviews.apache.org/r/43882/#comment182184>

It seems a general functional class, can we move it into `process/metrics`?


- Klaus Ma


On Feb. 25, 2016, 7:37 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43882/
> ---
> 
> (Updated Feb. 25, 2016, 7:37 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4721
> https://issues.apache.org/jira/browse/MESOS-4721
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocation metrics for allocation time.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 323d01d99456a71bd384faf186264e3fc4bf2207 
>   src/master/allocator/mesos/hierarchical.hpp 
> 3043888630b066505410d3b32c5b3f813cc458c1 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5ef29f26ec8071f79c2f4f78dbe2bb0a613cc92d 
>   src/tests/hierarchical_allocator_tests.cpp 
> 5f771f02db9bd098f3cd36730cd84bf2f5e87a33 
> 
> Diff: https://reviews.apache.org/r/43882/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X)
> 
> I confirmed that this does not lead to general performance regressions in the 
> allocator; this is partially expected since the added code only inserts 
> metrics in the allocator while the actual work is perform asynchronously. 
> These tests where performed with 
> `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an optimized build 
> under OS X using clang(trunk) as compiler.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 43561: Improve Ranges parsing to handle single values.

[Klaus Ma]

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

(Updated Feb. 29, 2016, 5:57 p.m.)


Review request for mesos and Ben Mahler.


Changes
---

Rebase


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


Repository: mesos


Description
---

Improve Ranges parsing to handle single values.


Diffs (updated)
-

  src/common/values.cpp 9f3c0b9930bd3dd9d84d97119419825862f531c3 
  src/tests/resources_tests.cpp a545100522bf4b1f03e50656d461b3cda6b41e11 
  src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 

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


Testing
---

make
make check GTEST_FILTER=~"*"
./src/mesos-test


Thanks,

Klaus Ma

Re: Review Request 43838: Added note about not implemented requestResources call.

[Klaus Ma]

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




docs/app-framework-development-guide.md (line 172)
<https://reviews.apache.org/r/43838/#comment181681>

It seems scheduler did not send `ResourceRequestMessage` to the master; so 
other allocator also can not handle it. I'd like to say Master ignore this call.


- Klaus Ma


On Feb. 23, 2016, 12:54 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43838/
> ---
> 
> (Updated Feb. 23, 2016, 12:54 a.m.)
> 
> 
> Review request for mesos, Adam B and Guangya Liu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added note about not implemented requestResources call.
> 
> 
> Diffs
> -
> 
>   docs/app-framework-development-guide.md 
> e0f40adacf96bdf0c510b3400eb0ed0cd964ab9d 
>   include/mesos/scheduler.hpp 14c7ff964aa7b94f439d16e605380661d2279d54 
>   src/java/src/org/apache/mesos/SchedulerDriver.java 
> bf866f5ebece2505eaa27bf39a1382cd1a2a069a 
> 
> Diff: https://reviews.apache.org/r/43838/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Review Request 43864: Fix typo of roles doc.

[Klaus Ma]

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

Review request for mesos and Neil Conway.


Repository: mesos


Description
---

Fix typo of roles doc.


Diffs
-

  docs/endpoints/master/roles.json.md d67779c246cceae2209f2611f32ada4493ae6f83 
  docs/endpoints/master/roles.md 976a9b7891a17652289126ec7e7ee73cea0c2e35 

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


Testing
---


Thanks,

Klaus Ma