Re: Review Request 50181: Fixed the flaky test case `MasterAPITest.GetTasks`.

2016-07-24 Thread Mesos ReviewBot 

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



Patch looks great!

Reviews applied: [50181]

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 25, 2016, 1:41 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50181/
> ---
> 
> (Updated July 25, 2016, 1:41 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Jay Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5860
> https://issues.apache.org/jira/browse/MESOS-5860
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This changes to wait for every `StatusUpdateAcknowledgementMessage`
> exactly in this test case to make sure they don't conflict each other.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 88523fdb3469c40b7ff71d5d753b645d20bc51bd 
> 
> Diff: https://reviews.apache.org/r/50181/diff/
> 
> 
> Testing
> ---
> 
> Before apply the patch, could reproduce by
> 
> ```
> $ stress --cpu 4 --timeout 3600s &
> $ GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*MasterAPITest.GetTasks*" 
> --gtest_break_on_failure --gtest_repeat=-1 --verbose
> ```
> 
> After apply the patch, could not reproduce by
> 
> ```
> $ stress --cpu 4 --timeout 3600s &
> $ GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*MasterAPITest.GetTasks*" 
> --gtest_break_on_failure --gtest_repeat=-1 --verbose
> ```
> 
> So verified this patch resolve the flaky.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 50246: Modified 'master' to accecpt request with charset in Content-Type.

2016-07-24 Thread zhou xing 

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



is that possible to add one more test case to send a request with 
"application/json; charset=UTF-8" content-type to our operator API?

- zhou xing


On July 20, 2016, 6:11 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50246/
> ---
> 
> (Updated July 20, 2016, 6:11 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, zhou xing, Jay Guo, and Vinod Kone.
> 
> 
> Bugs: MESOS-5858
> https://issues.apache.org/jira/browse/MESOS-5858
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Earlier operator v1 api was not supporting charset utf-8
> specified in 'Content Type'. In this patch, mesos master
> is modified to support contentType with and without
> charset utf-8 specified.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 1cbec976ccedae81def388c62889624aaf9a794e 
>   src/master/master.hpp 6decff6f4b9c3434de030fd5c06df4c683a7abad 
> 
> Diff: https://reviews.apache.org/r/50246/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 16.04:
> sudo GTEST_FILTER="*MasterAPITest.*" make -j4 check
> 
> Manual Testing:
> 1. sudo  ./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/var/lib/mesos
> 2. Ran Poster application in firefox and sent a POST data for GET_VERSION 
> call to http://127.0.0.1:5050/api/v1 with contentType as application/json; 
> charset=utf-8
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

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

2016-07-24 Thread Mesos ReviewBot 

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



Patch looks great!

Reviews applied: [50374, 50382]

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 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 filtered out 
> 18000 offers
> round 18 allocate() took 769331us to make 1000 offers after filtered out 
> 19000 offers
> round 19 allocate() took 769225us to make 1000 offers after filtered out 
> 2 offers
> round 20 allocate() took 780541us to make 1000 offers after filtered out 
> 21000 offers
> ...
> ```
> ```
> ./bin/mesos-tests.sh --benchmark  
>

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

2016-07-24 Thread haosdent huang 

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

(Updated July 25, 2016, 2:21 a.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Address @jieyu's comments.


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


Repository: mesos


Description
---

Implemented `CgroupsIsolatorProcess::create`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 50181: Fixed the flaky test case `MasterAPITest.GetTasks`.

2016-07-24 Thread haosdent huang 

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

(Updated July 25, 2016, 1:41 a.m.)


Review request for mesos, Anand Mazumdar, Jay Guo, and Vinod Kone.


Changes
---

Add `Testing Done`.


Summary (updated)
-

Fixed the flaky test case `MasterAPITest.GetTasks`.


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


Repository: mesos


Description
---

This changes to wait for every `StatusUpdateAcknowledgementMessage`
exactly in this test case to make sure they don't conflict each other.


Diffs (updated)
-

  src/tests/api_tests.cpp 88523fdb3469c40b7ff71d5d753b645d20bc51bd 

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


Testing (updated)
---

Before apply the patch, could reproduce by

```
$ stress --cpu 4 --timeout 3600s &
$ GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*MasterAPITest.GetTasks*" 
--gtest_break_on_failure --gtest_repeat=-1 --verbose
```

After apply the patch, could not reproduce by

```
$ stress --cpu 4 --timeout 3600s &
$ GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*MasterAPITest.GetTasks*" 
--gtest_break_on_failure --gtest_repeat=-1 --verbose
```

So verified this patch resolve the flaky.


Thanks,

haosdent huang

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

2016-07-24 Thread Guangya Liu 

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

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 filtered out 18000 
offers
round 18 allocate() took 769331us to make 1000 offers after filtered out 19000 
offers
round 19 allocate() took 769225us to make 1000 offers after filtered out 2 
offers
round 20 allocate() took 780541us to make 1000 offers after filtered out 21000 
offers
...
```
```
./bin/mesos-tests.sh --benchmark  
--gtest_filter="SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.ResourceLabels/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.ResourceLabels/0
Using 1000 agents and 1 frameworks
Added 1 frameworks in 760us
Added 1000 agents in 566673us
round 0 allocate() took 254148us to make 0 offers after filtered out 1000 offers
round 1 allocate() took 254993us to make 0 offers after filtered out 1000 offers
[   OK ] 
SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.ResourceLabels/0 
(1825 ms)
[--] 1 test from 
SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test (1825 ms total)

[--] Global test environment tear-down
[==] 1

Re: Review Request 50339: Fixed comment typos and whitespace infelicities.

2016-07-24 Thread Alexander Rojas 

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


Ship it!




Ship It!

- Alexander Rojas


On July 22, 2016, 3:35 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50339/
> ---
> 
> (Updated July 22, 2016, 3:35 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Vinod Kone.
> 
> 
> Bugs: MESOS-5888
> https://issues.apache.org/jira/browse/MESOS-5888
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed comment typos and whitespace infelicities.
> 
> 
> Diffs
> -
> 
>   src/tests/authorization_tests.cpp 38885caa664d1f495a8dbf369d6e2bf9e8fdb0e1 
>   src/tests/slave_authorization_tests.cpp 
> f76ed3a4c77ba4109d1b9e2083bd3dfbfef41b43 
> 
> Diff: https://reviews.apache.org/r/50339/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 50338: Fixed flakiness in SlaveAuthorizerTest.ViewFlags.

2016-07-24 Thread Alexander Rojas 

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



Looks much more simpler than what we discussed with the guys in the office 
(which is the reason I didn't propose a patch yet). The error occurs in Linux 
only so I will try it and then give it a ship it if it solves the issue.

- Alexander Rojas


On July 22, 2016, 3:35 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50338/
> ---
> 
> (Updated July 22, 2016, 3:35 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Vinod Kone.
> 
> 
> Bugs: MESOS-5888
> https://issues.apache.org/jira/browse/MESOS-5888
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed flakiness in SlaveAuthorizerTest.ViewFlags.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_authorization_tests.cpp 
> f76ed3a4c77ba4109d1b9e2083bd3dfbfef41b43 
> 
> Diff: https://reviews.apache.org/r/50338/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Validated that with this change, slave recovery always finishes before any of 
> the subsequent HTTP requests are sent. Note that I couldn't actually trigger 
> the test case failure on my machine.
> 
> If there's a more precise way to have the test case wait for the slave to 
> finish recovery, let me know.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

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

2016-07-24 Thread 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);
> > 
> > ...
> >   }
> > }
> > ```

Do we ask user to pass `cgroups/cpuacct` exactly in `isolation` flag eventually?


- haosdent


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


On July 24, 2016, 12:52 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49814/
> ---
> 
> (Updated July 24, 2016, 12:52 p.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`.

2016-07-24 Thread Jie Yu 

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




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);

...
  }
}
```


- Jie Yu


On July 24, 2016, 12:52 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49814/
> ---
> 
> (Updated July 24, 2016, 12:52 p.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 49828: Added default methods implementations for `Subsystem` base class.

2016-07-24 Thread haosdent huang 

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

(Updated July 24, 2016, 3:16 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Address @qianzhang's comments.


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


Repository: mesos


Description
---

Added default methods implementations for `Subsystem` base class.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49827: Implemented `CgroupsIsolatorProcess::cleanup`.

2016-07-24 Thread haosdent huang 

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

(Updated July 24, 2016, 3:13 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Address @qianzhang's comments.


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


Repository: mesos


Description
---

Implemented `CgroupsIsolatorProcess::cleanup`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
c57baec88437f68886702a40ec8a6a6458546119 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49825: Implemented `CgroupsIsolatorProcess::status`.

2016-07-24 Thread haosdent huang 

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

(Updated July 24, 2016, 2:39 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Address @qianzhang's comments.


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


Repository: mesos


Description
---

Implemented `CgroupsIsolatorProcess::status`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49828: Added default methods implementations for `Subsystem` base class.

2016-07-24 Thread Qian Zhang 

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




src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (lines 87 - 88)


I would suggest to merge these two line into a single line `return 
ResourceStatistics();`.



src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp (lines 94 - 95)


Ditto.


- Qian Zhang


On July 22, 2016, 3:07 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49828/
> ---
> 
> (Updated July 22, 2016, 3:07 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Bugs: MESOS-5039
> https://issues.apache.org/jira/browse/MESOS-5039
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added default methods implementations for `Subsystem` base class.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 
> 
> Diff: https://reviews.apache.org/r/49828/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49824: Implemented `CgroupsIsolatorProcess::usage`.

2016-07-24 Thread haosdent huang 

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

(Updated July 24, 2016, 2:36 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Address @qianzhang's comments.


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


Repository: mesos


Description
---

Implemented `CgroupsIsolatorProcess::usage`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49827: Implemented `CgroupsIsolatorProcess::cleanup`.

2016-07-24 Thread Qian Zhang 

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




src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp (lines 540 - 546)


I do not think we need this `onAny()` call.



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


I do not think you need this parameter.



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


Ditto.



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


Suggest to add a newline before this line.



src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp (lines 570 - 575)


I would suggest to change these code to:
```
  return collect(futures)
