Re: Review Request 50123: Added GPU scheduler for docker containerizer.

[Yubo Li]

> On 七月 27, 2016, 2:15 a.m., Guangya Liu wrote:
> > src/slave/containerizer/docker.cpp, line 1333
> > 
> >
> > 1) s/allocator.get().allocate(requested)/allocator->allocate(requested)
> > 
> > 2) I think it is not right to use `Option` here as the 
> > `allocator->allocate(requested)` is returning a `Future`.
> > 
> > One proposal is as following:
> > ```
> > allocator->allocate(requested)
> >   .then(defer(...));
> > ```
> > 
> > You can also refer to 
> > https://github.com/apache/mesos/blob/master/src/slave/containerizer/mesos/isolators/gpu/isolator.cpp#L356
> >  to get some detail.
> 
> Yubo Li wrote:
> `Future.get()` will block the host threading and wait for the 
> sub-threading return its value. For docker containerizer, we need to wait  
> `Option allocated` return, and pass it to mesos-docker-executor, 
> it's hard to follow the async calling style. For mesos containerizer, it uses 
> cgroup to set device control directly, so the async call is fine.

Since here we must block the host threading to get GPU allocated results, The 
logic fixed as:
+  if (requested > 0) {
+// Some GPUs requested.
+Future future = allocator->allocate(requested);
+
+// We block the code until the GPU allocation finished.
+// For docker containerizer, we must wait for GPUs allocated
+// completed, and pass allocated GPUs to mesos-docker-executor.
+future.await();
+if (!future.isReady()) {
+  return Failure("GPU allocator allocating GPU failed");
+}
+
+set allocated = future.get();
+
+foreach (const Gpu& gpu, allocated) {
+  container->gpuAllocated.push_back(gpu);
+}
The logic should be safe because I refer to: 
https://github.com/liyubobj/mesos/blob/master/src/slave/containerizer/fetcher.cpp#L326


- Yubo


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


On 七月 18, 2016, 9:17 a.m., Yubo Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50123/
> ---
> 
> (Updated 七月 18, 2016, 9:17 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Kevin Klues, and Rajat Phull.
> 
> 
> Bugs: MESOS-5795
> https://issues.apache.org/jira/browse/MESOS-5795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This added 'NvidiaGpuAllocator' to docker containerizer so that the
> docker containerizer can use it to allocate GPUs to the task with 'gpus'
> resource. Also, allocated GPUs will automatically deallocated after the
> job destroyed.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp 43ca4317d608b3b43dd7bd0d1b55c721e7364885 
>   src/slave/containerizer/docker.cpp f1ecf3b25d85597f6c3dcaa47968860ed119dbd5 
> 
> Diff: https://reviews.apache.org/r/50123/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yubo Li
> 
>

Re: Review Request 50482: Fixed the CORS error when redirect in WEB UI.

[haosdent huang]

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

(Updated July 29, 2016, 4:35 a.m.)


Review request for mesos, Adam B, Jie Yu, Michael Lunøe, Vinod Kone, and Jiang 
Yan Xu.


Changes
---

Rebase.


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


Repository: mesos


Description
---

The redirection in WEB UI is broken since the CORS restriction after
we enabled redirection in `master/state` endpoint in
https://reviews.apache.org/r/34646/. We change the request way to
`master/state` endpoint from xhr to jsonp for bypassing the CORS
restriction.


Diffs (updated)
-

  src/webui/master/static/index.html a083537cd718162d1913842bddbd2653d8c52337 
  src/webui/master/static/js/controllers.js 
ceaf1402ebdae4efd1a8a6fc9c7b795de69e2fc0 

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


Testing
---

Testing video record

![http_redirect.gif](https://issues.apache.org/jira/secure/attachment/12820401/http_redirect.gif)


Thanks,

haosdent huang

Re: Review Request 50482: Fixed the CORS error when redirect in WEB UI.

[haosdent huang]

> On July 27, 2016, 11:32 p.m., Vinod Kone wrote:
> > src/webui/master/static/js/controllers.js, line 60
> > 
> >
> > why is this scope.state instead of scope.data?

In the previous code, we didn't use JSONP, so the type of `data` and 
`$scope.data` here are string. `data` need to `JSON.parse` first then assign to 
`$scope.state` as a JSON object.
After we use JSONP here, the object passed into `updateState` is a parsed JSON 
object actually. Continue to use `data` as the variable name here is a bit 
confuse.
I change `data` to `state` now.


- haosdent


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


On July 29, 2016, 4:24 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50482/
> ---
> 
> (Updated July 29, 2016, 4:24 a.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Lunøe, Vinod Kone, and 
> Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5911
> https://issues.apache.org/jira/browse/MESOS-5911
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The redirection in WEB UI is broken since the CORS restriction after
> we enabled redirection in `master/state` endpoint in
> https://reviews.apache.org/r/34646/. We change the request way to
> `master/state` endpoint from xhr to jsonp for bypassing the CORS
> restriction.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/index.html a083537cd718162d1913842bddbd2653d8c52337 
>   src/webui/master/static/js/controllers.js 
> ceaf1402ebdae4efd1a8a6fc9c7b795de69e2fc0 
> 
> Diff: https://reviews.apache.org/r/50482/diff/
> 
> 
> Testing
> ---
> 
> Testing video record
> 
> ![http_redirect.gif](https://issues.apache.org/jira/secure/attachment/12820401/http_redirect.gif)
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 50482: Fixed the CORS error when redirect in WEB UI.

[haosdent huang]

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

(Updated July 29, 2016, 4:24 a.m.)


Review request for mesos, Adam B, Jie Yu, Michael Lunøe, Vinod Kone, and Jiang 
Yan Xu.


Changes
---

Address @vinodkone's comment.


Summary (updated)
-

Fixed the CORS error when redirect in WEB UI.


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


Repository: mesos


Description
---

The redirection in WEB UI is broken since the CORS restriction after
we enabled redirection in `master/state` endpoint in
https://reviews.apache.org/r/34646/. We change the request way to
`master/state` endpoint from xhr to jsonp for bypassing the CORS
restriction.


Diffs (updated)
-

  src/webui/master/static/index.html a083537cd718162d1913842bddbd2653d8c52337 
  src/webui/master/static/js/controllers.js 
ceaf1402ebdae4efd1a8a6fc9c7b795de69e2fc0 

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


Testing
---

Testing video record

![http_redirect.gif](https://issues.apache.org/jira/secure/attachment/12820401/http_redirect.gif)


Thanks,

haosdent huang

Review Request 50593: Added `URL::isRelative` to check if the URL is relative.

[haosdent huang]

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

Review request for mesos, Adam B, Benjamin Mahler, Jie Yu, Michael Lunøe, Vinod 
Kone, and Jiang Yan Xu.


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


Repository: mesos


Description
---

Added `URL::isRelative` to check if the URL is relative.


Diffs
-

  3rdparty/libprocess/include/process/http.hpp 
c17c047de050da47989a97395c731686f0dfa548 
  3rdparty/libprocess/src/http.cpp 1bf1f3b1e6b94f19d82d4b23eed7da7da0b25e5a 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 50488: Fixed the incomplete redirect url in `Master::Http::redirect`.

[haosdent huang]

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

(Updated July 29, 2016, 4:07 a.m.)


Review request for mesos, Adam B, Benjamin Mahler, Jie Yu, Michael Lunøe, Vinod 
Kone, and Jiang Yan Xu.


Changes
---

Address @xujyan's comment.


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


Repository: mesos


Description
---

When the request which contains query parameters or fragment sent to a
non-leading master, the master would redirect to an URL that ignored
the query parameters and fragment. This changes the redirect URL
include the query parameters or fragment.


Diffs (updated)
-

  src/master/http.cpp 1cbec976ccedae81def388c62889624aaf9a794e 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49571: Added a benchmark test for allocations.

[Guangya Liu]

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



I would suggest you take a look at https://reviews.apache.org/r/49616 to see 
more comments from Ben Mahler.


src/tests/hierarchical_allocator_tests.cpp (line 84)


new line here



src/tests/hierarchical_allocator_tests.cpp (line 252)


new line here



src/tests/hierarchical_allocator_tests.cpp (line 265)


new line here



src/tests/hierarchical_allocator_tests.cpp (lines 3483 - 3484)


s/unsigned/size_t



src/tests/hierarchical_allocator_tests.cpp (lines 3487 - 3489)


Do we really need this as we already using `Clock::pause();`, I think that 
you can just kill this.

I'm planning to do a clean up for other benchmark tests, such as 
`DeclineOffers`, `ResourceLabels` etc.



src/tests/hierarchical_allocator_tests.cpp (line 3498)


1) The braces go on the next line.
2) How about s/OfferedResources/Allocation/ ? This would match the pattern 
used in the HierarchicalAllocatorTestBase.



src/tests/hierarchical_allocator_tests.cpp (lines 3510 - 3513)


first and second is not clear, what about the following?

```
foreachpair (const SlaveID& slaveId, const Resources& r, resources) {
  Offer offer;
  offer.frameworkId = frameworkId;
  offer.slaveId = slaveId;
  offer.resources = r;

  offers.push_back(std::move(offer));
}
```

I would also prefer that you update line 3504 to 
```
vector allocations;
```

Then the first code section will be `allocations` but not `offers` since 
offers is a master concept but not allocator concept.



src/tests/hierarchical_allocator_tests.cpp (line 3515)


The `offerCount` was already removed from other benchmark tests and we are 
using `offers.size()` instead.



src/tests/hierarchical_allocator_tests.cpp (line 3518)


We tend to declare variables close to where they are needed, seems this can 
be moved down to where we create the slaves below?



src/tests/hierarchical_allocator_tests.cpp (line 3540)


I prefer rename this as variable as `allocation`



src/tests/hierarchical_allocator_tests.cpp (line 3549)


s/unsigned/size_t



src/tests/hierarchical_allocator_tests.cpp (lines 3550 - 3552)


move this out of the loop



src/tests/hierarchical_allocator_tests.cpp (line 3578)


we actually have one more port allocated but not `a random port`, and also 
the allocator do not know task, I would suggest that we only keep `// Add some 
used resources on each slave.` or just remove the comments here.



src/tests/hierarchical_allocator_tests.cpp (line 3589)


s/unsigned/size_t



src/tests/hierarchical_allocator_tests.cpp (line 3590)


Can you avoid auto here? It would be more readable to have the type, and we 
only use 'auto' when the type is very verbose or is clear using only local 
reasonsing



src/tests/hierarchical_allocator_tests.cpp (line 3597)


kill this



src/tests/hierarchical_allocator_tests.cpp (line 3599)


kill this brace, I think it is not needed



src/tests/hierarchical_allocator_tests.cpp (line 3604)


s/background/batch


- Guangya Liu


On 七月 28, 2016, 10:46 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49571/
> ---
> 
> (Updated 七月 28, 2016, 10:46 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5771
> https://issues.apache.org/jira/browse/MESOS-5771
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allocations test has the following configurations:
> (1) REGULAR: Offers from every slave have regular resources.
> (2) SHARED: Offers from every slave include a shared resource.
> (3) REGULAR: Offers from every alternate slave contain only 

Review Request 50592: Updated NvidiaVolume to mount as 'tmpfs' if parent fs is 'noexec'.

[Kevin Klues]

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

Review request for mesos, Benjamin Mahler and Jie Yu.


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


Repository: mesos


Description
---

This patch is in response to an issue we ran into on Ubuntu 14.04,
where '/run' is being mounted as 'noexec' (MESOS-5923). Since our
NvidiaVolume is created below this mount point, we are unable to
execute any binaries we add to this volume. This causes problems, for
example, when trying to execute 'nvidia-smi' from within a container
that has this volume mounted in.

To work around this issue, we detect if any mount point above the path
where we create the volume is marked as 'noexec', and if so, we create
a new 'tmpfs' mount for the volume without 'noexec' set.


Diffs
-

  src/slave/containerizer/mesos/isolators/gpu/volume.cpp 
4b3651a8266b1ba193f6d207cd3be1ce55629703 

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


Testing
---

Remount /run as 'noexec'

`GTEST_FILTER="*NVIDIA*" make -j check`

Ran a master/agent/execute set running 'nvidia-smi' both inside and outside a 
docker container.
Both setups ran successfully.


Thanks,

Kevin Klues

Re: Review Request 50205: Enhanced benchmark test for resources to include shared resources.

[Guangya Liu]

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




src/tests/resources_tests.cpp (lines 2480 - 2487)


I prefer that you adjust the position of `shared` to the place where it was 
used.

```
// Test a typical vector of scalars which include shared resources
// (viz, shared persistent volumes).
Resource disk = createDiskResource(
"256", "test", "persistentId", "/volume", None(), true);

Parameter shared;
shared.resources = Resources::parse("cpus:1;mem:128").get() + disk;
shared.totalOperations = 5;
```


- Guangya Liu


On 七月 19, 2016, 10:52 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50205/
> ---
> 
> (Updated 七月 19, 2016, 10:52 p.m.)
> 
> 
> Review request for mesos, Klaus Ma and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4892
> https://issues.apache.org/jira/browse/MESOS-4892
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhanced benchmark test for resources to include shared resources.
> 
> 
> Diffs
> -
> 
>   src/tests/resources_tests.cpp 4111e080b84079e100b731c9a56861b204f17388 
> 
> Diff: https://reviews.apache.org/r/50205/diff/
> 
> 
> Testing
> ---
> 
> Tests passed. Results for resources benchmark is as follows:
> 
> Minimal impact seen in Resources arithmetic with the Resources refactor 
> changes to incorporate shared resources.
> 
> With shared resources patch (note that 4th test below is for shared resources 
> for scalars)
> 
> [--] 4 tests from ResourcesOperators/Resources_BENCHMARK_Test
> [ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/0
> [   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/0 (964 ms)
> [ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/1
> [   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/1 (17099 
> ms)
> [ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2
> [   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2 (9222 
> ms)
> [ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/3
> [   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/3 (1049 
> ms)
> [--] 4 tests from ResourcesOperators/Resources_BENCHMARK_Test (28334 
> ms total)
> 
> HEAD
> 
> [--] 3 tests from ResourcesOperators/Resources_BENCHMARK_Test
> [ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/0
> [   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/0 (866 ms)
> [ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/1
> [   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/1 (17563 
> ms)
> [ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2
> [   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2 (9218 
> ms)
> [--] 3 tests from ResourcesOperators/Resources_BENCHMARK_Test (27647 
> ms total)
> 
> Output from stdout is:
> 
> [==] Running 4 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 4 tests from ResourcesOperators/Resources_BENCHMARK_Test
> [ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/0
> Took 113429us to perform 5 'total += r' operations on cpus(*):1; 
> gpus(*):1; mem(*):128; disk(*):256
> Took 154104us to perform 5 'total -= r' operations on cpus(*):1; 
> gpus(*):1; mem(*):128; disk(*):256
> Took 327479us to perform 5 'total = total + r' operations on cpus(*):1; 
> gpus(*):1; mem(*):128; disk(*):256
> Took 365752us to perform 5 'total = total - r' operations on cpus(*):1; 
> gpus(*):1; mem(*):128; disk(*):256
> [   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/0 (964 ms)
> [ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/1
> Took 4.105799secs to perform 10 'total += r' operations on cpus(0, 
> principal_0, {key_0: value_0}):1; gpus(...
> Took 4.325702secs to perform 10 'total -= r' operations on cpus(0, 
> principal_0, {key_0: value_0}):1; gpus(...
> Took 4.188394secs to perform 10 'total = total + r' operations on cpus(0, 
> principal_0, {key_0: value_0}):1; gpus(...
> Took 4.428872secs to perform 10 'total = total - r' operations on cpus(0, 
> principal_0, {key_0: value_0}):1; gpus(...
> [   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/1 (17099 
> ms)
> [ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2
> Took 2.515553secs to perform 1000 'total += r' operations on ports(*):[1-2, 
> 4-5, 7-8, 10-11, 13-14, 16-17, 1...
> Took 2.038598secs to perform 1000 'total -= r' operations on 

Re: Review Request 50568: Updated -=/+= to subtract/add for resource object.

[Guangya Liu]

> On 七月 28, 2016, 11:13 p.m., Benjamin Mahler wrote:
> > src/common/resources.cpp, line 458
> > 
> >
> > Hm.. why isn't this just:
> > 
> > ```
> > result.add(resource);
> > ```

Sorry, I missed this part when uploading the patch, updated now. Thanks.


- Guangya


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


On 七月 29, 2016, 1:34 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50568/
> ---
> 
> (Updated 七月 29, 2016, 1:34 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joris Van Remoortere, and Klaus Ma.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is a follow up action for MESOS-5919. Based on the patch of
> https://reviews.apache.org/r/50553/ and
> https://reviews.apache.org/r/50557/ , we should update -=/+= to
> subtract/add if the resource object is from resources object.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 88a9feabf66ed34e7e5b1c6cb7e831818e7f7883 
>   include/mesos/v1/resources.hpp 054ed00a03319ae5e350542add34f497eaf79152 
>   src/common/resources.cpp 3dbff24d6859d3b1ed8589cec50170a5202cfbcb 
>   src/v1/resources.cpp 3c85dc8aa8125962b44e60806ece83a7653d0dc7 
> 
> Diff: https://reviews.apache.org/r/50568/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 50568: Updated -=/+= to subtract/add for resource object.

[Guangya Liu]

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

(Updated 七月 29, 2016, 1:34 a.m.)


Review request for mesos, Benjamin Mahler, Joris Van Remoortere, and Klaus Ma.


Repository: mesos


Description
---

This is a follow up action for MESOS-5919. Based on the patch of
https://reviews.apache.org/r/50553/ and
https://reviews.apache.org/r/50557/ , we should update -=/+= to
subtract/add if the resource object is from resources object.


Diffs (updated)
-

  include/mesos/resources.hpp 88a9feabf66ed34e7e5b1c6cb7e831818e7f7883 
  include/mesos/v1/resources.hpp 054ed00a03319ae5e350542add34f497eaf79152 
  src/common/resources.cpp 3dbff24d6859d3b1ed8589cec50170a5202cfbcb 
  src/v1/resources.cpp 3c85dc8aa8125962b44e60806ece83a7653d0dc7 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

[Jiang Yan Xu]

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



This is done before the you rebased but it should be straightforward once we 
address these remaining issues and we should be good to go!


include/mesos/resources.hpp (line 64)


Perhaps a short note:

```
// The rest of the private section is below the public section. We need to 
define Resource_ first because the public typedefs below depend on it.
```



include/mesos/resources.hpp (line 95)


Can we reword it as

```
The `Resource_` arithmetric and comparison operators and `contains()` 
method require...
```



include/mesos/resources.hpp (line 96)


I didn't raise this earlier but to maintain consistency in comments let's 
just do

s/shared property/sharedness/



include/mesos/resources.hpp (line 101)


s/comparison/`contains()` method/

comparisions (i.e., ==, !=) are operators too.



include/mesos/resources.hpp (line 471)


s/is/are/



src/common/resources.cpp (lines 255 - 279)


Why the change from the last revision?

Sharedness check at the top allows us to short-circuit the checking for 
shared resources and allow us to not have to consider shared resources as a 
possbility in subsequent if conditions which is better IMO both for simplicity 
and performance (even if the difference in performance may not be significant).



src/common/resources.cpp (lines 323 - 346)


Ditto.



src/common/resources.cpp (lines 842 - 843)


Just trying to see if we can make the comments more concise.

```
// Same sharedness required.
```



src/common/resources.cpp (lines 848 - 850)


Just trying to see if the comments can be made more concise:

```
Assuming the wrapped Resource objects are equal, the 'contains' 
relationship is determined by the relationship of the counters for shared 
resources.
```



src/common/resources.cpp (line 856)


Here an `else` clause is more explicit.

```
else {
  return internal::contains(resource, that.resource);
}
```



src/common/resources.cpp (lines 900 - 901)


Just trying to see if we can make the comments more concise.

```
// Same sharedness required.
```



src/common/resources.cpp (line 979)


Looks like we don't need `const Resource& resource = resource_;` and we can 
just do:

```
if (resource_.resource == that) {
...
}
```

?



src/tests/resources_tests.cpp (line 2675)


In my previous comment I was suggesting that let's make sections of the 
test not depend on one another. i.e., r2 doesn't depend on r1. (Of course as a 
reader I still need to know the definition of disk1, disk2 and disk3). Speaking 
of which, disk3 and disk2 are the same so perhaps we don't need a disk3 (once 
we established in simple tests that the equality operator works).

If it's written this way, it's then very clear to me what exactly we are 
testing in each section and whether some tests are redundant.

e.g.,

```
Resources r2 = Resources(disk1) + disk2; (Addition)

...

Resources r3 = Resources(disk1) + disk2 - disk1; (Can substract)

...

Resources r4 = Resources(disk1) - disk2; (Cannot substract)
```



src/tests/resources_tests.cpp (line 2700)


Seems like adding disk1 twice doesn't particularly help with anything? For 
symmetry maybe just 

```
Resources r1 = Resources::parse("cpus:1;mem:5").get() + disk1;

Resources r2 = Resources::parse("cpus:1;mem:5").get() + disk2;
```



src/tests/resources_tests.cpp (line 2702)


Kill this line. We don't have it for r2 and we it's already covered in 
other tests.



src/tests/resources_tests.cpp (line 2707)


Simple heading.

// r1 and r2 don't contain each other because of disk1 and disk2's 
different sharedness.



src/tests/resources_tests.cpp (line 2710)

Re: Review Request 50324: Added script to build mesos-execute.

[Joseph Wu]

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


Ship it!





src/cli/CMakeLists.txt (line 22)


How about:
```
Utility used to schedule and run a command in a mesos cluster
```


- Joseph Wu


On July 26, 2016, 4:43 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50324/
> ---
> 
> (Updated July 26, 2016, 4:43 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Bugs: MESOS-5792
> https://issues.apache.org/jira/browse/MESOS-5792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The mesos-execute executable helps in running a task 
> in mesos cluster.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt bde76f7840afe55f20d7551b3f7e5fe522f7f326 
>   src/cli/CMakeLists.txt PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50324/diff/
> 
> 
> Testing
> ---
> 
> cmake .. && make mesos-execute
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 50323: Added build script for mesos-local executable.

[Joseph Wu]

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


Ship it!




LGTM!


src/local/CMakeLists.txt (lines 20 - 22)


I would mention that `mesos-local` is for testing purposes.


- Joseph Wu


On July 26, 2016, 4:42 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50323/
> ---
> 
> (Updated July 26, 2016, 4:42 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Bugs: MESOS-5792
> https://issues.apache.org/jira/browse/MESOS-5792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The executable will help to run a local mesos cluster.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt bde76f7840afe55f20d7551b3f7e5fe522f7f326 
>   src/local/CMakeLists.txt PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/50323/diff/
> 
> 
> Testing
> ---
> 
> cmake .. && make mesos-local
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 50477: Fixed use-after-close bug when using libevent and SSL.

[Joseph Wu]

> On July 27, 2016, 1:31 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/src/libevent_ssl_socket.cpp, lines 106-110
> > 
> >
> > This comment still seems relevant.
> 
> Benjamin Mahler wrote:
> Yeah, it's still relevant. However, I removed this comment because it 
> seemed like a very lengthy way of saying: don't access 'this' after the 
> 'this' is destructed. When I was reading this code and trying to understand 
> all of it, this was one comment that didn't help (it seemed very clear to me 
> from the code). Are you ok with removing it?

Yup, removing it is fine with me.


- Joseph


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


On July 26, 2016, 7:03 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50477/
> ---
> 
> (Updated July 26, 2016, 7:03 p.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Joseph Wu.
> 
> 
> Bugs: MESOS-5913
> https://issues.apache.org/jira/browse/MESOS-5913
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The deferred call to SSL_shutdown within ~LibeventSSLSocketImpl
> can occur after the socket fd has been closed by Socket::~Impl.
> 
> This can lead to a TLS Alert message being sent on any fd if
> it the fd is re-used between the close and the SSL_shutdown!
> 
> Thanks to Jan-Philip Gehrcke for reporting the issue.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/socket.hpp 
> 881b44b987e5894cac838dae046ab7dbad20b000 
>   3rdparty/libprocess/src/libevent_ssl_socket.hpp 
> 4d376d8b7c1b29105de69bed2e4077f8c94fed0b 
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 
> f4c0b0b97960400b0282837979bf0ed17f56a068 
> 
> Diff: https://reviews.apache.org/r/50477/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Ran my repro steps (issue HTTP requests while hammering the master with HTTPS 
> requests). Issue disappears after this fix.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 50477: Fixed use-after-close bug when using libevent and SSL.

[Benjamin Mahler]

> On July 27, 2016, 8:31 p.m., Joseph Wu wrote:
> > LGTM + some comment tweaks.
> > 
> > This review is almost a revert of this patch: 
> > https://github.com/apache/mesos/commit/ca3667f4e97e11ad30811753fdb52bc02854113f
> > You may want to reference that commit and/or the reasoning behind it.

Will do, thanks!


> On July 27, 2016, 8:31 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/src/libevent_ssl_socket.cpp, lines 144-146
> > 
> >
> > This comment is no longer accurate.

Good eye!


> On July 27, 2016, 8:31 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/src/libevent_ssl_socket.cpp, lines 106-110
> > 
> >
> > This comment still seems relevant.

Yeah, it's still relevant. However, I removed this comment because it seemed 
like a very lengthy way of saying: don't access 'this' after the 'this' is 
destructed. When I was reading this code and trying to understand all of it, 
this was one comment that didn't help (it seemed very clear to me from the 
code). Are you ok with removing it?


- Benjamin


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


On July 27, 2016, 2:03 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50477/
> ---
> 
> (Updated July 27, 2016, 2:03 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Joseph Wu.
> 
> 
> Bugs: MESOS-5913
> https://issues.apache.org/jira/browse/MESOS-5913
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The deferred call to SSL_shutdown within ~LibeventSSLSocketImpl
> can occur after the socket fd has been closed by Socket::~Impl.
> 
> This can lead to a TLS Alert message being sent on any fd if
> it the fd is re-used between the close and the SSL_shutdown!
> 
> Thanks to Jan-Philip Gehrcke for reporting the issue.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/socket.hpp 
> 881b44b987e5894cac838dae046ab7dbad20b000 
>   3rdparty/libprocess/src/libevent_ssl_socket.hpp 
> 4d376d8b7c1b29105de69bed2e4077f8c94fed0b 
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 
> f4c0b0b97960400b0282837979bf0ed17f56a068 
> 
> Diff: https://reviews.apache.org/r/50477/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Ran my repro steps (issue HTTP requests while hammering the master with HTTPS 
> requests). Issue disappears after this fix.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 50477: Fixed use-after-close bug when using libevent and SSL.

[Benjamin Mahler]

> On July 27, 2016, 8:33 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/src/libevent_ssl_socket.cpp, line 508
> > 
> >
> > why `s` vs `get()`?

I didn't think of using get() at the time. Looking at it now, `s` seems a bit 
clearer vs a free-standing `get()`, and I notice `s` is already used in another 
location this file (whereas get() is not used). So I'll stick with `s` for now.


- Benjamin


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


On July 27, 2016, 2:03 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50477/
> ---
> 
> (Updated July 27, 2016, 2:03 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Joseph Wu.
> 
> 
> Bugs: MESOS-5913
> https://issues.apache.org/jira/browse/MESOS-5913
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The deferred call to SSL_shutdown within ~LibeventSSLSocketImpl
> can occur after the socket fd has been closed by Socket::~Impl.
> 
> This can lead to a TLS Alert message being sent on any fd if
> it the fd is re-used between the close and the SSL_shutdown!
> 
> Thanks to Jan-Philip Gehrcke for reporting the issue.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/socket.hpp 
> 881b44b987e5894cac838dae046ab7dbad20b000 
>   3rdparty/libprocess/src/libevent_ssl_socket.hpp 
> 4d376d8b7c1b29105de69bed2e4077f8c94fed0b 
>   3rdparty/libprocess/src/libevent_ssl_socket.cpp 
> f4c0b0b97960400b0282837979bf0ed17f56a068 
> 
> Diff: https://reviews.apache.org/r/50477/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Ran my repro steps (issue HTTP requests while hammering the master with HTTPS 
> requests). Issue disappears after this fix.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 49616: Add suppression benchmark.

[Guangya Liu]

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




src/tests/hierarchical_allocator_tests.cpp (line 3878)


// Wait for all the `suppressOffers` operations to be processed.

The above comments seems more clear than just saying `Settle clock...`



src/tests/hierarchical_allocator_tests.cpp (lines 3883 - 3887)


What about the following, I think this is good enough and also other 
benchmark test are using such format.

```
// Advance the clock and trigger a batch allocation.
Clock::advance(flags.allocation_interval);
Clock::settle();
```


- Guangya Liu


On 七月 28, 2016, 8:42 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49616/
> ---
> 
> (Updated 七月 28, 2016, 8:42 p.m.)
> 
> 
> Review request for mesos, James Peach, Joris Van Remoortere, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5781
> https://issues.apache.org/jira/browse/MESOS-5781
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - Useful for high framework clusters which carry
>   many suppressed frameworks that are considered
>   during allocation.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> bb6947fcecb5b78047e98d10fc1278c612a69548 
> 
> Diff: https://reviews.apache.org/r/49616/diff/
> 
> 
> Testing
> ---
> 
> MESOS_BENCHMARK=1 GTEST_FILTER="*BENCHMARK_Test.SuppressOffers*" make check
> 
> ...
> [ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/12
> Using 5000 agents and 500 frameworks
> Added 500 frameworks in 9300us
> Added 5000 agents in 15.881391secs
> allocate() took 15.537025secs to make 5000 offers with 100 out of 500 
> frameworks suppressing offers
> allocate() took 15.175028secs to make 5000 offers with 200 out of 500 
> frameworks suppressing offers
> allocate() took 15.554843secs to make 5000 offers with 300 out of 500 
> frameworks suppressing offers
> allocate() took 15.740833secs to make 5000 offers with 400 out of 500 
> frameworks suppressing offers
> allocate() took 1.311314secs to make 0 offers with 500 out of 500 frameworks 
> suppressing offers
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/12 
> (93269 ms)
> [ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/13
> Using 5000 agents and 1000 frameworks
> Added 1000 frameworks in 17917us
> Added 5000 agents in 31.450037secs
> allocate() took 31.807624secs to make 5000 offers with 200 out of 1000 
> frameworks suppressing offers
> allocate() took 32.039667secs to make 5000 offers with 400 out of 1000 
> frameworks suppressing offers
> allocate() took 32.333716secs to make 5000 offers with 600 out of 1000 
> frameworks suppressing offers
> allocate() took 32.938456secs to make 5000 offers with 800 out of 1000 
> frameworks suppressing offers
> allocate() took 2.69174secs to make 0 offers with 1000 out of 1000 frameworks 
> suppressing offers
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/13 
> (178827 ms)
> [ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/14
> Using 5000 agents and 3000 frameworks
> Added 3000 frameworks in 55821us
> Added 5000 agents in 1.6420837333mins
> allocate() took 1.6388713167mins to make 5000 offers with 600 out of 3000 
> frameworks suppressing offers
> allocate() took 1.6645890833mins to make 5000 offers with 1200 out of 
> 3000 frameworks suppressing offers
> allocate() took 1.6524411667mins to make 5000 offers with 1800 out of 
> 3000 frameworks suppressing offers
> allocate() took 1.71699065mins to make 5000 offers with 2400 out of 3000 
> frameworks suppressing offers
> allocate() took 8.371823secs to make 0 offers with 3000 out of 3000 
> frameworks suppressing offers
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/14 
> (528494 ms)
> [ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/15
> Using 5000 agents and 6000 frameworks
> Added 6000 frameworks in 110493us
> Added 5000 agents in 3.342676mins
> allocate() took 3.6809198667mins to make 5000 offers with 1200 out of 
> 6000 frameworks suppressing offers
> allocate() took 3.6710132333mins to make 5000 offers with 2400 out of 
> 6000 frameworks suppressing offers
> allocate() took 3.6804925167mins to make 5000 offers with 3600 out of 
> 6000 frameworks suppressing offers
> allocate() took 3.7197498333mins to make 5000 

Re: Review Request 50568: Updated -=/+= to subtract/add for resource object.

[Benjamin Mahler]

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


Fix it, then Ship it!





src/common/resources.cpp (line 458)


Hm.. why isn't this just:

```
result.add(resource);
```



src/v1/resources.cpp (line 460)


Hm.. why isn't this just:

```
result.add(resource);
```


- Benjamin Mahler


On July 28, 2016, 2:03 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50568/
> ---
> 
> (Updated July 28, 2016, 2:03 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joris Van Remoortere, and Klaus Ma.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is a follow up action for MESOS-5919. Based on the patch of
> https://reviews.apache.org/r/50553/ and
> https://reviews.apache.org/r/50557/ , we should update -=/+= to
> subtract/add if the resource object is from resources object.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 88a9feabf66ed34e7e5b1c6cb7e831818e7f7883 
>   include/mesos/v1/resources.hpp 054ed00a03319ae5e350542add34f497eaf79152 
>   src/common/resources.cpp 3dbff24d6859d3b1ed8589cec50170a5202cfbcb 
>   src/v1/resources.cpp 3c85dc8aa8125962b44e60806ece83a7653d0dc7 
> 
> Diff: https://reviews.apache.org/r/50568/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 50179: Added mesos-logrotate-logger utility executable.

[Joseph Wu]

> On July 28, 2016, 3:12 p.m., Joseph Wu wrote:
> > A few minor nits.  I can fix before committing.

Oh, and I will disable this target on Windows for now.  The implementation 
currently includes some non-Windows headers.


- Joseph


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


On July 22, 2016, 12:29 a.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50179/
> ---
> 
> (Updated July 22, 2016, 12:29 a.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Bugs: MESOS-5792
> https://issues.apache.org/jira/browse/MESOS-5792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added mesos-logrotate-logger utility executable.
> 
> 
> Diffs
> -
> 
>   src/slave/CMakeLists.txt bb9ad62b2372ca038c43a20d9906aaf43d9ead41 
>   src/slave/cmake/SlaveConfigure.cmake 
> ced57496970f1d7edf9e7e443b22d14d2ee948f0 
> 
> Diff: https://reviews.apache.org/r/50179/diff/
> 
> 
> Testing
> ---
> 
> cmake .. && make mesos-logrotate-logger
> GLOG_v=1 ./src/mesos-tests 
> --gtest_filter="ContainerLogger*.LOGROTATE_RotateInSandbox" should run 
> successfully.
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 50382: Added number of filtered offers to allocator benchmark test.

[Benjamin Mahler]

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


Ship it!





src/tests/hierarchical_allocator_tests.cpp (line 3537)


Hm.. can we do without this comment? It seems pretty clear to me without 
the comment.



src/tests/hierarchical_allocator_tests.cpp (line 3568)


s/after filtered out/after filtering/



src/tests/hierarchical_allocator_tests.cpp (line 3723)


Ditto here.



src/tests/hierarchical_allocator_tests.cpp (line 3754)


s/after filtered out/after filtering/


- Benjamin Mahler


On July 25, 2016, 12:29 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50382/
> ---
> 
> (Updated July 25, 2016, 12:29 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix added number of  filtered offers to two test cases:
> 1) HierarchicalAllocator_BENCHMARK_Test.ResourceLabels
> 2) HierarchicalAllocator_BENCHMARK_Test.DeclineOffers
> 
> This can help evaluate how much time does `Resources::contains`
> contribute to the time of each allocation.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> bb6947fcecb5b78047e98d10fc1278c612a69548 
> 
> Diff: https://reviews.apache.org/r/50382/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> ```
>  ./bin/mesos-tests.sh --benchmark  
> --gtest_filter="SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.DeclineOffers/0"
>  [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test
> [ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.DeclineOffers/0
> Using 1000 agents and 1 frameworks
> Added 1 frameworks in 3100us
> Added 1000 agents in 426289us
> round 0 allocate() took 295008us to make 0 offers after filtered out 1000 
> offers
> round 1 allocate() took 261507us to make 0 offers after filtered out 1000 
> offers
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.DeclineOffers/0 
> (1683 ms)
> [--] 1 test from 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test (1683 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (1694 ms total)
> [  PASSED  ] 1 test.
> ```
> ```
> ./bin/mesos-tests.sh --benchmark  
> --gtest_filter="SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.DeclineOffers/1"
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test
> [ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.DeclineOffers/1
> Using 1000 agents and 50 frameworks
> Added 50 frameworks in 1722us
> Added 1000 agents in 705104us
> round 0 allocate() took 568385us to make 1000 offers after filtered out 1000 
> offers
> round 1 allocate() took 567718us to make 1000 offers after filtered out 2000 
> offers
> round 2 allocate() took 572209us to make 1000 offers after filtered out 3000 
> offers
> round 3 allocate() took 584650us to make 1000 offers after filtered out 4000 
> offers
> round 4 allocate() took 581486us to make 1000 offers after filtered out 5000 
> offers
> round 5 allocate() took 594665us to make 1000 offers after filtered out 6000 
> offers
> round 6 allocate() took 608935us to make 1000 offers after filtered out 7000 
> offers
> round 7 allocate() took 614147us to make 1000 offers after filtered out 8000 
> offers
> round 8 allocate() took 625020us to make 1000 offers after filtered out 9000 
> offers
> round 9 allocate() took 631848us to make 1000 offers after filtered out 1 
> offers
> round 10 allocate() took 644921us to make 1000 offers after filtered out 
> 11000 offers
> round 11 allocate() took 654093us to make 1000 offers after filtered out 
> 12000 offers
> round 12 allocate() took 661563us to make 1000 offers after filtered out 
> 13000 offers
> round 13 allocate() took 688864us to make 1000 offers after filtered out 
> 14000 offers
> round 14 allocate() took 685055us to make 1000 offers after filtered out 
> 15000 offers
> round 15 allocate() took 703696us to make 1000 offers after filtered out 
> 16000 offers
> round 16 allocate() took 735020us to make 1000 offers after filtered out 
> 17000 offers
> round 17 allocate() took 754733us to make 1000 offers after 

Re: Review Request 50374: Used `size_t` as the type of "size", "count" and "index".

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On July 23, 2016, 9:54 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50374/
> ---
> 
> (Updated July 23, 2016, 9:54 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We should use `size_t` as the type of "size", "count" and "index"
> variables in general, as it provides some additional information
> to the reader.
> 
> Some reference as following:
> 1) http://www.cplusplus.com/reference/cstring/size_t/
> 2) http://stackoverflow.com/questions/131803/unsigned-int-vs-size-t
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/gpu/allocator.cpp 
> 769dfa242ed5322a72b65fbb422894e70af2ad0c 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 3675c02453160417b24c8b3b0d001208504515a5 
>   src/tests/hierarchical_allocator_tests.cpp 
> bb6947fcecb5b78047e98d10fc1278c612a69548 
> 
> Diff: https://reviews.apache.org/r/50374/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45961: Support sharing of resources through reference counting of resources.

[Anindya Sinha]

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




src/master/master.cpp (line 3936)


We now replaces `slave->pendingResources` with `slave->pendingTasks`.


- Anindya Sinha


On July 28, 2016, 10:44 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45961/
> ---
> 
> (Updated July 28, 2016, 10:44 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joris Van Remoortere, Michael 
> Park, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4431
> https://issues.apache.org/jira/browse/MESOS-4431
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> o Each shared resource is accouted via its share count. This count is
>   updated based on the resource operations (such as add and subtract)
>   in various scenarios such as task launch and terminate at multiple
>   modules such as master, allocator, sorter, etc.
> o Only allow DESTROY if there are no running or pending tasks using
>   the volume. However, if the volume is in a pending offer to one or
>   more frameworks, rescind that offer before processing the DESTROY.
> o To allow multiple tasks to be launched in the same ACCEPT call
>   using the same shared resource, we update the allocator and
>   sorter with additional copies of shared resources to reflect the
>   true shared count of allocated shared resources.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 3dbff24d6859d3b1ed8589cec50170a5202cfbcb 
>   src/common/resources_utils.cpp 8e881a09500b0966b577e514077460f724868a8d 
>   src/master/allocator/mesos/hierarchical.hpp 
> b72ba16277a3210e4d309b616d185a10e2029a66 
>   src/master/allocator/mesos/hierarchical.cpp 
> 7d4064535a20b93950f5a95eef1ad3f0d37d305b 
>   src/master/allocator/sorter/drf/sorter.hpp 
> bc6bfb2d5d3b32d55be055a0514861b4e7d889bb 
>   src/master/allocator/sorter/drf/sorter.cpp 
> ac85b327fc33d34246788e6a8c8bf5a486c61434 
>   src/master/http.cpp 1cbec976ccedae81def388c62889624aaf9a794e 
>   src/master/master.hpp 6decff6f4b9c3434de030fd5c06df4c683a7abad 
>   src/master/master.cpp 060dc7f9730808c7fd9b8f9ecdbde0aac14d135c 
>   src/master/validation.hpp 43d876b00511d4ef652c2685d4950c994b5eef31 
>   src/master/validation.cpp 52002beac29c371411348cb026a216e99ac96ab2 
>   src/tests/master_validation_tests.cpp 
> 9eb82a569f7c48caa96d4d54a93b199ccac74385 
>   src/tests/sorter_tests.cpp ade356cbbba5b93a6d3e5c9de30eefd3982d15c1 
>   src/v1/resources.cpp 3c85dc8aa8125962b44e60806ece83a7653d0dc7 
> 
> Diff: https://reviews.apache.org/r/45961/diff/
> 
> 
> Testing
> ---
> 
> Tests successful.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 49571: Added a benchmark test for allocations.

[Anindya Sinha]

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

(Updated July 28, 2016, 10:46 p.m.)


Review request for mesos and Jiang Yan Xu.


Changes
---

Rebase with master.


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


Repository: mesos


Description
---

Allocations test has the following configurations:
(1) REGULAR: Offers from every slave have regular resources.
(2) SHARED: Offers from every slave include a shared resource.
(3) REGULAR: Offers from every alternate slave contain only regular
resources; and offers from every other alternate slave contains
a shared resource.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
bb6947fcecb5b78047e98d10fc1278c612a69548 

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


Testing
---

All tests passed.

Allocations benchmark test results
==
Support of shared resources has a small impact on runtime performance in 
allocations. Also, there is no visible impact in performance when shared 
resources are added in the tests.

With the patch (and no shared resources)

round 0 allocate took 3.19704secs to make 200 offers
round 50 allocate took 3.240605secs to make 200 offers
round 100 allocate took 3.227024secs to make 200 offers
round 150 allocate took 3.225281secs to make 200 offers
round 199 allocate took 3.26036secs to make 200 offers

With the patch (and shared resources on all agents)
---
round 0 allocate took 3.279115secs to make 200 offers
round 50 allocate took 3.273396secs to make 200 offers
round 100 allocate took 3.278509secs to make 200 offers
round 150 allocate took 3.275959secs to make 200 offers
round 199 allocate took 3.278151secs to make 200 offers

With the patch (and shared resources on alternate agents)
-
round 0 allocate took 3.251739secs to make 200 offers
round 50 allocate took 3.263777secs to make 200 offers
round 100 allocate took 3.263079secs to make 200 offers
round 150 allocate took 3.263114secs to make 200 offers
round 199 allocate took 3.236228secs to make 200 offers

Based on HEAD, with all regular resources (no shared resources in HEAD 
supported)
-
round 0 allocate took 2.925681secs to make 200 offers
round 50 allocate took 2.922036secs to make 200 offers
round 100 allocate took 2.909337secs to make 200 offers
round 150 allocate took 2.914093secs to make 200 offers
round 199 allocate took 2.923762secs to make 200 offers


Thanks,

Anindya Sinha

Re: Review Request 45961: Support sharing of resources through reference counting of resources.

[Anindya Sinha]

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

(Updated July 28, 2016, 10:44 p.m.)


Review request for mesos, Benjamin Mahler, Joris Van Remoortere, Michael Park, 
and Jiang Yan Xu.


Changes
---

Rebase with master and addressed review comments.


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


Repository: mesos


Description (updated)
---

o Each shared resource is accouted via its share count. This count is
  updated based on the resource operations (such as add and subtract)
  in various scenarios such as task launch and terminate at multiple
  modules such as master, allocator, sorter, etc.
o Only allow DESTROY if there are no running or pending tasks using
  the volume. However, if the volume is in a pending offer to one or
  more frameworks, rescind that offer before processing the DESTROY.
o To allow multiple tasks to be launched in the same ACCEPT call
  using the same shared resource, we update the allocator and
  sorter with additional copies of shared resources to reflect the
  true shared count of allocated shared resources.


Diffs (updated)
-

  src/common/resources.cpp 3dbff24d6859d3b1ed8589cec50170a5202cfbcb 
  src/common/resources_utils.cpp 8e881a09500b0966b577e514077460f724868a8d 
  src/master/allocator/mesos/hierarchical.hpp 
b72ba16277a3210e4d309b616d185a10e2029a66 
  src/master/allocator/mesos/hierarchical.cpp 
7d4064535a20b93950f5a95eef1ad3f0d37d305b 
  src/master/allocator/sorter/drf/sorter.hpp 
bc6bfb2d5d3b32d55be055a0514861b4e7d889bb 
  src/master/allocator/sorter/drf/sorter.cpp 
ac85b327fc33d34246788e6a8c8bf5a486c61434 
  src/master/http.cpp 1cbec976ccedae81def388c62889624aaf9a794e 
  src/master/master.hpp 6decff6f4b9c3434de030fd5c06df4c683a7abad 
  src/master/master.cpp 060dc7f9730808c7fd9b8f9ecdbde0aac14d135c 
  src/master/validation.hpp 43d876b00511d4ef652c2685d4950c994b5eef31 
  src/master/validation.cpp 52002beac29c371411348cb026a216e99ac96ab2 
  src/tests/master_validation_tests.cpp 
9eb82a569f7c48caa96d4d54a93b199ccac74385 
  src/tests/sorter_tests.cpp ade356cbbba5b93a6d3e5c9de30eefd3982d15c1 
  src/v1/resources.cpp 3c85dc8aa8125962b44e60806ece83a7653d0dc7 

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


Testing
---

Tests successful.


Thanks,

Anindya Sinha

Re: Review Request 45961: Support sharing of resources through reference counting of resources.

[Anindya Sinha]

> On July 26, 2016, 4:06 p.m., Jiang Yan Xu wrote:
> > src/master/master.hpp, lines 817-822
> > 
> >
> > As commented below, I realized that we may not need this. We shouldn't 
> > calculate total task resources as it's done in `Master::addTask()`.

Removed this method.


> On July 26, 2016, 4:06 p.m., Jiang Yan Xu wrote:
> > src/master/master.hpp, line 331
> > 
> >
> > s/assigned/requested/ (assigned is a bit ambiguous, at this point 
> > resources are still not given to the task yet).

Removed `slave->pendingResources` and replaced with `slave->pendingTasks`.


> On July 26, 2016, 4:06 p.m., Jiang Yan Xu wrote:
> > src/master/master.hpp, line 340
> > 
> >
> > s/accept/ACCEPT/

Removed `slave->pendingResources` and replaced with `slave->pendingTasks`.


> On July 26, 2016, 4:06 p.m., Jiang Yan Xu wrote:
> > src/master/master.hpp, line 341
> > 
> >
> > s/removing it/removing them/

Removed `slave->pendingResources` and replaced with `slave->pendingTasks`.


> On July 26, 2016, 4:06 p.m., Jiang Yan Xu wrote:
> > src/master/master.hpp, line 344
> > 
> >
> > With `hashmap pendingResources` we are 
> > preventing DESTROY from affecting pending tasks of the same framework but 
> > letting DESTROY to go through under pending tasks of other frameworks (of 
> > the same role).
> > 
> > If we use `pendingResources` it should be across frameworks right?
> > 
> > But because we already need to add
> > 
> > ```
> > multihashmap pendingTasks;
> > ```
> > 
> > for /r/47082/,
> > 
> > we can just do that instead in this review, the number of tasks pending 
> > for each slave should be small enough to loop through quickly.
> > 
> > Overall this should be more straightfoward than maintaining 
> > `pendingResources`.

Removed `slave->pendingResources` and replaced with `slave->pendingTasks`.


> On July 26, 2016, 4:06 p.m., Jiang Yan Xu wrote:
> > src/master/master.hpp, lines 2501-2502
> > 
> >
> > Prbably can't store them here as we don't aggregate identity-based 
> > resources across agents.
> > 
> > Additionally it looks like we don't need them per comments below.

Removed these.


> On July 26, 2016, 4:06 p.m., Jiang Yan Xu wrote:
> > src/master/master.cpp, lines 3672-3673
> > 
> >
> > Shouldn't need to keep track of used shared resources.

Not needed any more.


> On July 26, 2016, 4:06 p.m., Jiang Yan Xu wrote:
> > src/master/validation.hpp, line 158
> > 
> >
> > This will not take pendingTasks on the agent as the last argument.

I think you meant s/not/now. Yes.


> On July 26, 2016, 4:06 p.m., Jiang Yan Xu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 604-607
> > 
> >
> > Now we have to modify this method to process the LAUNCH call and adjust 
> > the API docs accordingly.

As discussed, we are not going to comment in the base class regarding using 
`Offer::Operation::LAUNCH` in `updateAllocation()` since it is specific to our 
implementation. Appropriate comments added in `hierarchical.cpp`.


- Anindya


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


On July 19, 2016, 10:52 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45961/
> ---
> 
> (Updated July 19, 2016, 10:52 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joris Van Remoortere, Michael 
> Park, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4431
> https://issues.apache.org/jira/browse/MESOS-4431
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> o Each shared resource is accouted via its share count. This count is
>   updated based on the resource operations (such as add and subtract)
>   in various scenarios such as task launch and terminate at multiple
>   modules such as master, allocator, sorter, etc.
> o Only allow DESTROY if there are no running or pending tasks using
>   the volume. However, if the volume is in a pending 

Re: Review Request 45962: Added a persistent volume test framework for shared volumes.

[Anindya Sinha]

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

(Updated July 28, 2016, 10:43 p.m.)


Review request for mesos and Jiang Yan Xu.


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


Repository: mesos


Description
---

Added a persistent volume test framework for shared volumes.


Diffs (updated)
-

  src/Makefile.am a531a50c7fd881445decc1b30b420c3ad5d2fb1e 
  src/examples/persistent_shared_volume_framework.cpp PRE-CREATION 
  src/tests/examples_tests.cpp cac5304f968693be963c9038fa36b0535fb56c3e 
  src/tests/persistent_shared_volume_framework_test.sh PRE-CREATION 

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


Testing
---

New test framework for shared resources added.
Tests successful.


Thanks,

Anindya Sinha

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

[Anindya Sinha]

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

(Updated July 28, 2016, 10:42 p.m.)


Review request for mesos, Benjamin Mahler, Joris Van Remoortere, and Jiang Yan 
Xu.


Changes
---

Rebase with master.


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


Repository: mesos


Description
---

A new class Resource_ is added that allows 'Resources' to group
identical shared resource objects together into a single 'Resource_'
object and tracked by its shared count. Non-shared resource objects
are not grouped.

For resource addition and subtraction, the shared count is adjusted for
shared resources as follows:
a) Addition: If shared resource is absent from original, then the
   resource is added initialized with a consumer count of 1. Otherwise,
   the share count for the shared resource is incremented.
b) Subtraction: If shared resource's share count is already 1, then
   the shared resource is removed from the original. Otherwise, its
   consumer count is decremented.

Note that v1 changes for shared resources are in the next commit.


Diffs (updated)
-

  include/mesos/resources.hpp 88a9feabf66ed34e7e5b1c6cb7e831818e7f7883 
  src/common/resources.cpp 3dbff24d6859d3b1ed8589cec50170a5202cfbcb 
  src/master/validation.cpp 52002beac29c371411348cb026a216e99ac96ab2 
  src/tests/mesos.hpp 51c66f175c80ebacd5af230222ea7e4c81dfc1e8 
  src/tests/resources_tests.cpp 4111e080b84079e100b731c9a56861b204f17388 

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


Testing
---

New tests added to demonstrate arithmetic operations for shared resources with 
consumer counts.
Tests successful.


Thanks,

Anindya Sinha

Re: Review Request 48616: Add v1 changes for shared resources.

[Anindya Sinha]

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

(Updated July 28, 2016, 10:42 p.m.)


Review request for mesos and Jiang Yan Xu.


Changes
---

Rebase with master.


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


Repository: mesos


Description
---

Add v1 changes for shared resources.


Diffs (updated)
-

  include/mesos/v1/resources.hpp 054ed00a03319ae5e350542add34f497eaf79152 
  src/v1/resources.cpp 3c85dc8aa8125962b44e60806ece83a7653d0dc7 

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


Testing
---


Thanks,

Anindya Sinha

Re: Review Request 50064: Added setns and active user test binaries.

[Joseph Wu]

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


Ship it!




LGTM!


src/tests/CMakeLists.txt (line 26)


This TODO still seems relevant.  I'll add it back in before committing.


- Joseph Wu


On July 21, 2016, 11:37 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50064/
> ---
> 
> (Updated July 21, 2016, 11:37 p.m.)
> 
> 
> Review request for mesos, Alex Clemmer and Joseph Wu.
> 
> 
> Bugs: MESOS-5792
> https://issues.apache.org/jira/browse/MESOS-5792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added setns and active user test binaries.
> 
> 
> Diffs
> -
> 
>   src/tests/CMakeLists.txt 3c530631d22aa1cfdc2c600112059601bba7d6b7 
>   src/tests/cmake/MesosTestsConfigure.cmake 
> caecce14ca884dcc09ae4ba7649a09f7ae7c1fdf 
>   src/tests/containerizer/CMakeLists.txt 
> 41e792a2c9ec588d4897d60d012e67c606bbe601 
> 
> Diff: https://reviews.apache.org/r/50064/diff/
> 
> 
> Testing
> ---
> 
> cmake .. && make check
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 40512: Libprocess Reinit: Add a test-only method to reinitialize libprocess.

[Greg Mann]

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




3rdparty/libprocess/src/process.cpp (line 871)


`authenticationRealm` is currently unused in this function. The signature 
should probably be altered to include both `readonlyAuthenticationRealm` and 
`readwriteAuthenticationRealm`, as these recently replaced the single 
`authenticationRealm` in libprocess.

We could either propagate the authentication realms from the original 
initialization by default, or require them to be specified with every 
initialization.


- Greg Mann


On July 26, 2016, 9:17 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40512/
> ---
> 
> (Updated July 26, 2016, 9:17 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3820
> https://issues.apache.org/jira/browse/MESOS-3820
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This builds upon earlier changes to complete `process::finalize`.  
> Tests which need to reconfigure libprocess, such as some SSL-related 
> tests, can use `process::reinitialize` to do so.  
> 
> This method may also be useful for providing additional isolation 
> between tests, such as cleaning up all processes after each test case.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/openssl.cpp 
> a73313b3221d6c80b35f21c00f35d9f9c795f1ec 
>   3rdparty/libprocess/src/process.cpp 
> 7f331b812de2f0437838f48e0959441c8e04c358 
> 
> Diff: https://reviews.apache.org/r/40512/diff/
> 
> 
> Testing
> ---
> 
> TODO: Define `process::reinitialize` in several tests, such as `HTTPTest`, 
> `MetricsTest`, and `ReapTest` and call `process::reinitialize` before and 
> after tests.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 49616: Add suppression benchmark.

[Jacob Janco]

> On July 28, 2016, 4:02 p.m., Jiang Yan Xu wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 3813
> > 
> >
> > The asymmetry between the string typed `agentTotal` and the `Resources` 
> > typed `allocation` looks jarring to me, how about just:
> > 
> > ```
> > SlaveInfo agent = 
> > createSlaveInfo("cpus:24;mem:4096;disk:4096;ports:[31000-32000]");
> > 
> > // Then later,
> > 
> > agents.push_back(agent);
> > ```

Need to call createSlaveInfo in the loop to generate unique slaveIds on each 
pass.


- Jacob


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


On July 28, 2016, 8:42 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49616/
> ---
> 
> (Updated July 28, 2016, 8:42 p.m.)
> 
> 
> Review request for mesos, James Peach, Joris Van Remoortere, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5781
> https://issues.apache.org/jira/browse/MESOS-5781
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> - Useful for high framework clusters which carry
>   many suppressed frameworks that are considered
>   during allocation.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> bb6947fcecb5b78047e98d10fc1278c612a69548 
> 
> Diff: https://reviews.apache.org/r/49616/diff/
> 
> 
> Testing
> ---
> 
> MESOS_BENCHMARK=1 GTEST_FILTER="*BENCHMARK_Test.SuppressOffers*" make check
> 
> ...
> [ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/12
> Using 5000 agents and 500 frameworks
> Added 500 frameworks in 9300us
> Added 5000 agents in 15.881391secs
> allocate() took 15.537025secs to make 5000 offers with 100 out of 500 
> frameworks suppressing offers
> allocate() took 15.175028secs to make 5000 offers with 200 out of 500 
> frameworks suppressing offers
> allocate() took 15.554843secs to make 5000 offers with 300 out of 500 
> frameworks suppressing offers
> allocate() took 15.740833secs to make 5000 offers with 400 out of 500 
> frameworks suppressing offers
> allocate() took 1.311314secs to make 0 offers with 500 out of 500 frameworks 
> suppressing offers
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/12 
> (93269 ms)
> [ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/13
> Using 5000 agents and 1000 frameworks
> Added 1000 frameworks in 17917us
> Added 5000 agents in 31.450037secs
> allocate() took 31.807624secs to make 5000 offers with 200 out of 1000 
> frameworks suppressing offers
> allocate() took 32.039667secs to make 5000 offers with 400 out of 1000 
> frameworks suppressing offers
> allocate() took 32.333716secs to make 5000 offers with 600 out of 1000 
> frameworks suppressing offers
> allocate() took 32.938456secs to make 5000 offers with 800 out of 1000 
> frameworks suppressing offers
> allocate() took 2.69174secs to make 0 offers with 1000 out of 1000 frameworks 
> suppressing offers
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/13 
> (178827 ms)
> [ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/14
> Using 5000 agents and 3000 frameworks
> Added 3000 frameworks in 55821us
> Added 5000 agents in 1.6420837333mins
> allocate() took 1.6388713167mins to make 5000 offers with 600 out of 3000 
> frameworks suppressing offers
> allocate() took 1.6645890833mins to make 5000 offers with 1200 out of 
> 3000 frameworks suppressing offers
> allocate() took 1.6524411667mins to make 5000 offers with 1800 out of 
> 3000 frameworks suppressing offers
> allocate() took 1.71699065mins to make 5000 offers with 2400 out of 3000 
> frameworks suppressing offers
> allocate() took 8.371823secs to make 0 offers with 3000 out of 3000 
> frameworks suppressing offers
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/14 
> (528494 ms)
> [ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/15
> Using 5000 agents and 6000 frameworks
> Added 6000 frameworks in 110493us
> Added 5000 agents in 3.342676mins
> allocate() took 3.6809198667mins to make 5000 offers with 1200 out of 
> 6000 frameworks suppressing offers
> allocate() took 3.6710132333mins to make 5000 offers with 2400 out of 
> 6000 frameworks suppressing offers
> allocate() took 3.6804925167mins to make 5000 offers with 3600 out of 
> 6000 frameworks suppressing offers
> allocate() took 3.7197498333mins to make 5000 offers with 

Re: Review Request 49616: Add suppression benchmark.

[Jacob Janco]

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

(Updated July 28, 2016, 8:42 p.m.)


Review request for mesos, James Peach, Joris Van Remoortere, and Jiang Yan Xu.


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


Repository: mesos


Description
---

- Useful for high framework clusters which carry
  many suppressed frameworks that are considered
  during allocation.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
bb6947fcecb5b78047e98d10fc1278c612a69548 

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


Testing (updated)
---

MESOS_BENCHMARK=1 GTEST_FILTER="*BENCHMARK_Test.SuppressOffers*" make check

...
[ RUN  ] 
SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/12
Using 5000 agents and 500 frameworks
Added 500 frameworks in 9300us
Added 5000 agents in 15.881391secs
allocate() took 15.537025secs to make 5000 offers with 100 out of 500 
frameworks suppressing offers
allocate() took 15.175028secs to make 5000 offers with 200 out of 500 
frameworks suppressing offers
allocate() took 15.554843secs to make 5000 offers with 300 out of 500 
frameworks suppressing offers
allocate() took 15.740833secs to make 5000 offers with 400 out of 500 
frameworks suppressing offers
allocate() took 1.311314secs to make 0 offers with 500 out of 500 frameworks 
suppressing offers
[   OK ] 
SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/12 
(93269 ms)
[ RUN  ] 
SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/13
Using 5000 agents and 1000 frameworks
Added 1000 frameworks in 17917us
Added 5000 agents in 31.450037secs
allocate() took 31.807624secs to make 5000 offers with 200 out of 1000 
frameworks suppressing offers
allocate() took 32.039667secs to make 5000 offers with 400 out of 1000 
frameworks suppressing offers
allocate() took 32.333716secs to make 5000 offers with 600 out of 1000 
frameworks suppressing offers
allocate() took 32.938456secs to make 5000 offers with 800 out of 1000 
frameworks suppressing offers
allocate() took 2.69174secs to make 0 offers with 1000 out of 1000 frameworks 
suppressing offers
[   OK ] 
SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/13 
(178827 ms)
[ RUN  ] 
SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/14
Using 5000 agents and 3000 frameworks
Added 3000 frameworks in 55821us
Added 5000 agents in 1.6420837333mins
allocate() took 1.6388713167mins to make 5000 offers with 600 out of 3000 
frameworks suppressing offers
allocate() took 1.6645890833mins to make 5000 offers with 1200 out of 3000 
frameworks suppressing offers
allocate() took 1.6524411667mins to make 5000 offers with 1800 out of 3000 
frameworks suppressing offers
allocate() took 1.71699065mins to make 5000 offers with 2400 out of 3000 
frameworks suppressing offers
allocate() took 8.371823secs to make 0 offers with 3000 out of 3000 frameworks 
suppressing offers
[   OK ] 
SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/14 
(528494 ms)
[ RUN  ] 
SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.SuppressOffers/15
Using 5000 agents and 6000 frameworks
Added 6000 frameworks in 110493us
Added 5000 agents in 3.342676mins
allocate() took 3.6809198667mins to make 5000 offers with 1200 out of 6000 
frameworks suppressing offers
allocate() took 3.6710132333mins to make 5000 offers with 2400 out of 6000 
frameworks suppressing offers
allocate() took 3.6804925167mins to make 5000 offers with 3600 out of 6000 
frameworks suppressing offers
allocate() took 3.7197498333mins to make 5000 offers with 4800 out of 6000 
frameworks suppressing offers
allocate() took 17.318823secs to make 0 offers with 6000 out of 6000 frameworks 
suppressing offers
...


Thanks,

Jacob Janco

Re: Review Request 40268: Libprocess Reinit: Change Socket::DEFAULT_KIND to a non-static value.

[Greg Mann]

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




3rdparty/libprocess/src/socket.cpp (line 108)


Use `return` directly here?


- Greg Mann


On July 26, 2016, 9:15 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40268/
> ---
> 
> (Updated July 26, 2016, 9:15 p.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3820
> https://issues.apache.org/jira/browse/MESOS-3820
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is needed for tests that utilize the test-only 
> `process::reinitialize` function.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/socket.hpp 
> 881b44b987e5894cac838dae046ab7dbad20b000 
>   3rdparty/libprocess/src/socket.cpp 668f26d85cf3097ff9b90a10be14c5833fafd558 
> 
> Diff: https://reviews.apache.org/r/40268/diff/
> 
> 
> Testing
> ---
> 
> Some test cleanup issues were uncovered by this change; fixed here: 
> https://reviews.apache.org/r/40453/
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 50205: Enhanced benchmark test for resources to include shared resources.

[Jiang Yan Xu]

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


Ship it!




BTW the method `static vector parameters()` should probably 
eventually be splitted into individual clearly named methods when it gets even 
longer but it's fine for now I guess.

- Jiang Yan Xu


On July 19, 2016, 3:52 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50205/
> ---
> 
> (Updated July 19, 2016, 3:52 p.m.)
> 
> 
> Review request for mesos, Klaus Ma and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4892
> https://issues.apache.org/jira/browse/MESOS-4892
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhanced benchmark test for resources to include shared resources.
> 
> 
> Diffs
> -
> 
>   src/tests/resources_tests.cpp 4111e080b84079e100b731c9a56861b204f17388 
> 
> Diff: https://reviews.apache.org/r/50205/diff/
> 
> 
> Testing
> ---
> 
> Tests passed. Results for resources benchmark is as follows:
> 
> Minimal impact seen in Resources arithmetic with the Resources refactor 
> changes to incorporate shared resources.
> 
> With shared resources patch (note that 4th test below is for shared resources 
> for scalars)
> 
> [--] 4 tests from ResourcesOperators/Resources_BENCHMARK_Test
> [ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/0
> [   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/0 (964 ms)
> [ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/1
> [   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/1 (17099 
> ms)
> [ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2
> [   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2 (9222 
> ms)
> [ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/3
> [   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/3 (1049 
> ms)
> [--] 4 tests from ResourcesOperators/Resources_BENCHMARK_Test (28334 
> ms total)
> 
> HEAD
> 
> [--] 3 tests from ResourcesOperators/Resources_BENCHMARK_Test
> [ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/0
> [   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/0 (866 ms)
> [ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/1
> [   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/1 (17563 
> ms)
> [ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2
> [   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2 (9218 
> ms)
> [--] 3 tests from ResourcesOperators/Resources_BENCHMARK_Test (27647 
> ms total)
> 
> Output from stdout is:
> 
> [==] Running 4 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 4 tests from ResourcesOperators/Resources_BENCHMARK_Test
> [ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/0
> Took 113429us to perform 5 'total += r' operations on cpus(*):1; 
> gpus(*):1; mem(*):128; disk(*):256
> Took 154104us to perform 5 'total -= r' operations on cpus(*):1; 
> gpus(*):1; mem(*):128; disk(*):256
> Took 327479us to perform 5 'total = total + r' operations on cpus(*):1; 
> gpus(*):1; mem(*):128; disk(*):256
> Took 365752us to perform 5 'total = total - r' operations on cpus(*):1; 
> gpus(*):1; mem(*):128; disk(*):256
> [   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/0 (964 ms)
> [ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/1
> Took 4.105799secs to perform 10 'total += r' operations on cpus(0, 
> principal_0, {key_0: value_0}):1; gpus(...
> Took 4.325702secs to perform 10 'total -= r' operations on cpus(0, 
> principal_0, {key_0: value_0}):1; gpus(...
> Took 4.188394secs to perform 10 'total = total + r' operations on cpus(0, 
> principal_0, {key_0: value_0}):1; gpus(...
> Took 4.428872secs to perform 10 'total = total - r' operations on cpus(0, 
> principal_0, {key_0: value_0}):1; gpus(...
> [   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/1 (17099 
> ms)
> [ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2
> Took 2.515553secs to perform 1000 'total += r' operations on ports(*):[1-2, 
> 4-5, 7-8, 10-11, 13-14, 16-17, 1...
> Took 2.038598secs to perform 1000 'total -= r' operations on ports(*):[1-2, 
> 4-5, 7-8, 10-11, 13-14, 16-17, 1...
> Took 2.62789secs to perform 1000 'total = total + r' operations on 
> ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
> Took 2.03868secs to perform 1000 'total = total - r' operations on 
> ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
> [   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2 (9222 
> 

Re: Review Request 40413: Libprocess Reinit: Move ReaperProcess instantiation into process.cpp.

[Greg Mann]

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




3rdparty/libprocess/include/process/reap.hpp (line 22)


I tend to prefer leaving includes when we explicitly make use of the type, 
as is the case with `Option` here. What do you think?


- Greg Mann


On July 14, 2016, 12:21 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40413/
> ---
> 
> (Updated July 14, 2016, 12:21 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-3934
> https://issues.apache.org/jira/browse/MESOS-3934
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The reaper singleton must be unified with `process::initialize` so 
> that it also falls under the scope of reinitialization.  The singleton 
> must also not be guarded by `Once`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/reap.hpp 
> 1a9709c618c5ddc9d2b7492cc1855a11f1fc4fb9 
>   3rdparty/libprocess/src/process.cpp 
> 9661386afd4fddd1877d55941fa403afc9230280 
>   3rdparty/libprocess/src/reap.cpp ac60c6d769076912293950432266c956d6c7e705 
> 
> Diff: https://reviews.apache.org/r/40413/diff/
> 
> 
> Testing
> ---
> 
> Tests done in a subsequent review.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Review Request 50582: Fixed OsTest.User failure due to unsorted gids returned by getgrouplist.

[Gilbert Song]

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

Review request for mesos, Artem Harutyunyan, Jie Yu, Kapil Arya, and Timothy 
Chen.


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


Repository: mesos


Description
---

Fixed OsTest.User failure due to unsorted gids returned by getgrouplist.


Diffs
-

  3rdparty/stout/tests/os_tests.cpp e67444077eae55fd25945b451164b5bcc37552b0 

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


Testing
---

make check 

sudo ./bin/mesos-tests.sh


Thanks,

Gilbert Song

Review Request 50580: Removed unused user variable in filesystem linux isolator prepare.

[Gilbert Song]

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

Review request for mesos, Artem Harutyunyan, Jie Yu, Timothy Chen, and Jiang 
Yan Xu.


Repository: mesos


Description
---

Removed unused user variable in filesystem linux isolator prepare.


Diffs
-

  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
db3ed8f69de8b52633194b252b0e5aba38ec69c0 

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


Testing
---

make check

sudo ./bin/mesos-tests.sh


Thanks,

Gilbert Song

Review Request 50581: Added logs for pre-exec commands to sandbox in MesosContainerizerLaunch.

[Gilbert Song]

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

Review request for mesos, Artem Harutyunyan, Jie Yu, and Timothy Chen.


Repository: mesos


Description
---

Added logs for pre-exec commands to sandbox in MesosContainerizerLaunch.


Diffs
-

  src/slave/containerizer/mesos/launch.cpp 
51f0c110ff0c414837fd69db81047979a0093388 

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


Testing
---

make check

sudo ./bin/mesos-tests.sh

The sandbox log will look as the following:
```
Executing pre-exec non-shell command 
'/home/vagrant/mesos/build/src/mesos-containerizer' with arguments [ 
mesos-containerizer, mount, , --help=false, --operation=make-rslave, --path=/ ] 
as a subprocess
Executing pre-exec non-shell command 'mount' with arguments [ mount, -n, 
--rbind, 
/tmp/LinuxFilesystemIsolatorTest_ROOT_ChangeRootFilesystemCommandExecutorWithVolumes_CAPegh/slaves/8b348595-e921-4a3b-a7f8-549510982b96-S0/frameworks/8b348595-e921-4a3b-a7f8-549510982b96-/executors/f2b0ca11-2747-4e06-a8b2-5afda1438afe/runs/6466b38d-75a2-4e7b-a74a-c49d4cafbf5c,
 
/tmp/LinuxFilesystemIsolatorTest_ROOT_ChangeRootFilesystemCommandExecutorWithVolumes_CAPegh/provisioner/containers/6466b38d-75a2-4e7b-a74a-c49d4cafbf5c/backends/copy/rootfses/54985528-873a-4999-bbab-dab6e7d914c9/mnt/mesos/sandbox
 ] as a subprocess
Executing pre-exec non-shell command 'mount' with arguments [ mount, -n, 
--rbind, /tmp/DPe91y/dir1, 
/tmp/LinuxFilesystemIsolatorTest_ROOT_ChangeRootFilesystemCommandExecutorWithVolumes_CAPegh/provisioner/containers/6466b38d-75a2-4e7b-a74a-c49d4cafbf5c/backends/copy/rootfses/54985528-873a-4999-bbab-dab6e7d914c9/tmp
 ] as a subprocess
Executing pre-exec non-shell command 'mount' with arguments [ mount, -n, 
--rbind, /tmp/DPe91y/dir2, 
/tmp/LinuxFilesystemIsolatorTest_ROOT_ChangeRootFilesystemCommandExecutorWithVolumes_CAPegh/provisioner/containers/6466b38d-75a2-4e7b-a74a-c49d4cafbf5c/backends/copy/rootfses/54985528-873a-4999-bbab-dab6e7d914c9/mnt/mesos/sandbox/relative_dir
 ] as a subprocess
Changing root to 
/tmp/LinuxFilesystemIsolatorTest_ROOT_ChangeRootFilesystemCommandExecutorWithVolumes_CAPegh/provisioner/containers/6466b38d-75a2-4e7b-a74a-c49d4cafbf5c/backends/copy/rootfses/54985528-873a-4999-bbab-dab6e7d914c9
```


Thanks,

Gilbert Song

Re: Review Request 50216: Updated filesystem linux isolator pre exec commands to be non-shell.

[Gilbert Song]

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

(Updated July 28, 2016, 12:39 p.m.)


Review request for mesos, Artem Harutyunyan, Jie Yu, Timothy Chen, and Jiang 
Yan Xu.


Summary (updated)
-

Updated filesystem linux isolator pre exec commands to be non-shell.


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


Repository: mesos


Description (updated)
---

Updated filesystem linux isolator pre exec commands to be non-shell.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/filesystem/linux.hpp 
0afe9272900cfa4b39887eb259070a9a2df2ab93 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
db3ed8f69de8b52633194b252b0e5aba38ec69c0 

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


Testing
---

make check

sudo ./bin/mesos-tests.sh


Thanks,

Gilbert Song

Re: Review Request 50570: Mesos-slave --help should not return as failed.

[Pierre Cheynier]

> On juil. 28, 2016, 5:09 après-midi, Greg Mann wrote:
> > src/slave/main.cpp, lines 192-195
> > 
> >
> > Perhaps we could move this block above the `load.isError()` check, 
> > rather than adding the additional check above?

Sure ! I was not entirely confident in doing this since I guessed it was done 
on purpose to fail for non-existing args before applying some logic.


- Pierre


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


On juil. 28, 2016, 5:24 après-midi, Pierre Cheynier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50570/
> ---
> 
> (Updated juil. 28, 2016, 5:24 après-midi)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-5922
> https://issues.apache.org/jira/browse/MESOS-5922
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Doing a `mesos-agent --help` should return with exit_status=0.
> It's not the case since work_dir is considered as mandatory (1.0.0 / 
> 95e6e3063c64ecdcf371181f98d0b2621be6186f)
> 
> Should fix MESOS-5922
> 
> 
> Diffs
> -
> 
>   src/slave/main.cpp a4d971a00f18af661a8bf2cb4f580d35f7c76d10 
> 
> Diff: https://reviews.apache.org/r/50570/diff/
> 
> 
> Testing
> ---
> 
> $ ./boostrap
> $ mkdir build && cd build
> $ ../configure
> $ make
> $ make check
> $ ./build/src/mesos-agent --help > /dev/null; echo $?
> 0
> $ ./build/src/mesos-agent --work_dir='/mnt/mesos' > /dev/null; echo $?
> 0
> $ ./build/src/mesos-agent --work_dir='/mnt/mesos' --master=localhost:5050; 
> echo $?
> WARNING: Logging before InitGoogleLogging() is written to STDERR
> I0728 17:01:43.261836 23247 main.cpp:243] Build: 2016-07-28 16:16:49 by pierre
> I0728 17:01:43.261925 23247 main.cpp:244] Version: 1.1.0
> I0728 17:01:43.261930 23247 main.cpp:251] Git SHA: 
> beda6ffb72687d9f5c23b07552cadf42756bcd75
> (...)
> 
> 
> Thanks,
> 
> Pierre Cheynier
> 
>

Re: Review Request 50570: Mesos-slave --help should not return as failed.

[Pierre Cheynier]

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

(Updated juil. 28, 2016, 5:24 après-midi)


Review request for mesos and Greg Mann.


Changes
---

Add reviewer


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


Repository: mesos


Description
---

Doing a `mesos-agent --help` should return with exit_status=0.
It's not the case since work_dir is considered as mandatory (1.0.0 / 
95e6e3063c64ecdcf371181f98d0b2621be6186f)

Should fix MESOS-5922


Diffs
-

  src/slave/main.cpp a4d971a00f18af661a8bf2cb4f580d35f7c76d10 

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


Testing
---

$ ./boostrap
$ mkdir build && cd build
$ ../configure
$ make
$ make check
$ ./build/src/mesos-agent --help > /dev/null; echo $?
0
$ ./build/src/mesos-agent --work_dir='/mnt/mesos' > /dev/null; echo $?
0
$ ./build/src/mesos-agent --work_dir='/mnt/mesos' --master=localhost:5050; echo 
$?
WARNING: Logging before InitGoogleLogging() is written to STDERR
I0728 17:01:43.261836 23247 main.cpp:243] Build: 2016-07-28 16:16:49 by pierre
I0728 17:01:43.261925 23247 main.cpp:244] Version: 1.1.0
I0728 17:01:43.261930 23247 main.cpp:251] Git SHA: 
beda6ffb72687d9f5c23b07552cadf42756bcd75
(...)


Thanks,

Pierre Cheynier

Re: Review Request 50570: Mesos-slave --help should not return as failed.

[Mesos ReviewBot]

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



Bad review!

Reviews applied: []

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos ReviewBot


On July 28, 2016, 3:14 p.m., Pierre Cheynier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50570/
> ---
> 
> (Updated July 28, 2016, 3:14 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Doing a `mesos-agent --help` should return with exit_status=0.
> It's not the case since work_dir is considered as mandatory (1.0.0 / 
> 95e6e3063c64ecdcf371181f98d0b2621be6186f)
> 
> Should fix MESOS-5922
> 
> 
> Diffs
> -
> 
>   src/slave/main.cpp a4d971a00f18af661a8bf2cb4f580d35f7c76d10 
> 
> Diff: https://reviews.apache.org/r/50570/diff/
> 
> 
> Testing
> ---
> 
> $ ./boostrap
> $ mkdir build && cd build
> $ ../configure
> $ make
> $ make check
> $ ./build/src/mesos-agent --help > /dev/null; echo $?
> 0
> $ ./build/src/mesos-agent --work_dir='/mnt/mesos' > /dev/null; echo $?
> 0
> $ ./build/src/mesos-agent --work_dir='/mnt/mesos' --master=localhost:5050; 
> echo $?
> WARNING: Logging before InitGoogleLogging() is written to STDERR
> I0728 17:01:43.261836 23247 main.cpp:243] Build: 2016-07-28 16:16:49 by pierre
> I0728 17:01:43.261925 23247 main.cpp:244] Version: 1.1.0
> I0728 17:01:43.261930 23247 main.cpp:251] Git SHA: 
> beda6ffb72687d9f5c23b07552cadf42756bcd75
> (...)
> 
> 
> Thanks,
> 
> Pierre Cheynier
> 
>

Re: Review Request 49852: Implemented `NetClsSubsystem`.

[Qian Zhang]

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




src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (line 359)


Where is the implementation of this class `NetClsHandleManager`?


- Qian Zhang


On July 27, 2016, 1:15 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49852/
> ---
> 
> (Updated July 27, 2016, 1:15 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Bugs: MESOS-5046
> https://issues.apache.org/jira/browse/MESOS-5046
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented `NetClsSubsystem`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 348f105f9c3109a02f1dde0649f1b829cb9ddd04 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
> 
> Diff: https://reviews.apache.org/r/49852/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 50552: Fixed a typo in cgroups.hpp.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On July 27, 2016, 11:58 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50552/
> ---
> 
> (Updated July 27, 2016, 11:58 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a typo in cgroups.hpp.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> c57baec88437f68886702a40ec8a6a6458546119 
> 
> Diff: https://reviews.apache.org/r/50552/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Review Request 50570: Mesos-slave --help should not return as failed.

[Pierre Cheynier]

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

Review request for mesos.


Repository: mesos


Description
---

Doing a `mesos-agent --help` should return with exit_status=0.
It's not the case since work_dir is considered as mandatory (1.0.0 / 
95e6e3063c64ecdcf371181f98d0b2621be6186f)

Should fix MESOS-5922


Diffs
-

  src/slave/main.cpp a4d971a00f18af661a8bf2cb4f580d35f7c76d10 

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


Testing
---

$ ./boostrap
$ mkdir build && cd build
$ ../configure
$ make
$ make check
$ ./build/src/mesos-agent --help > /dev/null; echo $?
0
$ ./build/src/mesos-agent --work_dir='/mnt/mesos' > /dev/null; echo $?
0
$ ./build/src/mesos-agent --work_dir='/mnt/mesos' --master=localhost:5050; echo 
$?
WARNING: Logging before InitGoogleLogging() is written to STDERR
I0728 17:01:43.261836 23247 main.cpp:243] Build: 2016-07-28 16:16:49 by pierre
I0728 17:01:43.261925 23247 main.cpp:244] Version: 1.1.0
I0728 17:01:43.261930 23247 main.cpp:251] Git SHA: 
beda6ffb72687d9f5c23b07552cadf42756bcd75
(...)


Thanks,

Pierre Cheynier

Re: Review Request 50568: Updated -=/+= to subtract/add for resource object.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [50380, 50551, 50553, 50556, 50557, 50568]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On July 28, 2016, 2:03 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50568/
> ---
> 
> (Updated July 28, 2016, 2:03 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joris Van Remoortere, and Klaus Ma.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is a follow up action for MESOS-5919. Based on the patch of
> https://reviews.apache.org/r/50553/ and
> https://reviews.apache.org/r/50557/ , we should update -=/+= to
> subtract/add if the resource object is from resources object.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp 88a9feabf66ed34e7e5b1c6cb7e831818e7f7883 
>   include/mesos/v1/resources.hpp 054ed00a03319ae5e350542add34f497eaf79152 
>   src/common/resources.cpp 3dbff24d6859d3b1ed8589cec50170a5202cfbcb 
>   src/v1/resources.cpp 3c85dc8aa8125962b44e60806ece83a7653d0dc7 
> 
> Diff: https://reviews.apache.org/r/50568/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 49852: Implemented `NetClsSubsystem`.

[Qian Zhang]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (lines 961 - 965)


Should this be a CHECK()?



src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (line 1026)


I think you also need to do `infos.erase(containerId);` here.


- Qian Zhang


On July 27, 2016, 1:15 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49852/
> ---
> 
> (Updated July 27, 2016, 1:15 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Bugs: MESOS-5046
> https://issues.apache.org/jira/browse/MESOS-5046
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented `NetClsSubsystem`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 348f105f9c3109a02f1dde0649f1b829cb9ddd04 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
> 
> Diff: https://reviews.apache.org/r/49852/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 50568: Updated -=/+= to subtract/add for resource object.

[Guangya Liu]

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

Review request for mesos, Benjamin Mahler, Joris Van Remoortere, and Klaus Ma.


Repository: mesos


Description
---

This is a follow up action for MESOS-5919. Based on the patch of
https://reviews.apache.org/r/50553/ and
https://reviews.apache.org/r/50557/ , we should update -=/+= to
subtract/add if the resource object is from resources object.


Diffs
-

  include/mesos/resources.hpp 88a9feabf66ed34e7e5b1c6cb7e831818e7f7883 
  include/mesos/v1/resources.hpp 054ed00a03319ae5e350542add34f497eaf79152 
  src/common/resources.cpp 3dbff24d6859d3b1ed8589cec50170a5202cfbcb 
  src/v1/resources.cpp 3c85dc8aa8125962b44e60806ece83a7653d0dc7 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 50422: Future-proofed some slave removal tests.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [50235, 50416, 50417, 50418, 50422]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On July 28, 2016, 10:14 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50422/
> ---
> 
> (Updated July 28, 2016, 10:14 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These tests relied on the implementation detail that when an agent is
> removed from the list of registered agents, the master sends a
> ShutdownSlaveMessage to the agent. That will change in the future
> (MESOS-4049). To prepare for this future planned behavior, adjust these
> tests to be more robust by instead checking for the invocation of the
> `slaveLost` scheduler callback.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp b9fa85dc1ae0922a100786fcb01156b90a013d2a 
> 
> Diff: https://reviews.apache.org/r/50422/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Validated that when the "cancel pending slave removal on receipt of ping" 
> code is removed, `CancelSlaveRemoval` still fails.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 50557: Used `add` instead of `+=` for `Resources::filter`.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [50380, 50551, 50553, 50556, 50557]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On July 28, 2016, 9:29 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50557/
> ---
> 
> (Updated July 28, 2016, 9:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joris Van Remoortere, and Klaus Ma.
> 
> 
> Bugs: MESOS-5919
> https://issues.apache.org/jira/browse/MESOS-5919
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The current logic of `Resources::filter` is using `+=` to add
> the filtered resource object and the `+=` resource object will
> invoke `validate` which is not necessary as all of the resource
> objects are valid.
> 
> The fix is using `add` instead of `+=` for `Resources::filter`
> , this can make sure there is no validation and can improve
> performance of `Resources::filter` for resources object.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp 3dbff24d6859d3b1ed8589cec50170a5202cfbcb 
>   src/v1/resources.cpp 3c85dc8aa8125962b44e60806ece83a7653d0dc7 
> 
> Diff: https://reviews.apache.org/r/50557/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> The performance for `ports` resources of `r.nonRevocable()` was improved 
> about `8s` with 1000 operations.
> 
> Before fix with validation.
> ```
> [ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2
> Took 2.832429secs to perform 1000 'total += r' operations on ports(*):[1-2, 
> 4-5, 7-8, 10-11, 13-14, 16-17, 1...
> Took 22.034252secs to perform 1000 'total.contains(r)' operations on 
> ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
> Took 3.628885secs to perform 1000 'total -= r' operations on ports(*):[1-2, 
> 4-5, 7-8, 10-11, 13-14, 16-17, 1...
> Took 3.113548secs to perform 1000 'total = total + r' operations on 
> ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
> Took 3.854608secs to perform 1000 'total = total - r' operations on 
> ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
> Took 8.517941secs to perform 1000 'r.nonRevocable()' operations on 
> ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
> [   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2 (43985 
> ms)
> ```
> 
> After fix without validation.
> ```
> [ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2
> Took 2.834335secs to perform 1000 'total += r' operations on ports(*):[1-2, 
> 4-5, 7-8, 10-11, 13-14, 16-17, 1...
> Took 21.828846secs to perform 1000 'total.contains(r)' operations on 
> ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
> Took 3.689858secs to perform 1000 'total -= r' operations on ports(*):[1-2, 
> 4-5, 7-8, 10-11, 13-14, 16-17, 1...
> Took 2.962259secs to perform 1000 'total = total + r' operations on 
> ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
> Took 3.760281secs to perform 1000 'total = total - r' operations on 
> ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
> Took 125159us to perform 1000 'r.nonRevocable()' operations on ports(*):[1-2, 
> 4-5, 7-8, 10-11, 13-14, 16-17, 1...
> [   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2 (35204 
> ms)
> ```
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 50235: Added more expectations to TASK_LOST test cases.

[Neil Conway]

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

(Updated July 28, 2016, 11:04 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Check more metrics after expiry of agent_reregiter_timeout.


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


Repository: mesos


Description
---

Check the reason and source of TASK_LOST status updates, replaced
ASSERT_ with EXPECT_ in various places where EXPECT_ is more
appropriate.


Diffs (updated)
-

  src/tests/fault_tolerance_tests.cpp 79f36a5a855ea1795be83840479e71f0e417298b 
  src/tests/master_authorization_tests.cpp 
e43b264b9f67d4cd965aee143cc42a1034ac9952 
  src/tests/master_maintenance_tests.cpp 
2d4b45f78a234118df9e58a69f5587dd4d37956c 
  src/tests/master_slave_reconciliation_tests.cpp 
fdb9bb651c682fb302bb2a533d2dda9bf090839a 
  src/tests/master_tests.cpp 6709818d599c068c289bcb714446018577082d8b 
  src/tests/partition_tests.cpp 91969e4c3196a4f36c19abf38e229f3a36e87ea1 
  src/tests/slave_recovery_tests.cpp 470fb26a2985f912b2b91d647cd7a27b8748c2a5 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 50422: Future-proofed some slave removal tests.

[Neil Conway]

> On July 27, 2016, 11:54 p.m., Vinod Kone wrote:
> > src/tests/slave_tests.cpp, line 2428
> > 
> >
> > Are you sure pausing the clock at the beginning is safe? IIRC, the 
> > scheduler driver does reliable registration which relies on timers and with 
> > esp auth enabled there is no guarantee that auth and registration go 
> > through in the first try?
> > 
> > Also, please run these tests in a loop if you haven't already.

Hmmm -- we pause the clock as the first step of a lot of tests, so if it isn't 
safe that is a bigger issue. I do think pausing the clock for the entirety of 
the test has some advantages in terms of code clarity (and I'd like to see us 
consider running all tests with the clock paused by default). Anyway, I 
reverted this change for now.


- Neil


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


On July 28, 2016, 10:14 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50422/
> ---
> 
> (Updated July 28, 2016, 10:14 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These tests relied on the implementation detail that when an agent is
> removed from the list of registered agents, the master sends a
> ShutdownSlaveMessage to the agent. That will change in the future
> (MESOS-4049). To prepare for this future planned behavior, adjust these
> tests to be more robust by instead checking for the invocation of the
> `slaveLost` scheduler callback.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp b9fa85dc1ae0922a100786fcb01156b90a013d2a 
> 
> Diff: https://reviews.apache.org/r/50422/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Validated that when the "cancel pending slave removal on receipt of ping" 
> code is removed, `CancelSlaveRemoval` still fails.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 50422: Future-proofed some slave removal tests.

[Neil Conway]

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




src/tests/slave_tests.cpp (line 2564)


Fair enough -- we do resume the clock unnecessarily in a lot of tests, so I 
was following that pattern for consistency. It would be good to go through and 
remove the unnecessary `resume`s from all tests.


- Neil Conway


On July 28, 2016, 10:14 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50422/
> ---
> 
> (Updated July 28, 2016, 10:14 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These tests relied on the implementation detail that when an agent is
> removed from the list of registered agents, the master sends a
> ShutdownSlaveMessage to the agent. That will change in the future
> (MESOS-4049). To prepare for this future planned behavior, adjust these
> tests to be more robust by instead checking for the invocation of the
> `slaveLost` scheduler callback.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp b9fa85dc1ae0922a100786fcb01156b90a013d2a 
> 
> Diff: https://reviews.apache.org/r/50422/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Validated that when the "cancel pending slave removal on receipt of ping" 
> code is removed, `CancelSlaveRemoval` still fails.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 50422: Future-proofed some slave removal tests.

[Neil Conway]

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

(Updated July 28, 2016, 10:14 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Address comments.


Repository: mesos


Description (updated)
---

These tests relied on the implementation detail that when an agent is
removed from the list of registered agents, the master sends a
ShutdownSlaveMessage to the agent. That will change in the future
(MESOS-4049). To prepare for this future planned behavior, adjust these
tests to be more robust by instead checking for the invocation of the
`slaveLost` scheduler callback.


Diffs (updated)
-

  src/tests/slave_tests.cpp b9fa85dc1ae0922a100786fcb01156b90a013d2a 

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


Testing
---

make check

Validated that when the "cancel pending slave removal on receipt of ping" code 
is removed, `CancelSlaveRemoval` still fails.


Thanks,

Neil Conway

Re: Review Request 50417: Improved consistency of test code for partitioning an agent.

[Neil Conway]

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

(Updated July 28, 2016, 9:42 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Update commit message.


Repository: mesos


Description (updated)
---

Removed unnecessary `Clock::settle` calls: `Clock::settle` should
typically only be used when a test case does not have an easy way to
wait for a _specific_ event to occur. In this case, `Clock::settle` was
unnecessary because the test code immediately proceeded to `AWAIT_READY`
for a more specific event.

Also fixed up some whitespace.


Diffs (updated)
-

  src/tests/partition_tests.cpp 91969e4c3196a4f36c19abf38e229f3a36e87ea1 
  src/tests/slave_recovery_tests.cpp 470fb26a2985f912b2b91d647cd7a27b8748c2a5 
  src/tests/slave_tests.cpp b9fa85dc1ae0922a100786fcb01156b90a013d2a 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 50417: Improved consistency of test code for partitioning an agent.

[Neil Conway]

> On July 27, 2016, 11:44 p.m., Vinod Kone wrote:
> > I'm assuming the `Clock::settle()` is no longer necessary because `AWAIT_*` 
> > does Clock::settle() implicitly? Can you add the reason in the description 
> > for posterity?
> > 
> > Also, I'm guessing you ran these tests in a loop to check for flakiness?

Re: `Clock::settle()`, yeah -- `AWAIT_*` sorta does an implicit 
`Clock::settle`. In other words, `Clock::settle` waits for all pending events 
to be handled, `AWAIT_READY` waits for a single, specific future to be ready. I 
updated the commit comment.

Yep, I ran these tests in a loop to check for flakiness.


- Neil


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


On July 25, 2016, 9:35 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50417/
> ---
> 
> (Updated July 25, 2016, 9:35 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Remove unnecessary `Clock::settle` calls, whitespace fixes.
> 
> 
> Diffs
> -
> 
>   src/tests/partition_tests.cpp 91969e4c3196a4f36c19abf38e229f3a36e87ea1 
>   src/tests/slave_recovery_tests.cpp 470fb26a2985f912b2b91d647cd7a27b8748c2a5 
>   src/tests/slave_tests.cpp b9fa85dc1ae0922a100786fcb01156b90a013d2a 
> 
> Diff: https://reviews.apache.org/r/50417/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 50553: Used `subtract` instead of `-=` for `Resources::contains`.

[Guangya Liu]

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

(Updated 七月 28, 2016, 9:30 a.m.)


Review request for mesos, Benjamin Mahler, Joris Van Remoortere, and Klaus Ma.


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


Repository: mesos


Description
---

The current logic of `Resources::contains` is using `-=` to remove
the contained resource object and the `-=` resource object will
invoke `validate` which is not necessary as all of the resource
objects are valid.

The fix is using `subtract` instead of `-=` for `Resources::contains`
, this can make sure there is no validation and can improve
performance of `Resources::contains` for resources object.


Diffs
-

  src/common/resources.cpp 3dbff24d6859d3b1ed8589cec50170a5202cfbcb 
  src/v1/resources.cpp 3c85dc8aa8125962b44e60806ece83a7653d0dc7 

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


Testing (updated)
---

make
make check

The performance for `ports` resources of `total.contains(r)` was improved about 
`9s` with 1000 operations.

Before fix with validation.
```
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2
Took 2.705859secs to perform 1000 'total += r' operations on ports(*):[1-2, 
4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 30.612245secs to perform 1000 'total.contains(r)' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 3.532576secs to perform 1000 'total -= r' operations on ports(*):[1-2, 
4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 2.902233secs to perform 1000 'total = total + r' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 3.608806secs to perform 1000 'total = total - r' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2 (43364 ms)
[--] 3 tests from ResourcesOperators/Resources_BENCHMARK_Test (193001 
ms total)
```

After fix without validation.
```
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2
Took 2.899972secs to perform 1000 'total += r' operations on ports(*):[1-2, 
4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 21.577654secs to perform 1000 'total.contains(r)' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 3.580736secs to perform 1000 'total -= r' operations on ports(*):[1-2, 
4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 3.028626secs to perform 1000 'total = total + r' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 3.699126secs to perform 1000 'total = total - r' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2 (34789 ms)
```


Thanks,

Guangya Liu

Re: Review Request 50557: Used `add` instead of `+=` for `Resources::filter`.

[Guangya Liu]

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

(Updated 七月 28, 2016, 9:29 a.m.)


Review request for mesos, Benjamin Mahler, Joris Van Remoortere, and Klaus Ma.


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


Repository: mesos


Description
---

The current logic of `Resources::filter` is using `+=` to add
the filtered resource object and the `+=` resource object will
invoke `validate` which is not necessary as all of the resource
objects are valid.

The fix is using `add` instead of `+=` for `Resources::filter`
, this can make sure there is no validation and can improve
performance of `Resources::filter` for resources object.


Diffs
-

  src/common/resources.cpp 3dbff24d6859d3b1ed8589cec50170a5202cfbcb 
  src/v1/resources.cpp 3c85dc8aa8125962b44e60806ece83a7653d0dc7 

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


Testing (updated)
---

make
make check

The performance for `ports` resources of `r.nonRevocable()` was improved about 
`8s` with 1000 operations.

Before fix with validation.
```
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2
Took 2.832429secs to perform 1000 'total += r' operations on ports(*):[1-2, 
4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 22.034252secs to perform 1000 'total.contains(r)' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 3.628885secs to perform 1000 'total -= r' operations on ports(*):[1-2, 
4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 3.113548secs to perform 1000 'total = total + r' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 3.854608secs to perform 1000 'total = total - r' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 8.517941secs to perform 1000 'r.nonRevocable()' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2 (43985 ms)
```

After fix without validation.
```
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2
Took 2.834335secs to perform 1000 'total += r' operations on ports(*):[1-2, 
4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 21.828846secs to perform 1000 'total.contains(r)' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 3.689858secs to perform 1000 'total -= r' operations on ports(*):[1-2, 
4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 2.962259secs to perform 1000 'total = total + r' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 3.760281secs to perform 1000 'total = total - r' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 125159us to perform 1000 'r.nonRevocable()' operations on ports(*):[1-2, 
4-5, 7-8, 10-11, 13-14, 16-17, 1...
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2 (35204 ms)
```


Thanks,

Guangya Liu

Re: Review Request 50552: Fixed a typo in cgroups.hpp.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [50552]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On July 28, 2016, 6:58 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50552/
> ---
> 
> (Updated July 28, 2016, 6:58 a.m.)
> 
> 
> Review request for mesos, haosdent huang and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a typo in cgroups.hpp.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> c57baec88437f68886702a40ec8a6a6458546119 
> 
> Diff: https://reviews.apache.org/r/50552/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 50235: Added more expectations to TASK_LOST test cases.

[Neil Conway]

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

(Updated July 28, 2016, 9:06 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Replaced more ASSERTs with EXPECTs per RR comments.


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


Repository: mesos


Description (updated)
---

Check the reason and source of TASK_LOST status updates, replaced
ASSERT_ with EXPECT_ in various places where EXPECT_ is more
appropriate.


Diffs (updated)
-

  src/tests/fault_tolerance_tests.cpp 79f36a5a855ea1795be83840479e71f0e417298b 
  src/tests/master_authorization_tests.cpp 
e43b264b9f67d4cd965aee143cc42a1034ac9952 
  src/tests/master_maintenance_tests.cpp 
2d4b45f78a234118df9e58a69f5587dd4d37956c 
  src/tests/master_slave_reconciliation_tests.cpp 
fdb9bb651c682fb302bb2a533d2dda9bf090839a 
  src/tests/master_tests.cpp 6709818d599c068c289bcb714446018577082d8b 
  src/tests/partition_tests.cpp 91969e4c3196a4f36c19abf38e229f3a36e87ea1 
  src/tests/slave_recovery_tests.cpp 470fb26a2985f912b2b91d647cd7a27b8748c2a5 

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


Testing (updated)
---

make check


Thanks,

Neil Conway

Review Request 50557: Used `add` instead of `+=` for `Resources::filter`.

[Guangya Liu]

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

Review request for mesos, Benjamin Mahler, Joris Van Remoortere, and Klaus Ma.


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


Repository: mesos


Description
---

The current logic of `Resources::filter` is using `+=` to add
the filtered resource object and the `+=` resource object will
invoke `validate` which is not necessary as all of the resource
objects are valid.

The fix is using `add` instead of `+=` for `Resources::filter`
, this can make sure there is no validation and can improve
performance of `Resources::filter` for resources object.


Diffs
-

  src/common/resources.cpp 3dbff24d6859d3b1ed8589cec50170a5202cfbcb 
  src/v1/resources.cpp 3c85dc8aa8125962b44e60806ece83a7653d0dc7 

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


Testing
---

make
make check

The performance for `ports` resources of `r.nonRevocable()` was improved about 
`8s` with 5000 operations.

Before fix with validation.
```
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2
Took 2.832429secs to perform 1000 'total += r' operations on ports(*):[1-2, 
4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 22.034252secs to perform 1000 'total.contains(r)' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 3.628885secs to perform 1000 'total -= r' operations on ports(*):[1-2, 
4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 3.113548secs to perform 1000 'total = total + r' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 3.854608secs to perform 1000 'total = total - r' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 8.517941secs to perform 1000 'r.nonRevocable()' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2 (43985 ms)
```

After fix without validation.
```
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2
Took 2.834335secs to perform 1000 'total += r' operations on ports(*):[1-2, 
4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 21.828846secs to perform 1000 'total.contains(r)' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 3.689858secs to perform 1000 'total -= r' operations on ports(*):[1-2, 
4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 2.962259secs to perform 1000 'total = total + r' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 3.760281secs to perform 1000 'total = total - r' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 125159us to perform 1000 'r.nonRevocable()' operations on ports(*):[1-2, 
4-5, 7-8, 10-11, 13-14, 16-17, 1...
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2 (35204 ms)
```


Thanks,

Guangya Liu

Re: Review Request 50553: Used `subtract` instead of `-=` for `Resources::contains`.

[Guangya Liu]

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

(Updated 七月 28, 2016, 8:51 a.m.)


Review request for mesos, Benjamin Mahler, Joris Van Remoortere, and Klaus Ma.


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


Repository: mesos


Description
---

The current logic of `Resources::contains` is using `-=` to remove
the contained resource object and the `-=` resource object will
invoke `validate` which is not necessary as all of the resource
objects are valid.

The fix is using `subtract` instead of `-=` for `Resources::contains`
, this can make sure there is no validation and can improve
performance of `Resources::contains` for resources object.


Diffs
-

  src/common/resources.cpp 3dbff24d6859d3b1ed8589cec50170a5202cfbcb 
  src/v1/resources.cpp 3c85dc8aa8125962b44e60806ece83a7653d0dc7 

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


Testing (updated)
---

make
make check

The performance for `ports` resources of `total.contains(r)` was improved about 
`9s` with 5000 operations.

Before fix with validation.
```
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2
Took 2.705859secs to perform 1000 'total += r' operations on ports(*):[1-2, 
4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 30.612245secs to perform 1000 'total.contains(r)' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 3.532576secs to perform 1000 'total -= r' operations on ports(*):[1-2, 
4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 2.902233secs to perform 1000 'total = total + r' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 3.608806secs to perform 1000 'total = total - r' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2 (43364 ms)
[--] 3 tests from ResourcesOperators/Resources_BENCHMARK_Test (193001 
ms total)
```

After fix without validation.
```
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2
Took 2.899972secs to perform 1000 'total += r' operations on ports(*):[1-2, 
4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 21.577654secs to perform 1000 'total.contains(r)' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 3.580736secs to perform 1000 'total -= r' operations on ports(*):[1-2, 
4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 3.028626secs to perform 1000 'total = total + r' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 3.699126secs to perform 1000 'total = total - r' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2 (34789 ms)
```


Thanks,

Guangya Liu

Review Request 50556: Added benchmark test for `Resources::nonRevocable`.

[Guangya Liu]

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

Review request for mesos, Benjamin Mahler, Joris Van Remoortere, and Klaus Ma.


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


Repository: mesos


Description
---

Added benchmark test for `Resources::nonRevocable`.


Diffs
-

  src/tests/resources_tests.cpp 4111e080b84079e100b731c9a56861b204f17388 

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


Testing
---

make
make check

```
[==] Running 3 tests from 1 test case.
[--] Global test environment set-up.
[--] 3 tests from ResourcesOperators/Resources_BENCHMARK_Test
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/0
Took 271717us to perform 5 'total += r' operations on cpus(*):1; gpus(*):1; 
mem(*):128; disk(*):256
Took 1.303957secs to perform 5 'total.contains(r)' operations on cpus(*):1; 
gpus(*):1; mem(*):128; disk(*):256
Took 526481us to perform 5 'total -= r' operations on cpus(*):1; gpus(*):1; 
mem(*):128; disk(*):256
Took 968073us to perform 5 'total = total + r' operations on cpus(*):1; 
gpus(*):1; mem(*):128; disk(*):256
Took 1.270586secs to perform 5 'total = total - r' operations on cpus(*):1; 
gpus(*):1; mem(*):128; disk(*):256
Took 775512us to perform 5 'r.nonRevocable()' operations on cpus(*):1; 
gpus(*):1; mem(*):128; disk(*):256
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/0 (5118 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/1
Took 23.026846secs to perform 10 'total += r' operations on cpus(0, 
principal_0, {key_0: value_0}):1; gpus(...
Took 49.498008secs to perform 10 'total.contains(r)' operations on cpus(0, 
principal_0, {key_0: value_0}):1; gpus(...
Took 24.343378secs to perform 10 'total -= r' operations on cpus(0, 
principal_0, {key_0: value_0}):1; gpus(...
Took 24.444537secs to perform 10 'total = total + r' operations on cpus(0, 
principal_0, {key_0: value_0}):1; gpus(...
Took 24.803728secs to perform 10 'total = total - r' operations on cpus(0, 
principal_0, {key_0: value_0}):1; gpus(...
Took 23.992406secs to perform 10 'r.nonRevocable()' operations on cpus(0, 
principal_0, {key_0: value_0}):1; gpus(...
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/1 (170300 
ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2
Took 2.832429secs to perform 1000 'total += r' operations on ports(*):[1-2, 
4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 22.034252secs to perform 1000 'total.contains(r)' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 3.628885secs to perform 1000 'total -= r' operations on ports(*):[1-2, 
4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 3.113548secs to perform 1000 'total = total + r' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 3.854608secs to perform 1000 'total = total - r' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 8.517941secs to perform 1000 'r.nonRevocable()' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2 (43985 ms)
[--] 3 tests from ResourcesOperators/Resources_BENCHMARK_Test (219403 
ms total)

[--] Global test environment tear-down
[==] 3 tests from 1 test case ran. (219420 ms total)
[  PASSED  ] 3 tests.
```


Thanks,

Guangya Liu

Re: Review Request 49851: Implemented `MemorySubsystem`.

[Qian Zhang]

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




src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (lines 532 - 556)


I see you have changed the implementation of `usage()` and `_usage()` which 
are actually slightly different from the counterparts in the existing cgroups 
mem isolator, can you please elaborate why? And can we just keep consistent 
with cgroups mem isolator?


- Qian Zhang


On July 27, 2016, 1:13 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49851/
> ---
> 
> (Updated July 27, 2016, 1:13 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Bugs: MESOS-5045
> https://issues.apache.org/jira/browse/MESOS-5045
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented `MemorySubsystem`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> c57baec88437f68886702a40ec8a6a6458546119 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 348f105f9c3109a02f1dde0649f1b829cb9ddd04 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
> 
> Diff: https://reviews.apache.org/r/49851/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 50398: Added libz to Ubuntu build dependencies.

[Tomasz Janiszewski]

> On July 28, 2016, 12:01 a.m., Vinod Kone wrote:
> > docs/getting-started.md, line 50
> > 
> >
> > what's libz needed for? we don't seem to install it in our docker 
> > builds? https://github.com/apache/mesos/blob/master/support/docker_build.sh

It looks like zlib is installed indirectly with other dependecies 
`libsvn-dev`,` libcurl4-nss-dev`. I double checkd it and `./configure` errors 
must be my fault.


- Tomasz


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


On July 25, 2016, 5:39 p.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50398/
> ---
> 
> (Updated July 25, 2016, 5:39 p.m.)
> 
> 
> Review request for mesos, Dave Lester, Kevin Klues, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added libz to Ubuntu build dependencies.
> 
> 
> Diffs
> -
> 
>   docs/getting-started.md ebe52705b8c8757d4a507ce3ae75f56d535a39d1 
> 
> Diff: https://reviews.apache.org/r/50398/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Review Request 50553: Used `subtract` instead of `-=` for `Resources::contains`.

[Guangya Liu]

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

Review request for mesos, Benjamin Mahler, Joris Van Remoortere, and Klaus Ma.


Repository: mesos


Description
---

The current logic of `Resources::contains` is using `-=` to remove
the contained resource object and the `-=` resource object will
invoke `validate` which is not necessary as all of the resource
objects are valid.

The fix is using `subtract` instead of `-=` for `Resources::contains`
, this can make sure there is no validation and can improve
performance of `Resources::contains` for resources object.


Diffs
-

  src/common/resources.cpp 3dbff24d6859d3b1ed8589cec50170a5202cfbcb 
  src/v1/resources.cpp 3c85dc8aa8125962b44e60806ece83a7653d0dc7 

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


Testing
---

make
make check

The performance for `ports` resources was improved about `9s` with 5000 
operations.

Before fix with validation.
```
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2
Took 2.705859secs to perform 1000 'total += r' operations on ports(*):[1-2, 
4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 30.612245secs to perform 1000 'total.contains(r)' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 3.532576secs to perform 1000 'total -= r' operations on ports(*):[1-2, 
4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 2.902233secs to perform 1000 'total = total + r' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 3.608806secs to perform 1000 'total = total - r' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2 (43364 ms)
[--] 3 tests from ResourcesOperators/Resources_BENCHMARK_Test (193001 
ms total)
```

After fix without validation.
```
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2
Took 2.899972secs to perform 1000 'total += r' operations on ports(*):[1-2, 
4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 21.577654secs to perform 1000 'total.contains(r)' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 3.580736secs to perform 1000 'total -= r' operations on ports(*):[1-2, 
4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 3.028626secs to perform 1000 'total = total + r' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 3.699126secs to perform 1000 'total = total - r' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2 (34789 ms)
```


Thanks,

Guangya Liu

Re: Review Request 50552: Fixed a typo in cgroups.hpp.

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On July 28, 2016, 6:58 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50552/
> ---
> 
> (Updated July 28, 2016, 6:58 a.m.)
> 
> 
> Review request for mesos, haosdent huang and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a typo in cgroups.hpp.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> c57baec88437f68886702a40ec8a6a6458546119 
> 
> Diff: https://reviews.apache.org/r/50552/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 50499: Added logic in master/main.cpp to use log network module.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [50490, 50491, 50492, 50493, 50494, 50495, 50496, 50497, 
50498, 50499]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On July 28, 2016, 5:28 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50499/
> ---
> 
> (Updated July 28, 2016, 5:28 a.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-5829
> https://issues.apache.org/jira/browse/MESOS-5829
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added logic in master/main.cpp to use log network module.
> 
> 
> Diffs
> -
> 
>   src/master/main.cpp e202973e92ee065b93c0b431cae0bc066cbd7dc7 
> 
> Diff: https://reviews.apache.org/r/50499/diff/
> 
> 
> Testing
> ---
> 
> make check on Ubuntu 14.04
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 50552: Fixed a typo in cgroups.hpp.

[haosdent huang]

> On July 28, 2016, 7 a.m., haosdent huang wrote:
> > Ship It!

Thank you for correting the typo!


- haosdent


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


On July 28, 2016, 6:58 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50552/
> ---
> 
> (Updated July 28, 2016, 6:58 a.m.)
> 
> 
> Review request for mesos, haosdent huang and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a typo in cgroups.hpp.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> c57baec88437f68886702a40ec8a6a6458546119 
> 
> Diff: https://reviews.apache.org/r/50552/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 49814: Implemented `CgroupsIsolatorProcess::create`.

[haosdent huang]

> On July 24, 2016, 5:50 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp, lines 78-84
> > 
> >
> > I just realize that the old `cgroups/cpu` isolator actually handles two 
> > subsystems: cpu and cpuacct. Therefore, I suggest we use the following 
> > logic:
> > ```
> > // Multipmap: isolator name -> subsystem name.
> > multihashmap table = {
> >   {"cpu", "cpu"},
> >   {"cpu", "cpuacct"},
> >   {"mem", "memory"},
> >   ...
> > };
> > 
> > foreach (const string& _isolator?...) {
> >   if (!strings::startsWith(_isolator, "cgroups/")) {
> > continue;
> >   }
> >   
> >   string isolator = strings::remove(
> >   isolator,
> >   "cgroups/",
> >   strings::mode::PREFIX);
> >   
> >   foreach (const string& subsystemName, table.get(isolator)) {
> > if (hierarchies.contains(subsystemName)) {
> >   continue;
> > }
> > 
> > Try hierarchy = cgroups::prepare(
> > flags.cgroups_hierarchy,
> > subsystemName,
> > flags.cgroups_root);
> > 
> > ...
> >   }
> > }
> > ```
> 
> haosdent huang wrote:
> Do we ask user to pass `cgroups/cpuacct` exactly in `isolation` flag 
> eventually?
> 
> Qian Zhang wrote:
> With the logic suggested by Jie, I think user does not need to specify 
> `cgroups/cpuacct` in `--isolation` flag, just `cgroups/cpu` should be enough.

Yes, have removed `cgroups/cpuacct` in `--isolation` flag.


- haosdent


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


On July 25, 2016, 2:35 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49814/
> ---
> 
> (Updated July 25, 2016, 2:35 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Bugs: MESOS-5041
> https://issues.apache.org/jira/browse/MESOS-5041
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented `CgroupsIsolatorProcess::create`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 4a9f55bf3b217405bf90943f27a976422877a99e 
> 
> Diff: https://reviews.apache.org/r/49814/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49814: Implemented `CgroupsIsolatorProcess::create`.

[Qian Zhang]

> On July 25, 2016, 1:50 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp, lines 78-84
> > 
> >
> > I just realize that the old `cgroups/cpu` isolator actually handles two 
> > subsystems: cpu and cpuacct. Therefore, I suggest we use the following 
> > logic:
> > ```
> > // Multipmap: isolator name -> subsystem name.
> > multihashmap table = {
> >   {"cpu", "cpu"},
> >   {"cpu", "cpuacct"},
> >   {"mem", "memory"},
> >   ...
> > };
> > 
> > foreach (const string& _isolator?...) {
> >   if (!strings::startsWith(_isolator, "cgroups/")) {
> > continue;
> >   }
> >   
> >   string isolator = strings::remove(
> >   isolator,
> >   "cgroups/",
> >   strings::mode::PREFIX);
> >   
> >   foreach (const string& subsystemName, table.get(isolator)) {
> > if (hierarchies.contains(subsystemName)) {
> >   continue;
> > }
> > 
> > Try hierarchy = cgroups::prepare(
> > flags.cgroups_hierarchy,
> > subsystemName,
> > flags.cgroups_root);
> > 
> > ...
> >   }
> > }
> > ```
> 
> haosdent huang wrote:
> Do we ask user to pass `cgroups/cpuacct` exactly in `isolation` flag 
> eventually?

With the logic suggested by Jie, I think user does not need to specify 
`cgroups/cpuacct` in `--isolation` flag, just `cgroups/cpu` should be enough.


- Qian


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


On July 25, 2016, 10:35 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49814/
> ---
> 
> (Updated July 25, 2016, 10:35 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Bugs: MESOS-5041
> https://issues.apache.org/jira/browse/MESOS-5041
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented `CgroupsIsolatorProcess::create`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 4a9f55bf3b217405bf90943f27a976422877a99e 
> 
> Diff: https://reviews.apache.org/r/49814/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 50552: Fixed a typo in cgroups.hpp.

[Qian Zhang]

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

Review request for mesos, haosdent huang and Jie Yu.


Repository: mesos


Description
---

Fixed a typo in cgroups.hpp.


Diffs
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
c57baec88437f68886702a40ec8a6a6458546119 

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


Testing
---


Thanks,

Qian Zhang

Review Request 50551: Added benchmark test for `Resources::contains`.

[Guangya Liu]

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

Review request for mesos, Benjamin Mahler and Klaus Ma.


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


Repository: mesos


Description
---

Added benchmark test for `Resources::contains`.


Diffs
-

  src/tests/resources_tests.cpp 4111e080b84079e100b731c9a56861b204f17388 

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


Testing
---

make
make check

```
./bin/mesos-tests.sh --benchmark  
--gtest_filter="ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/*"
[==] Running 3 tests from 1 test case.
[--] Global test environment set-up.
[--] 3 tests from ResourcesOperators/Resources_BENCHMARK_Test
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/0
Took 276934us to perform 5 'total += r' operations on cpus(*):1; gpus(*):1; 
mem(*):128; disk(*):256
Took 1.395498secs to perform 5 'total.contains(r)' operations on cpus(*):1; 
gpus(*):1; mem(*):128; disk(*):256
Took 479028us to perform 5 'total -= r' operations on cpus(*):1; gpus(*):1; 
mem(*):128; disk(*):256
Took 951110us to perform 5 'total = total + r' operations on cpus(*):1; 
gpus(*):1; mem(*):128; disk(*):256
Took 1.307601secs to perform 5 'total = total - r' operations on cpus(*):1; 
gpus(*):1; mem(*):128; disk(*):256
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/0 (4413 ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/1
Took 23.67492secs to perform 10 'total += r' operations on cpus(0, principal_0, 
{key_0: value_0}):1; gpus(...
Took 49.557323secs to perform 10 'total.contains(r)' operations on cpus(0, 
principal_0, {key_0: value_0}):1; gpus(...
Took 23.956811secs to perform 10 'total -= r' operations on cpus(0, 
principal_0, {key_0: value_0}):1; gpus(...
Took 23.374198secs to perform 10 'total = total + r' operations on cpus(0, 
principal_0, {key_0: value_0}):1; gpus(...
Took 24.492203secs to perform 10 'total = total - r' operations on cpus(0, 
principal_0, {key_0: value_0}):1; gpus(...
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/1 (145223 
ms)
[ RUN  ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2
Took 2.705859secs to perform 1000 'total += r' operations on ports(*):[1-2, 
4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 30.612245secs to perform 1000 'total.contains(r)' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 3.532576secs to perform 1000 'total -= r' operations on ports(*):[1-2, 
4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 2.902233secs to perform 1000 'total = total + r' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
Took 3.608806secs to perform 1000 'total = total - r' operations on 
ports(*):[1-2, 4-5, 7-8, 10-11, 13-14, 16-17, 1...
[   OK ] ResourcesOperators/Resources_BENCHMARK_Test.Arithmetic/2 (43364 ms)
[--] 3 tests from ResourcesOperators/Resources_BENCHMARK_Test (193001 
ms total)

[--] Global test environment tear-down
[==] 3 tests from 1 test case ran. (193024 ms total)
[  PASSED  ] 3 tests.
```


Thanks,

Guangya Liu

Re: Review Request 49849: Implemented `CpuSubsystem`.

[Qian Zhang]

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




src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp (line 39)


I see there is a file 
(https://github.com/apache/mesos/blob/1.0.0/src/slave/containerizer/mesos/isolators/cgroups/constants.hpp)
 to keep all the cgroups-related constants, you may put this constant there as 
well.


- Qian Zhang


On July 27, 2016, 1:12 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49849/
> ---
> 
> (Updated July 27, 2016, 1:12 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Bugs: MESOS-5042
> https://issues.apache.org/jira/browse/MESOS-5042
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented `CpuSubsystem`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 348f105f9c3109a02f1dde0649f1b829cb9ddd04 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
> 
> Diff: https://reviews.apache.org/r/49849/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>