Re: Review Request 67104: Fixed a race condition in the allocator metrics.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67104 was successfully built and tested.

Reviews applied: `['67104']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67104

- Mesos Reviewbot Windows


On May 12, 2018, 1:02 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67104/
> ---
> 
> (Updated May 12, 2018, 1:02 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-8904
> https://issues.apache.org/jira/browse/MESOS-8904
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch addresses a race condition in which the removal of
> a role from the allocator's quota sorter races with execution
> of a callback tied to a `PullGauge`. The gauge's callback
> assumed that the role would be present in the sorter, but it's
> possible for the role to be removed before the callback is
> executed.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 1000968be6a2935a4cac571414d7f06d7df7acf0 
> 
> 
> Diff: https://reviews.apache.org/r/67104/diff/1/
> 
> 
> Testing
> ---
> 
> The test `MasterQuotaTest.RemoveSingleQuota` was modified to include a call 
> to '/metrics/snapshot', both with and without a framework registered in the 
> role for which quota is set. It's not easy to ensure that such a test 
> provokes the race condition 100% of the time, but the test would always 
> expose it within a few iterations.
> 
> Before this patch, the test would crash reliably within several iterations. 
> After this patch, the test can be run many times with no failures.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 67099: Updated support scripts to check for Python 3.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [67055, 67059, 67099]

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

- Mesos Reviewbot


On May 11, 2018, 10:01 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67099/
> ---
> 
> (Updated May 11, 2018, 10:01 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Eric Chung, 
> and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated support scripts to check for Python 3.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py 0c744a90f63ec78badad627e90599a3e68791d5f 
>   support/check-python3 PRE-CREATION 
>   support/generate-endpoint-help.py 7e59b358aa3d2cd1d490099fc4cca2e7808ed67a 
>   support/jsonurl.py 72fe682bf7d70507621181f48375d69b1d1c073b 
>   support/mesos-gtest-runner.py 4ae5fe3fa8e936d380876d9bd1bf3f47c48bc9be 
>   support/mesos-split.py 7ef5c86d55e61b80b5a7c8f137323aa056f272ed 
>   support/mesos-style.py 07074daa245ab503cf551201ccadeac8cc10206d 
>   support/post-reviews.py a6646f2eca45f911aad2403b8b0fef3a9323 
>   support/push-commits.py cd751cbf3c7e64005f1da6c96eb33c6fc62c7ae3 
>   support/test-upgrade.py 53c068871765de702c71d1a4260558eefcfb0a34 
>   support/verify-reviews.py fbc2460051e96761c9669db001c2fa13906f0cca 
> 
> 
> Diff: https://reviews.apache.org/r/67099/diff/1/
> 
> 
> Testing
> ---
> 
> Tested manually by changing my environment variable and seeing the output of 
> the scripts.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 67101: Displayed modifications of all support scripts to Python 3.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67101 was successfully built and tested.

Reviews applied: `['67055', '67101']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67101

- Mesos Reviewbot Windows


On May 11, 2018, 2:59 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67101/
> ---
> 
> (Updated May 11, 2018, 2:59 p.m.)
> 
> 
> Review request for Andrew Schwartzmeyer.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Displayed modifications of /r/67059/
> 
> 
> Diffs
> -
> 
>   support/README.md e812e9638ca555c56e4c52e52485240e75cb8229 
>   support/apply-reviews.py 0c744a90f63ec78badad627e90599a3e68791d5f 
>   support/build-virtualenv 850af89326721f34de20eb45a7e78fa391d031be 
>   support/generate-endpoint-help.py 7e59b358aa3d2cd1d490099fc4cca2e7808ed67a 
>   support/jsonurl.py 72fe682bf7d70507621181f48375d69b1d1c073b 
>   support/mesos-gtest-runner.py 4ae5fe3fa8e936d380876d9bd1bf3f47c48bc9be 
>   support/mesos-split.py 7ef5c86d55e61b80b5a7c8f137323aa056f272ed 
>   support/mesos-style.py 07074daa245ab503cf551201ccadeac8cc10206d 
>   support/post-reviews.py a6646f2eca45f911aad2403b8b0fef3a9323 
>   support/push-commits.py cd751cbf3c7e64005f1da6c96eb33c6fc62c7ae3 
>   support/test-upgrade.py 53c068871765de702c71d1a4260558eefcfb0a34 
>   support/verify-reviews.py fbc2460051e96761c9669db001c2fa13906f0cca 
> 
> 
> Diff: https://reviews.apache.org/r/67101/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Review Request 67104: Fixed a race condition in the allocator metrics.

[Greg Mann]

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

Review request for mesos, Benjamin Mahler and Vinod Kone.


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


Repository: mesos


Description
---

This patch addresses a race condition in which the removal of
a role from the allocator's quota sorter races with execution
of a callback tied to a `PullGauge`. The gauge's callback
assumed that the role would be present in the sorter, but it's
possible for the role to be removed before the callback is
executed.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
1000968be6a2935a4cac571414d7f06d7df7acf0 


Diff: https://reviews.apache.org/r/67104/diff/1/


Testing
---

The test `MasterQuotaTest.RemoveSingleQuota` was modified to include a call to 
'/metrics/snapshot', both with and without a framework registered in the role 
for which quota is set. It's not easy to ensure that such a test provokes the 
race condition 100% of the time, but the test would always expose it within a 
few iterations.

Before this patch, the test would crash reliably within several iterations. 
After this patch, the test can be run many times with no failures.


Thanks,

Greg Mann

Re: Review Request 67099: Updated support scripts to check for Python 3.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67099 was successfully built and tested.