.then(defer(PID(this),
::__cleanup,
containerId))
```
And to make the above code work, we also need to change the return value of 
`__cleanup` from `void` to `Future`. You may take a look at the 
following code as a reference:

https://github.com/apache/mesos/blob/1.0.0-rc4/src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp#L627:L634


- Qian Zhang


On July 22, 2016, 3:11 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49827/
> ---
> 
> (Updated July 22, 2016, 3:11 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::cleanup`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> c57baec88437f68886702a40ec8a6a6458546119 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 4a9f55bf3b217405bf90943f27a976422877a99e 
> 
> Diff: https://reviews.apache.org/r/49827/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49825: Implemented `CgroupsIsolatorProcess::status`.

2016-07-24 Thread Qian Zhang 

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


Fix it, then Ship it!





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


To be consistent with the code: 
https://github.com/apache/mesos/blob/1.0.0-rc4/src/slave/containerizer/mesos/containerizer.cpp#L1554
 , I would suggest to do:
`s/Skipping get the status information of container/Skipping status for 
container/`


- Qian Zhang


On July 22, 2016, 3:07 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49825/
> ---
> 
> (Updated July 22, 2016, 3:07 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::status`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 4a9f55bf3b217405bf90943f27a976422877a99e 
> 
> Diff: https://reviews.apache.org/r/49825/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49824: Implemented `CgroupsIsolatorProcess::usage`.

2016-07-24 Thread Qian Zhang 


> On July 24, 2016, 9:05 p.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp, line 475
> > 
> >
> > s/Skipping get/Skipping/

Second thought, to be consistent with the code here: 
https://github.com/apache/mesos/blob/1.0.0-rc4/src/slave/containerizer/mesos/containerizer.cpp#L1496,
 I think we should do:
`s/Skipping get the usage information of container/Skipping resource statistic 
for container/`.


- Qian


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


On July 22, 2016, 3:07 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49824/
> ---
> 
> (Updated July 22, 2016, 3:07 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::usage`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 4a9f55bf3b217405bf90943f27a976422877a99e 
> 
> Diff: https://reviews.apache.org/r/49824/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49824: Implemented `CgroupsIsolatorProcess::usage`.

2016-07-24 Thread Qian Zhang 

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


Fix it, then Ship it!





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


s/Skipping get/Skipping/


- Qian Zhang


On July 22, 2016, 3:07 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49824/
> ---
> 
> (Updated July 22, 2016, 3:07 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::usage`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 4a9f55bf3b217405bf90943f27a976422877a99e 
> 
> Diff: https://reviews.apache.org/r/49824/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 49823: Implemented `CgroupsIsolatorProcess::update`.

2016-07-24 Thread haosdent huang 

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

(Updated July 24, 2016, 12:56 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Address @qianzhang's comments.


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


Repository: mesos


Description
---

Implemented `CgroupsIsolatorProcess::update`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49819: Implemented `CgroupsIsolatorProcess::prepare`.

2016-07-24 Thread haosdent huang 

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

(Updated July 24, 2016, 12:55 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Address @qianzhang's comments.


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


Repository: mesos


Description
---

Implemented `CgroupsIsolatorProcess::prepare`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
c57baec88437f68886702a40ec8a6a6458546119 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49820: Implemented `CgroupsIsolatorProcess::isolate`.

2016-07-24 Thread haosdent huang 

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

(Updated July 24, 2016, 12:55 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Address @qianzhang's comments.


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


Repository: mesos


Description
---

Implemented `CgroupsIsolatorProcess::isolate`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 

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


Testing
---


Thanks,

haosdent huang

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

2016-07-24 Thread haosdent huang 

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

(Updated July 24, 2016, 12:52 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.


Changes
---

Update the comment in code.


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


Repository: mesos


Description
---

Implemented `CgroupsIsolatorProcess::create`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49823: Implemented `CgroupsIsolatorProcess::update`.

2016-07-24 Thread Qian Zhang 

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




src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp (lines 440 - 448)


I would suggest to merge these codes into a single line `return 
collect(updates).then([]() { return Nothing(); });`.


- Qian Zhang


On July 22, 2016, 3:06 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49823/
> ---
> 
> (Updated July 22, 2016, 3:06 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::update`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 4a9f55bf3b217405bf90943f27a976422877a99e 
> 
> Diff: https://reviews.apache.org/r/49823/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 50341: Ensured actor ID references class name in ProtobufProcess<> instances.

2016-07-24 Thread Alexander Rukletsov 

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

(Updated July 24, 2016, 11:16 a.m.)


Review request for mesos and Vinod Kone.


Summary (updated)
-

Ensured actor ID references class name in ProtobufProcess<> instances.


Repository: mesos


Description (updated)
---

To facilitate debugging and improve the output of
__processes__ endpoint, add a distinguishable actor
ID to to types derived from ProtobufProcess<>.


Diffs
-

  src/docker/executor.cpp 0d1fd653c873a2d5e9d0cbaf782aba695234aa8d 
  src/health-check/health_checker.hpp f61e80d8e5eb09f8f71e80b6600f7c5b1548bd62 
  src/launcher/executor.cpp 5a5f95f04a6ce096079b67397cb324575409f795 
  src/log/network.hpp 34843c9abb7e1756a32a19d26a449e22e1faea7e 
  src/slave/status_update_manager.cpp 5caa2aa9e5413b93b3e064fe1b42ced6c01193a5 

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


Testing (updated)
---

make check


Thanks,

Alexander Rukletsov

Re: Review Request 50342: Tweaked LocalAuthorizerProcess actor ID for clarity.

2016-07-24 Thread Alexander Rukletsov 

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

(Updated July 24, 2016, 11:16 a.m.)


Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/authorizer/local/authorizer.cpp f979e83b8a227fb54d5914e22e1d22ce6f7b9ba5 

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


Testing (updated)
---

None: not a functional change.


Thanks,

Alexander Rukletsov

Re: Review Request 50341: Ensured actor ID references class name in StatusUpdateManager.

2016-07-24 Thread Alexander Rukletsov 

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

(Updated July 24, 2016, 11:12 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Squashed similar subsequent commits in the chain.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/docker/executor.cpp 0d1fd653c873a2d5e9d0cbaf782aba695234aa8d 
  src/health-check/health_checker.hpp f61e80d8e5eb09f8f71e80b6600f7c5b1548bd62 
  src/launcher/executor.cpp 5a5f95f04a6ce096079b67397cb324575409f795 
  src/log/network.hpp 34843c9abb7e1756a32a19d26a449e22e1faea7e 
  src/slave/status_update_manager.cpp 5caa2aa9e5413b93b3e064fe1b42ced6c01193a5 

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


Testing
---

See https://reviews.apache.org/r/50346/


Thanks,

Alexander Rukletsov

Re: Review Request 50342: Tweaked LocalAuthorizerProcess actor ID for clarity.

2016-07-24 Thread Alexander Rukletsov 

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

(Updated July 24, 2016, 11:11 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Rebased.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/authorizer/local/authorizer.cpp f979e83b8a227fb54d5914e22e1d22ce6f7b9ba5 

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


Testing
---

See https://reviews.apache.org/r/50346/


Thanks,

Alexander Rukletsov

Re: Review Request 49820: Implemented `CgroupsIsolatorProcess::isolate`.

2016-07-24 Thread Qian Zhang 

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




src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp (lines 399 - 407)


I would suggest to merge these codes into a single line `return 
collect(isolates).then([]() { return Nothing(); });` just like 
https://github.com/apache/mesos/blob/1.0.0-rc4/src/slave/containerizer/mesos/isolators/gpu/isolator.cpp#L241


- Qian Zhang


On July 22, 2016, 3:06 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49820/
> ---
> 
> (Updated July 22, 2016, 3:06 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::isolate`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 4a9f55bf3b217405bf90943f27a976422877a99e 
> 
> Diff: https://reviews.apache.org/r/49820/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 50365: Removed `C` framework support from framework developer guide.

2016-07-24 Thread haosdent huang 

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


Ship it!




Ship It!

- haosdent huang


On July 22, 2016, 10:59 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50365/
> ---
> 
> (Updated July 22, 2016, 10:59 p.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, mesos do not support writting `C` frameworks with the
> scheduler driver, we should remove it from document.
> 
> 
> Diffs
> -
> 
>   docs/app-framework-development-guide.md 
> 6d283db357dcf5a61f99800429cdfe90ccbcd8c3 
> 
> Diff: https://reviews.apache.org/r/50365/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>