Re: Review Request 49938: Updated Master::GET_METRICS call to return metrics grouped by types.

[Abhishek Dasgupta]

> On July 15, 2016, 6:55 a.m., Abhishek Dasgupta wrote:
> > src/master/http.cpp, lines 1917-1928
> > 
> >
> > You may do it this way:
> >  // TODO(tuananh): I don't know why I cannot use foreachpair instead.
> > foreachpair (const MetricType& metricType, const auto& metrics, 
> > metricsMap) {
> >   foreachpair (const string& key, double value, metrics) {
> > switch (metricType) {
> >   case MetricType::COUNTER:
> >   {
> > Metric metric;
> > metric.set_name(key);
> > metric.set_value(value);
> > _getMetrics->add_counters()->CopyFrom(metric);
> > break;
> >   }

Please, notice that I changed blocks inside switch cases a little. Can you do 
likewise for other blocks in this switch case if you address this comment of 
mine?


- Abhishek


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


On July 14, 2016, 9:35 p.m., Tuan-Anh Hoang-Vu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49938/
> ---
> 
> (Updated July 14, 2016, 9:35 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-5731
> https://issues.apache.org/jira/browse/MESOS-5731
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated Master::GET_METRICS call to return metrics grouped by types.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto 7fb7153b5498a55469d2c72873c45fb534c0e60d 
>   include/mesos/v1/master/master.proto 
> c9e4359fff018de55d83da5cde0eb6d59dacfc3e 
>   src/master/http.cpp 42ba3643daf8f6fe63152178214b57e3b773d93e 
>   src/tests/api_tests.cpp 55e825ea6a3bd43c76dc67e8b90a97e8c9530a47 
> 
> Diff: https://reviews.apache.org/r/49938/diff/
> 
> 
> Testing
> ---
> 
> Modified MasterAPITest.GetMetrics to make sure we return metrics grouped by 
> types.
> 
> 
> 1. Run master: ./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/var/lib/mesos
> 2. Call GET_METRICS: curl --header "Content-Type:application/json" --data 
> '{"type": "GET_METRICS", "get_metrics":{}}' http://localhost:5050/api/v1 | 
> python -m json.tool
> 
> 
> Thanks,
> 
> Tuan-Anh Hoang-Vu
> 
>

Re: Review Request 49938: Updated Master::GET_METRICS call to return metrics grouped by types.

[Abhishek Dasgupta]

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




src/master/http.cpp (lines 1917 - 1928)


You may do it this way:
 // TODO(tuananh): I don't know why I cannot use foreachpair instead.
foreachpair (const MetricType& metricType, const auto& metrics, 
metricsMap) {
  foreachpair (const string& key, double value, metrics) {
switch (metricType) {
  case MetricType::COUNTER:
  {
Metric metric;
metric.set_name(key);
metric.set_value(value);
_getMetrics->add_counters()->CopyFrom(metric);
break;
  }


- Abhishek Dasgupta


On July 14, 2016, 9:35 p.m., Tuan-Anh Hoang-Vu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49938/
> ---
> 
> (Updated July 14, 2016, 9:35 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-5731
> https://issues.apache.org/jira/browse/MESOS-5731
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated Master::GET_METRICS call to return metrics grouped by types.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto 7fb7153b5498a55469d2c72873c45fb534c0e60d 
>   include/mesos/v1/master/master.proto 
> c9e4359fff018de55d83da5cde0eb6d59dacfc3e 
>   src/master/http.cpp 42ba3643daf8f6fe63152178214b57e3b773d93e 
>   src/tests/api_tests.cpp 55e825ea6a3bd43c76dc67e8b90a97e8c9530a47 
> 
> Diff: https://reviews.apache.org/r/49938/diff/
> 
> 
> Testing
> ---
> 
> Modified MasterAPITest.GetMetrics to make sure we return metrics grouped by 
> types.
> 
> 
> 1. Run master: ./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/var/lib/mesos
> 2. Call GET_METRICS: curl --header "Content-Type:application/json" --data 
> '{"type": "GET_METRICS", "get_metrics":{}}' http://localhost:5050/api/v1 | 
> python -m json.tool
> 
> 
> Thanks,
> 
> Tuan-Anh Hoang-Vu
> 
>

Re: Review Request 49938: Updated Master::GET_METRICS call to return metrics grouped by types.

[Abhishek Dasgupta]

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




src/tests/api_tests.cpp (line 335)


Can you be specific here? eg., // Verifies that the response for `COUNTER` 
type metrics is not empty. Likewise, in the following ASSERT_LE tests.


- Abhishek Dasgupta


On July 14, 2016, 9:35 p.m., Tuan-Anh Hoang-Vu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49938/
> ---
> 
> (Updated July 14, 2016, 9:35 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-5731
> https://issues.apache.org/jira/browse/MESOS-5731
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated Master::GET_METRICS call to return metrics grouped by types.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto 7fb7153b5498a55469d2c72873c45fb534c0e60d 
>   include/mesos/v1/master/master.proto 
> c9e4359fff018de55d83da5cde0eb6d59dacfc3e 
>   src/master/http.cpp 42ba3643daf8f6fe63152178214b57e3b773d93e 
>   src/tests/api_tests.cpp 55e825ea6a3bd43c76dc67e8b90a97e8c9530a47 
> 
> Diff: https://reviews.apache.org/r/49938/diff/
> 
> 
> Testing
> ---
> 
> Modified MasterAPITest.GetMetrics to make sure we return metrics grouped by 
> types.
> 
> 
> 1. Run master: ./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/var/lib/mesos
> 2. Call GET_METRICS: curl --header "Content-Type:application/json" --data 
> '{"type": "GET_METRICS", "get_metrics":{}}' http://localhost:5050/api/v1 | 
> python -m json.tool
> 
> 
> Thanks,
> 
> Tuan-Anh Hoang-Vu
> 
>

Re: Review Request 49943: Made vector reserve some spaces for allocator benchmark test.

[Jiang Yan Xu]

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


Ship it!




Ship It!

- Jiang Yan Xu


On July 14, 2016, 8:14 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49943/
> ---
> 
> (Updated July 14, 2016, 8:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Jie Yu, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated HierarchicalAllocator_BENCHMARK_Test.AddAndUpdateSlave to
> call vector reserve to reserve soem spaces before using, this can
> help improve the performance of the test.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 153c9b4cf4819e976910c5a7ad9602028e2d22eb 
> 
> Diff: https://reviews.apache.org/r/49943/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> ./bin/mesos-tests.sh --benchmark 
> --gtest_filter="SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.AddAndUpdateSlave/*"
> [==] Running 36 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 36 tests from 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test
> [ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.AddAndUpdateSlave/0
> Using 1000 agents and 1 frameworks
> Added 1 frameworks in 102us
> Added 1000 agents in 1.110443secs
> Updated 1000 agents in 583296us
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.AddAndUpdateSlave/0
>  (1993 ms)
> [ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.AddAndUpdateSlave/1
> Using 1000 agents and 50 frameworks
> Added 50 frameworks in 1826us
> Added 1000 agents in 2.577197secs
> Updated 1000 agents in 2.412386secs
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.AddAndUpdateSlave/1
>  (5265 ms)
> [ RUN  ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.AddAndUpdateSlave/2
> Using 1000 agents and 100 frameworks
> Added 100 frameworks in 4260us
> Added 1000 agents in 4.275021secs
> Updated 1000 agents in 3.902358secs
> [   OK ] 
> SlaveAndFrameworkCount/HierarchicalAllocator_BENCHMARK_Test.AddAndUpdateSlave/2
>  (8494 ms)
> ...
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Review Request 50065: Enhancement for containers which have image and join host network.

[Qian Zhang]

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

Review request for mesos, Avinash sridharan and Jie Yu.


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


Repository: mesos


Description
---

For the containers which have image and join host network, we enhanced
'network/cni' isolator to make sure they have access to host /etc/hosts
, /etc/hostname and /etc/resolv.conf files.


Diffs
-

  src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
09890cedf2e7a1846bd1cb250e117be1680a1b80 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
92b33111799cb4e1c8bc2051381e1254d701d95c 

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


Testing
---

make check

1. Start Mesos master
```
sudo ./bin/mesos-master.sh --work_dir=/opt/mesos
```

2. Start Mesos agent
```
sudo ./bin/mesos-slave.sh --master=192.168.122.171:5050 --containerizers=mesos 
--image_providers=appc,docker 
--isolation=filesystem/linux,docker/runtime,network/cni,cgroups/cpu,cgroups/mem 
--network_cni_config_dir=/opt/cni/net_configs 
--network_cni_plugins_dir=/opt/cni/plugins --work_dir=/opt/mesos
```

3. Launch a container which has image and joins host network
```
sudo src/mesos-execute --master=192.168.122.171:5050 --name=test 
--docker_image=library/busybox --command="sleep 120"
```

4. Check if the container has access to host network files
```
# 26927 is the PID of the container

$ sudo nsenter -t 26927 -m -n -u ls -la /etc/
total 36
drwxr-xr-x2 root root  4096 Jul 15 05:52 .
drwxr-xr-x   13 root root  4096 Jul 15 05:52 ..
-rw-rw-r--1 root root   304 Dec  5  2015 group
-rw-r--r--1 root root 6 Aug 10  2015 hostname
-rw-r--r--1 root root   272 Apr 18 08:37 hosts
-rw-r--r--1 root root   118 Jun  8 16:29 localtime
-rw-rw-r--1 root root   334 Dec  5  2015 passwd
-rw-r--r--1 root root   176 Jul  9 14:44 resolv.conf
-rw-rw-r--1 root root   243 Dec  5  2015 shadow

$ sudo nsenter -t 26927 -m -n -u mount 
rootfs on / type rootfs (rw)
/dev/mapper/u1404u1--vg-root on / type ext4 
(rw,relatime,errors=remount-ro,data=ordered)
tmpfs on /etc/resolv.conf type tmpfs 
(rw,nosuid,noexec,relatime,size=1643388k,mode=755)
/dev/mapper/u1404u1--vg-root on /etc/hostname type ext4 
(rw,relatime,errors=remount-ro,data=ordered)
/dev/mapper/u1404u1--vg-root on /etc/hosts type ext4 
(rw,relatime,errors=remount-ro,data=ordered)
/dev/mapper/u1404u1--vg-root on /mnt/mesos/sandbox type ext4 
(rw,relatime,errors=remount-ro,data=ordered)
proc on /proc type proc (rw,nosuid,nodev,noexec,relatime)
proc on /proc/sys type proc (ro,relatime)
sysfs on /sys type sysfs (ro,nosuid,nodev,noexec,relatime)
tmpfs on /dev type tmpfs (rw,nosuid,mode=755)
devpts on /dev/pts type devpts (rw,nosuid,noexec,relatime,mode=600,ptmxmode=666)
tmpfs on /dev/shm type tmpfs (rw,nosuid,nodev)
```


Thanks,

Qian Zhang

Re: Review Request 50061: Removed the trailing semi-colon when parse resources.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49943, 49781, 50060, 50061]

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 15, 2016, 5:16 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50061/
> ---
> 
> (Updated July 15, 2016, 5:16 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Klaus Ma, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed the trailing semi-colon when parse resources.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 4d049ed160041748f6150d78bdf65a71baa891bd 
>   src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
> 7944c0da716b2438fa2b40f5504aefa346e31046 
>   src/tests/hierarchical_allocator_tests.cpp 
> 153c9b4cf4819e976910c5a7ad9602028e2d22eb 
>   src/tests/resources_tests.cpp 40d290ac540d26373c5fb7c2a93d27d1aa61d722 
> 
> Diff: https://reviews.apache.org/r/50061/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 50061: Removed the trailing semi-colon when parse resources.

[Guangya Liu]

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

(Updated 七月 15, 2016, 5:16 a.m.)


Review request for mesos, Benjamin Mahler, Klaus Ma, and Jiang Yan Xu.


Repository: mesos


Description
---

Removed the trailing semi-colon when parse resources.


Diffs (updated)
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
4d049ed160041748f6150d78bdf65a71baa891bd 
  src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
7944c0da716b2438fa2b40f5504aefa346e31046 
  src/tests/hierarchical_allocator_tests.cpp 
153c9b4cf4819e976910c5a7ad9602028e2d22eb 
  src/tests/resources_tests.cpp 40d290ac540d26373c5fb7c2a93d27d1aa61d722 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 50060: Always call watch.stop() to get an accurate time for benchmark test.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49943, 49781, 50060]

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 15, 2016, 2:27 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50060/
> ---
> 
> (Updated July 15, 2016, 2:27 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Klaus Ma, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This path include two fixes:
> 1) Always call watch.stop() explicitly to get an accurate time
> for benchmark test.
> 2) Move `watch.start()` right before `allocator->addSlave` to
> make the time more accurate.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 153c9b4cf4819e976910c5a7ad9602028e2d22eb 
> 
> Diff: https://reviews.apache.org/r/50060/diff/
> 
> 
> Testing
> ---
> 
> ./bin/mesos-tests.sh --benchmark 
> --gtest_filter="*HierarchicalAllocator_BENCHMARK_Test.*/1"
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 49178: Configured single output binary folder.

[Daniel Pravat]

> On June 24, 2016, 8:41 p.m., Joseph Wu wrote:
> > src/CMakeLists.txt, line 462
> > 
> >
> > Would it be worthwhile to get rid of the "Debug" and "Release" build 
> > configuration folders too?
> > 
> > i.e. 
> > ```
> > SET(CMAKE_RUNTIME_OUTPUT_DIRECTORY_DEBUG ${CMAKE_BINARY_DIR}/src)
> > SET(CMAKE_RUNTIME_OUTPUT_DIRECTORY_RELEASE ${CMAKE_BINARY_DIR}/src)
> > ```
> > 
> > See: 
> > https://cmake.org/cmake/help/v3.5/prop_tgt/RUNTIME_OUTPUT_DIRECTORY_CONFIG.html

Agree


- Daniel


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


On June 24, 2016, 4:04 a.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49178/
> ---
> 
> (Updated June 24, 2016, 4:04 a.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Configured single output binary folder.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt ecdeee75abfca944bc2ac1da4962d8d50e236d2d 
> 
> Diff: https://reviews.apache.org/r/49178/diff/
> 
> 
> Testing
> ---
> 
> Windows: build/run
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

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

[haosdent huang]

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

(Updated July 15, 2016, 3:33 a.m.)


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


Changes
---

Rebase.


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 49823: Implemented `CgroupsIsolatorProcess::update`.

[haosdent huang]

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

(Updated July 15, 2016, 3:33 a.m.)


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


Changes
---

Rebase.


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.hpp 
c57baec88437f68886702a40ec8a6a6458546119 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 

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


Testing
---


Thanks,

haosdent huang

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

[haosdent huang]

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

(Updated July 15, 2016, 3:33 a.m.)


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


Changes
---

Rebase.


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.hpp 
c57baec88437f68886702a40ec8a6a6458546119 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49853: Implemented `PerfEventSubsystem`.

[haosdent huang]

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

(Updated July 15, 2016, 3:33 a.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `PerfEventSubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49817: Implemented `CgroupsIsolatorProcess::recover`.

[haosdent huang]

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

(Updated July 15, 2016, 3:32 a.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `CgroupsIsolatorProcess::recover`.


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/49817/diff/


Testing
---


Thanks,

haosdent huang

Re: Review Request 49850: Implemented `CpuacctSubsystem`.

[haosdent huang]

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

(Updated July 15, 2016, 3:33 a.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `CpuacctSubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49849: Implemented `CpuSubsystem`.

[haosdent huang]

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

(Updated July 15, 2016, 3:33 a.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `CpuSubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 
  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

Re: Review Request 49855: Enabled cgroups unified isolator in isolation.

[haosdent huang]

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

(Updated July 15, 2016, 3:33 a.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Enabled cgroups unified isolator in isolation.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
e900d752b70165236a198f70c8cb24876a678e4b 

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


Testing
---


Thanks,

haosdent huang

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

[haosdent huang]

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

(Updated July 15, 2016, 3:33 a.m.)


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


Changes
---

Rebase.


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.hpp 
c57baec88437f68886702a40ec8a6a6458546119 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 45573: Add `PerfEventHandleManager`.

[haosdent huang]

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

(Updated July 15, 2016, 3:33 a.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Add `PerfEventHandleManager`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/perf_event.hpp 
1de38491a33e5782b20e5ee491412779d09ee391 
  src/slave/containerizer/mesos/isolators/cgroups/perf_event.cpp 
31f35385691681ef5da14be747edfb5f57c5d05a 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49851: Implemented `MemorySubsystem`.

[haosdent huang]

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

(Updated July 15, 2016, 3:33 a.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `MemorySubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
c57baec88437f68886702a40ec8a6a6458546119 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 
  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 49820: Implemented `CgroupsIsolatorProcess::isolate`.

[haosdent huang]

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

(Updated July 15, 2016, 3:32 a.m.)


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


Changes
---

Rebase.


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.hpp 
c57baec88437f68886702a40ec8a6a6458546119 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49854: Implemented `DevicesSubsystem`.

[haosdent huang]

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

(Updated July 15, 2016, 3:33 a.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `DevicesSubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
5f52a076a1fa3a21d886cb961ddeed5046a38d7c 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
a30ecafcbecc9d3b6eeea2b04dcb4d278750af41 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49821: Implemented `CgroupsIsolatorProcess::watch`.

[haosdent huang]

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

(Updated July 15, 2016, 3:33 a.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `CgroupsIsolatorProcess::watch`.


Diffs (updated)
-

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

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


Testing
---


Thanks,

haosdent huang

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

[haosdent huang]

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

(Updated July 15, 2016, 3:33 a.m.)


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


Changes
---

Rebase.


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 49852: Implemented `NetClsSubsystem`.

[haosdent huang]

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

(Updated July 15, 2016, 3:33 a.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `NetClsSubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 
  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 49819: Implemented `CgroupsIsolatorProcess::prepare`.

[haosdent huang]

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

(Updated July 15, 2016, 3:32 a.m.)


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


Changes
---

Rebase.


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 49814: Implemented `CgroupsIsolatorProcess::create`.

[haosdent huang]

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

(Updated July 15, 2016, 3:31 a.m.)


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


Changes
---

Rebase.


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.hpp 
c57baec88437f68886702a40ec8a6a6458546119 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 50061: Removed the trailing semi-colon when parse resources.

[Guangya Liu]

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

(Updated 七月 15, 2016, 3:20 a.m.)


Review request for mesos, Benjamin Mahler, Klaus Ma, and Jiang Yan Xu.


Repository: mesos


Description
---

Removed the trailing semi-colon when parse resources.


Diffs (updated)
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
4d049ed160041748f6150d78bdf65a71baa891bd 
  src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
7944c0da716b2438fa2b40f5504aefa346e31046 
  src/tests/hierarchical_allocator_tests.cpp 
153c9b4cf4819e976910c5a7ad9602028e2d22eb 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Review Request 50061: Removed the trailing semi-colon when parse resources.

[Guangya Liu]

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

Review request for mesos, Benjamin Mahler, Klaus Ma, and Jiang Yan Xu.


Repository: mesos


Description
---

Removed the trailing semi-colon when parse resources.


Diffs
-

  src/tests/containerizer/docker_containerizer_tests.cpp 
4d049ed160041748f6150d78bdf65a71baa891bd 
  src/tests/hierarchical_allocator_tests.cpp 
153c9b4cf4819e976910c5a7ad9602028e2d22eb 

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


Testing
---

make
make check


Thanks,

Guangya Liu

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

[haosdent huang]

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

(Updated July 15, 2016, 3 a.m.)


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


Changes
---

Address 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 49827: Implemented `CgroupsIsolatorProcess::cleanup`.

[haosdent huang]

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

(Updated July 15, 2016, 3 a.m.)


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


Changes
---

Address 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 49843: Added benchmark test for sorter.

[Guangya Liu]

> On 七月 14, 2016, 7:14 p.m., Benjamin Mahler wrote:
> >

Thanks Ben for the cleanup and helping merge the code. Please check my answers 
inline.


> On 七月 14, 2016, 7:14 p.m., Benjamin Mahler wrote:
> > src/tests/sorter_tests.cpp, lines 504-542
> > 
> >
> > It looks like we can simplify these two functions down to the following 
> > single function?
> > 
> > ```
> > // Returns a "ports" resource with the number of ranges
> > // specified as: [1-2, 4-5, 7-8, 10-11, ...]
> > static Resource makePortRanges(size_t numRanges)
> > {
> >   ::mesos::Value::Ranges ranges;
> > 
> >   for (size_t i = 0; i < numRanges; ++i) {
> > Value::Range* range = ranges.add_range();
> > range->set_begin((3 * i) + 1);
> > range->set_end(range->begin() + 1);
> >   }
> > 
> >   Value value;
> >   value.set_type(Value::RANGES);
> >   value.mutable_ranges()->CopyFrom(ranges);
> > 
> >   Resource resource;
> >   resource.set_role("*");
> >   resource.set_name("ports");
> >   resource.set_type(Value::RANGES);
> >   resource.mutable_ranges()->CopyFrom(value.ranges());
> > 
> >   return resource;
> > }
> > ```
> > 
> > Since we only care about the number of ranges, we don't need to specify 
> > the bounds, right?

Yes, my orignial thinking was keep this same as 
https://github.com/apache/mesos/blob/master/src/tests/hierarchical_allocator_tests.cpp#L85-L122
 in case we may need specify bounds in future, but since this is only for test 
and we do not need to care for bounds, so seems no need to have `makeRange` 
here. I was thinking we may also need to update 
https://github.com/apache/mesos/blob/master/src/tests/hierarchical_allocator_tests.cpp#L85-L122
 as well to use a single function to create port ranges.


> On 七月 14, 2016, 7:14 p.m., Benjamin Mahler wrote:
> > src/tests/sorter_tests.cpp, line 511
> > 
> >
> > Is this an optimization?

Yes, but I saw that you removed this part when merge the code, do we need to 
add it back?

```
// Reserve space to expand the field to at least the given size.  This only
// resizes the pointer array; it doesn't allocate any objects.  If the
// array is grown, it will always be at least doubled in size.
void Reserve(int new_size);
```


> On 七月 14, 2016, 7:14 p.m., Benjamin Mahler wrote:
> > src/tests/sorter_tests.cpp, line 536
> > 
> >
> > We tend to use 'unsigned int' rather than just 'unsigned'.

>From http://en.cppreference.com/w/cpp/types/size_t , it seems it is 
>recommended to use `size_t` with following reason:

```
std::size_t is commonly used for array indexing and loop counting. Programs 
that use other types, such as unsigned int, for array indexing may fail on, 
e.g. 64-bit systems when the index exceeds UINT_MAX or if it relies on 32-bit 
modular arithmetic.
```


> On 七月 14, 2016, 7:14 p.m., Benjamin Mahler wrote:
> > src/tests/sorter_tests.cpp, line 577
> > 
> >
> > Seems we should move this down to at the start of the loop to avoid 
> > timing the resource parsing?

Yes.


> On 七月 14, 2016, 7:14 p.m., Benjamin Mahler wrote:
> > src/tests/sorter_tests.cpp, line 595
> > 
> >
> > No need for the trailing semi-colon after disk?

I will do a clean up for this in mesos test cause many `Resources::parse` 
including a trailing semi-colon.


> On 七月 14, 2016, 7:14 p.m., Benjamin Mahler wrote:
> > src/tests/sorter_tests.cpp, lines 600-603
> > 
> >
> > Oh.. this isn't allocating resources on all of the agents! The 
> > following will do it:
> > 
> > ```
> >   watch.start();
> >   size_t clientIndex = 0;
> >   foreach (const SlaveID& slaveId, agents) {
> > const string& client = clients[clientIndex++ % clients.size()];
> > sorter.allocated(client, slaveId, allocated);
> >   }
> >   watch.stop();
> > ```
> > 
> > Note that this makes things slower, as expected.

Sorry I missed that part, I was only thinking to make all clients get resources 
and then check the time of `sorter`, yes, we should also make the time of 
adding agents accurate.


> On 七月 14, 2016, 7:14 p.m., Benjamin Mahler wrote:
> > src/tests/sorter_tests.cpp, lines 515-516
> > 
> >
> > Why unsigned when these are actually uint64_t?

I should check this when get it from 
https://github.com/apache/mesos/blob/master/src/tests/hierarchical_allocator_tests.cpp#L85-L122


- Guangya


-

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

[haosdent huang]

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




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


Add a blank line below.


- haosdent huang


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

Review Request 50060: Always call watch.stop() to get an accurate time for benchmark test.

[Guangya Liu]

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

Review request for mesos, Benjamin Mahler, Klaus Ma, and Jiang Yan Xu.


Repository: mesos


Description
---

This path include two fixes:
1) Always call watch.stop() explicitly to get an accurate time
for benchmark test.
2) Move `watch.start()` right before `allocator->addSlave` to
make the time more accurate.


Diffs
-

  src/tests/hierarchical_allocator_tests.cpp 
153c9b4cf4819e976910c5a7ad9602028e2d22eb 

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


Testing
---

./bin/mesos-tests.sh --benchmark 
--gtest_filter="*HierarchicalAllocator_BENCHMARK_Test.*/1"


Thanks,

Guangya Liu

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

[haosdent huang]

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




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


Update the comment with the reason why we need save the info here.



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


s/Create cgroup/Create cgroup in/



src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp (lines 373 - 382)


This method is unnecessary after we remove `cleanup`.


- haosdent huang


On July 14, 2016, 5:15 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49819/
> ---
> 
> (Updated July 14, 2016, 5:15 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::prepare`.
> 
> 
> 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/49819/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 50003: Propagate work_dir flag from local runs to agents/masters.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [50002, 50003]

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 14, 2016, 10:11 p.m., Ammar Askar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50003/
> ---
> 
> (Updated July 14, 2016, 10:11 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Bugs: MESOS-5613
> https://issues.apache.org/jira/browse/MESOS-5613
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Propagate work_dir flag from local runs to agents/masters.
> 
> 
> Diffs
> -
> 
>   src/local/flags.hpp f0af0d2 
>   src/local/local.cpp a543aef 
> 
> Diff: https://reviews.apache.org/r/50003/diff/
> 
> 
> Testing
> ---
> 
> Manually tested that `mesos local` and `mesos-local` run now. Not sure if a 
> regression test for this would be good, please advise.
> 
> 
> Thanks,
> 
> Ammar Askar
> 
>

Re: Review Request 50044: Updated Sorter::sort to return a vector rather than list.

[Klaus Ma]

> On July 15, 2016, 7:49 a.m., Klaus Ma wrote:
> > src/master/allocator/sorter/drf/sorter.hpp, line 22
> > 
> >
> > This's not necessary, `sorter/sorter.hpp` had included it.
> 
> Guangya Liu wrote:
> I think the reason here is we generally don't rely on transitive includes 
> as it's harder to maintain.

We should keep small list of header; if anyone missed removing, it'll impact 
compile performance.


- Klaus


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


On July 15, 2016, 3:35 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50044/
> ---
> 
> (Updated July 15, 2016, 3:35 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, Klaus Ma, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We now prefer to use vector rather than list in general for
> efficiency reasons, unless we need to take advantage of the
> operations that are efficient on a linked-list.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.hpp 
> e29feebd70277c79f7c3f6fb233e7a36501cf220 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 7df4dd641b21ea0705368861bf4679fed1ef078d 
>   src/master/allocator/sorter/sorter.hpp 
> f5f0b086cb95eb2ab70b3f67e5b20814925bf702 
>   src/tests/sorter_tests.cpp bdd4355bfcd7b1fa1c22983f8e0ee6f20906917a 
> 
> Diff: https://reviews.apache.org/r/50044/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> benchmarks: with 1000 clients, sort time is reduced by 72 us
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 49939: Updated Agent::GET_METRICS call to return metrics grouped by types.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [49936, 49937, 49938, 49939]

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 14, 2016, 9:32 p.m., Tuan-Anh Hoang-Vu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49939/
> ---
> 
> (Updated July 14, 2016, 9:32 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-5731
> https://issues.apache.org/jira/browse/MESOS-5731
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated Agent::GET_METRICS call to return metrics grouped by types.
> 
> 
> Diffs
> -
> 
>   include/mesos/agent/agent.proto cfd117de81396bf79049b7642f1ccd1ff4fbb676 
>   include/mesos/v1/agent/agent.proto 213c428d424d8e4f0cc07bd86f1ed59b60df107c 
>   src/slave/http.cpp 21c7ebf7c23fd06bee7125c90576eb892b249b4d 
>   src/tests/api_tests.cpp 55e825ea6a3bd43c76dc67e8b90a97e8c9530a47 
> 
> Diff: https://reviews.apache.org/r/49939/diff/
> 
> 
> Testing
> ---
> 
> Modified AgentAPITest.GetMetrics to make sure we return metrics grouped by 
> types.
> 
> 
> 1. Run master: ./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/var/lib/mesos
> 2. Run agent: ./bin/mesos-slave.sh --master=127.0.0.1:5050 
> --work_dir=/var/lib/mesos
> 2. Call GET_METRICS: curl --header "Content-Type:application/json" --data 
> '{"type": "GET_METRICS", "get_metrics":{}}' http://localhost:5051/api/v1 | 
> python -m json.tool
> 
> 
> Thanks,
> 
> Tuan-Anh Hoang-Vu
> 
>

Re: Review Request 50044: Updated Sorter::sort to return a vector rather than list.

[Guangya Liu]

> On 七月 14, 2016, 11:49 p.m., Klaus Ma wrote:
> > src/master/allocator/sorter/drf/sorter.hpp, line 22
> > 
> >
> > This's not necessary, `sorter/sorter.hpp` had included it.

I think the reason here is we generally don't rely on transitive includes as 
it's harder to maintain.


- Guangya


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


On 七月 14, 2016, 7:35 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50044/
> ---
> 
> (Updated 七月 14, 2016, 7:35 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Klaus Ma, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We now prefer to use vector rather than list in general for
> efficiency reasons, unless we need to take advantage of the
> operations that are efficient on a linked-list.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.hpp 
> e29feebd70277c79f7c3f6fb233e7a36501cf220 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 7df4dd641b21ea0705368861bf4679fed1ef078d 
>   src/master/allocator/sorter/sorter.hpp 
> f5f0b086cb95eb2ab70b3f67e5b20814925bf702 
>   src/tests/sorter_tests.cpp bdd4355bfcd7b1fa1c22983f8e0ee6f20906917a 
> 
> Diff: https://reviews.apache.org/r/50044/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> benchmarks: with 1000 clients, sort time is reduced by 72 us
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 50044: Updated Sorter::sort to return a vector rather than list.

[Guangya Liu]

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




src/master/allocator/sorter/sorter.hpp (line 131)


s/list/vector



src/tests/sorter_tests.cpp (line 21)


remove this



src/tests/sorter_tests.cpp (line 23)


```
#include 
```



src/tests/sorter_tests.cpp (line 40)


remove this


- Guangya Liu


On 七月 14, 2016, 7:35 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50044/
> ---
> 
> (Updated 七月 14, 2016, 7:35 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Klaus Ma, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We now prefer to use vector rather than list in general for
> efficiency reasons, unless we need to take advantage of the
> operations that are efficient on a linked-list.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.hpp 
> e29feebd70277c79f7c3f6fb233e7a36501cf220 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 7df4dd641b21ea0705368861bf4679fed1ef078d 
>   src/master/allocator/sorter/sorter.hpp 
> f5f0b086cb95eb2ab70b3f67e5b20814925bf702 
>   src/tests/sorter_tests.cpp bdd4355bfcd7b1fa1c22983f8e0ee6f20906917a 
> 
> Diff: https://reviews.apache.org/r/50044/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> benchmarks: with 1000 clients, sort time is reduced by 72 us
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 50044: Updated Sorter::sort to return a vector rather than list.

[Klaus Ma]

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


Ship it!




Ship It!

- Klaus Ma


On July 15, 2016, 3:35 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50044/
> ---
> 
> (Updated July 15, 2016, 3:35 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, Klaus Ma, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We now prefer to use vector rather than list in general for
> efficiency reasons, unless we need to take advantage of the
> operations that are efficient on a linked-list.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.hpp 
> e29feebd70277c79f7c3f6fb233e7a36501cf220 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 7df4dd641b21ea0705368861bf4679fed1ef078d 
>   src/master/allocator/sorter/sorter.hpp 
> f5f0b086cb95eb2ab70b3f67e5b20814925bf702 
>   src/tests/sorter_tests.cpp bdd4355bfcd7b1fa1c22983f8e0ee6f20906917a 
> 
> Diff: https://reviews.apache.org/r/50044/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> benchmarks: with 1000 clients, sort time is reduced by 72 us
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 50044: Updated Sorter::sort to return a vector rather than list.

[Klaus Ma]

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




src/master/allocator/sorter/drf/sorter.hpp (line 22)


This's not necessary, `sorter/sorter.hpp` had included it.


- Klaus Ma


On July 15, 2016, 3:35 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50044/
> ---
> 
> (Updated July 15, 2016, 3:35 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, Klaus Ma, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We now prefer to use vector rather than list in general for
> efficiency reasons, unless we need to take advantage of the
> operations that are efficient on a linked-list.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.hpp 
> e29feebd70277c79f7c3f6fb233e7a36501cf220 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 7df4dd641b21ea0705368861bf4679fed1ef078d 
>   src/master/allocator/sorter/sorter.hpp 
> f5f0b086cb95eb2ab70b3f67e5b20814925bf702 
>   src/tests/sorter_tests.cpp bdd4355bfcd7b1fa1c22983f8e0ee6f20906917a 
> 
> Diff: https://reviews.apache.org/r/50044/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> benchmarks: with 1000 clients, sort time is reduced by 72 us
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 50044: Updated Sorter::sort to return a vector rather than list.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [50044]

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 14, 2016, 7:35 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50044/
> ---
> 
> (Updated July 14, 2016, 7:35 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Klaus Ma, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We now prefer to use vector rather than list in general for
> efficiency reasons, unless we need to take advantage of the
> operations that are efficient on a linked-list.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/sorter.hpp 
> e29feebd70277c79f7c3f6fb233e7a36501cf220 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 7df4dd641b21ea0705368861bf4679fed1ef078d 
>   src/master/allocator/sorter/sorter.hpp 
> f5f0b086cb95eb2ab70b3f67e5b20814925bf702 
>   src/tests/sorter_tests.cpp bdd4355bfcd7b1fa1c22983f8e0ee6f20906917a 
> 
> Diff: https://reviews.apache.org/r/50044/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> benchmarks: with 1000 clients, sort time is reduced by 72 us
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 50003: Propagate work_dir flag from local runs to agents/masters.

[Ammar Askar]

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

(Updated July 14, 2016, 10:11 p.m.)


Review request for mesos, Greg Mann and Vinod Kone.


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


Repository: mesos


Description
---

Propagate work_dir flag from local runs to agents/masters.


Diffs (updated)
-

  src/local/flags.hpp f0af0d2 
  src/local/local.cpp a543aef 

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


Testing
---

Manually tested that `mesos local` and `mesos-local` run now. Not sure if a 
regression test for this would be good, please advise.


Thanks,

Ammar Askar

Re: Review Request 50002: Allow all flags load methods to specify a prefix.

[Ammar Askar]

> On July 14, 2016, 1:27 a.m., Vinod Kone wrote:
> > 3rdparty/stout/tests/flags_tests.cpp, line 546
> > 
> >
> > s/test_option_map/values2/

Scoped so they can share the same name of `values`


- Ammar


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


On July 14, 2016, 10:10 p.m., Ammar Askar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50002/
> ---
> 
> (Updated July 14, 2016, 10:10 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Bugs: MESOS-5613
> https://issues.apache.org/jira/browse/MESOS-5613
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow all flags load methods to specify a prefix.
> 
> This also refactors the prefix logic into one place, so that's nice.
> Required for the actual fix for passing work_dir in local.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/flags/flags.hpp dd93627 
>   3rdparty/stout/tests/flags_tests.cpp 77f3a6a 
> 
> Diff: https://reviews.apache.org/r/50002/diff/
> 
> 
> Testing
> ---
> 
> Added additional tests to flags_tests.cpp to ensure that prefix works on the 
> other methods.
> 
> `make check` passes
> 
> 
> Thanks,
> 
> Ammar Askar
> 
>

Re: Review Request 50002: Allow all flags load methods to specify a prefix.

[Ammar Askar]

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

(Updated July 14, 2016, 10:10 p.m.)


Review request for mesos, Greg Mann and Vinod Kone.


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


Repository: mesos


Description
---

Allow all flags load methods to specify a prefix.

This also refactors the prefix logic into one place, so that's nice.
Required for the actual fix for passing work_dir in local.


Diffs (updated)
-

  3rdparty/stout/include/stout/flags/flags.hpp dd93627 
  3rdparty/stout/tests/flags_tests.cpp 77f3a6a 

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


Testing
---

Added additional tests to flags_tests.cpp to ensure that prefix works on the 
other methods.

`make check` passes


Thanks,

Ammar Askar

Re: Review Request 49938: Updated Master::GET_METRICS call to return metrics grouped by types.

[Tuan-Anh Hoang-Vu]

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

(Updated July 14, 2016, 2:35 p.m.)


Review request for mesos, Benjamin Mahler and Vinod Kone.


Summary (updated)
-

Updated Master::GET_METRICS call to return metrics grouped by types.


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


Repository: mesos


Description (updated)
---

Updated Master::GET_METRICS call to return metrics grouped by types.


Diffs (updated)
-

  include/mesos/master/master.proto 7fb7153b5498a55469d2c72873c45fb534c0e60d 
  include/mesos/v1/master/master.proto c9e4359fff018de55d83da5cde0eb6d59dacfc3e 
  src/master/http.cpp 42ba3643daf8f6fe63152178214b57e3b773d93e 
  src/tests/api_tests.cpp 55e825ea6a3bd43c76dc67e8b90a97e8c9530a47 

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


Testing
---

Modified MasterAPITest.GetMetrics to make sure we return metrics grouped by 
types.


1. Run master: ./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/var/lib/mesos
2. Call GET_METRICS: curl --header "Content-Type:application/json" --data 
'{"type": "GET_METRICS", "get_metrics":{}}' http://localhost:5050/api/v1 | 
python -m json.tool


Thanks,

Tuan-Anh Hoang-Vu

Re: Review Request 49939: Updated Agent::GET_METRICS call to return metrics grouped by types.

[Tuan-Anh Hoang-Vu]

> On July 14, 2016, 12:31 a.m., Mesos ReviewBot wrote:
> > Bad patch!
> > 
> > Reviews applied: [49939, 49938, 49937, 49936]
> > 
> > Failed command: ./support/apply-review.sh -n -r 49938
> > 
> > Error:
> > 2016-07-14 07:31:47 URL:https://reviews.apache.org/r/49938/diff/raw/ 
> > [4422/4422] -> "49938.patch" [1]
> > Total errors found: 0
> > Checking 2 files
> > Error: No line in the commit message summary may exceed 72 characters.
> > 
> > Full log: https://builds.apache.org/job/mesos-reviewbot/14309/console
> 
> Abhishek Dasgupta wrote:
> I think the problem will be solved if you divide the description section 
> for this review(and r449938) into two lines where each containing not more 
> than 72 characters. Thank you for your contribution.

Yes I reworded two commits to fit the messages into < 72 chars.


- Tuan-Anh


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


On July 14, 2016, 2:32 p.m., Tuan-Anh Hoang-Vu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49939/
> ---
> 
> (Updated July 14, 2016, 2:32 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-5731
> https://issues.apache.org/jira/browse/MESOS-5731
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated Agent::GET_METRICS call to return metrics grouped by types.
> 
> 
> Diffs
> -
> 
>   include/mesos/agent/agent.proto cfd117de81396bf79049b7642f1ccd1ff4fbb676 
>   include/mesos/v1/agent/agent.proto 213c428d424d8e4f0cc07bd86f1ed59b60df107c 
>   src/slave/http.cpp 21c7ebf7c23fd06bee7125c90576eb892b249b4d 
>   src/tests/api_tests.cpp 55e825ea6a3bd43c76dc67e8b90a97e8c9530a47 
> 
> Diff: https://reviews.apache.org/r/49939/diff/
> 
> 
> Testing
> ---
> 
> Modified AgentAPITest.GetMetrics to make sure we return metrics grouped by 
> types.
> 
> 
> 1. Run master: ./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/var/lib/mesos
> 2. Run agent: ./bin/mesos-slave.sh --master=127.0.0.1:5050 
> --work_dir=/var/lib/mesos
> 2. Call GET_METRICS: curl --header "Content-Type:application/json" --data 
> '{"type": "GET_METRICS", "get_metrics":{}}' http://localhost:5051/api/v1 | 
> python -m json.tool
> 
> 
> Thanks,
> 
> Tuan-Anh Hoang-Vu
> 
>

Re: Review Request 49939: Updated Agent::GET_METRICS call to return metrics grouped by types.

[Tuan-Anh Hoang-Vu]

> On July 14, 2016, 3:19 a.m., Abhishek Dasgupta wrote:
> > If you address my comments in master, pls do them for agent as well.

Yes thank you for your reviews, I updated both Master and Agent


- Tuan-Anh


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


On July 14, 2016, 2:32 p.m., Tuan-Anh Hoang-Vu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49939/
> ---
> 
> (Updated July 14, 2016, 2:32 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-5731
> https://issues.apache.org/jira/browse/MESOS-5731
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated Agent::GET_METRICS call to return metrics grouped by types.
> 
> 
> Diffs
> -
> 
>   include/mesos/agent/agent.proto cfd117de81396bf79049b7642f1ccd1ff4fbb676 
>   include/mesos/v1/agent/agent.proto 213c428d424d8e4f0cc07bd86f1ed59b60df107c 
>   src/slave/http.cpp 21c7ebf7c23fd06bee7125c90576eb892b249b4d 
>   src/tests/api_tests.cpp 55e825ea6a3bd43c76dc67e8b90a97e8c9530a47 
> 
> Diff: https://reviews.apache.org/r/49939/diff/
> 
> 
> Testing
> ---
> 
> Modified AgentAPITest.GetMetrics to make sure we return metrics grouped by 
> types.
> 
> 
> 1. Run master: ./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/var/lib/mesos
> 2. Run agent: ./bin/mesos-slave.sh --master=127.0.0.1:5050 
> --work_dir=/var/lib/mesos
> 2. Call GET_METRICS: curl --header "Content-Type:application/json" --data 
> '{"type": "GET_METRICS", "get_metrics":{}}' http://localhost:5051/api/v1 | 
> python -m json.tool
> 
> 
> Thanks,
> 
> Tuan-Anh Hoang-Vu
> 
>

Re: Review Request 49939: Updated Agent::GET_METRICS call to return metrics grouped by types.

[Tuan-Anh Hoang-Vu]

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

(Updated July 14, 2016, 2:32 p.m.)


Review request for mesos, Benjamin Mahler and Vinod Kone.


Summary (updated)
-

Updated Agent::GET_METRICS call to return metrics grouped by types.


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


Repository: mesos


Description (updated)
---

Updated Agent::GET_METRICS call to return metrics grouped by types.


Diffs (updated)
-

  include/mesos/agent/agent.proto cfd117de81396bf79049b7642f1ccd1ff4fbb676 
  include/mesos/v1/agent/agent.proto 213c428d424d8e4f0cc07bd86f1ed59b60df107c 
  src/slave/http.cpp 21c7ebf7c23fd06bee7125c90576eb892b249b4d 
  src/tests/api_tests.cpp 55e825ea6a3bd43c76dc67e8b90a97e8c9530a47 

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


Testing
---

Modified AgentAPITest.GetMetrics to make sure we return metrics grouped by 
types.


1. Run master: ./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/var/lib/mesos
2. Run agent: ./bin/mesos-slave.sh --master=127.0.0.1:5050 
--work_dir=/var/lib/mesos
2. Call GET_METRICS: curl --header "Content-Type:application/json" --data 
'{"type": "GET_METRICS", "get_metrics":{}}' http://localhost:5051/api/v1 | 
python -m json.tool


Thanks,

Tuan-Anh Hoang-Vu

Re: Review Request 50041: Supported docker container running as a defined user.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [50041]

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 14, 2016, 6:17 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50041/
> ---
> 
> (Updated July 14, 2016, 6:17 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Artem Harutyunyan, Jie Yu, and 
> Timothy Chen.
> 
> 
> Bugs: MESOS-5754
> https://issues.apache.org/jira/browse/MESOS-5754
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch allows the container launched by docker containerizer
> to run as a user defined user (either the user from FrameworkInfo
> or CommandInfo).
> 
> Important NOTE:
> 1. This is a behavior change, since the agent flag 'switch_user'
>defaults to be true. So the docker container will defaultly
>run as the framework/command user.
> 2. The user has to be existed in both host machine and the
>docker image. Otherwise, either the host would fail to
>chown to a non-existed user, or the docker daemon cannot
>start the container due to unable to find the user.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
>   src/docker/docker.cpp 515842d381ca8a91ad481f66c7be057dff2f3f28 
>   src/docker/executor.hpp 7b63d784d6b8685912598b77fb38cf6e70646ae3 
>   src/docker/executor.cpp 69511044e39bc05d7d6240264ec70b6e6f44edba 
>   src/slave/containerizer/docker.cpp f1ecf3b25d85597f6c3dcaa47968860ed119dbd5 
>   src/tests/containerizer/docker_tests.cpp 
> 7b73a4939da33d0b7dd08fdcf56525b6403eada3 
>   src/tests/mesos.hpp e4eccfc3810bed3649a3ab80e252849470de4c72 
> 
> Diff: https://reviews.apache.org/r/50041/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-tests.sh
> 
> Manually tested by /src/mesos-execute with CommandInfo.user set as `nobody` 
> using docker image `alpine`.
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 49938: Updated GET_METRICS call in v1 api to return metrics grouped by types.

[Tuan-Anh Hoang-Vu]

> On July 14, 2016, 1:47 a.m., Abhishek Dasgupta wrote:
> > src/master/http.cpp, lines 1916-1944
> > 
> >
> > Can we consider to use a switch here?

I don't know why I can't use foreachpair for MetricMap. I got this error 
message:

../../src/master/http.cpp:1918:94: error: macro "foreachpair" passed 4 
arguments, but takes just 3
 foreachpair (const MetricType metricType, hashmap 
metrics, metricsMap) {


- Tuan-Anh


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


On July 14, 2016, 1:31 p.m., Tuan-Anh Hoang-Vu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49938/
> ---
> 
> (Updated July 14, 2016, 1:31 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-5731
> https://issues.apache.org/jira/browse/MESOS-5731
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated GET_METRICS call in v1 api to return metrics grouped by types.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/master.proto 7fb7153b5498a55469d2c72873c45fb534c0e60d 
>   include/mesos/v1/master/master.proto 
> c9e4359fff018de55d83da5cde0eb6d59dacfc3e 
>   src/master/http.cpp 42ba3643daf8f6fe63152178214b57e3b773d93e 
>   src/tests/api_tests.cpp 55e825ea6a3bd43c76dc67e8b90a97e8c9530a47 
> 
> Diff: https://reviews.apache.org/r/49938/diff/
> 
> 
> Testing
> ---
> 
> Modified MasterAPITest.GetMetrics to make sure we return metrics grouped by 
> types.
> 
> 
> 1. Run master: ./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/var/lib/mesos
> 2. Call GET_METRICS: curl --header "Content-Type:application/json" --data 
> '{"type": "GET_METRICS", "get_metrics":{}}' http://localhost:5050/api/v1 | 
> python -m json.tool
> 
> 
> Thanks,
> 
> Tuan-Anh Hoang-Vu
> 
>

Re: Review Request 49938: Updated GET_METRICS call in v1 api to return metrics grouped by types.

[Tuan-Anh Hoang-Vu]

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

(Updated July 14, 2016, 1:31 p.m.)


Review request for mesos, Benjamin Mahler and Vinod Kone.


Summary (updated)
-

Updated GET_METRICS call in v1 api to return metrics grouped by types.


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


Repository: mesos


Description (updated)
---

Updated GET_METRICS call in v1 api to return metrics grouped by types.


Diffs (updated)
-

  include/mesos/master/master.proto 7fb7153b5498a55469d2c72873c45fb534c0e60d 
  include/mesos/v1/master/master.proto c9e4359fff018de55d83da5cde0eb6d59dacfc3e 
  src/master/http.cpp 42ba3643daf8f6fe63152178214b57e3b773d93e 
  src/tests/api_tests.cpp 55e825ea6a3bd43c76dc67e8b90a97e8c9530a47 

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


Testing
---

Modified MasterAPITest.GetMetrics to make sure we return metrics grouped by 
types.


1. Run master: ./bin/mesos-master.sh --ip=127.0.0.1 --work_dir=/var/lib/mesos
2. Call GET_METRICS: curl --header "Content-Type:application/json" --data 
'{"type": "GET_METRICS", "get_metrics":{}}' http://localhost:5050/api/v1 | 
python -m json.tool


Thanks,

Tuan-Anh Hoang-Vu

Review Request 50044: Updated Sorter::sort to return a vector rather than list.

[Benjamin Mahler]

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

Review request for mesos, Guangya Liu, Klaus Ma, and Jiang Yan Xu.


Repository: mesos


Description
---

We now prefer to use vector rather than list in general for
efficiency reasons, unless we need to take advantage of the
operations that are efficient on a linked-list.


Diffs
-

  src/master/allocator/sorter/drf/sorter.hpp 
e29feebd70277c79f7c3f6fb233e7a36501cf220 
  src/master/allocator/sorter/drf/sorter.cpp 
7df4dd641b21ea0705368861bf4679fed1ef078d 
  src/master/allocator/sorter/sorter.hpp 
f5f0b086cb95eb2ab70b3f67e5b20814925bf702 
  src/tests/sorter_tests.cpp bdd4355bfcd7b1fa1c22983f8e0ee6f20906917a 

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


Testing
---

make check

benchmarks: with 1000 clients, sort time is reduced by 72 us


Thanks,

Benjamin Mahler

Re: Review Request 49843: Added benchmark test for sorter.

[Benjamin Mahler]

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


Fix it, then Ship it!





src/tests/sorter_tests.cpp (lines 504 - 542)


It looks like we can simplify these two functions down to the following 
single function?

```
// Returns a "ports" resource with the number of ranges
// specified as: [1-2, 4-5, 7-8, 10-11, ...]
static Resource makePortRanges(size_t numRanges)
{
  ::mesos::Value::Ranges ranges;

  for (size_t i = 0; i < numRanges; ++i) {
Value::Range* range = ranges.add_range();
range->set_begin((3 * i) + 1);
range->set_end(range->begin() + 1);
  }

  Value value;
  value.set_type(Value::RANGES);
  value.mutable_ranges()->CopyFrom(ranges);

  Resource resource;
  resource.set_role("*");
  resource.set_name("ports");
  resource.set_type(Value::RANGES);
  resource.mutable_ranges()->CopyFrom(value.ranges());

  return resource;
}
```

Since we only care about the number of ranges, we don't need to specify the 
bounds, right?



src/tests/sorter_tests.cpp (line 505)


For counts like 'numRanges', we'll use size_t instead of unsigned.



src/tests/sorter_tests.cpp (line 511)


Is this an optimization?



src/tests/sorter_tests.cpp (line 514)


* goes against the type instead of the variable name



src/tests/sorter_tests.cpp (lines 515 - 516)


Why unsigned when these are actually uint64_t?



src/tests/sorter_tests.cpp (line 536)


We tend to use 'unsigned int' rather than just 'unsigned'.



src/tests/sorter_tests.cpp (line 555)


Need a using statement for std::vector, this is only compiling because of 
the subprocess headers leaking using statements.



src/tests/sorter_tests.cpp (line 577)


Seems we should move this down to at the start of the loop to avoid timing 
the resource parsing?



src/tests/sorter_tests.cpp (line 595)


No need for the trailing semi-colon after disk?



src/tests/sorter_tests.cpp (lines 600 - 603)


Oh.. this isn't allocating resources on all of the agents! The following 
will do it:

```
  watch.start();
  size_t clientIndex = 0;
  foreach (const SlaveID& slaveId, agents) {
const string& client = clients[clientIndex++ % clients.size()];
sorter.allocated(client, slaveId, allocated);
  }
  watch.stop();
```

Note that this makes things slower, as expected.



src/tests/sorter_tests.cpp (lines 607 - 610)


Let's add an additional call to sort to see how much time we spend copying 
all the clients out to the caller.


- Benjamin Mahler


On July 12, 2016, 9:09 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49843/
> ---
> 
> (Updated July 12, 2016, 9:09 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Klaus Ma.
> 
> 
> Bugs: MESOS-5701
> https://issues.apache.org/jira/browse/MESOS-5701
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added benchmark test for sorter.
> 
> 
> Diffs
> -
> 
>   src/tests/sorter_tests.cpp 20e42419934e81b97676569876cd63ee0a550da3 
> 
> Diff: https://reviews.apache.org/r/49843/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
>  
>  ./bin/mesos-tests.sh --benchmark 
> --gtest_filter="AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/*"
> [==] Running 36 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 36 tests from AgentAndClientCount/Sorter_BENCHMARK_Test
> [ RUN  ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/0
> Using 1000 agents and 1 clients
> Added 1 clients in 1047us
> Added 1000 agents in 30147us
> Sorted 1 clients in 87us
> [   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/0 (33 ms)
> [ RUN  ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/1
> Using 1000 agents and 50 clients
> Added 50 clients in 884us
> Added 1000 agents in 30129us
> Sorted 50 clients in 1284us
> [   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/1 (37 ms)
> [ RUN  ] AgentAndClientCount/Sorter_BENCHMARK_T

Re: Review Request 49914: Improved the speed of 'MasterAPITest.UnreserveResources'.

[Abhishek Dasgupta]

> On July 13, 2016, 10:09 a.m., Neil Conway wrote:
> > src/tests/api_tests.cpp, line 95
> > 
> >
> > Rather than changing this for all tests implicitly, I'd prefer creating 
> > a `master::Flags` and changing the allocation interval explicitly in the 
> > tests that benefit from it.

Ok.. I was working over this. Do you mean to say to write a createMasterFlags() 
in tests/mesos.hpp as inline function and use that instead of current virtual 
function? It needs a long streak of small changes in many of the current tests.


- Abhishek


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


On July 11, 2016, 7:39 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49914/
> ---
> 
> (Updated July 11, 2016, 7:39 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Neil Conway, and Vinod Kone.
> 
> 
> Bugs: MESOS-5732
> https://issues.apache.org/jira/browse/MESOS-5732
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Overloaded createMasterFlags in MasterAPITest to increase
> the speed of 'MasterAPITest.ReserveResources' and
> 'MasterAPITest.UnreserveResources'.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp 55e825ea6a3bd43c76dc67e8b90a97e8c9530a47 
> 
> Diff: https://reviews.apache.org/r/49914/diff/
> 
> 
> Testing
> ---
> 
> On Ubuntu 16.04:
> sudo make -j4 check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 49855: Enabled cgroups unified isolator in isolation.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [49855, 49854, 49853, 45573, 49852, 49851, 49850, 49849, 
49828, 49827, 49825, 49824, 49823, 49821, 49820, 49819, 49817, 49814, 50038]

Failed command: ./support/apply-review.sh -n -r 49851

Error:
2016-07-14 18:51:01 URL:https://reviews.apache.org/r/49851/diff/raw/ 
[23799/23799] -> "49851.patch" [1]
src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp:177:  Weird 
number of spaces at line-start.  Are you using a 2-space indent?  
[whitespace/indent] [3]
Total errors found: 1
Checking 4 files

Full log: https://builds.apache.org/job/mesos-reviewbot/14317/console

- Mesos ReviewBot


On July 14, 2016, 5:30 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49855/
> ---
> 
> (Updated July 14, 2016, 5:30 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Bugs: MESOS-5652
> https://issues.apache.org/jira/browse/MESOS-5652
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled cgroups unified isolator in isolation.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> e900d752b70165236a198f70c8cb24876a678e4b 
> 
> Diff: https://reviews.apache.org/r/49855/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 50002: Allow all flags load methods to specify a prefix.

[Ammar Askar]

> On July 14, 2016, 4:44 p.m., Greg Mann wrote:
> > 3rdparty/stout/tests/flags_tests.cpp, line 546
> > 
> >
> > Another option would be to scope the flag loading/checking so that you 
> > can use the same variable name for each map:
> > ```cpp
> > {
> >   flags = TestFlags();
> >   std::map> values;
> >   values["name1"] = "billy joel";
> >   values["name2"] = "51";
> > 
> >   // load(map, unknowns, prefix)
> >   Try load = flags.load(values, false, "FLAGSTEST_");
> >   
> >   EXPECT_SOME(load);
> >   EXPECT_EQ(0, load->warnings.size());
> > 
> >   EXPECT_EQ(flags.name1, "billy joel");
> >   EXPECT_EQ(flags.name2, 51);
> > 
> >   EXPECT_SOME(load);
> >   EXPECT_EQ(0, load->warnings.size());
> > 
> >   EXPECT_EQ(flags.name1, "billy joel");
> >   EXPECT_EQ(flags.name2, 51);
> > }
> > ```

This sounds good, it also helps seperate which method is being tested than 
whitespace.


- Ammar


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


On July 13, 2016, 8:53 p.m., Ammar Askar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50002/
> ---
> 
> (Updated July 13, 2016, 8:53 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Bugs: MESOS-5613
> https://issues.apache.org/jira/browse/MESOS-5613
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow all flags load methods to specify a prefix.
> 
> This also refactors the prefix logic into one place, so that's nice.
> Required for the actual fix for passing work_dir in local.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/flags/flags.hpp 
> dd9362772d1fbd32638fc7e70126fd49d4a03c68 
>   3rdparty/stout/tests/flags_tests.cpp 
> 77f3a6af110da1ffcdf2b7ab2b66431a6b5c91d3 
> 
> Diff: https://reviews.apache.org/r/50002/diff/
> 
> 
> Testing
> ---
> 
> Added additional tests to flags_tests.cpp to ensure that prefix works on the 
> other methods.
> 
> `make check` passes
> 
> 
> Thanks,
> 
> Ammar Askar
> 
>

Review Request 50041: Supported docker container running as a defined user.

[Gilbert Song]

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

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


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


Repository: mesos


Description
---

This patch allows the container launched by docker containerizer
to run as a user defined user (either the user from FrameworkInfo
or CommandInfo).

Important NOTE:
1. This is a behavior change, since the agent flag 'switch_user'
   defaults to be true. So the docker container will defaultly
   run as the framework/command user.
2. The user has to be existed in both host machine and the
   docker image. Otherwise, either the host would fail to
   chown to a non-existed user, or the docker daemon cannot
   start the container due to unable to find the user.


Diffs
-

  src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
  src/docker/docker.cpp 515842d381ca8a91ad481f66c7be057dff2f3f28 
  src/docker/executor.hpp 7b63d784d6b8685912598b77fb38cf6e70646ae3 
  src/docker/executor.cpp 69511044e39bc05d7d6240264ec70b6e6f44edba 
  src/slave/containerizer/docker.cpp f1ecf3b25d85597f6c3dcaa47968860ed119dbd5 
  src/tests/containerizer/docker_tests.cpp 
7b73a4939da33d0b7dd08fdcf56525b6403eada3 
  src/tests/mesos.hpp e4eccfc3810bed3649a3ab80e252849470de4c72 

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


Testing
---

make check

sudo ./bin/mesos-tests.sh

Manually tested by /src/mesos-execute with CommandInfo.user set as `nobody` 
using docker image `alpine`.


Thanks,

Gilbert Song

Re: Review Request 49689: Added Appc runtime isolator tests.

[Mesos ReviewBot]

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



Bad review!

Reviews applied: [49689]

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

- Mesos ReviewBot


On July 14, 2016, 5:28 p.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49689/
> ---
> 
> (Updated July 14, 2016, 5:28 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-4778
> https://issues.apache.org/jira/browse/MESOS-4778
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Appc runtime isolator tests.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9f2b0585fa901918431530b967b6bbb0f286c595 
>   src/tests/containerizer/appc_archive.hpp PRE-CREATION 
>   src/tests/containerizer/appc_runtime_isolator_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/49689/diff/
> 
> 
> Testing
> ---
> 
> Make check
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 50017: WIP: Validated the resources when parsing it.

[Benjamin Mahler]

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




include/mesos/resources.hpp (lines 135 - 147)


This suggests a change of semantics to `Resources`.

Currently: `Resources` always refers to a valid collection resources. This 
function would never return Some(Error) unless we change the semantics.

If we want to remove the unnecessary validation, we can do this by making 
'`operator += (const Resources& that)`' avoid validating '`that`' since it's 
already valid. Currently it just calls '`operator += (const Resource& r)`' 
which is unaware of '`r`' already being valid since it comes from a 
'`Resources`'.

However, we have to keep the validation in '`operator += (const Resource& 
r)`', since '`r`' is just a protobuf coming from an arbitrary caller, it may be 
invalid and we need to validate.

Make sense?


- Benjamin Mahler


On July 14, 2016, 3:59 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50017/
> ---
> 
> (Updated July 14, 2016, 3:59 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Klaus Ma.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The "validation" API was called in a huge number of times, but this
> was only needed when parsing resources and we do not need to do
> other validation when doing resources operations, such as add etc.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp a557e97c65194d4aad879fb88d8edefd1c95b8d8 
>   include/mesos/v1/resources.hpp a5ba8fec4c9c3643646308f75a4b28cefe0b3df3 
>   src/common/resources.cpp f6ff92b591c15bc8e93fd85e1896349c3a7bb968 
>   src/v1/resources.cpp 8c3f2d1c1529915a59d47fe37bb3fc7a3267079a 
> 
> Diff: https://reviews.apache.org/r/50017/diff/
> 
> 
> Testing
> ---
> 
> The `qcachegrind` result here: 
> https://docs.google.com/document/d/1oilen04e8trIOgYbj-jxAd7esTRDaySll8y2BQ6p5nU/edit?usp=sharing
> 
> After the fix, the performance of `adding` resources increased 30+% when 
> adding 5 agents to the cluster.
> 
> Before fix:
> [ RUN  ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/30
> Using 5 agents and 1 clients
> Added 1 clients in 47us
> **Added 5 agents in 1.312497secs**
> Sorted 1 clients in 43us
> [   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/30 (1321 ms)
> [ RUN  ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/31
> Using 5 agents and 50 clients
> Added 50 clients in 948us
> **Added 5 agents in 1.325987secs**
> Sorted 50 clients in 1165us
> [   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/31 (1340 ms)
> [ RUN  ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/32
> Using 5 agents and 100 clients
> Added 100 clients in 1697us
> **Added 5 agents in 1.409478secs**
> Sorted 100 clients in 2876us
> [   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/32 (1432 ms)
> [ RUN  ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/33
> Using 5 agents and 200 clients
> Added 200 clients in 4553us
> **Added 5 agents in 1.371473secs**
> Sorted 200 clients in 5371us
> [   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/33 (1412 ms)
> [ RUN  ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/34
> Using 5 agents and 500 clients
> Added 500 clients in 8836us
> **Added 5 agents in 1.304245secs**
> Sorted 500 clients in 14697us
> [   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/34 (1387 ms)
> [ RUN  ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/35
> Using 5 agents and 1000 clients
> Added 1000 clients in 19508us
> **Added 5 agents in 1.270555secs**
> Sorted 1000 clients in 32575us
> [   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/35 (1433 ms)
> 
> 
> After the fix:
> [ RUN  ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/30
> Using 5 agents and 1 clients
> Added 1 clients in 42us
> **Added 5 agents in 891266us**
> Sorted 1 clients in 59us
> [   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/30 (902 ms)
> [ RUN  ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/31
> Using 5 agents and 50 clients
> Added 50 clients in 933us
> **Added 5 agents in 885006us**
> Sorted 50 clients in 1220us
> [   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/31 (899 ms)
> [ RUN  ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/32
> Using 5 agents and 100 clients
> Added 100 clients in 1879us
> **Added 5 agents in 903112us**
> Sorted 100 clients in 2800us
> [   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/32 (922 ms)
> [ RUN  ] A

Re: Review Request 50003: Propagate work_dir flag from local runs to agents/masters.

[Greg Mann]

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




src/local/flags.hpp (line 36)


This keeps the newlines here consistent with other help strings, but it 
does look a bit messy. Perhaps we should move the newlines to accommodate the 
line wrapping here?



src/local/flags.hpp (line 39)


s/masters/masters and agents/



src/local/flags.hpp (line 41)


`/var/lib/mesos` might be more appropriate here, since this isn't master or 
agent specific?


- Greg Mann


On July 13, 2016, 8:54 p.m., Ammar Askar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50003/
> ---
> 
> (Updated July 13, 2016, 8:54 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Bugs: MESOS-5613
> https://issues.apache.org/jira/browse/MESOS-5613
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Propagate work_dir flag from local runs to agents/masters.
> 
> 
> Diffs
> -
> 
>   src/local/flags.hpp f0af0d251e004f305726e7e4fe7941c2d4081183 
>   src/local/local.cpp a543aef117fea62660d55435be4d66d30f8ee860 
> 
> Diff: https://reviews.apache.org/r/50003/diff/
> 
> 
> Testing
> ---
> 
> Manually tested that `mesos local` and `mesos-local` run now. Not sure if a 
> regression test for this would be good, please advise.
> 
> 
> Thanks,
> 
> Ammar Askar
> 
>

Re: Review Request 49855: Enabled cgroups unified isolator in isolation.

[haosdent huang]

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

(Updated July 14, 2016, 5:30 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Enabled cgroups unified isolator in isolation.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
e900d752b70165236a198f70c8cb24876a678e4b 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49854: Implemented `DevicesSubsystem`.

[haosdent huang]

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

(Updated July 14, 2016, 5:30 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `DevicesSubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
f677da3035181855f6490e3e516d13963ec43b51 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
64fca3c8776248fcbf2d5605966db60952149b2f 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 45573: Add `PerfEventHandleManager`.

[haosdent huang]

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

(Updated July 14, 2016, 5:30 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Add `PerfEventHandleManager`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/perf_event.hpp 
1de38491a33e5782b20e5ee491412779d09ee391 
  src/slave/containerizer/mesos/isolators/cgroups/perf_event.cpp 
31f35385691681ef5da14be747edfb5f57c5d05a 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49853: Implemented `PerfEventSubsystem`.

[haosdent huang]

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

(Updated July 14, 2016, 5:30 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `PerfEventSubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
f677da3035181855f6490e3e516d13963ec43b51 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
64fca3c8776248fcbf2d5605966db60952149b2f 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49852: Implemented `NetClsSubsystem`.

[haosdent huang]

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

(Updated July 14, 2016, 5:29 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `NetClsSubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
f677da3035181855f6490e3e516d13963ec43b51 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
64fca3c8776248fcbf2d5605966db60952149b2f 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49851: Implemented `MemorySubsystem`.

[haosdent huang]

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

(Updated July 14, 2016, 5:29 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `MemorySubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
c57baec88437f68886702a40ec8a6a6458546119 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
f677da3035181855f6490e3e516d13963ec43b51 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
64fca3c8776248fcbf2d5605966db60952149b2f 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49232: Added appcManifest to ImageInfo and ProvisionInfo.

[Srinivas Brahmaroutu]

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

(Updated July 14, 2016, 5:28 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Added appcManifest to ImageInfo and ProvisionInfo.


Diffs (updated)
-

  src/slave/containerizer/mesos/containerizer.cpp 
e900d752b70165236a198f70c8cb24876a678e4b 
  src/slave/containerizer/mesos/provisioner/appc/store.cpp 
aaa0efe63e587b9e604082b52a3cb8c11545fbb9 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
48a05059969e068a0ee0d38b61be9e7104e3188d 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
6825742546a87c8148097e6e13a94592e3b6754e 
  src/slave/containerizer/mesos/provisioner/store.hpp 
1d477ef13ddd24fd8badae0decaa4a2271ecc746 

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


Testing
---

Make Check.


Thanks,

Srinivas Brahmaroutu

Re: Review Request 49348: Added implementation to Appc Runtime Isolator.

[Srinivas Brahmaroutu]

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

(Updated July 14, 2016, 5:27 p.m.)


Review request for mesos.


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


Repository: mesos


Description
---

Added implementation to Appc Runtime Isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/appc/runtime.cpp PRE-CREATION 

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


Testing
---

make check


Thanks,

Srinivas Brahmaroutu

Re: Review Request 49689: Added Appc runtime isolator tests.

[Srinivas Brahmaroutu]

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

(Updated July 14, 2016, 5:28 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

Added Appc runtime isolator tests.


Diffs (updated)
-

  src/Makefile.am 9f2b0585fa901918431530b967b6bbb0f286c595 
  src/tests/containerizer/appc_archive.hpp PRE-CREATION 
  src/tests/containerizer/appc_runtime_isolator_tests.cpp PRE-CREATION 

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


Testing
---

Make check


Thanks,

Srinivas Brahmaroutu

Re: Review Request 49936: Added metric types in libprocess.

[Tuan-Anh Hoang-Vu]

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

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


Review request for mesos, Benjamin Mahler and Vinod Kone.


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


Repository: mesos


Description
---

Added metric types in libprocess.


Diffs (updated)
-

  3rdparty/libprocess/include/process/metrics/counter.hpp 
a13cc7e18c8b23eae83c326d63874d9d2aaedc0d 
  3rdparty/libprocess/include/process/metrics/gauge.hpp 
c5bbeaf5d53c60f3636d1778db47cdb6cdda34e8 
  3rdparty/libprocess/include/process/metrics/metric.hpp 
21f162d5b7d9e56dc3289d65b6d86deb4c2fa721 
  3rdparty/libprocess/include/process/metrics/timer.hpp 
0a9c0227c457c6c81a59f65f901a5464ee00983d 
  3rdparty/libprocess/src/tests/metrics_tests.cpp 
5a82f4f49aecd03d12687de629516be5b7895036 

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


Testing
---

Modified MetricsTest.Counter, MetricsTest.Gauge and MetricsTest.Timer to make 
sure we set metric types correctly when creating them.


Thanks,

Tuan-Anh Hoang-Vu

Re: Review Request 49850: Implemented `CpuacctSubsystem`.

[haosdent huang]

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

(Updated July 14, 2016, 5:27 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `CpuacctSubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
f677da3035181855f6490e3e516d13963ec43b51 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
64fca3c8776248fcbf2d5605966db60952149b2f 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49849: Implemented `CpuSubsystem`.

[haosdent huang]

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

(Updated July 14, 2016, 5:25 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `CpuSubsystem`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
f677da3035181855f6490e3e516d13963ec43b51 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
64fca3c8776248fcbf2d5605966db60952149b2f 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49821: Implemented `CgroupsIsolatorProcess::watch`.

[haosdent huang]

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

(Updated July 14, 2016, 5:16 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `CgroupsIsolatorProcess::watch`.


Diffs (updated)
-

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

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 50038: Removed unnecessary forward declaration in subsystem.hpp.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On July 14, 2016, 10:11 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50038/
> ---
> 
> (Updated July 14, 2016, 10: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
> ---
> 
> Removed unnecessary forward declaration in subsystem.hpp.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> f677da3035181855f6490e3e516d13963ec43b51 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> 64fca3c8776248fcbf2d5605966db60952149b2f 
> 
> Diff: https://reviews.apache.org/r/50038/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

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

[haosdent huang]

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

(Updated July 14, 2016, 5:19 p.m.)


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


Changes
---

Rebase.


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 
64fca3c8776248fcbf2d5605966db60952149b2f 

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


Testing
---


Thanks,

haosdent huang

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

[haosdent huang]

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

(Updated July 14, 2016, 5:18 p.m.)


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


Changes
---

Rebase.


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.hpp 
c57baec88437f68886702a40ec8a6a6458546119 
  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`.

[haosdent huang]

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

(Updated July 14, 2016, 5:17 p.m.)


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


Changes
---

Rebase.


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.hpp 
c57baec88437f68886702a40ec8a6a6458546119 
  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`.

[haosdent huang]

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

(Updated July 14, 2016, 5:18 p.m.)


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


Changes
---

Rebase.


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 49823: Implemented `CgroupsIsolatorProcess::update`.

[haosdent huang]

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

(Updated July 14, 2016, 5:17 p.m.)


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


Changes
---

Rebase.


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.hpp 
c57baec88437f68886702a40ec8a6a6458546119 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 

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


Testing
---


Thanks,

haosdent huang

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

[haosdent huang]

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

(Updated July 14, 2016, 5:15 p.m.)


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


Changes
---

Rebase.


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.hpp 
c57baec88437f68886702a40ec8a6a6458546119 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4a9f55bf3b217405bf90943f27a976422877a99e 

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


Testing
---


Thanks,

haosdent huang

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

[haosdent huang]

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

(Updated July 14, 2016, 5:15 p.m.)


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


Changes
---

Rebase.


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 49816: Implemented `CgroupsIsolatorProcess::initialize`.

[haosdent huang]

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

(Updated July 14, 2016, 5:13 p.m.)


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


Repository: mesos


Description
---

Implemented `CgroupsIsolatorProcess::initialize`.


Diffs
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49817: Implemented `CgroupsIsolatorProcess::recover`.

[haosdent huang]

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

(Updated July 14, 2016, 5:14 p.m.)


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


Changes
---

Rebase.


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


Repository: mesos


Description
---

Implemented `CgroupsIsolatorProcess::recover`.


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/49817/diff/


Testing
---


Thanks,

haosdent huang

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

[haosdent huang]

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

(Updated July 14, 2016, 5:12 p.m.)


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


Changes
---

Update ticket field.


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.hpp 
c57baec88437f68886702a40ec8a6a6458546119 
  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`.

[haosdent huang]

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

(Updated July 14, 2016, 5:12 p.m.)


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


Changes
---

Rebase.


Repository: mesos


Description
---

Implemented `CgroupsIsolatorProcess::create`.


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/49814/diff/


Testing
---


Thanks,

haosdent huang

Review Request 50038: Removed unnecessary forward declaration in subsystem.hpp.

[haosdent huang]

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

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

Removed unnecessary forward declaration in subsystem.hpp.


Diffs
-

  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
f677da3035181855f6490e3e516d13963ec43b51 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
64fca3c8776248fcbf2d5605966db60952149b2f 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49813: Added stubs for the unified cgroups isolator.

[haosdent huang]

> On July 10, 2016, 9:50 a.m., Qian Zhang wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp, lines 36-37
> > 
> >
> > Why do you do the forward declaration rather than include cgroups.hpp?
> 
> haosdent huang wrote:
> Because we include 
> `slave/containerizer/mesos/isolators/cgroups/subsystem.hpp` in 
> `src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp`. If we include 
> `src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp` in 
> `slave/containerizer/mesos/isolators/cgroups/subsystem.hpp`, it would bring 
> circular dependencies. So I forward declaration here.

Because we use callback now, we don't need include `cgroups.hpp` in 
`subsystem.hpp` which bring circular dependencies.


- haosdent


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


On July 14, 2016, 4:43 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49813/
> ---
> 
> (Updated July 14, 2016, 4:43 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Qian Zhang, and Zhengju Sha.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added stubs for the unified cgroups isolator.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 9d8a8ba674f0bf2d081acf0447dc2de28cfcf010 
>   src/Makefile.am 1d798d6c02b97c15e15d8216158676b91cbccaff 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/49813/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

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

[haosdent huang]

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

(Updated July 14, 2016, 4:44 p.m.)


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


Changes
---

Rebase.


Repository: mesos


Description
---

Implemented `CgroupsIsolatorProcess::create`.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 50002: Allow all flags load methods to specify a prefix.

[Greg Mann]

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




3rdparty/stout/include/stout/flags/flags.hpp (lines 905 - 906)


Might want to either remove the comment about command-line flags, or expand 
it to point out that the other overloads handle command-line arguments and then 
forward them to this function.



3rdparty/stout/tests/flags_tests.cpp (line 546)


Another option would be to scope the flag loading/checking so that you can 
use the same variable name for each map:
```cpp
{
  flags = TestFlags();
  std::map> values;
  values["name1"] = "billy joel";
  values["name2"] = "51";

  // load(map, unknowns, prefix)
  Try load = flags.load(values, false, "FLAGSTEST_");
  
  EXPECT_SOME(load);
  EXPECT_EQ(0, load->warnings.size());

  EXPECT_EQ(flags.name1, "billy joel");
  EXPECT_EQ(flags.name2, 51);

  EXPECT_SOME(load);
  EXPECT_EQ(0, load->warnings.size());

  EXPECT_EQ(flags.name1, "billy joel");
  EXPECT_EQ(flags.name2, 51);
}
```


- Greg Mann


On July 13, 2016, 8:53 p.m., Ammar Askar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50002/
> ---
> 
> (Updated July 13, 2016, 8:53 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Bugs: MESOS-5613
> https://issues.apache.org/jira/browse/MESOS-5613
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allow all flags load methods to specify a prefix.
> 
> This also refactors the prefix logic into one place, so that's nice.
> Required for the actual fix for passing work_dir in local.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/flags/flags.hpp 
> dd9362772d1fbd32638fc7e70126fd49d4a03c68 
>   3rdparty/stout/tests/flags_tests.cpp 
> 77f3a6af110da1ffcdf2b7ab2b66431a6b5c91d3 
> 
> Diff: https://reviews.apache.org/r/50002/diff/
> 
> 
> Testing
> ---
> 
> Added additional tests to flags_tests.cpp to ensure that prefix works on the 
> other methods.
> 
> `make check` passes
> 
> 
> Thanks,
> 
> Ammar Askar
> 
>

Re: Review Request 49813: Added stubs for the unified cgroups isolator.

[haosdent huang]

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

(Updated July 14, 2016, 4:42 p.m.)


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


Changes
---

Address comments.


Repository: mesos


Description
---

Added stubs for the unified cgroups isolator.


Diffs (updated)
-

  src/CMakeLists.txt 9d8a8ba674f0bf2d081acf0447dc2de28cfcf010 
  src/Makefile.am 1d798d6c02b97c15e15d8216158676b91cbccaff 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp PRE-CREATION 

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


Testing
---


Thanks,

haosdent huang

Re: Review Request 49812: Added missing header `version.hpp` in `src/linux/perf.hpp`.

[haosdent huang]

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

(Updated July 14, 2016, 4:42 p.m.)


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


Changes
---

Rebase.


Repository: mesos


Description
---

`version.hpp` is required in `perf.hpp`, otherwise compile would failed
if include `perf.hpp` without `version.hpp`.


Diffs (updated)
-

  src/linux/perf.hpp 674d5f886ea41b939a8e48832ee6595a78b2f6ce 

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


Testing
---


Thanks,

haosdent huang

Review Request 50017: WIP: Validated the resources when parsing it.

[Guangya Liu]

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

Review request for mesos, Benjamin Mahler and Klaus Ma.


Repository: mesos


Description
---

The "validation" API was called in a huge number of times, but this
was only needed when parsing resources and we do not need to do
other validation when doing resources operations, such as add etc.


Diffs
-

  include/mesos/resources.hpp a557e97c65194d4aad879fb88d8edefd1c95b8d8 
  include/mesos/v1/resources.hpp a5ba8fec4c9c3643646308f75a4b28cefe0b3df3 
  src/common/resources.cpp f6ff92b591c15bc8e93fd85e1896349c3a7bb968 
  src/v1/resources.cpp 8c3f2d1c1529915a59d47fe37bb3fc7a3267079a 

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


Testing
---

The `qcachegrind` result here: 
https://docs.google.com/document/d/1oilen04e8trIOgYbj-jxAd7esTRDaySll8y2BQ6p5nU/edit?usp=sharing

After the fix, the performance of `adding` resources increased 30+% when adding 
5 agents to the cluster.

Before fix:
[ RUN  ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/30
Using 5 agents and 1 clients
Added 1 clients in 47us
**Added 5 agents in 1.312497secs**
Sorted 1 clients in 43us
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/30 (1321 ms)
[ RUN  ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/31
Using 5 agents and 50 clients
Added 50 clients in 948us
**Added 5 agents in 1.325987secs**
Sorted 50 clients in 1165us
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/31 (1340 ms)
[ RUN  ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/32
Using 5 agents and 100 clients
Added 100 clients in 1697us
**Added 5 agents in 1.409478secs**
Sorted 100 clients in 2876us
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/32 (1432 ms)
[ RUN  ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/33
Using 5 agents and 200 clients
Added 200 clients in 4553us
**Added 5 agents in 1.371473secs**
Sorted 200 clients in 5371us
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/33 (1412 ms)
[ RUN  ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/34
Using 5 agents and 500 clients
Added 500 clients in 8836us
**Added 5 agents in 1.304245secs**
Sorted 500 clients in 14697us
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/34 (1387 ms)
[ RUN  ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/35
Using 5 agents and 1000 clients
Added 1000 clients in 19508us
**Added 5 agents in 1.270555secs**
Sorted 1000 clients in 32575us
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/35 (1433 ms)


After the fix:
[ RUN  ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/30
Using 5 agents and 1 clients
Added 1 clients in 42us
**Added 5 agents in 891266us**
Sorted 1 clients in 59us
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/30 (902 ms)
[ RUN  ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/31
Using 5 agents and 50 clients
Added 50 clients in 933us
**Added 5 agents in 885006us**
Sorted 50 clients in 1220us
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/31 (899 ms)
[ RUN  ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/32
Using 5 agents and 100 clients
Added 100 clients in 1879us
**Added 5 agents in 903112us**
Sorted 100 clients in 2800us
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/32 (922 ms)
[ RUN  ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/33
Using 5 agents and 200 clients
Added 200 clients in 3893us
**Added 5 agents in 881240us**
Sorted 200 clients in 5802us
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/33 (912 ms)
[ RUN  ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/34
Using 5 agents and 500 clients
Added 500 clients in 10712us
**Added 5 agents in 877442us**
Sorted 500 clients in 17887us
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/34 (949 ms)
[ RUN  ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/35
Using 5 agents and 1000 clients
Added 1000 clients in 21472us
**Added 5 agents in 916653us**
Sorted 1000 clients in 37369us
[   OK ] AgentAndClientCount/Sorter_BENCHMARK_Test.FullSort/35 (1057 ms)


Thanks,

Guangya Liu

Re: Review Request 42441: Speeded up the `ExamplesTest.*` test cases.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [50026, 42441]

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 14, 2016, 9:58 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42441/
> ---
> 
> (Updated July 14, 2016, 9:58 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Jian Qiu.
> 
> 
> Bugs: MESOS-4155
> https://issues.apache.org/jira/browse/MESOS-4155
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This lowers the scheduler authentication timeout to speed up the
> `ExamplesTest.*` test cases. Because the master may drop the
> authentication message from the scheduler while it is recovering,
> lowering the authentication timeout could speed up the scheduler
> retries to connect the master.
> 
> 
> Diffs
> -
> 
>   src/tests/dynamic_reservation_framework_test.sh 
> 448dfee8704e39a552fbe2340de6fd1be20b5950 
>   src/tests/java_exception_test.sh fd02322096d09cee6cc22340f18681ad372867b5 
>   src/tests/java_framework_test.sh 8fccc699133c6db6130153c367ce01ba3931c1a2 
>   src/tests/no_executor_framework_test.sh 
> 4fa154ee52454fe3f56161abaa29e58793e627e3 
>   src/tests/persistent_volume_framework_test.sh 
> 074cdcbc4a738170e84887c1773cd7c118112d58 
>   src/tests/python_framework_test.sh dc3a50f4fe8d3340429996622e62f732941985a2 
>   src/tests/test_framework_test.sh 617ca5283bd626b535d0f8091bd1d622d175f6d8 
>   src/tests/test_http_framework_test.sh 
> 0e04ff689d008dea5d2e565b0c58096ecdd24b08 
> 
> Diff: https://reviews.apache.org/r/42441/diff/
> 
> 
> Testing
> ---
> 
> Repeat test in CentOS 7.1
> ```
> $ sudo ./bin/mesos-tests.sh --gtest_filter="ExamplesTest.*" 
> --gtest_repeat=200 --gtest_break_on_failure
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 50022: Fixed incorrected command in documentation.

[Joerg Schad]

> On July 14, 2016, 10:35 a.m., Guangya Liu wrote:
> > I think we should not merge this, the mesos-execute should use mesos 
> > containerizer but not docker containerizer here, comments?
> 
> Joerg Schad wrote:
> Good point. I was mostly concerned with the command not working. I will 
> check with containerizers=mesos and then update.
> 
> Guangya Liu wrote:
> The default containerizer in mesos-execute is `mesos`, so you do not need 
> to specify it in the command. Adding `containerizr=docker` here will cause 
> task launch failed cause the agent default `containerizer` is `mesos`.

Ok, asked alex to revert the commit. Thanks for checking.


- Joerg


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


On July 14, 2016, 8:32 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50022/
> ---
> 
> (Updated July 14, 2016, 8:32 a.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Command was not working as specified, we need
> to specify the containerizer parameter.
> 
> 
> Diffs
> -
> 
>   docs/container-image.md 745f6b045d11de6f54323347ebadac9e3ae6cab2 
> 
> Diff: https://reviews.apache.org/r/50022/diff/
> 
> 
> Testing
> ---
> 
> Viewed via gist: 
> https://gist.github.com/joerg84/aec465f898b9186aa3c26acad7671d20
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 50022: Fixed incorrected command in documentation.

[Guangya Liu]

> On 七月 14, 2016, 10:35 a.m., Guangya Liu wrote:
> > I think we should not merge this, the mesos-execute should use mesos 
> > containerizer but not docker containerizer here, comments?
> 
> Joerg Schad wrote:
> Good point. I was mostly concerned with the command not working. I will 
> check with containerizers=mesos and then update.

The default containerizer in mesos-execute is `mesos`, so you do not need to 
specify it in the command. Adding `containerizr=docker` here will cause task 
launch failed cause the agent default `containerizer` is `mesos`.


- Guangya


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


On 七月 14, 2016, 8:32 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50022/
> ---
> 
> (Updated 七月 14, 2016, 8:32 a.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Command was not working as specified, we need
> to specify the containerizer parameter.
> 
> 
> Diffs
> -
> 
>   docs/container-image.md 745f6b045d11de6f54323347ebadac9e3ae6cab2 
> 
> Diff: https://reviews.apache.org/r/50022/diff/
> 
> 
> Testing
> ---
> 
> Viewed via gist: 
> https://gist.github.com/joerg84/aec465f898b9186aa3c26acad7671d20
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 50022: Fixed incorrected command in documentation.

[Joerg Schad]

> On July 14, 2016, 10:35 a.m., Guangya Liu wrote:
> > I think we should not merge this, the mesos-execute should use mesos 
> > containerizer but not docker containerizer here, comments?

Good point. I was mostly concerned with the command not working. I will check 
with containerizers=mesos and then update.


- Joerg


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


On July 14, 2016, 8:32 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50022/
> ---
> 
> (Updated July 14, 2016, 8:32 a.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Command was not working as specified, we need
> to specify the containerizer parameter.
> 
> 
> Diffs
> -
> 
>   docs/container-image.md 745f6b045d11de6f54323347ebadac9e3ae6cab2 
> 
> Diff: https://reviews.apache.org/r/50022/diff/
> 
> 
> Testing
> ---
> 
> Viewed via gist: 
> https://gist.github.com/joerg84/aec465f898b9186aa3c26acad7671d20
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 49936: Added metric types in libprocess.

[Abhishek Dasgupta]

> On July 14, 2016, 8:26 a.m., Abhishek Dasgupta wrote:
> > 3rdparty/libprocess/include/process/metrics/metric.hpp, line 74
> > 
> >
> > We may modify it to data(new Data(name, UNKNOWN, window) if we follow 
> > the below comment of mine.

*if we follow the above comment of mine.


- Abhishek


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


On July 12, 2016, 2:02 a.m., Tuan-Anh Hoang-Vu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49936/
> ---
> 
> (Updated July 12, 2016, 2:02 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-5731
> https://issues.apache.org/jira/browse/MESOS-5731
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added metric types in libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/metrics/counter.hpp 
> a13cc7e18c8b23eae83c326d63874d9d2aaedc0d 
>   3rdparty/libprocess/include/process/metrics/gauge.hpp 
> c5bbeaf5d53c60f3636d1778db47cdb6cdda34e8 
>   3rdparty/libprocess/include/process/metrics/metric.hpp 
> 21f162d5b7d9e56dc3289d65b6d86deb4c2fa721 
>   3rdparty/libprocess/include/process/metrics/timer.hpp 
> 0a9c0227c457c6c81a59f65f901a5464ee00983d 
>   3rdparty/libprocess/src/tests/metrics_tests.cpp 
> 5a82f4f49aecd03d12687de629516be5b7895036 
> 
> Diff: https://reviews.apache.org/r/49936/diff/
> 
> 
> Testing
> ---
> 
> Modified MetricsTest.Counter, MetricsTest.Gauge and MetricsTest.Timer to make 
> sure we set metric types correctly when creating them.
> 
> 
> Thanks,
> 
> Tuan-Anh Hoang-Vu
> 
>

Re: Review Request 50023: Fixed flakiness in persistent volume test case.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [50023]

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 14, 2016, 8:33 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50023/
> ---
> 
> (Updated July 14, 2016, 8:33 a.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-5844
> https://issues.apache.org/jira/browse/MESOS-5844
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> PersistentVolumeEndpointsTest.OfferCreateThenEndpointRemove. Along the
> way, cleanup some comments.
> 
> 
> Diffs
> -
> 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> 2a22f3b0da817e650a25e5e2c951adb7462718b4 
> 
> Diff: https://reviews.apache.org/r/50023/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>