Reviews applied: `['67055', '67059', '67099']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67099

- Mesos Reviewbot Windows


On May 11, 2018, 3:01 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67099/
> ---
> 
> (Updated May 11, 2018, 3:01 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Eric Chung, 
> and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated support scripts to check for Python 3.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py 0c744a90f63ec78badad627e90599a3e68791d5f 
>   support/check-python3 PRE-CREATION 
>   support/generate-endpoint-help.py 7e59b358aa3d2cd1d490099fc4cca2e7808ed67a 
>   support/jsonurl.py 72fe682bf7d70507621181f48375d69b1d1c073b 
>   support/mesos-gtest-runner.py 4ae5fe3fa8e936d380876d9bd1bf3f47c48bc9be 
>   support/mesos-split.py 7ef5c86d55e61b80b5a7c8f137323aa056f272ed 
>   support/mesos-style.py 07074daa245ab503cf551201ccadeac8cc10206d 
>   support/post-reviews.py a6646f2eca45f911aad2403b8b0fef3a9323 
>   support/push-commits.py cd751cbf3c7e64005f1da6c96eb33c6fc62c7ae3 
>   support/test-upgrade.py 53c068871765de702c71d1a4260558eefcfb0a34 
>   support/verify-reviews.py fbc2460051e96761c9669db001c2fa13906f0cca 
> 
> 
> Diff: https://reviews.apache.org/r/67099/diff/1/
> 
> 
> Testing
> ---
> 
> Tested manually by changing my environment variable and seeing the output of 
> the scripts.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 67078: Made `UriDiskProfileAdaptor` be able to update profile selectors.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [67078]

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

- Mesos Reviewbot


On May 11, 2018, 9:28 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67078/
> ---
> 
> (Updated May 11, 2018, 9:28 p.m.)
> 
> 
> Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8906
> https://issues.apache.org/jira/browse/MESOS-8906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made `UriDiskProfileAdaptor` be able to update profile selectors.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/uri_disk_profile_adaptor.cpp 
> 300ea12687de487737ce91066ab4e74d9b3430e6 
> 
> 
> Diff: https://reviews.apache.org/r/67078/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 67098: Updated the container launcher mount sequence.

[Jie Yu]

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



high level comments: should we just create `/dev` in the linux devices 
isolator "prepare" phase? The linux devices isolator should also prepare those 
standard devices. Logically, this makes sense because `/dev` should just be 
owned by linux devices isolator?


src/slave/containerizer/mesos/launch.cpp
Lines 399-410 (patched)


+ @jasonlai

This might be related to your chain. Please let us know what's the best way 
forward to best aligh with your goal.


- Jie Yu


On May 11, 2018, 6:32 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67098/
> ---
> 
> (Updated May 11, 2018, 6:32 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Zhitao Li.
> 
> 
> Bugs: MESOS-8792
> https://issues.apache.org/jira/browse/MESOS-8792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `linux/devices` isolator needs to make bind mounts into
> the `/dev` directory of the container. However, the container
> mounts are made before the container `/dev` is mounted as part
> of the chroot preparation. We need to prepare the chroot,
> then make any necessary container mounts, and finally enter
> the chroot. This sequence of operations also requires us to
> touch the target mount point, since we can't do it from the
> isolator because the '/dev' directory doesn't exist yet.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> f25d90651ef32495c9161c3eaed8a327d1b2b926 
> 
> 
> Diff: https://reviews.apache.org/r/67098/diff/1/
> 
> 
> Testing
> ---
> 
> manual
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 67097: Added `linux/devices` isolator whitelist support.

[James Peach]

> On May 11, 2018, 11:34 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/linux/devices.cpp
> > Lines 73 (patched)
> > 
> >
> > I'd suggest we just skip this whitelist entry, instead of fail the 
> > agent launch.
> > 
> > If the operator does not specify the device path, it indicate that he 
> > does not want the device path to show up in the container. He just want the 
> > device cgroup to allow the device to be whitelisted. Thoughts?

I thought about that, and I'm happy to change to that behaviour. However, even 
in the devices cgroup isolator, the path is actually required .. it just 
silently doesn't work if you omit the path. Maybe we should require the path in 
both cases?


- James


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


On May 11, 2018, 6:32 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67097/
> ---
> 
> (Updated May 11, 2018, 6:32 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Zhitao Li.
> 
> 
> Bugs: MESOS-8792
> https://issues.apache.org/jira/browse/MESOS-8792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `linux/devices` isolator support for populating the container
> devices.  This introduces a general mechanism for populating devices
> into a specific container but currently only implements devices for all
> containers based on the devices specified by the `--allowed_devices`
> agent flag.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/linux/devices.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/devices.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67097/diff/1/
> 
> 
> Testing
> ---
> 
> manual
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 67097: Added `linux/devices` isolator whitelist support.

[Jie Yu]

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




src/slave/containerizer/mesos/isolators/linux/devices.cpp
Lines 71 (patched)


use `deviceAccess` please :)



src/slave/containerizer/mesos/isolators/linux/devices.cpp
Lines 73 (patched)


I'd suggest we just skip this whitelist entry, instead of fail the agent 
launch.

If the operator does not specify the device path, it indicate that he does 
not want the device path to show up in the container. He just want the device 
cgroup to allow the device to be whitelisted. Thoughts?


- Jie Yu


On May 11, 2018, 6:32 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67097/
> ---
> 
> (Updated May 11, 2018, 6:32 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Zhitao Li.
> 
> 
> Bugs: MESOS-8792
> https://issues.apache.org/jira/browse/MESOS-8792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `linux/devices` isolator support for populating the container
> devices.  This introduces a general mechanism for populating devices
> into a specific container but currently only implements devices for all
> containers based on the devices specified by the `--allowed_devices`
> agent flag.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/linux/devices.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/devices.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67097/diff/1/
> 
> 
> Testing
> ---
> 
> manual
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 67094: Split linux chroot into prepare and enter phases.

[Jason Lai]

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




src/linux/fs.cpp
Lines 968-1034 (original), 973-1041 (patched)


We can also consider changing `fs::chroot::enter` with the [`pivotRoot` 
implementation in runc's 
`rootfs_linux.go`](https://github.com/opencontainers/runc/blob/0cbfd8392fff2462701507296081e835b3b0b99a/libcontainer/rootfs_linux.go#L645-L702)


- Jason Lai


On May 11, 2018, 6:31 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67094/
> ---
> 
> (Updated May 11, 2018, 6:31 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Zhitao Li.
> 
> 
> Bugs: MESOS-8792
> https://issues.apache.org/jira/browse/MESOS-8792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since we will need to perform additional work to configure
> the chroot before entering it, split the Linux chroot API
> into `fs::chroot::prepare()` and `fs::chroot::enter()`.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp 76dc09c38996eefd8487ba6ef4977ef2f7c9df4c 
>   src/linux/fs.cpp fbd03b19abb9b56dbf3fb8a84d09a39171bbc1b0 
>   src/slave/containerizer/mesos/launch.cpp 
> f25d90651ef32495c9161c3eaed8a327d1b2b926 
> 
> 
> Diff: https://reviews.apache.org/r/67094/diff/1/
> 
> 
> Testing
> ---
> 
> manual
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 67096: Added a `linux/devices` isolator skeleton.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On May 11, 2018, 6:32 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67096/
> ---
> 
> (Updated May 11, 2018, 6:32 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Zhitao Li.
> 
> 
> Bugs: MESOS-8792
> https://issues.apache.org/jira/browse/MESOS-8792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the skeleton of a `linux/devices` isolator and wired it into
> the build and the Mesos containerizer.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt d4881318d6751fe1475521ecc2669f147c15c2c0 
>   src/Makefile.am c08ac6e2f5deec4d05f59f71ff6c51382f216708 
>   src/slave/containerizer/mesos/containerizer.cpp 
> eac1d16f2388385fec04ff8f013ce0ebf4e97f0f 
>   src/slave/containerizer/mesos/isolators/linux/devices.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/linux/devices.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67096/diff/1/
> 
> 
> Testing
> ---
> 
> manual
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 67078: Made `UriDiskProfileAdaptor` be able to update profile selectors.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67078 was successfully built and tested.

Reviews applied: `['67078']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67078

- Mesos Reviewbot Windows


On May 11, 2018, 9:28 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67078/
> ---
> 
> (Updated May 11, 2018, 9:28 p.m.)
> 
> 
> Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8906
> https://issues.apache.org/jira/browse/MESOS-8906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made `UriDiskProfileAdaptor` be able to update profile selectors.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/uri_disk_profile_adaptor.cpp 
> 300ea12687de487737ce91066ab4e74d9b3430e6 
> 
> 
> Diff: https://reviews.apache.org/r/67078/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 67095: Added a containerizer devices path helper.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On May 11, 2018, 6:32 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67095/
> ---
> 
> (Updated May 11, 2018, 6:32 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Zhitao Li.
> 
> 
> Bugs: MESOS-8792
> https://issues.apache.org/jira/browse/MESOS-8792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a helper to define a per-container directory for storing
> container device nodes.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/paths.hpp 
> b9f0f454ada3ee4e6648916a2c582bcfeebe1732 
>   src/slave/containerizer/mesos/paths.cpp 
> cf7d47bf501f6401183c0026cf1aca98395351a0 
> 
> 
> Diff: https://reviews.apache.org/r/67095/diff/1/
> 
> 
> Testing
> ---
> 
> manual
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 67094: Split linux chroot into prepare and enter phases.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/launch.cpp
Lines 465 (patched)


Fix the TODO format here:
```
// TODO(jpeach): xxx
```


- Jie Yu


On May 11, 2018, 6:31 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67094/
> ---
> 
> (Updated May 11, 2018, 6:31 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Zhitao Li.
> 
> 
> Bugs: MESOS-8792
> https://issues.apache.org/jira/browse/MESOS-8792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since we will need to perform additional work to configure
> the chroot before entering it, split the Linux chroot API
> into `fs::chroot::prepare()` and `fs::chroot::enter()`.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp 76dc09c38996eefd8487ba6ef4977ef2f7c9df4c 
>   src/linux/fs.cpp fbd03b19abb9b56dbf3fb8a84d09a39171bbc1b0 
>   src/slave/containerizer/mesos/launch.cpp 
> f25d90651ef32495c9161c3eaed8a327d1b2b926 
> 
> 
> Diff: https://reviews.apache.org/r/67094/diff/1/
> 
> 
> Testing
> ---
> 
> manual
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 67059: Ported all support scripts to Python 3.

[Armand Grillet]

> On mai 10, 2018, 8:34 après-midi, Andrew Schwartzmeyer wrote:
> > support/build-virtualenv
> > Lines 49-51 (original), 49-51 (patched)
> > 
> >
> > I didn't know where else to comment to open this issue:
> > 
> > We should add a BIG AND LOUD deprecation warning to the existing 
> > scripts that devs must start using the Python 3 scripts, as on "June 1st" 
> > the existing Python 3 scripts WILL BE DELETED.
> > 
> > Something to that effect :)

/r/67099/


> On mai 10, 2018, 8:34 après-midi, Andrew Schwartzmeyer wrote:
> > support/python3/apply-reviews.py
> > Lines 2-17 (patched)
> > 
> >
> > It's unfortunate that due to a staged replacement, we can't review the 
> > diffs of these on ReviewBoard :/
> > 
> > Armand, for the purposes of review, could you put up a parallel review 
> > where the scripts are replaced so that we can review the diff, and then 
> > when it's given ship-its, we take the finished ported scripts and put them 
> > here (trusting they're the same as the reviewed ones).

/r/67101/


- Armand


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


On mai 11, 2018, 9:57 après-midi, Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67059/
> ---
> 
> (Updated mai 11, 2018, 9:57 après-midi)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Eric Chung, 
> and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The scripts are in a temporary directory, support/python3.
> 
> The scripts have been ported using 2to3, the official tool to do so.
> Many of these scripts require testing from the community before being
> used by default.
> 
> The script building the virtual environment and the git hooks have
> been updated to use the new scripts if the environment variable
> `MESOSSUPPORTPYTHON` is set to `3` by the user.
> 
> 
> Diffs
> -
> 
>   support/README.md e812e9638ca555c56e4c52e52485240e75cb8229 
>   support/build-virtualenv 850af89326721f34de20eb45a7e78fa391d031be 
>   support/hooks/post-rewrite 1ab14abf711d1923a7ae69beb33581317009a94a 
>   support/hooks/pre-commit 6faba98ab6db68aef1a54091a08b8db1eaac8701 
>   support/python3/apply-reviews.py PRE-CREATION 
>   support/python3/generate-endpoint-help.py PRE-CREATION 
>   support/python3/jsonurl.py PRE-CREATION 
>   support/python3/mesos-gtest-runner.py PRE-CREATION 
>   support/python3/mesos-split.py PRE-CREATION 
>   support/python3/mesos-style.py PRE-CREATION 
>   support/python3/post-reviews.py PRE-CREATION 
>   support/python3/push-commits.py PRE-CREATION 
>   support/python3/test-upgrade.py PRE-CREATION 
>   support/python3/verify-reviews.py PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67059/diff/2/
> 
> 
> Testing
> ---
> 
> All the files are OK for our linter, I have tested `mesos-style.py` and 
> `post-reviews.py` (used for this review request).
> 
> We will likely see error messages `TypeError: cannot use a string pattern on 
> a bytes-like object`, they are very easy to fix (we just need to add 
> `.decode("utf-8")` but detecting all of them requires to use the scripts. 
> Please do so and create issues.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Review Request 67099: Updated support scripts to check for Python 3.

[Armand Grillet]

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

Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Eric Chung, 
and Kevin Klues.


Repository: mesos


Description
---

Updated support scripts to check for Python 3.


Diffs
-

  support/apply-reviews.py 0c744a90f63ec78badad627e90599a3e68791d5f 
  support/check-python3 PRE-CREATION 
  support/generate-endpoint-help.py 7e59b358aa3d2cd1d490099fc4cca2e7808ed67a 
  support/jsonurl.py 72fe682bf7d70507621181f48375d69b1d1c073b 
  support/mesos-gtest-runner.py 4ae5fe3fa8e936d380876d9bd1bf3f47c48bc9be 
  support/mesos-split.py 7ef5c86d55e61b80b5a7c8f137323aa056f272ed 
  support/mesos-style.py 07074daa245ab503cf551201ccadeac8cc10206d 
  support/post-reviews.py a6646f2eca45f911aad2403b8b0fef3a9323 
  support/push-commits.py cd751cbf3c7e64005f1da6c96eb33c6fc62c7ae3 
  support/test-upgrade.py 53c068871765de702c71d1a4260558eefcfb0a34 
  support/verify-reviews.py fbc2460051e96761c9669db001c2fa13906f0cca 


Diff: https://reviews.apache.org/r/67099/diff/1/


Testing
---

Tested manually by changing my environment variable and seeing the output of 
the scripts.


Thanks,

Armand Grillet

Re: Review Request 67059: Ported all support scripts to Python 3.

[Armand Grillet]

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

(Updated mai 11, 2018, 9:57 après-midi)


Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Eric Chung, 
and Kevin Klues.


Changes
---

Fixed some issues, new review requests for the ones not fixed.


Repository: mesos


Description (updated)
---

The scripts are in a temporary directory, support/python3.

The scripts have been ported using 2to3, the official tool to do so.
Many of these scripts require testing from the community before being
used by default.

The script building the virtual environment and the git hooks have
been updated to use the new scripts if the environment variable
`MESOSSUPPORTPYTHON` is set to `3` by the user.


Diffs (updated)
-

  support/README.md e812e9638ca555c56e4c52e52485240e75cb8229 
  support/build-virtualenv 850af89326721f34de20eb45a7e78fa391d031be 
  support/hooks/post-rewrite 1ab14abf711d1923a7ae69beb33581317009a94a 
  support/hooks/pre-commit 6faba98ab6db68aef1a54091a08b8db1eaac8701 
  support/python3/apply-reviews.py PRE-CREATION 
  support/python3/generate-endpoint-help.py PRE-CREATION 
  support/python3/jsonurl.py PRE-CREATION 
  support/python3/mesos-gtest-runner.py PRE-CREATION 
  support/python3/mesos-split.py PRE-CREATION 
  support/python3/mesos-style.py PRE-CREATION 
  support/python3/post-reviews.py PRE-CREATION 
  support/python3/push-commits.py PRE-CREATION 
  support/python3/test-upgrade.py PRE-CREATION 
  support/python3/verify-reviews.py PRE-CREATION 


Diff: https://reviews.apache.org/r/67059/diff/2/

Changes: https://reviews.apache.org/r/67059/diff/1-2/


Testing
---

All the files are OK for our linter, I have tested `mesos-style.py` and 
`post-reviews.py` (used for this review request).

We will likely see error messages `TypeError: cannot use a string pattern on a 
bytes-like object`, they are very easy to fix (we just need to add 
`.decode("utf-8")` but detecting all of them requires to use the scripts. 
Please do so and create issues.


Thanks,

Armand Grillet

Re: Review Request 67055: Added Python 3 version of cpplint.

[Armand Grillet]

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

(Updated mai 11, 2018, 9:49 après-midi)


Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Eric Chung, 
and Kevin Klues.


Changes
---

Changed Python version.


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


Repository: mesos


Description
---

The Python 3 supports scripts currently live in support/python3.
Once all support scripts will have been updated to Python 3 and
tested by the developers, we will replace the Python 2 scripts by
the new ones in an ongoing effort of modernizing our Python codebase.


Diffs (updated)
-

  support/python3/cpplint.patch PRE-CREATION 
  support/python3/cpplint.py PRE-CREATION 


Diff: https://reviews.apache.org/r/67055/diff/2/

Changes: https://reviews.apache.org/r/67055/diff/1-2/


Testing
---

Used `mesos-style.py` with the old `cpplint.py` (no change on `mesos-style.py`):
```
apache-mesos (cpplint) $ ./support/mesos-style.py
Checking 1289 C++ files
Total errors found: 0
Checking 4 JavaScript files
Total errors found: 0
Checking 47 Python files
  py27-lint: commands succeeded
  congratulations :)
  py27-lint: commands succeeded
  congratulations :)
Total errors found: 0
```
Updated `mesos-style.py` to use the new version of cpplint: 
```
# We do not use a version of cpplint available through pip as
# we use a custom version (see cpplint.path) to lint C++ files.
process = subprocess.Popen(
['python3', 'support/python3/cpplint.py', rules_filter] + 
source_paths,
stderr=subprocess.PIPE,
close_fds=True)
```

Run `mesos-style.py` again:
```
apache-mesos (cpplint) $ ./support/mesos-style.py
Checking 1289 C++ files
Total errors found: 0
Checking 4 JavaScript files
Total errors found: 0
Checking 47 Python files
  py27-lint: commands succeeded
  congratulations :)
  py27-lint: commands succeeded
  congratulations :)
* Module mesos-style
C:279, 0: Line too long (83/80) (line-too-long)
Total errors found: 1
```
As we can see, the number of errors found by cpplint is the same as before.

I have also used cpplint separately bu running `python3 
support/python3/cpplint` on a few C++ files to see that it was running as 
expected.


Thanks,

Armand Grillet

Re: Review Request 67094: Split linux chroot into prepare and enter phases.

[Jason Lai]

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



Great to see the split of `fs::enter::prepare` from `fs::enter::chroot`. Let's 
make sure the parts dealing with `$rootfs/tmp` are symmetric within 
`fs::enter::chroot` first and I'm fine with the rest.

We could even consider moving some parts of `fs::enter::prepare` into 
`launch.cpp` after my chain gets merged into master.


src/linux/fs.cpp
Lines 940-966 (original), 940-966 (patched)


I would suggest that `fs::chroot::enter` starts here, as later in 
`fs::chroot::enter` we have `fs::umount("/tmp")`. We should consider making 
`fs::chroot::enter` symmetric when dealing with `$rootfs/tmp`, as well as 
making it an extended `pivot_root(2)` with no unnecessary preparations and 
cleanups.


- Jason Lai


On May 11, 2018, 6:31 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67094/
> ---
> 
> (Updated May 11, 2018, 6:31 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Zhitao Li.
> 
> 
> Bugs: MESOS-8792
> https://issues.apache.org/jira/browse/MESOS-8792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since we will need to perform additional work to configure
> the chroot before entering it, split the Linux chroot API
> into `fs::chroot::prepare()` and `fs::chroot::enter()`.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp 76dc09c38996eefd8487ba6ef4977ef2f7c9df4c 
>   src/linux/fs.cpp fbd03b19abb9b56dbf3fb8a84d09a39171bbc1b0 
>   src/slave/containerizer/mesos/launch.cpp 
> f25d90651ef32495c9161c3eaed8a327d1b2b926 
> 
> 
> Diff: https://reviews.apache.org/r/67094/diff/1/
> 
> 
> Testing
> ---
> 
> manual
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 52695: Harden libprocess

[Benjamin Mahler]

> On Nov. 2, 2016, 9:32 a.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/Makefile.am
> > Lines 30 (patched)
> > 
> >
> > I am not a big fan of unconditionally omitting frame pointers as this 
> > gives the optimizer one less register to work with. Unfortunately one 
> > cannot easily tell the actual impact of this from the info here. Is this 
> > strictly needed here or just nice to have?
> 
> James Peach wrote:
> The performance benefit of omitting frame pointers is likely to be 
> marginal on x64_64, if it is a win at all. The rationale for adding this is 
> that it makes stack walking reliable in all cases, so debugability is 
> improved and you can get reasonable results when uting `perf`. Since most 
> users will build with default options I suggested to Aaron that we should 
> make it the default.
> 
> Benjamin Bannier wrote:
> Thanks James, that makes sense.
> 
> Since this seems all related to debugability what about enabling it _only 
> for builds with `--enable-debug`_ (e.g., perf results already now also don't 
> necessarily give full info w/o debug symbols)? Tangentially related, tcmalloc 
> can fail in debug builds with omitted frame pointers, so disabling 
> `omit-frame-pointer` in debug builds might safe us from some future 
> headaches, https://bugs.chromium.org/p/chromium/issues/detail?id=636489.
> 
> `stack-protector-strong` can significantly increase the binary size, and 
> we should either only enable it for e.g., debug builds, or give users a 
> `configure` knob to disable it.
> 
> For using `FORTIFY_SOURCE` I think we also need be a little more careful. 
> Support for it is somewhat broken in clang 
> (https://llvm.org/bugs/show_bug.cgi?id=16821), it only has useful effects in 
> builds with some level of optimization, and can e.g., mess up reports from 
> sanitizers injected by users. I can see good uses for a `configure` flag to 
> disable this compiler flag, but I am not sure what the default should be.
> 
> Aaron Wood wrote:
> Going to drop this since we've all agreed on Slack to have the frame 
> pointer modification done in a separate patch.

Looks like there wasn't a patch for `-fno-omit-frame-pointer`?

Filed: https://issues.apache.org/jira/browse/MESOS-8908


- Benjamin


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


On Nov. 30, 2016, 8:52 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52695/
> ---
> 
> (Updated Nov. 30, 2016, 8:52 p.m.)
> 
> 
> Review request for mesos, James Peach, Michael Park, and Neil Conway.
> 
> 
> Bugs: MESOS-6229
> https://issues.apache.org/jira/browse/MESOS-6229
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add hardened flags for libprocess.
> Take compile flag macro at 391cb680171d3889965b1ead43d3a326c913bc25.
> The macro at 1a869696e4129279f7b99c3f9052717354b79a86 requires autoconf 2.64 
> which breaks on CentOS 6.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 9d496b8 
>   3rdparty/libprocess/configure.ac e65e5ca 
>   3rdparty/libprocess/m4/ax_check_compile_flag.m4 PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/52695/diff/9/
> 
> 
> Testing
> ---
> 
> Compared the benchmarks with and without the flags being used. Also did a 
> comparsion with the flags being used with and without optimizations and 
> without the flags being used with and without optimizations. Overall the 
> performance hit was very small with a 3-8% overhead (optimizations brings 
> this down slightly). Most benchmarks were about 5% (or less) slower.
> 
> 
> File Attachments
> 
> 
> --enable-optimized with hardening applied
>   
> https://reviews.apache.org/media/uploaded/files/2016/11/02/875c9e6e-c73b-4e3c-8265-0f7c6dc00351__hardened-optimized.txt
> Hardening applied but no --enable-optimized
>   
> https://reviews.apache.org/media/uploaded/files/2016/11/02/932d28a7-2d31-471a-b438-647841a6853c__hardened-unoptimized.txt
> --enable-optimized with no hardening applied
>   
> https://reviews.apache.org/media/uploaded/files/2016/11/02/896944ea-9b31-4d62-b1b9-97fb4700a882__optimized.txt
> No hardening applied and no --enable-optimized
>   
> https://reviews.apache.org/media/uploaded/files/2016/11/02/b32667ce-3e3b-4d2b-b4f8-4c2404a0fc1c__unoptimized.txt
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 67078: Made `UriDiskProfileAdaptor` be able to update profile selectors.

[Chun-Hung Hsiao]

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

(Updated May 11, 2018, 9:28 p.m.)


Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.


Changes
---

Addressed James' comment by adding a TODO.


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


Repository: mesos


Description
---

Made `UriDiskProfileAdaptor` be able to update profile selectors.


Diffs (updated)
-

  src/resource_provider/storage/uri_disk_profile_adaptor.cpp 
300ea12687de487737ce91066ab4e74d9b3430e6 


Diff: https://reviews.apache.org/r/67078/diff/2/

Changes: https://reviews.apache.org/r/67078/diff/1-2/


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 67078: Made `UriDiskProfileAdaptor` be able to update profile selectors.

[James DeFelice]

> On May 11, 2018, 12:05 p.m., James DeFelice wrote:
> > src/resource_provider/storage/uri_disk_profile_adaptor.cpp
> > Line 281 (original)
> > 
> >
> > this seems like a pretty aggressive change. instead of removing the 
> > attempted optimization completely, why not fix it instead?
> > 
> > IMO it seems like it would be cheaper to do an equivalence check here 
> > and skip the subsequent notifications if nothing has changed. we don't 
> > expect profiles to change very frequently.
> 
> Chun-Hung Hsiao wrote:
> The equivalence operation for proto messages does not come for free in 
> C++, and we need to either use the `MessageDifferencer` or define it by 
> ourselves. Since this needs to be backported I'd like to make the patch as 
> simple as possible. Dropping this issue.

OK. Please add a TODO as suggested by Jie.


- James


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


On May 11, 2018, 3:32 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67078/
> ---
> 
> (Updated May 11, 2018, 3:32 a.m.)
> 
> 
> Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8906
> https://issues.apache.org/jira/browse/MESOS-8906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made `UriDiskProfileAdaptor` be able to update profile selectors.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/uri_disk_profile_adaptor.cpp 
> 300ea12687de487737ce91066ab4e74d9b3430e6 
> 
> 
> Diff: https://reviews.apache.org/r/67078/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 67078: Made `UriDiskProfileAdaptor` be able to update profile selectors.

[Chun-Hung Hsiao]

> On May 11, 2018, 12:05 p.m., James DeFelice wrote:
> > src/resource_provider/storage/uri_disk_profile_adaptor.cpp
> > Line 281 (original)
> > 
> >
> > this seems like a pretty aggressive change. instead of removing the 
> > attempted optimization completely, why not fix it instead?
> > 
> > IMO it seems like it would be cheaper to do an equivalence check here 
> > and skip the subsequent notifications if nothing has changed. we don't 
> > expect profiles to change very frequently.

The equivalence operation for proto messages does not come for free in C++, and 
we need to either use the `MessageDifferencer` or define it by ourselves. Since 
this needs to be backported I'd like to make the patch as simple as possible. 
Dropping this issue.


- Chun-Hung


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


On May 11, 2018, 3:32 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67078/
> ---
> 
> (Updated May 11, 2018, 3:32 a.m.)
> 
> 
> Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8906
> https://issues.apache.org/jira/browse/MESOS-8906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made `UriDiskProfileAdaptor` be able to update profile selectors.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/uri_disk_profile_adaptor.cpp 
> 300ea12687de487737ce91066ab4e74d9b3430e6 
> 
> 
> Diff: https://reviews.apache.org/r/67078/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 67098: Updated the container launcher mount sequence.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [67094, 67095, 67096, 67097, 67098]

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

- Mesos Reviewbot


On May 11, 2018, 6:32 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67098/
> ---
> 
> (Updated May 11, 2018, 6:32 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Zhitao Li.
> 
> 
> Bugs: MESOS-8792
> https://issues.apache.org/jira/browse/MESOS-8792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `linux/devices` isolator needs to make bind mounts into
> the `/dev` directory of the container. However, the container
> mounts are made before the container `/dev` is mounted as part
> of the chroot preparation. We need to prepare the chroot,
> then make any necessary container mounts, and finally enter
> the chroot. This sequence of operations also requires us to
> touch the target mount point, since we can't do it from the
> isolator because the '/dev' directory doesn't exist yet.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> f25d90651ef32495c9161c3eaed8a327d1b2b926 
> 
> 
> Diff: https://reviews.apache.org/r/67098/diff/1/
> 
> 
> Testing
> ---
> 
> manual
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 67098: Updated the container launcher mount sequence.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67098 was successfully built and tested.

Reviews applied: `['67094', '67095', '67096', '67097', '67098']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67098

- Mesos Reviewbot Windows


On May 11, 2018, 6:32 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67098/
> ---
> 
> (Updated May 11, 2018, 6:32 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Zhitao Li.
> 
> 
> Bugs: MESOS-8792
> https://issues.apache.org/jira/browse/MESOS-8792
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `linux/devices` isolator needs to make bind mounts into
> the `/dev` directory of the container. However, the container
> mounts are made before the container `/dev` is mounted as part
> of the chroot preparation. We need to prepare the chroot,
> then make any necessary container mounts, and finally enter
> the chroot. This sequence of operations also requires us to
> touch the target mount point, since we can't do it from the
> isolator because the '/dev' directory doesn't exist yet.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> f25d90651ef32495c9161c3eaed8a327d1b2b926 
> 
> 
> Diff: https://reviews.apache.org/r/67098/diff/1/
> 
> 
> Testing
> ---
> 
> manual
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 66669: Added clean up of `containers_` map in composing containerizer.

[Andrei Budnik]

> On May 10, 2018, 9:23 p.m., Greg Mann wrote:
> > src/slave/containerizer/composing.cpp
> > Lines 488-502 (original), 493-512 (patched)
> > 
> >
> > I'm wondering if, instead of also adding a callback for this in 
> > `destroy()`, perhaps we should unconditionally remove the container from 
> > the `containers_` map in the callbacks which are registered on `wait()`?
> > 
> > i.e. we would do the following here:
> > ```
> >   if (launchResult == Containerizer::LaunchResult::SUCCESS) {
> > // Note that we don't update the state if a destroy is in progress.
> > if (container->state == LAUNCHING) {
> >   container->state = LAUNCHED;
> > }
> > 
> > // This is needed for eventually removing the given container from
> > // the list of active containers.
> > container->containerizer->wait(containerId)
> >   .onAny(defer(self(), [=](const 
> > Future&) {
> > if (containers_.contains(containerId)) {
> >   delete containers_.at(containerId);
> >   containers_.erase(containerId);
> > }
> >   }));
> > 
> > // Note that the return value is not impacted
> > // by whether a destroy is currently in progress.
> > return Containerizer::LaunchResult::SUCCESS;
> >   }
> > ```
> > 
> > and NOT register the callback in `destroy()`.
> > 
> > This might also mean that we could remove `if 
> > (containers_.contains(containerId))` and instead do 
> > `CHECK(containers_.contains(containerId));`?
> > 
> > WDYT?

Composing c'zer adds a container to the `containers_` map and then calls 
`launch()` on underlying c'zer. However, composing c'zer doesn't handle the 
case when an underlying containerizer returns a failure during the launch, so 
the container will not be removed from the `containers_` map. If we want to 
call `destroy()` without the callback, then there is no chance to get the 
container removed from the `containers_` map in this case.

To check that, you can remove the callback from `destroy()` (and optionally 
copy-paste `containerizer->wait(containerId).onAny(...)` to `_recover()`), then 
run tests and you'll get a few tests failing, including 
`ParentChildContainerTypeAndContentType/AgentContainerAPITest.NestedContainerFailLaunch/0`.

We could address the issue by handling the launch failures, but there are 3 
places where we should handle it. That change would make composing c'zer more 
complex IMO.


- Andrei


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


On April 17, 2018, 3:23 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9/
> ---
> 
> (Updated April 17, 2018, 3:23 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, Jie Yu, and Qian 
> Zhang.
> 
> 
> Bugs: MESOS-8714
> https://issues.apache.org/jira/browse/MESOS-8714
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds callbacks on `wait()` and `destroy()` in composing
> containerizer to remove a container from the `containers_` map after
> the container's termination.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.cpp 
> 186102c66d373dcd799cadd9fed7d1c8cb894971 
> 
> 
> Diff: https://reviews.apache.org/r/9/diff/2/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Review Request 67098: Updated the container launcher mount sequence.

[James Peach]

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

Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Zhitao Li.


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


Repository: mesos


Description
---

The `linux/devices` isolator needs to make bind mounts into
the `/dev` directory of the container. However, the container
mounts are made before the container `/dev` is mounted as part
of the chroot preparation. We need to prepare the chroot,
then make any necessary container mounts, and finally enter
the chroot. This sequence of operations also requires us to
touch the target mount point, since we can't do it from the
isolator because the '/dev' directory doesn't exist yet.


Diffs
-

  src/slave/containerizer/mesos/launch.cpp 
f25d90651ef32495c9161c3eaed8a327d1b2b926 


Diff: https://reviews.apache.org/r/67098/diff/1/


Testing
---

manual


Thanks,

James Peach

Review Request 67097: Added `linux/devices` isolator whitelist support.

[James Peach]

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

Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Zhitao Li.


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


Repository: mesos


Description
---

Added `linux/devices` isolator support for populating the container
devices.  This introduces a general mechanism for populating devices
into a specific container but currently only implements devices for all
containers based on the devices specified by the `--allowed_devices`
agent flag.


Diffs
-

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


Diff: https://reviews.apache.org/r/67097/diff/1/


Testing
---

manual


Thanks,

James Peach

Review Request 67096: Added a `linux/devices` isolator skeleton.

[James Peach]

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

Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Zhitao Li.


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


Repository: mesos


Description
---

Added the skeleton of a `linux/devices` isolator and wired it into
the build and the Mesos containerizer.


Diffs
-

  src/CMakeLists.txt d4881318d6751fe1475521ecc2669f147c15c2c0 
  src/Makefile.am c08ac6e2f5deec4d05f59f71ff6c51382f216708 
  src/slave/containerizer/mesos/containerizer.cpp 
eac1d16f2388385fec04ff8f013ce0ebf4e97f0f 
  src/slave/containerizer/mesos/isolators/linux/devices.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/linux/devices.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/67096/diff/1/


Testing
---

manual


Thanks,

James Peach

Review Request 67095: Added a containerizer devices path helper.

[James Peach]

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

Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Zhitao Li.


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


Repository: mesos


Description
---

Added a helper to define a per-container directory for storing
container device nodes.


Diffs
-

  src/slave/containerizer/mesos/paths.hpp 
b9f0f454ada3ee4e6648916a2c582bcfeebe1732 
  src/slave/containerizer/mesos/paths.cpp 
cf7d47bf501f6401183c0026cf1aca98395351a0 


Diff: https://reviews.apache.org/r/67095/diff/1/


Testing
---

manual


Thanks,

James Peach

Review Request 67094: Split linux chroot into prepare and enter phases.

[James Peach]

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

Review request for mesos, Gilbert Song, Jason Lai, Jie Yu, and Zhitao Li.


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


Repository: mesos


Description
---

Since we will need to perform additional work to configure
the chroot before entering it, split the Linux chroot API
into `fs::chroot::prepare()` and `fs::chroot::enter()`.


Diffs
-

  src/linux/fs.hpp 76dc09c38996eefd8487ba6ef4977ef2f7c9df4c 
  src/linux/fs.cpp fbd03b19abb9b56dbf3fb8a84d09a39171bbc1b0 
  src/slave/containerizer/mesos/launch.cpp 
f25d90651ef32495c9161c3eaed8a327d1b2b926 


Diff: https://reviews.apache.org/r/67094/diff/1/


Testing
---

manual


Thanks,

James Peach

Re: Review Request 67078: Made `UriDiskProfileAdaptor` be able to update profile selectors.

[Jie Yu]

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


Ship it!




I am fine with this. Let's add a TODO about a potential equality check here.

- Jie Yu


On May 11, 2018, 3:32 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67078/
> ---
> 
> (Updated May 11, 2018, 3:32 a.m.)
> 
> 
> Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8906
> https://issues.apache.org/jira/browse/MESOS-8906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made `UriDiskProfileAdaptor` be able to update profile selectors.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/uri_disk_profile_adaptor.cpp 
> 300ea12687de487737ce91066ab4e74d9b3430e6 
> 
> 
> Diff: https://reviews.apache.org/r/67078/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66650: Removed an invalid TODO in puller.cpp.

[Gilbert Song]

> On April 17, 2018, 6:33 p.m., Qian Zhang wrote:
> > Mind to explain why this TODO is invalid in the description? :-)

It means registry per container I believe. We already support registry prefix 
like quay.io/alpine:lastest


- Gilbert


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


On April 16, 2018, 10:32 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66650/
> ---
> 
> (Updated April 16, 2018, 10:32 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Qian Zhang.
> 
> 
> Bugs: MESOS-8794
> https://issues.apache.org/jira/browse/MESOS-8794
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed an invalid TODO in puller.cpp.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/docker/puller.cpp 
> d7d8987d493a37d20f32ddd254dc0c3b15159951 
> 
> 
> Diff: https://reviews.apache.org/r/66650/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 66817: Enabled composing containerizer as a default containerizer in tests.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [8, 66670, 66671, 66817]

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

- Mesos Reviewbot


On May 11, 2018, 1:55 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66817/
> ---
> 
> (Updated May 11, 2018, 1:55 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, Jie Yu, and Qian 
> Zhang.
> 
> 
> Bugs: MESOS-8732
> https://issues.apache.org/jira/browse/MESOS-8732
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch enforces all tests that start an agent to use composing
> containerizer. This is needed to make sure that composing containerizer
> is fairly covered by tests.
> 
> 
> Diffs
> -
> 
>   src/tests/cluster.cpp b56212f6529a4d307e65797ad9bb34f2104fc832 
> 
> 
> Diff: https://reviews.apache.org/r/66817/diff/3/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 66669: Added clean up of `containers_` map in composing containerizer.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the current review.

Failed command: `python.exe .\support\apply-reviews.py -n -r 9`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/9

Relevant logs:

- 
[apply-review-9-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/9/logs/apply-review-9-stdout.log):

```
error: patch failed: src/slave/containerizer/composing.cpp:355
error: src/slave/containerizer/composing.cpp: patch does not apply
```

- Mesos Reviewbot Windows


On April 17, 2018, 3:23 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9/
> ---
> 
> (Updated April 17, 2018, 3:23 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, Jie Yu, and Qian 
> Zhang.
> 
> 
> Bugs: MESOS-8714
> https://issues.apache.org/jira/browse/MESOS-8714
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds callbacks on `wait()` and `destroy()` in composing
> containerizer to remove a container from the `containers_` map after
> the container's termination.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.cpp 
> 186102c66d373dcd799cadd9fed7d1c8cb894971 
> 
> 
> Diff: https://reviews.apache.org/r/9/diff/2/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 66817: Enabled composing containerizer as a default containerizer in tests.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66817 was successfully built and tested.

Reviews applied: `['8', '66670', '66671', '66817']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66817

- Mesos Reviewbot Windows


On May 11, 2018, 1:55 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66817/
> ---
> 
> (Updated May 11, 2018, 1:55 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, Jie Yu, and Qian 
> Zhang.
> 
> 
> Bugs: MESOS-8732
> https://issues.apache.org/jira/browse/MESOS-8732
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch enforces all tests that start an agent to use composing
> containerizer. This is needed to make sure that composing containerizer
> is fairly covered by tests.
> 
> 
> Diffs
> -
> 
>   src/tests/cluster.cpp b56212f6529a4d307e65797ad9bb34f2104fc832 
> 
> 
> Diff: https://reviews.apache.org/r/66817/diff/3/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 66669: Added clean up of `containers_` map in composing containerizer.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [9, 8]

Failed command: python support/apply-reviews.py -n -r 9

Error:
2018-05-11 14:43:27 URL:https://reviews.apache.org/r/9/diff/raw/ 
[2148/2148] -> "9.patch" [1]
error: patch failed: src/slave/containerizer/composing.cpp:355
error: src/slave/containerizer/composing.cpp: patch does not apply

Full log: https://builds.apache.org/job/Mesos-Reviewbot/22433/console

- Mesos Reviewbot


On April 17, 2018, 3:23 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9/
> ---
> 
> (Updated April 17, 2018, 3:23 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, Jie Yu, and Qian 
> Zhang.
> 
> 
> Bugs: MESOS-8714
> https://issues.apache.org/jira/browse/MESOS-8714
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds callbacks on `wait()` and `destroy()` in composing
> containerizer to remove a container from the `containers_` map after
> the container's termination.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.cpp 
> 186102c66d373dcd799cadd9fed7d1c8cb894971 
> 
> 
> Diff: https://reviews.apache.org/r/9/diff/2/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 66817: Enabled composing containerizer as a default containerizer in tests.

[Andrei Budnik]

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

(Updated May 11, 2018, 1:55 p.m.)


Review request for mesos, Alexander Rukletsov, Greg Mann, Jie Yu, and Qian 
Zhang.


Changes
---

rebased


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


Repository: mesos


Description
---

This patch enforces all tests that start an agent to use composing
containerizer. This is needed to make sure that composing containerizer
is fairly covered by tests.


Diffs (updated)
-

  src/tests/cluster.cpp b56212f6529a4d307e65797ad9bb34f2104fc832 


Diff: https://reviews.apache.org/r/66817/diff/3/

Changes: https://reviews.apache.org/r/66817/diff/2-3/


Testing
---

internal CI


Thanks,

Andrei Budnik

Re: Review Request 66668: Removed `destroyed` from `Container` struct in composing containerizer.

[Andrei Budnik]

> On May 9, 2018, 3:15 p.m., Qian Zhang wrote:
> > src/slave/containerizer/composing.cpp
> > Lines 658-661 (original)
> > 
> >
> > If we remove these codes, will the container be missed to delete and 
> > erased from `containers_`?
> 
> Qian Zhang wrote:
> Ah, Just saw the code to delete it in the next patch. Anyway I think it 
> may be better to put that change into this patch :-)
> 
> Andrei Budnik wrote:
> `container_` cleanup is implemented in the following patch `/r/9/`.

Merged the next patch into this one.


- Andrei


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


On April 17, 2018, 3:23 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8/
> ---
> 
> (Updated April 17, 2018, 3:23 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, Jie Yu, and Qian 
> Zhang.
> 
> 
> Bugs: MESOS-8712
> https://issues.apache.org/jira/browse/MESOS-8712
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, we were using `destroyed` promise for each container to
> guarantee that calling `destroy()` on a container returns a non-empty
> value when the destroy-in-progress stops an launch-in-progress using
> the next containerizer. Since we are unifying the semantics of the
> return type for both `wait()` and `destroy()` operations, we can
> remove the `destroyed` promise.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.cpp 
> 186102c66d373dcd799cadd9fed7d1c8cb894971 
> 
> 
> Diff: https://reviews.apache.org/r/8/diff/2/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 66668: Removed `destroyed` from `Container` struct in composing containerizer.

[Andrei Budnik]

> On May 9, 2018, 3:15 p.m., Qian Zhang wrote:
> > I checked the callers of `Containerizer::destroy()`, it seems no one 
> > actually cares about its return value (`Option`), so 
> > why do we need to return that? Can we just make it return `Future`?
> 
> Qian Zhang wrote:
> And then for the last line of `ComposingContainerizerProcess::destroy`:
> ```
> return container->containerizer->wait(containerId);
> ```
> We could change it to something like:
> ```
> return container->containerizer->wait(containerId)
> .onAny([](const Future& termination) 
> {return Nothing();})
> ```

There are plans to use the return value of `destroy()` in tests, see 
[MESOS-8829](https://issues.apache.org/jira/browse/MESOS-8829).

Also, as `destroy()` depends on `wait()` method, it'd be necessary to implement 
wrappers like your example above, if we want them to return different type. 
Those wrappers introduce some extra complexity, which IMO doesn't facilitate 
achieving one of the goals "to simplify composing containerizer".


> On May 9, 2018, 3:15 p.m., Qian Zhang wrote:
> > src/slave/containerizer/composing.cpp
> > Lines 658-661 (original)
> > 
> >
> > If we remove these codes, will the container be missed to delete and 
> > erased from `containers_`?
> 
> Qian Zhang wrote:
> Ah, Just saw the code to delete it in the next patch. Anyway I think it 
> may be better to put that change into this patch :-)

`container_` cleanup is implemented in the following patch `/r/9/`.


- Andrei


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


On April 17, 2018, 3:23 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8/
> ---
> 
> (Updated April 17, 2018, 3:23 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, Jie Yu, and Qian 
> Zhang.
> 
> 
> Bugs: MESOS-8712
> https://issues.apache.org/jira/browse/MESOS-8712
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, we were using `destroyed` promise for each container to
> guarantee that calling `destroy()` on a container returns a non-empty
> value when the destroy-in-progress stops an launch-in-progress using
> the next containerizer. Since we are unifying the semantics of the
> return type for both `wait()` and `destroy()` operations, we can
> remove the `destroyed` promise.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.cpp 
> 186102c66d373dcd799cadd9fed7d1c8cb894971 
> 
> 
> Diff: https://reviews.apache.org/r/8/diff/2/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 67078: Made `UriDiskProfileAdaptor` be able to update profile selectors.

[James DeFelice]

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




src/resource_provider/storage/uri_disk_profile_adaptor.cpp
Line 281 (original)


this seems like a pretty aggressive change. instead of removing the 
attempted optimization completely, why not fix it instead?

IMO it seems like it would be cheaper to do an equivalence check here and 
skip the subsequent notifications if nothing has changed. we don't expect 
profiles to change very frequently.


- James DeFelice


On May 11, 2018, 3:32 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67078/
> ---
> 
> (Updated May 11, 2018, 3:32 a.m.)
> 
> 
> Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8906
> https://issues.apache.org/jira/browse/MESOS-8906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made `UriDiskProfileAdaptor` be able to update profile selectors.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/uri_disk_profile_adaptor.cpp 
> 300ea12687de487737ce91066ab4e74d9b3430e6 
> 
> 
> Diff: https://reviews.apache.org/r/67078/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 67054: Added .tox to files excluded by Python linter.

[Kevin Klues]

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



This looks good, but I think we should also add the .virtualenv and .tox 
directories to our .gitignore. They should never be part of the list of files 
that are being processed by mesos-style.py during a commit hook anyway.

- Kevin Klues


On May 10, 2018, 10:56 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67054/
> ---
> 
> (Updated May 10, 2018, 10:56 a.m.)
> 
> 
> Review request for mesos, Eric Chung and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added .tox to files excluded by Python linter.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 07074daa245ab503cf551201ccadeac8cc10206d 
> 
> 
> Diff: https://reviews.apache.org/r/67054/diff/1/
> 
> 
> Testing
> ---
> 
> Before change, when running `python mesos-style.py`:
> ```
> Checking 1769 Python files
> src/python/cli_new/.tox/py27-lint/lib/python2.7/site-packages/pylint/test/functional/redefined_builtin.py:1:
>  A license header should appear's on one of the first line of the file 
> starting with '# Licensed'.: """Tests for redefining builtins."""
> from __future__ import print_function
> 
> src/python/cli_new/.tox/py27-lint/lib/python2.7/site-packages/astroid/tests/testdata/python2/data/all.py:1:
>  A license header should appear's on one of the first line of the file 
> starting with '# Licensed'.:
> name = 'a'
> _bla = 2
> src/python/cli_new/.tox/py27-lint/lib/python2.7/site-packages/_pytest/monkeypatch.py:1:
>  A license header should appear's on one of the first line of the file 
> starting with '# Licensed'.: """ monkeypatching and mocking functionality.  
> """
> from __future__ import absolute_import, division, print_function
> 
> src/python/cli_new/.tox/py27-lint/lib/python2.7/site-packages/PyInstaller/lib/altgraph/Graph.py:1:
>  A license header should appear's on one of the first line of the file 
> starting with '# Licensed'.: """
> altgraph.Graph - Base Graph class
> =
> src/python/cli_new/.tox/py27-lint/lib/python2.7/site-packages/pip/_vendor/requests/packages/chardet/euctwprober.py:1:
>  A license header should appear's on one of the first line of the file 
> starting with '# Licensed'.:  BEGIN LICENSE BLOCK 
> 
> # The Original Code is mozilla.org code.
> #
> src/python/cli_new/.tox/py27-lint/lib/python2.7/site-packages/astroid/tests/testdata/python2/data/package/absimport.py:1:
>  A license header should appear's on one of the first line of the file 
> starting with '# Licensed'.: from __future__ import absolute_import, 
> print_function
> import import_package_subpackage_module # fail
> print(import_package_subpackage_module)
> src/python/cli_new/.tox/py27-lint/lib/python2.7/site-packages/kazoo/protocol/states.py:1:
>  A license header should appear's on one of the first line of the file 
> starting with '# Licensed'.: """Kazoo State and Event objects"""
> from collections import namedtuple
> 
> src/python/cli_new/.tox/py27-lint/lib/python2.7/site-packages/pylint/test/input/func_no_dummy_redefined.py:1:
>  A license header should appear's on one of the first line of the file 
> starting with '# Licensed'.: """Make sure warnings about redefinitions do not 
> trigger for dummy variables."""
> from __future__ import print_function
> 
> src/python/cli_new/.tox/py27-lint/lib/python2.7/site-packages/pip/_vendor/requests/hooks.py:1:
>  A license header should appear's on one of the first line of the file 
> starting with '# Licensed'.: # -*- coding: utf-8 -*-
> 
> """
> src/python/cli_new/.tox/py27-lint/lib/python2.7/site-packages/pip/_vendor/lockfile/__init__.py:1:
>  A license header should appear's on one of the first line of the file 
> starting with '# Licensed'.: # -*- coding: utf-8 -*-
> 
> """
> src/python/cli_new/.tox/py27-lint/lib/python2.7/site-packages/pygments/lexers/installers.py:1:
>  A license header should appear's on one of the first line of the file 
> starting with '# Licensed'.: # -*- coding: utf-8 -*-
> """
> pygments.lexers.installers
> src/python/cli_new/.tox/py27-lint/lib/python2.7/site-packages/wheel/wininst2wheel.py:1:
>  A license header should appear's on one of the first line of the file 
> starting with '# Licensed'.: #!/usr/bin/env python
> import distutils.dist
> import os.path
> src/python/cli_new/.tox/py27-lint/lib/python2.7/site-packages/pip/vcs/__init__.py:1:
>  A license header should appear's on one of the first line of the file 
> starting with '# Licensed'.: """Handles all VCS (version control) support"""
> from __future__ import absolute_import
> 
> 

Re: Review Request 66847: Added per framework metric for offer operation.

[Gilbert Song]

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

(Updated May 11, 2018, 1:44 a.m.)


Review request for mesos, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jie Yu, 
and Vinod Kone.


Repository: mesos


Description
---

Added per framework metric for offer operation.


Diffs (updated)
-

  src/master/master.cpp 41862db9900acde85a62d2fea85459691c68556e 
  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


Diff: https://reviews.apache.org/r/66847/diff/4/

Changes: https://reviews.apache.org/r/66847/diff/3-4/


Testing
---


Thanks,

Gilbert Song

Re: Review Request 66846: Added per framework metrics support for terminal task state.

[Gilbert Song]

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

(Updated May 11, 2018, 1:44 a.m.)


Review request for mesos, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jie Yu, 
and Vinod Kone.


Repository: mesos


Description
---

Added per framework metrics support for terminal task state.


Diffs (updated)
-

  src/master/master.cpp 41862db9900acde85a62d2fea85459691c68556e 
  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


Diff: https://reviews.apache.org/r/66846/diff/4/

Changes: https://reviews.apache.org/r/66846/diff/3-4/


Testing
---


Thanks,

Gilbert Song

Re: Review Request 66823: Added per framework metrics for framework events.

[Gilbert Song]

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

(Updated May 11, 2018, 1:44 a.m.)


Review request for mesos, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jie Yu, 
and Vinod Kone.


Repository: mesos


Description
---

Added per framework metrics for framework events.


Diffs (updated)
-

  src/master/master.hpp 5ec764b5c7f96bab786084cccf20fd8a17319718 
  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


Diff: https://reviews.apache.org/r/66823/diff/5/

Changes: https://reviews.apache.org/r/66823/diff/4-5/


Testing
---


Thanks,

Gilbert Song

Re: Review Request 66822: Added per Framework Calls to metrics.

[Gilbert Song]

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

(Updated May 11, 2018, 1:44 a.m.)


Review request for mesos, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jie Yu, 
and Vinod Kone.


Repository: mesos


Description
---

Added per Framework Calls to metrics.


Diffs (updated)
-

  src/master/http.cpp 0492b979e4657a489ca3428e6f8022ef20cb05f5 
  src/master/master.cpp 41862db9900acde85a62d2fea85459691c68556e 
  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


Diff: https://reviews.apache.org/r/66822/diff/8/

Changes: https://reviews.apache.org/r/66822/diff/7-8/


Testing
---


Thanks,

Gilbert Song

Re: Review Request 66822: Added per Framework Calls to metrics.

[Gilbert Song]

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

(Updated May 11, 2018, 1:22 a.m.)


Review request for mesos, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jie Yu, 
and Vinod Kone.


Repository: mesos


Description
---

Added per Framework Calls to metrics.


Diffs (updated)
-

  src/master/http.cpp 0492b979e4657a489ca3428e6f8022ef20cb05f5 
  src/master/master.cpp 41862db9900acde85a62d2fea85459691c68556e 
  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


Diff: https://reviews.apache.org/r/66822/diff/7/

Changes: https://reviews.apache.org/r/66822/diff/6-7/


Testing
---


Thanks,

Gilbert Song

Re: Review Request 66847: Added per framework metric for offer operation.

[Gilbert Song]

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

(Updated May 11, 2018, 12:46 a.m.)


Review request for mesos, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jie Yu, 
and Vinod Kone.


Summary (updated)
-

Added per framework metric for offer operation.


Repository: mesos


Description (updated)
---

Added per framework metric for offer operation.


Diffs (updated)
-

  src/master/master.cpp 41862db9900acde85a62d2fea85459691c68556e 
  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


Diff: https://reviews.apache.org/r/66847/diff/3/

Changes: https://reviews.apache.org/r/66847/diff/2-3/


Testing
---


Thanks,

Gilbert Song

Re: Review Request 66846: Added per framework metrics support for terminal task state.

[Gilbert Song]

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

(Updated May 11, 2018, 12:44 a.m.)


Review request for mesos, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jie Yu, 
and Vinod Kone.


Summary (updated)
-

Added per framework metrics support for terminal task state.


Repository: mesos


Description (updated)
---

Added per framework metrics support for terminal task state.


Diffs (updated)
-

  src/master/master.cpp 41862db9900acde85a62d2fea85459691c68556e 
  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


Diff: https://reviews.apache.org/r/66846/diff/3/

Changes: https://reviews.apache.org/r/66846/diff/2-3/


Testing
---


Thanks,

Gilbert Song

Re: Review Request 66825: Added per framework offer metrics.

[Gilbert Song]

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

(Updated May 11, 2018, 12:41 a.m.)


Review request for mesos, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jie Yu, 
and Vinod Kone.


Repository: mesos


Description
---

Added per framework offer metrics.


Diffs (updated)
-

  src/master/master.cpp 41862db9900acde85a62d2fea85459691c68556e 
  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


Diff: https://reviews.apache.org/r/66825/diff/4/

Changes: https://reviews.apache.org/r/66825/diff/3-4/


Testing
---


Thanks,

Gilbert Song

Re: Review Request 66823: Added per framework metrics for framework events.

[Gilbert Song]

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

(Updated May 11, 2018, 12:40 a.m.)


Review request for mesos, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jie Yu, 
and Vinod Kone.


Repository: mesos


Description
---

Added per framework metrics for framework events.


Diffs (updated)
-

  src/master/master.hpp 5ec764b5c7f96bab786084cccf20fd8a17319718 
  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


Diff: https://reviews.apache.org/r/66823/diff/4/

Changes: https://reviews.apache.org/r/66823/diff/3-4/


Testing
---


Thanks,

Gilbert Song

Re: Review Request 66845: Made per-framework metrics count heartbeat events.

[Gilbert Song]

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

(Updated May 11, 2018, 12:40 a.m.)


Review request for mesos, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jie Yu, 
and Vinod Kone.


Summary (updated)
-

Made per-framework metrics count heartbeat events.


Repository: mesos


Description (updated)
---

Made per-framework metrics count heartbeat events.


Diffs (updated)
-

  src/master/master.hpp 5ec764b5c7f96bab786084cccf20fd8a17319718 


Diff: https://reviews.apache.org/r/66845/diff/3/

Changes: https://reviews.apache.org/r/66845/diff/2-3/


Testing
---


Thanks,

Gilbert Song

Re: Review Request 66822: Added per Framework Calls to metrics.

[Gilbert Song]

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

(Updated May 11, 2018, 12:39 a.m.)


Review request for mesos, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jie Yu, 
and Vinod Kone.


Repository: mesos


Description
---

Added per Framework Calls to metrics.


Diffs (updated)
-

  src/master/http.cpp 0492b979e4657a489ca3428e6f8022ef20cb05f5 
  src/master/master.cpp 41862db9900acde85a62d2fea85459691c68556e 
  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


Diff: https://reviews.apache.org/r/66822/diff/6/

Changes: https://reviews.apache.org/r/66822/diff/5-6/


Testing
---


Thanks,

Gilbert Song

Re: Review Request 67078: Made `UriDiskProfileAdaptor` be able to update profile selectors.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [67078]

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

- Mesos Reviewbot


On May 11, 2018, 3:32 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67078/
> ---
> 
> (Updated May 11, 2018, 3:32 a.m.)
> 
> 
> Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8906
> https://issues.apache.org/jira/browse/MESOS-8906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made `UriDiskProfileAdaptor` be able to update profile selectors.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/uri_disk_profile_adaptor.cpp 
> 300ea12687de487737ce91066ab4e74d9b3430e6 
> 
> 
> Diff: https://reviews.apache.org/r/67078/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66822: Added per Framework Calls to metrics.

[Gilbert Song]

> On May 8, 2018, 1:33 p.m., Gaston Kleiman wrote:
> > src/master/metrics.cpp
> > Lines 595-623 (patched)
> > 
> >
> > We could replace these two methods with a single method that that takes 
> > a `const scheduler::Call::Type&` instead of a `const scheduler::Call&`.
> > 
> > We use this pattern for a lot of metrics, so should consider adding 
> > something like:
> > 
> > ```
> > template 
> > class EnumCounter
> > {
> >   EnumCounter(const lambda::function 
> > _keyNameGenerator);
> >   
> >   ~EnumCounter();
> >   
> >   void increment(const Type& type);
> >   
> > private:
> >   hashmap counters;
> >   const lambda::function keyNameGenerator;
> > }
> > ```

Unified these two. Do we want to introduce such a template class now? 
Consdiering it may be consumed by multiple metrics, maybe we should add it when 
refactoring all.


- Gilbert


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


On May 11, 2018, 12:25 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66822/
> ---
> 
> (Updated May 11, 2018, 12:25 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jie Yu, 
> and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added per Framework Calls to metrics.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 0492b979e4657a489ca3428e6f8022ef20cb05f5 
>   src/master/master.cpp 41862db9900acde85a62d2fea85459691c68556e 
>   src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
>   src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 
> 
> 
> Diff: https://reviews.apache.org/r/66822/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 66822: Added per Framework Calls to metrics.

[Gilbert Song]

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

(Updated May 11, 2018, 12:25 a.m.)


Review request for mesos, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jie Yu, 
and Vinod Kone.


Changes
---

rebased.


Repository: mesos


Description
---

Added per Framework Calls to metrics.


Diffs (updated)
-

  src/master/http.cpp 0492b979e4657a489ca3428e6f8022ef20cb05f5 
  src/master/master.cpp 41862db9900acde85a62d2fea85459691c68556e 
  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


Diff: https://reviews.apache.org/r/66822/diff/5/

Changes: https://reviews.apache.org/r/66822/diff/4-5/


Testing
---


Thanks,

Gilbert Song

Re: Review Request 66820: Added per framework metrics 'subscribed' and helpers.

[Gilbert Song]

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

(Updated May 11, 2018, 12:24 a.m.)


Review request for mesos, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jie Yu, 
and Vinod Kone.


Changes
---

rebased.


Summary (updated)
-

Added per framework metrics 'subscribed' and helpers.


Repository: mesos


Description (updated)
---

Added per framework metrics 'subscribed' and helpers.


Diffs (updated)
-

  src/master/master.hpp 5ec764b5c7f96bab786084cccf20fd8a17319718 
  src/master/master.cpp 41862db9900acde85a62d2fea85459691c68556e 
  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


Diff: https://reviews.apache.org/r/66820/diff/5/

Changes: https://reviews.apache.org/r/66820/diff/4-5/


Testing
---


Thanks,

Gilbert Song

Re: Review Request 66819: Added FrameworkMetrics struct in framework struct.

[Gilbert Song]

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

(Updated May 11, 2018, 12:20 a.m.)


Review request for mesos, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jie Yu, 
and Vinod Kone.


Changes
---

rebased.


Repository: mesos


Description
---

Added FrameworkMetrics struct in framework struct.


Diffs (updated)
-

  src/master/master.hpp 5ec764b5c7f96bab786084cccf20fd8a17319718 
  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


Diff: https://reviews.apache.org/r/66819/diff/4/

Changes: https://reviews.apache.org/r/66819/diff/3-4/


Testing
---


Thanks,

Gilbert Song