Re: Review Request 42616: libprocess: Cleaned up usage of namespace-qualified identifiers.

[Neil Conway]

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

(Updated Feb. 1, 2016, 7:41 p.m.)


Review request for mesos and Michael Park.


Changes
---

Rebase.


Repository: mesos


Description
---

For example, avoid using `std::string` in a .cpp file that already does
`using std::string`.


Diffs (updated)
-

  3rdparty/libprocess/src/authenticator.cpp 
16b48cd599b10c09229fbf10f28139219855710b 
  3rdparty/libprocess/src/http.cpp 762da9a9038fc0a81156b5c03b556084df1bd7e0 
  3rdparty/libprocess/src/io.cpp 9530192e2c7afbfa4bd9aa2af2ff40f00ee505a9 
  3rdparty/libprocess/src/libevent_ssl_socket.cpp 
9321ce666a0c5c75cdbdd1c5a6d534fbc221bf5b 
  3rdparty/libprocess/src/metrics/metrics.cpp 
3083b92b3fb67421265950b03272b52cec68daf9 
  3rdparty/libprocess/src/pid.cpp 3cd04ecaddd045d9d5ecf132302417181d416f80 
  3rdparty/libprocess/src/process.cpp d8a74d7637d20c81f384e974e4fdeba22effb437 
  3rdparty/libprocess/src/socket.cpp 6c1b16f69a21d116a0ffef21097a1b4372977b2e 
  3rdparty/libprocess/src/tests/process_tests.cpp 
df1c7b3f37ab1103991a4f27a9e0359bcf06cc68 

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


Testing
---

"make check" on OSX and Arch Linux.


Thanks,

Neil Conway

Re: Review Request 42617: stout: Cleaned up usage of namespace-qualified identifiers.

[Neil Conway]

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

(Updated Feb. 1, 2016, 7:40 p.m.)


Review request for mesos and Michael Park.


Changes
---

Rebase.


Repository: mesos


Description
---

For example, avoid using `std::string` in a .cpp file that already does
`using std::string`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp 
5a10f59f3698a6ad85b7c2386def9686af8a23a6 
  3rdparty/libprocess/3rdparty/stout/tests/linkedhashmap_tests.cpp 
21a98b85277d946b887cb23c4ebb4d32cfcb0c6b 
  3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp 
01e4b2fff7cc05a47d7919d702543a38bf6c339d 
  3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
4b7d1347f93261df7a7d512721146edca2703b34 
  3rdparty/libprocess/3rdparty/stout/tests/some_tests.cpp 
c0be974ea2dc74c63ec3561403b3f4d3722a8df3 
  3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 
7715fa4baa150f370bdd0fd5c2f98dc4c6f5fc55 

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


Testing
---

"make check" on OSX and Arch Linux.


Thanks,

Neil Conway

Re: Review Request 43034: libprocess: Added EXPECT_NO_FUTURE_DISPATCHES.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On Jan. 31, 2016, 7:42 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43034/
> ---
> 
> (Updated Jan. 31, 2016, 7:42 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This allows a unit test to assert that a given libprocess message
> will not be dispatched.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/gmock.hpp 
> 22a0eac37b16f2ed6afd249f5ea911d81970ba6b 
> 
> Diff: https://reviews.apache.org/r/43034/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43014: Fix ShasumTest.SHA512SimpleFile failed on centos7.

[Joseph Wu]

> On Jan. 30, 2016, 9:11 a.m., Jie Yu wrote:
> > src/common/command_utils.cpp, line 172
> > 
> >
> > On ubuntu, we don't have sha512sum but shasum (and sha512).
> > 
> > Here is what I am thinking:
> > 1) we need to introduce another command util 'which' that searches PATH 
> > and returns the full path to the command (just shelling out to 'which' 
> > command).
> > 2) Then, we use 'which' to check for shasum, sha512sum, etc., and then 
> > use the one that's available.
> 
> haosdent huang wrote:
> which ubuntu version you use, I use ubuntu 14.04. Alos could found 
> `sha512sum` in `/usr/bin/sha512sum`.
> 
> Jojy Varghese wrote:
> I thought `which` will work only if the command is in the path.
> 
> Shuai Lin wrote:
> The command "which" is not avaiable in the `centos:7` image
> 
> ```sh
> $ docker run -it --rm centos:7 which ls
> exec: "which": executable file not found in $PATH
> Error response from daemon: Cannot start container 
> f13beade3abf9139ddad4ec9303a191a7e5a7cc28d011d378c31e59ecf360549: [8] System 
> error: exec: "which": executable file not found in $PATH
> ```
> 
> To get it one need to run "yum install which".
> 
> haosdent huang wrote:
> LoL, seems we should focus on `shasum` and `sha512sum ` command rather 
> than `which`.
> 
> Jie Yu wrote:
> OK, sounds good to me. (BTW, we can implement our own os::which which 
> uses os::envirionment and os::exists).

FYI: This topic came up a few times, so I filed this JIRA to track:
https://issues.apache.org/jira/browse/MESOS-4576


- Joseph


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


On Jan. 31, 2016, 10:21 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43014/
> ---
> 
> (Updated Jan. 31, 2016, 10:21 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, and Vinod Kone.
> 
> 
> Bugs: MESOS-4556
> https://issues.apache.org/jira/browse/MESOS-4556
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix ShasumTest.SHA512SimpleFile failed on centos7.
> 
> 
> Diffs
> -
> 
>   src/common/command_utils.cpp 722d0b42a995a6a5a6cfd08f3c356dabd1080cd1 
>   src/tests/common/command_utils_tests.cpp 
> e81c724fc28717f5cb1849d75a066b6da713cd44 
> 
> Diff: https://reviews.apache.org/r/43014/diff/
> 
> 
> Testing
> ---
> 
> # check in CentOS 7.
> sudo ./bin/mesos-tests.sh --gtest_filter="ShasumTest.SHA512SimpleFile"
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 42617: stout: Cleaned up usage of namespace-qualified identifiers.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [42615]

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

Error:
2016-02-01 20:27:18 URL:https://reviews.apache.org/r/42615/diff/raw/ 
[46356/46356] -> "42615.patch" [1]
Total errors found: 0
Checking 58 files
Error: Commit message summary (the first line) must start with a capital letter.

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

- Mesos ReviewBot


On Feb. 1, 2016, 7:40 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42617/
> ---
> 
> (Updated Feb. 1, 2016, 7:40 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For example, avoid using `std::string` in a .cpp file that already does
> `using std::string`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp 
> 5a10f59f3698a6ad85b7c2386def9686af8a23a6 
>   3rdparty/libprocess/3rdparty/stout/tests/linkedhashmap_tests.cpp 
> 21a98b85277d946b887cb23c4ebb4d32cfcb0c6b 
>   3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp 
> 01e4b2fff7cc05a47d7919d702543a38bf6c339d 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> 4b7d1347f93261df7a7d512721146edca2703b34 
>   3rdparty/libprocess/3rdparty/stout/tests/some_tests.cpp 
> c0be974ea2dc74c63ec3561403b3f4d3722a8df3 
>   3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 
> 7715fa4baa150f370bdd0fd5c2f98dc4c6f5fc55 
> 
> Diff: https://reviews.apache.org/r/42617/diff/
> 
> 
> Testing
> ---
> 
> "make check" on OSX and Arch Linux.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 42797: Fix expire CERN GPG key link.

[Joseph Wu]

> On Feb. 1, 2016, 8:12 a.m., Benjamin Bannier wrote:
> > docs/getting-started.md, lines 87-90
> > 
> >
> > While this appears to work, it seems weird to import the repo from 
> > `linuxsoft.cern.ch`, and the key from `ftp.scientificlinux.org`. Any reason 
> > this cannot be consistent? I am mostly worried about it running out of sync 
> > again.
> 
> haosdent huang wrote:
> Yes, you concern is reasonable. But this is the best link I could find by 
> google except we give up cern repo.

If we're worried about the key moving, it may be appropriate to copy the key to 
a Gist here: https://github.com/3rdparty


- Joseph


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


On Jan. 26, 2016, 6:13 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42797/
> ---
> 
> (Updated Jan. 26, 2016, 6:13 a.m.)
> 
> 
> Review request for mesos, Adam B and Timothy Chen.
> 
> 
> Bugs: MESOS-4511
> https://issues.apache.org/jira/browse/MESOS-4511
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix expire CERN GPG key link.
> 
> 
> Diffs
> -
> 
>   docs/getting-started.md 7177f9d5ae33dbabf0ed1bb1b9870873906e1a56 
> 
> Diff: https://reviews.apache.org/r/42797/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 43056: Edited comments in 'verify_reviews.py' for grammar and style.

[Mesos ReviewBot]

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



Bad review!

Reviews applied: []

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

- Mesos ReviewBot


On Feb. 1, 2016, 6:54 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43056/
> ---
> 
> (Updated Feb. 1, 2016, 6:54 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Edited comments in 'verify_reviews.py' for grammar and style.
> 
> 
> Diffs
> -
> 
>   support/verify_reviews.py 25f3892cf8ba58e4c3fd8a08063f251982876bf4 
> 
> Diff: https://reviews.apache.org/r/43056/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Review Request 43056: Edited comments in 'verify_reviews.py' for grammar and style.

[Greg Mann]

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

Review request for mesos.


Repository: mesos


Description
---

Edited comments in 'verify_reviews.py' for grammar and style.


Diffs
-

  support/verify_reviews.py 25f3892cf8ba58e4c3fd8a08063f251982876bf4 

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


Testing
---


Thanks,

Greg Mann

Re: Review Request 43014: Fix ShasumTest.SHA512SimpleFile failed on centos7.

[Jie Yu]

> On Jan. 30, 2016, 5:11 p.m., Jie Yu wrote:
> > src/common/command_utils.cpp, line 172
> > 
> >
> > On ubuntu, we don't have sha512sum but shasum (and sha512).
> > 
> > Here is what I am thinking:
> > 1) we need to introduce another command util 'which' that searches PATH 
> > and returns the full path to the command (just shelling out to 'which' 
> > command).
> > 2) Then, we use 'which' to check for shasum, sha512sum, etc., and then 
> > use the one that's available.
> 
> haosdent huang wrote:
> which ubuntu version you use, I use ubuntu 14.04. Alos could found 
> `sha512sum` in `/usr/bin/sha512sum`.
> 
> Jojy Varghese wrote:
> I thought `which` will work only if the command is in the path.
> 
> Shuai Lin wrote:
> The command "which" is not avaiable in the `centos:7` image
> 
> ```sh
> $ docker run -it --rm centos:7 which ls
> exec: "which": executable file not found in $PATH
> Error response from daemon: Cannot start container 
> f13beade3abf9139ddad4ec9303a191a7e5a7cc28d011d378c31e59ecf360549: [8] System 
> error: exec: "which": executable file not found in $PATH
> ```
> 
> To get it one need to run "yum install which".
> 
> haosdent huang wrote:
> LoL, seems we should focus on `shasum` and `sha512sum ` command rather 
> than `which`.
> 
> Jie Yu wrote:
> OK, sounds good to me. (BTW, we can implement our own os::which which 
> uses os::envirionment and os::exists).
> 
> Joseph Wu wrote:
> FYI: This topic came up a few times, so I filed this JIRA to track:
> https://issues.apache.org/jira/browse/MESOS-4576

Thanks Joseph!


- Jie


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


On Jan. 31, 2016, 6:21 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43014/
> ---
> 
> (Updated Jan. 31, 2016, 6:21 p.m.)
> 
> 
> Review request for mesos, Jie Yu, Jojy Varghese, and Vinod Kone.
> 
> 
> Bugs: MESOS-4556
> https://issues.apache.org/jira/browse/MESOS-4556
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix ShasumTest.SHA512SimpleFile failed on centos7.
> 
> 
> Diffs
> -
> 
>   src/common/command_utils.cpp 722d0b42a995a6a5a6cfd08f3c356dabd1080cd1 
>   src/tests/common/command_utils_tests.cpp 
> e81c724fc28717f5cb1849d75a066b6da713cd44 
> 
> Diff: https://reviews.apache.org/r/43014/diff/
> 
> 
> Testing
> ---
> 
> # check in CentOS 7.
> sudo ./bin/mesos-tests.sh --gtest_filter="ShasumTest.SHA512SimpleFile"
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 43056: Edited comments in 'verify_reviews.py' for grammar and style.

[Greg Mann]

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

(Updated Feb. 1, 2016, 10:10 p.m.)


Review request for mesos.


Repository: mesos


Description
---

Edited comments in 'verify_reviews.py' for grammar and style.


Diffs (updated)
-

  support/verify_reviews.py 25f3892cf8ba58e4c3fd8a08063f251982876bf4 

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


Testing
---


Thanks,

Greg Mann

Re: Review Request 42974: Removed unused variable in libprocess.

[Neil Conway]

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

(Updated Feb. 1, 2016, 10:08 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Fix commit message style


Summary (updated)
-

Removed unused variable in libprocess.


Repository: mesos


Description (updated)
---

Removed unused variable in libprocess.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp d8a74d7637d20c81f384e974e4fdeba22effb437 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 42618: Modified the `cgroups/net_cls` isolator to use the `NetClsHandleMgr`.

[Jie Yu]

> On Feb. 1, 2016, 9:40 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp, lines 454-455
> > 
> >
> > In fact, i think we should add a helper in src/linux/cgroup.hpp (like 
> > we did for other cgroups controls like cpu.shares, etc.)
> > 
> > ```
> > Try cgroups::net_cls::classid(uint32_t classid)
> > ```
> 
> Avinash sridharan wrote:
> Will do. Will create a separate patch and make this dependent on the new 
> patch.

Yeah, that's fine.


- Jie


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


On Jan. 31, 2016, 8:06 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42618/
> ---
> 
> (Updated Jan. 31, 2016, 8:06 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4345
> https://issues.apache.org/jira/browse/MESOS-4345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `NetClsHandleMgr` will be responsible for keeping track of the net_cls
> handles that will be used by the net_cls cgroup subsystem.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
> b4bc52114389d1c1efce2830f4292bd89bb0de7c 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
> ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 
> 
> Diff: https://reviews.apache.org/r/42618/diff/
> 
> 
> Testing
> ---
> 
> make and make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 42587: Implemented the `NetClsHandleMgr` class.

[Jie Yu]

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




src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (lines 137 - 139)


Hum, having two data fields here looks a little confusing to me. How about 
just merging them:

```
// A map for used handles: primary -> bitset for secondary.
hashmap> used;
```



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


Please insert a blank line above.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (lines 53 - 62)


This is a simple 'contains' check. For readability, I would rather inline 
this instead of creating a helper method.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (lines 73 - 78)


I got confused here. minor is a uint16 value. Why this check is necessary?



src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (lines 82 - 96)


For testin, I would probably just expose a 'isUsed' method:
```
bool isUsed(const NetClsHandle& handle);
```



src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (lines 102 - 105)


See my comments above. I would rather have the 'contains' check here.



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


why minorHandle -1 here?



src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp (lines 114 - 116)


Fix error message here (indentation and '+')



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


I would simply start from position 0 and find the first unset bit. We won't 
have a lot of containers (O(100) as most). So I won't be too worried about the 
performance here. Add a TODO to do it more efficiently should be sufficient.


- Jie Yu


On Jan. 31, 2016, 5:56 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42587/
> ---
> 
> (Updated Jan. 31, 2016, 5:56 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4345
> https://issues.apache.org/jira/browse/MESOS-4345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented the `NetClsHandleMgr` class.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
> b4bc52114389d1c1efce2830f4292bd89bb0de7c 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
> ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 
> 
> Diff: https://reviews.apache.org/r/42587/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Review Request 43072: Use full screen width for Mesos UI using Bootstraps (v3.3.6) container-fluid

[Michael Lunøe]

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

Review request for mesos, Bernd Mathiske and Thomas Rampelberg.


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


Repository: mesos


Description
---

The solution to our problem is solved by updating the Bootstrap CSS file, as 
container-fluid was introduced later in Bootstrap.

I have updated to lastest stable release of Bootstrap v3.3.6. Looking through 
the change log, the only thing that we are using in the Mesos UI and changed is 
the "hide" class (deprecated), so instances of these have been replaced by its 
successor ".hidden" class.


Diffs
-

  src/webui/master/static/browse.html 6b18056 
  src/webui/master/static/css/bootstrap-3.0.3.min.css c547283 
  src/webui/master/static/css/bootstrap-3.3.6.min.css PRE-CREATION 
  src/webui/master/static/framework.html 9b28820 
  src/webui/master/static/home.html d6cde1e 
  src/webui/master/static/index.html 25caf53 
  src/webui/master/static/slave.html bc46885 
  src/webui/master/static/slave_executor.html 9d582d5 
  src/webui/master/static/slave_framework.html 96d788f 

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


Testing
---

- Mobile: http://cl.ly/2R2O0m1a1G3Q/Image%202016-02-01%20at%2015.21.53.png
- 1680 x 1050px screen: 
http://cl.ly/0K1o112u3L2I/Image%202016-02-01%20at%2015.23.37.png
- 2880 x 1800px screen: 
http://cl.ly/1M0L3M2X0J2i/Image%202016-02-01%20at%2015.22.50.png

Visual test in latest Chrome, Firefox, Safari and IE. No functional changes 
introduced.


Thanks,

Michael Lunøe

Re: Review Request 42618: Modified the `cgroups/net_cls` isolator to use the `NetClsHandleMgr`.

[Avinash sridharan]

> On Feb. 1, 2016, 9:40 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp, lines 454-455
> > 
> >
> > In fact, i think we should add a helper in src/linux/cgroup.hpp (like 
> > we did for other cgroups controls like cpu.shares, etc.)
> > 
> > ```
> > Try cgroups::net_cls::classid(uint32_t classid)
> > ```

Will do. Will create a separate patch and make this dependent on the new patch.


- Avinash


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


On Jan. 31, 2016, 8:06 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42618/
> ---
> 
> (Updated Jan. 31, 2016, 8:06 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4345
> https://issues.apache.org/jira/browse/MESOS-4345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `NetClsHandleMgr` will be responsible for keeping track of the net_cls
> handles that will be used by the net_cls cgroup subsystem.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
> b4bc52114389d1c1efce2830f4292bd89bb0de7c 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
> ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 
> 
> Diff: https://reviews.apache.org/r/42618/diff/
> 
> 
> Testing
> ---
> 
> make and make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 43065: Fixed a leak of a `ZooKeeperMasterContender` in a test case.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43033, 43065]

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

- Mesos ReviewBot


On Feb. 1, 2016, 9:40 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43065/
> ---
> 
> (Updated Feb. 1, 2016, 9:40 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a leak of a `ZooKeeperMasterContender` in a test case.
> 
> 
> Diffs
> -
> 
>   src/tests/master_contender_detector_tests.cpp 
> 6375586c31b1fd406529bf299dad6e321b945de8 
> 
> Diff: https://reviews.apache.org/r/43065/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Note that the new test in https://reviews.apache.org/r/43035 fails if this 
> leak is not fixed, because advancing the clock causes the leaked 
> `GroupProcess` to fire an `expired` event.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 42616: Cleaned up usage of namespace-qualified identifiers in libprocess.

[Neil Conway]

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

(Updated Feb. 1, 2016, 10:07 p.m.)


Review request for mesos and Michael Park.


Changes
---

Fix commit message style


Summary (updated)
-

Cleaned up usage of namespace-qualified identifiers in libprocess.


Repository: mesos


Description
---

For example, avoid using `std::string` in a .cpp file that already does
`using std::string`.


Diffs (updated)
-

  3rdparty/libprocess/src/authenticator.cpp 
16b48cd599b10c09229fbf10f28139219855710b 
  3rdparty/libprocess/src/http.cpp 762da9a9038fc0a81156b5c03b556084df1bd7e0 
  3rdparty/libprocess/src/io.cpp 9530192e2c7afbfa4bd9aa2af2ff40f00ee505a9 
  3rdparty/libprocess/src/libevent_ssl_socket.cpp 
9321ce666a0c5c75cdbdd1c5a6d534fbc221bf5b 
  3rdparty/libprocess/src/metrics/metrics.cpp 
3083b92b3fb67421265950b03272b52cec68daf9 
  3rdparty/libprocess/src/pid.cpp 3cd04ecaddd045d9d5ecf132302417181d416f80 
  3rdparty/libprocess/src/process.cpp d8a74d7637d20c81f384e974e4fdeba22effb437 
  3rdparty/libprocess/src/socket.cpp 6c1b16f69a21d116a0ffef21097a1b4372977b2e 
  3rdparty/libprocess/src/tests/process_tests.cpp 
df1c7b3f37ab1103991a4f27a9e0359bcf06cc68 

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


Testing
---

"make check" on OSX and Arch Linux.


Thanks,

Neil Conway

Re: Review Request 42997: [6 of 7] Offer shareable resources to frameworks only if opted in.

[Anindya Sinha]

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




src/master/master.cpp (line 5452)


Adding gyliu's comments from RR 42992 to this RR for rebase issues.
move this comments to 5473


- Anindya Sinha


On Jan. 30, 2016, 1:35 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42997/
> ---
> 
> (Updated Jan. 30, 2016, 1:35 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-4325
> https://issues.apache.org/jira/browse/MESOS-4325
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a new capability SHAREABLE_RESOURCES that frameworks need to opt in
> if they are interested in receiving shared resources in their offers.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 194750e92020753e60154083a47bdc3398d31466 
>   include/mesos/v1/mesos.proto 1102bbc92f46f97c1915c03a71c7cf829003e0ed 
>   src/examples/persistent_shared_volume_framework.cpp PRE-CREATION 
>   src/master/master.cpp 98441a543611d4083b2495ee103f0ab5e2187e83 
> 
> Diff: https://reviews.apache.org/r/42997/diff/
> 
> 
> Testing
> ---
> 
> make check done.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 42615: Cleaned up usage of namespace-qualified identifiers in Mesos.

[Neil Conway]

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

(Updated Feb. 1, 2016, 10:07 p.m.)


Review request for mesos and Michael Park.


Changes
---

Fix commit message style


Summary (updated)
-

Cleaned up usage of namespace-qualified identifiers in Mesos.


Repository: mesos


Description
---

For example, avoid using `std::string` in a .cpp file that already does
`using std::string`.


Diffs (updated)
-

  src/authentication/http/basic_authenticator_factory.cpp 
6eb1c5bd09b136d3bc20481ddcc65cb8bd153682 
  src/cli/execute.cpp 0add77558e07ff49f0134ddb9085509501c61aab 
  src/common/http.cpp b7e71eb79ccb8ef1d0bbd168d77d4d4591ecb09b 
  src/common/protobuf_utils.cpp 53324ab569751924f25290641d1a70da790c2104 
  src/docker/docker.cpp a83172674b385a59cfd6f344836740a5719f954e 
  src/examples/balloon_framework.cpp 3c17676c92f72d41c501b2435a7017e3c9f01278 
  src/examples/test_container_logger_module.cpp 
6b1f4dbd63145afe4de17830c0a2a6202a896be7 
  src/examples/test_http_authenticator_module.cpp 
acf51a6deb8e7dc4ab6ac0cf70380ddbb1839906 
  src/examples/test_qos_controller_module.cpp 
f8aa1d730f6cdf5ab3f11789e9699682cc08ddd5 
  src/files/files.cpp dd64976ca454786152a8e29f590e8c1ed1df3b54 
  src/health-check/main.cpp 0beaed575ec865d81e6e3d83d8a0c894613acba4 
  src/hook/manager.cpp 6ee93038dce1247cffbc82c736a1c7a1ecb84bb0 
  src/launcher/fetcher.cpp 902e927ce0cbcf70e41041375e61987752629957 
  src/linux/routing/queueing/fq_codel.cpp 
d840e383f28481ba58d8269217a8a194c4e87db5 
  src/linux/routing/queueing/ingress.cpp 
c25a93966caca5c5a9eef91e6caa5d9e29091bee 
  src/linux/systemd.cpp d3f4a63d833a5bc599494e925d709ba0cc70d9ec 
  src/local/local.cpp 582d4a10444831b0753d20650698e5d3b51cca9f 
  src/log/tool/initialize.cpp c9706d1e33659f7b0e375419197b207cb96d4ca9 
  src/log/tool/read.cpp 9abf5a82338c554920283e0329cd83e4c787c103 
  src/log/tool/replica.cpp 8baf79755da420cf594d706f31aa561b5d665051 
  src/master/allocator/mesos/hierarchical.cpp 
1a07d69016407e5aad2209586da37fecbcddb765 
  src/master/allocator/sorter/drf/sorter.cpp 
db47d640e36c0302d7c6254a9c58caa878feac01 
  src/master/main.cpp 7ebca788c35d5bb6898ae65df91b899c2b74f3d4 
  src/master/maintenance.cpp df7cd6ca5a097611e5ca5a46cc988ef636da096b 
  src/master/master.cpp 98441a543611d4083b2495ee103f0ab5e2187e83 
  src/module/manager.cpp a53f71b9965f7ab85aadb6c0c7af18de958faf38 
  src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
  src/slave/containerizer/fetcher.cpp f7e3f7de6d0ebc253d2ccfe4a66b10ed2eca2c11 
  src/slave/containerizer/mesos/linux_launcher.cpp 
61801fffe9bac1a995a57d8b4e8c004d624bbd63 
  src/slave/containerizer/mesos/provisioner/docker/paths.cpp 
9c5c24992f9ea36159271691cca0147851240088 
  src/slave/containerizer/mesos/provisioner/docker/registry_client.cpp 
b74c760e90b12f5bf45b6e626e2b661f841e5a8d 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
71caeca9005b5846c56ff4dfd260b426958a73c5 
  src/slave/paths.cpp 77e86a07b7c64775d1cb1542527cdfc38b6748ff 
  src/slave/qos_controllers/load.cpp 52520d6220784423c09001b8bd59e090ebf787ec 
  src/slave/state.cpp fae7738a1cb7168abc0cfe35b075bbc73306b820 
  src/tests/attributes_tests.cpp 3f3dde1301566f279c3fc7dc24e3ef67039a4c14 
  src/tests/common/http_tests.cpp 0ea06341b092cd6ad278075b12dd970b84c84464 
  src/tests/containerizer/cgroups_isolator_tests.cpp 
0d24809701c42dc036bbd051791545f4742d1e9a 
  src/tests/containerizer/cgroups_tests.cpp 
5e15d44a472b2acd64475f69fd44cd4d2d9f729d 
  src/tests/containerizer/external_containerizer_test.cpp 
da52860f4a1d5363d7b61b9a8bb6abad02d89736 
  src/tests/containerizer/provisioner_docker_tests.cpp 
de99126aca8dccea0b4c04f975e4da4710b5a1f1 
  src/tests/credentials_tests.cpp 6e3725c61a55a2c1d183ee278cf3d72be16a6e40 
  src/tests/fetcher_cache_tests.cpp 2747b72ba49c9fde04e556b649601b037517283e 
  src/tests/gc_tests.cpp ef5544b627ac4a9f9c13fe13b0c51b21d5b98a12 
  src/tests/hierarchical_allocator_tests.cpp 
f18e6eb10572b0f5b8bbff338384d9406f6ad62b 
  src/tests/hook_tests.cpp 1a1bab49469485887e0a6a8fa1837bd70262d3b5 
  src/tests/http_authentication_tests.cpp 
bd622576973648e0dfeae1453a5ce631e4171352 
  src/tests/master_quota_tests.cpp 04efcf3362d3594e0ad8077793fa1f32536dd658 
  src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
  src/tests/monitor_tests.cpp 2226458b59b4a279a92e1353bd61457a0018d2a9 
  src/tests/registrar_tests.cpp 58b3fccf5450d20032c5c6d20d76dbd868c424a3 
  src/tests/registrar_zookeeper_tests.cpp 
3861dcf8341bfa520f94fd244b4814827299b861 
  src/tests/scheduler_driver_tests.cpp f35c4957b08803dba924ebf1a5f0af9daac9d0c5 
  src/tests/slave_tests.cpp ede2df08c2a9f7e1f010ff9dccbea0a1e94af55f 
  src/tests/values_tests.cpp a4eb68ad13407f471a07a9a923ed31c7890da9f7 
  src/uri/fetchers/curl.cpp 269df874f3a3a65d045a0822af57ba65e23a9fe0 
  

Re: Review Request 42971: Marked an unused function parameter appropriately in stout.

[Neil Conway]

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

(Updated Feb. 1, 2016, 10:08 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Fix commit message style


Summary (updated)
-

Marked an unused function parameter appropriately in stout.


Repository: mesos


Description (updated)
---

Marked an unused function parameter appropriately in stout.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/option.hpp 
a3dc7697f5f041a503e4aa84df67c915c43b76b5 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 42972: Fixed a typo (unintended inequality between pointer value and 0) in stout.

[Neil Conway]

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

(Updated Feb. 1, 2016, 10:08 p.m.)


Review request for mesos and Jie Yu.


Changes
---

Fix commit message style


Summary (updated)
-

Fixed a typo (unintended inequality between pointer value and 0) in stout.


Repository: mesos


Description (updated)
---

Fixed a typo (unintended inequality between pointer value and 0) in stout.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 
2a188459465a5203c56d788a74e69d403790c5bf 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 42618: Modified the `cgroups/net_cls` isolator to use the `NetClsHandleMgr`.

[Avinash sridharan]

> On Feb. 1, 2016, 8:14 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp, lines 499-503
> > 
> >
> > You should do this after the cgroup has been destroyed.

Makes sense. Planning to convert the lamdba being used in the defer for cleanup 
into a function, since we are adding more logic to the Lambda now. Hope that's 
fine?


- Avinash


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


On Jan. 31, 2016, 8:06 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42618/
> ---
> 
> (Updated Jan. 31, 2016, 8:06 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4345
> https://issues.apache.org/jira/browse/MESOS-4345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `NetClsHandleMgr` will be responsible for keeping track of the net_cls
> handles that will be used by the net_cls cgroup subsystem.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
> b4bc52114389d1c1efce2830f4292bd89bb0de7c 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
> ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 
> 
> Diff: https://reviews.apache.org/r/42618/diff/
> 
> 
> Testing
> ---
> 
> make and make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 43035: Added a test for the interaction between timers and destroyed Groups.

[Neil Conway]

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

(Updated Feb. 1, 2016, 9:40 p.m.)


Review request for mesos and Joris Van Remoortere.


Changes
---

Tweak expectation to be stronger.


Repository: mesos


Description
---

Check that even though we might fire a timer for a `GroupProcess` that has been
destroyed, this does not result in dispatching an event to a reclaimed process.


Diffs (updated)
-

  src/tests/group_tests.cpp 6344fade519f47cd4ac3a2dc36c3237d9adf4cf8 

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


Testing
---

`make check; ./src/mesos-tests --gtest_filter="GroupTest.TimerCleanup" 
--gtest_repeat=4000 --gtest_break_on_failure`

I also verified that if you remove the `delete` of the `GroupProcess`, the test 
fails because the `expired` callback is invoked (the `Clock::settle()` is 
necessary to ensure that this happens).


Thanks,

Neil Conway

Re: Review Request 42617: Cleaned up usage of namespace-qualified identifiers in stout.

[Neil Conway]

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

(Updated Feb. 1, 2016, 10:07 p.m.)


Review request for mesos and Michael Park.


Changes
---

Fix commit message style


Summary (updated)
-

Cleaned up usage of namespace-qualified identifiers in stout.


Repository: mesos


Description
---

For example, avoid using `std::string` in a .cpp file that already does
`using std::string`.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/tests/flags_tests.cpp 
5a10f59f3698a6ad85b7c2386def9686af8a23a6 
  3rdparty/libprocess/3rdparty/stout/tests/linkedhashmap_tests.cpp 
21a98b85277d946b887cb23c4ebb4d32cfcb0c6b 
  3rdparty/libprocess/3rdparty/stout/tests/os/filesystem_tests.cpp 
01e4b2fff7cc05a47d7919d702543a38bf6c339d 
  3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
4b7d1347f93261df7a7d512721146edca2703b34 
  3rdparty/libprocess/3rdparty/stout/tests/some_tests.cpp 
c0be974ea2dc74c63ec3561403b3f4d3722a8df3 
  3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 
7715fa4baa150f370bdd0fd5c2f98dc4c6f5fc55 

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


Testing
---

"make check" on OSX and Arch Linux.


Thanks,

Neil Conway

Review Request 43065: Fixed a leak of a `ZooKeeperMasterContender` in a test case.

[Neil Conway]

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

Review request for mesos and Joris Van Remoortere.


Repository: mesos


Description
---

Fixed a leak of a `ZooKeeperMasterContender` in a test case.


Diffs
-

  src/tests/master_contender_detector_tests.cpp 
6375586c31b1fd406529bf299dad6e321b945de8 

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


Testing
---

make check

Note that the new test in https://reviews.apache.org/r/43035 fails if this leak 
is not fixed, because advancing the clock causes the leaked `GroupProcess` to 
fire an `expired` event.


Thanks,

Neil Conway

Re: Review Request 43056: Fixed typo in 'verify_reviews.py'.

[Greg Mann]

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

(Updated Feb. 1, 2016, 10:11 p.m.)


Review request for mesos.


Summary (updated)
-

Fixed typo in 'verify_reviews.py'.


Repository: mesos


Description (updated)
---

Fixed typo in 'verify_reviews.py'.


Diffs
-

  support/verify_reviews.py 25f3892cf8ba58e4c3fd8a08063f251982876bf4 

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


Testing
---


Thanks,

Greg Mann

Re: Review Request 43035: Added a test for the interaction between timers and destroyed Groups.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43033, 43065, 43034, 43035]

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

- Mesos ReviewBot


On Feb. 1, 2016, 9:40 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43035/
> ---
> 
> (Updated Feb. 1, 2016, 9:40 p.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check that even though we might fire a timer for a `GroupProcess` that has 
> been
> destroyed, this does not result in dispatching an event to a reclaimed 
> process.
> 
> 
> Diffs
> -
> 
>   src/tests/group_tests.cpp 6344fade519f47cd4ac3a2dc36c3237d9adf4cf8 
> 
> Diff: https://reviews.apache.org/r/43035/diff/
> 
> 
> Testing
> ---
> 
> `make check; ./src/mesos-tests --gtest_filter="GroupTest.TimerCleanup" 
> --gtest_repeat=4000 --gtest_break_on_failure`
> 
> I also verified that if you remove the `delete` of the `GroupProcess`, the 
> test fails because the `expired` callback is invoked (the `Clock::settle()` 
> is necessary to ensure that this happens).
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 42997: Offer shareable resources to frameworks only if opted in.

[Anindya Sinha]

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

(Updated Feb. 2, 2016, 12:39 a.m.)


Review request for mesos and Adam B.


Summary (updated)
-

Offer shareable resources to frameworks only if opted in.


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


Repository: mesos


Description
---

Added a new capability SHAREABLE_RESOURCES that frameworks need to opt in
if they are interested in receiving shared resources in their offers.


Diffs (updated)
-

  include/mesos/mesos.proto 194750e92020753e60154083a47bdc3398d31466 
  include/mesos/v1/mesos.proto 1102bbc92f46f97c1915c03a71c7cf829003e0ed 
  src/examples/persistent_shared_volume_framework.cpp PRE-CREATION 
  src/master/master.cpp 98441a543611d4083b2495ee103f0ab5e2187e83 

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


Testing
---

make check done.


Thanks,

Anindya Sinha

Re: Review Request 42995: Allow tasks to set persistent volume as readonly or readwrite resource.

[Anindya Sinha]

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

(Updated Feb. 2, 2016, 12:38 a.m.)


Review request for mesos and Adam B.


Summary (updated)
-

Allow tasks to set persistent volume as readonly or readwrite resource.


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


Repository: mesos


Description (updated)
---

If the task is the first consumer of the shared persistent volume, then
set the ownership of the persistent volume to match that of the task.
Otherwise, allow the task to be executed only if the ownership of the
persistent volume matches that of the task.
Allow the task to specify the persistent volume access to be read-only
or read-write. Note that the persistent volume is always created as
read-write.


Diffs (updated)
-

  src/examples/persistent_shared_volume_framework.cpp PRE-CREATION 
  src/master/master.cpp 98441a543611d4083b2495ee103f0ab5e2187e83 
  src/master/validation.hpp 380b40279faf180a6f401a5e28280b601dbc648c 
  src/master/validation.cpp f2bc1bad79e3b0812c019be3774cd65b58ea2d07 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
24ff8194bb5ea86a31224d2d2c20d367472168aa 
  src/slave/containerizer/mesos/isolators/filesystem/posix.cpp 
bce5ab9a1913ee7cebaf55b5ee0b42fa4a325e87 

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


Testing
---

make check done.


Thanks,

Anindya Sinha

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

[Anindya Sinha]

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

(Updated Feb. 2, 2016, 12:38 a.m.)


Review request for mesos and Adam B.


Summary (updated)
-

Added a persistent volume test framework for shared volumes.


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


Repository: mesos


Description
---

Added a persistent volume test framework for shared volumes.


Diffs (updated)
-

  src/Makefile.am fac17f4bac3b2ddda0384dec8b0ed6f960bd24d1 
  src/examples/persistent_shared_volume_framework.cpp PRE-CREATION 
  src/tests/examples_tests.cpp a9b685b0290f23461f43b12f05f6071fa46412a6 
  src/tests/persistent_shared_volume_framework_test.sh PRE-CREATION 

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


Testing
---

make check done.


Thanks,

Anindya Sinha

Re: Review Request 42994: [3 of 7] Added a persistent volume test framework for shared volumes.

[Anindya Sinha]

> On Jan. 30, 2016, 10:30 a.m., Guangya Liu wrote:
> > src/tests/persistent_shared_volume_framework_test.sh, line 1
> > 
> >
> > Seems this file does not have execute permssion

Seems like it does.
-rwxr-xr-x  1   x  726 Feb  1 13:57 
persistent_shared_volume_framework_test.sh


- Anindya


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


On Jan. 30, 2016, 12:26 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42994/
> ---
> 
> (Updated Jan. 30, 2016, 12:26 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-4431
> https://issues.apache.org/jira/browse/MESOS-4431
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a persistent volume test framework for shared volumes.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am fac17f4bac3b2ddda0384dec8b0ed6f960bd24d1 
>   src/examples/persistent_shared_volume_framework.cpp PRE-CREATION 
>   src/tests/examples_tests.cpp a9b685b0290f23461f43b12f05f6071fa46412a6 
>   src/tests/persistent_shared_volume_framework_test.sh PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42994/diff/
> 
> 
> Testing
> ---
> 
> make check done.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 42998: Added docs for shareable resources.

[Anindya Sinha]

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

(Updated Feb. 2, 2016, 12:39 a.m.)


Review request for mesos and Adam B.


Summary (updated)
-

Added docs for shareable resources.


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


Repository: mesos


Description
---

Added docs for shareable resources.


Diffs (updated)
-

  docs/home.md dea6ec2605662dbd4b10d69b2bf8f35af50389ec 
  docs/shareable-resources.md PRE-CREATION 

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


Testing
---

Just documentation changes in this RR.


Thanks,

Anindya Sinha

Re: Review Request 42996: Add unit tests for sharing of resources.

[Anindya Sinha]

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

(Updated Feb. 2, 2016, 12:38 a.m.)


Review request for mesos and Adam B.


Summary (updated)
-

Add unit tests for sharing of resources.


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


Repository: mesos


Description
---

Add unit tests for sharing of resources.


Diffs (updated)
-

  src/tests/master_validation_tests.cpp 
6fae01fa1833ae05ec82618a4ae28ac5bd275bd5 
  src/tests/mesos.hpp c2bae4767ee7372c796bfad44ed1e86db7dd3488 
  src/tests/persistent_volume_tests.cpp 
cbf2bcedd5b4c14107d3b50a74f161aa9395d7d0 
  src/tests/resources_tests.cpp 4b25e82c13e4f46c73803f773db90f269c09c48a 

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


Testing
---

make check done.


Thanks,

Anindya Sinha

Re: Review Request 43072: Use full screen width for Mesos UI using Bootstraps (v3.3.6) container-fluid

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [43072]

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

Error:
2016-02-02 02:45:28 URL:https://reviews.apache.org/r/43072/diff/raw/ 
[226634/226634] -> "43072.patch" [1]
Traceback (most recent call last):
  File "support/apply-reviews.py", line 342, in 
reviewboard()
  File "support/apply-reviews.py", line 321, in reviewboard
apply_review()
  File "support/apply-reviews.py", line 139, in apply_review
commit_patch()
  File "support/apply-reviews.py", line 180, in commit_patch
data = patch_data()
  File "support/apply-reviews.py", line 199, in patch_data
return reviewboard_data()
  File "support/apply-reviews.py", line 252, in reviewboard_data
email=user.get('email'))
UnicodeEncodeError: 'ascii' codec can't encode character u'\xf8' in position 
11: ordinal not in range(128)

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

- Mesos ReviewBot


On Feb. 1, 2016, 11:36 p.m., Michael Lunøe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43072/
> ---
> 
> (Updated Feb. 1, 2016, 11:36 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Thomas Rampelberg.
> 
> 
> Bugs: MESOS-2585
> https://issues.apache.org/jira/browse/MESOS-2585
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The solution to our problem is solved by updating the Bootstrap CSS file, as 
> container-fluid was introduced later in Bootstrap.
> 
> I have updated to lastest stable release of Bootstrap v3.3.6. Looking through 
> the change log, the only thing that we are using in the Mesos UI and changed 
> is the "hide" class (deprecated), so instances of these have been replaced by 
> its successor ".hidden" class.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 6b18056 
>   src/webui/master/static/css/bootstrap-3.0.3.min.css c547283 
>   src/webui/master/static/css/bootstrap-3.3.6.min.css PRE-CREATION 
>   src/webui/master/static/framework.html 9b28820 
>   src/webui/master/static/home.html d6cde1e 
>   src/webui/master/static/index.html 25caf53 
>   src/webui/master/static/slave.html bc46885 
>   src/webui/master/static/slave_executor.html 9d582d5 
>   src/webui/master/static/slave_framework.html 96d788f 
> 
> Diff: https://reviews.apache.org/r/43072/diff/
> 
> 
> Testing
> ---
> 
> - Mobile: http://cl.ly/2R2O0m1a1G3Q/Image%202016-02-01%20at%2015.21.53.png
> - 1680 x 1050px screen: 
> http://cl.ly/0K1o112u3L2I/Image%202016-02-01%20at%2015.23.37.png
> - 2880 x 1800px screen: 
> http://cl.ly/1M0L3M2X0J2i/Image%202016-02-01%20at%2015.22.50.png
> 
> Visual test in latest Chrome, Firefox, Safari and IE. No functional changes 
> introduced.
> 
> 
> Thanks,
> 
> Michael Lunøe
> 
>

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

[Anindya Sinha]

> On Jan. 30, 2016, 4:02 a.m., Guangya Liu wrote:
> > src/common/resources.cpp, line 899
> > 
> >
> > Do we also need to check contain here?
> > 
> > I think that the unsharable should be failed if the current resoures 
> > does not contain the unshared resources.

No, since the passed Resource "that" needs to have share=true. If that is not 
the case, it is not a shared resource and hence cannot be unshared. If "that" 
is a shared resource, then it falls through to the call to 
`_getExistingConsumers(that)` where the logic for num_consumers==0 is done.


> On Jan. 30, 2016, 4:02 a.m., Guangya Liu wrote:
> > src/common/resources.cpp, line 1134
> > 
> >
> > Here not checking "if the volume exists" but "if the volume is a subset 
> > of current resources"

In `destroyable()`, check for `contains()` is done for non shareable resources. 
For shareable resources, we do a similar check in `_getExistingConsumers()` but 
extract the resource to ensure its num_consumers==0.
I think it covers both scenarios. Am I missing something?


> On Jan. 30, 2016, 4:02 a.m., Guangya Liu wrote:
> > src/common/resources.cpp, lines 1652-1654
> > 
> >
> > I think that we already set the num consumers as 0 when initialize, so 
> > when the logic can go here?
> > 
> > What is the difference of {None} and {0} here?

{None} is when num_consumers is not set, and 0 is when num_consumers is set to 
0. We do not send or expect to receive num_consumers for shareable resources in 
Resources for all messages going between master <-> framework. This condition 
handles the scenario when Resource is logged at these points when num_consumers 
is not set. An example is when we log the offer received while running all our 
test frameworks.


> On Jan. 30, 2016, 4:02 a.m., Guangya Liu wrote:
> > src/common/resources_utils.cpp, lines 71-73
> > 
> >
> > Do we need to move this under 68 under `isPersistentVolume` condition 
> > section?

We can but I think it is fine here as well, especially because shareable 
resources are supported for all resource types but enabled for persistent 
volumes right now. Except the check in `Option Resources::validate(const 
Resource& resource)`, I do not think there are any references to assert that 
only persistent volumes can be shared.
Also in future, when we do extend sharing of resources to other resource tyoes 
as well, we should be good to go here.


- Anindya


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


On Feb. 2, 2016, 12:36 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42992/
> ---
> 
> (Updated Feb. 2, 2016, 12:36 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-4431
> https://issues.apache.org/jira/browse/MESOS-4431
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added new Offer::Operation of SHARE and UNSHARE for resources.
> Added ShareInfo within Resources protobuf to allow for sharing of resources
> and keep track of consumers of such resources.
> Shareable resources keeps track of its consumers and makes decision based on
> that during task launch and task termination.
> Allow DESTROY or UNSHARE for shared volumes only if reference count is 0.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 194750e92020753e60154083a47bdc3398d31466 
>   include/mesos/resources.hpp 6bfac2e7e8799e74d87c7570fc5eef320ba76eb1 
>   include/mesos/v1/mesos.proto 1102bbc92f46f97c1915c03a71c7cf829003e0ed 
>   include/mesos/v1/resources.hpp 5a88c0756db2ea8db0f5df7ea3019b511ea135af 
>   src/common/resources.cpp 588a279c3cdebbeb58047bfbff5e78a42f53fc13 
>   src/common/resources_utils.cpp 70e6f025d89383084ab8b2cda23ab1cd55d959b2 
>   src/master/allocator/mesos/hierarchical.cpp 
> 1a07d69016407e5aad2209586da37fecbcddb765 
>   src/master/allocator/sorter/drf/sorter.cpp 
> db47d640e36c0302d7c6254a9c58caa878feac01 
>   src/master/master.hpp 3a7e18232323a1c051bcc97915484b1195fffe58 
>   src/master/master.cpp 98441a543611d4083b2495ee103f0ab5e2187e83 
>   src/master/validation.hpp 380b40279faf180a6f401a5e28280b601dbc648c 
>   src/master/validation.cpp f2bc1bad79e3b0812c019be3774cd65b58ea2d07 
>   src/v1/resources.cpp be4a5d153e9313cb71a6e85d1ed25a358537f2b7 
> 
> Diff: https://reviews.apache.org/r/42992/diff/
> 
> 
> Testing
> ---
> 
> make check done.
> 
> 
> Thanks,
> 
> Anindya 

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

[Anindya Sinha]

> On Feb. 1, 2016, 3:46 a.m., Guangya Liu wrote:
> > src/master/master.cpp, lines 5444-5445
> > 
> >
> > move this comments to 5452

To avoid rebase issues, I will fix this as a part of 
`https://reviews.apache.org/r/42997/`.


- Anindya


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


On Feb. 2, 2016, 12:36 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42992/
> ---
> 
> (Updated Feb. 2, 2016, 12:36 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-4431
> https://issues.apache.org/jira/browse/MESOS-4431
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added new Offer::Operation of SHARE and UNSHARE for resources.
> Added ShareInfo within Resources protobuf to allow for sharing of resources
> and keep track of consumers of such resources.
> Shareable resources keeps track of its consumers and makes decision based on
> that during task launch and task termination.
> Allow DESTROY or UNSHARE for shared volumes only if reference count is 0.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 194750e92020753e60154083a47bdc3398d31466 
>   include/mesos/resources.hpp 6bfac2e7e8799e74d87c7570fc5eef320ba76eb1 
>   include/mesos/v1/mesos.proto 1102bbc92f46f97c1915c03a71c7cf829003e0ed 
>   include/mesos/v1/resources.hpp 5a88c0756db2ea8db0f5df7ea3019b511ea135af 
>   src/common/resources.cpp 588a279c3cdebbeb58047bfbff5e78a42f53fc13 
>   src/common/resources_utils.cpp 70e6f025d89383084ab8b2cda23ab1cd55d959b2 
>   src/master/allocator/mesos/hierarchical.cpp 
> 1a07d69016407e5aad2209586da37fecbcddb765 
>   src/master/allocator/sorter/drf/sorter.cpp 
> db47d640e36c0302d7c6254a9c58caa878feac01 
>   src/master/master.hpp 3a7e18232323a1c051bcc97915484b1195fffe58 
>   src/master/master.cpp 98441a543611d4083b2495ee103f0ab5e2187e83 
>   src/master/validation.hpp 380b40279faf180a6f401a5e28280b601dbc648c 
>   src/master/validation.cpp f2bc1bad79e3b0812c019be3774cd65b58ea2d07 
>   src/v1/resources.cpp be4a5d153e9313cb71a6e85d1ed25a358537f2b7 
> 
> Diff: https://reviews.apache.org/r/42992/diff/
> 
> 
> Testing
> ---
> 
> make check done.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 42993: Support for multiple frameworks sharing the same resource.

[Anindya Sinha]

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

(Updated Feb. 2, 2016, 12:37 a.m.)


Review request for mesos and Adam B.


Summary (updated)
-

Support for multiple frameworks sharing the same resource.


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


Repository: mesos


Description (updated)
---

The allocator tracks allocated shared resources separately from the
remaining allocated resources so that multiple frameworks see the
same view of the shared resource at any given time. This avoids
unnecessary copies of the same shared resource when multiple
frameworks use such resources simultaneously.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
1a07d69016407e5aad2209586da37fecbcddb765 
  src/master/allocator/sorter/drf/sorter.hpp 
4669149b81de39b4bb921ef7cd6787aa583f6e40 
  src/master/allocator/sorter/drf/sorter.cpp 
db47d640e36c0302d7c6254a9c58caa878feac01 
  src/master/allocator/sorter/sorter.hpp 
a0a779b81f6d048271f15256b38ff907ae144b83 
  src/master/master.cpp 98441a543611d4083b2495ee103f0ab5e2187e83 

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


Testing
---

make check done.


Thanks,

Anindya Sinha

Re: Review Request 42974: Removed unused variable in libprocess.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [42971, 42972]

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

Error:
2016-02-02 01:48:21 URL:https://reviews.apache.org/r/42972/diff/raw/ [728/728] 
-> "42972.patch" [1]
Total errors found: 0
Checking 1 files
Error: Commit message summary (the first line) must not exceed 72 characters.

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

- Mesos ReviewBot


On Feb. 1, 2016, 10:08 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42974/
> ---
> 
> (Updated Feb. 1, 2016, 10:08 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed unused variable in libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> d8a74d7637d20c81f384e974e4fdeba22effb437 
> 
> Diff: https://reviews.apache.org/r/42974/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43056: Fixed typo in 'verify_reviews.py'.

[Mesos ReviewBot]

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



Bad review!

Reviews applied: []

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

- Mesos ReviewBot


On Feb. 1, 2016, 10:11 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43056/
> ---
> 
> (Updated Feb. 1, 2016, 10:11 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed typo in 'verify_reviews.py'.
> 
> 
> Diffs
> -
> 
>   support/verify_reviews.py 25f3892cf8ba58e4c3fd8a08063f251982876bf4 
> 
> Diff: https://reviews.apache.org/r/43056/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

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

[Anindya Sinha]

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

(Updated Feb. 2, 2016, 12:36 a.m.)


Review request for mesos and Adam B.


Summary (updated)
-

Support sharing of resources through reference counting of resources.


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


Repository: mesos


Description (updated)
---

Added new Offer::Operation of SHARE and UNSHARE for resources.
Added ShareInfo within Resources protobuf to allow for sharing of resources
and keep track of consumers of such resources.
Shareable resources keeps track of its consumers and makes decision based on
that during task launch and task termination.
Allow DESTROY or UNSHARE for shared volumes only if reference count is 0.


Diffs (updated)
-

  include/mesos/mesos.proto 194750e92020753e60154083a47bdc3398d31466 
  include/mesos/resources.hpp 6bfac2e7e8799e74d87c7570fc5eef320ba76eb1 
  include/mesos/v1/mesos.proto 1102bbc92f46f97c1915c03a71c7cf829003e0ed 
  include/mesos/v1/resources.hpp 5a88c0756db2ea8db0f5df7ea3019b511ea135af 
  src/common/resources.cpp 588a279c3cdebbeb58047bfbff5e78a42f53fc13 
  src/common/resources_utils.cpp 70e6f025d89383084ab8b2cda23ab1cd55d959b2 
  src/master/allocator/mesos/hierarchical.cpp 
1a07d69016407e5aad2209586da37fecbcddb765 
  src/master/allocator/sorter/drf/sorter.cpp 
db47d640e36c0302d7c6254a9c58caa878feac01 
  src/master/master.hpp 3a7e18232323a1c051bcc97915484b1195fffe58 
  src/master/master.cpp 98441a543611d4083b2495ee103f0ab5e2187e83 
  src/master/validation.hpp 380b40279faf180a6f401a5e28280b601dbc648c 
  src/master/validation.cpp f2bc1bad79e3b0812c019be3774cd65b58ea2d07 
  src/v1/resources.cpp be4a5d153e9313cb71a6e85d1ed25a358537f2b7 

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


Testing
---

make check done.


Thanks,

Anindya Sinha

Re: Review Request 42972: Fixed a minor bug in stout's Flags package.

[Neil Conway]

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

(Updated Feb. 2, 2016, 1:53 a.m.)


Review request for mesos and Jie Yu.


Changes
---

Fix commit summary.


Summary (updated)
-

Fixed a minor bug in stout's Flags package.


Repository: mesos


Description (updated)
---

Unintended inequality comparison between pointer value and 0.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 
2a188459465a5203c56d788a74e69d403790c5bf 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 42586: Defined the NetClsHandleMgr class.

[Jie Yu]

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




src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 40)


2 blank lines above please.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (lines 43 - 44)


I would say it's due to the fact that it depends on libnl headers and we 
cannot bundle libnl since it has GPL licensing.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (lines 51 - 52)


`s/majHandle/_major/`
`s/minHandle/_minor/`

Also, remove the space right after `NetClsHandle`



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 54)


Ditto on removing the extra space.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (lines 60 - 61)


s/majorHandle/major/
s/minorHandle/minor/



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (lines 65 - 75)


Why do we need this?



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (lines 78 - 85)


I would suggest that we don't define a new data structure for this. Can you 
just inline it in NetClsHandleManager?



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (lines 98 - 100)


```
NetClsHandleManager(const IntervalSet& _majors)
  : majors(_majors) {}
```



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (lines 101 - 104)


```
majors(_majors)
```



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (lines 108 - 113)


We do you need this in this patch?



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 117)


Remove the blank line here.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 118)


s/majorHandle/major/



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 121)


Remove the blank ine here.



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 129)


What's the key for this map?



src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp (line 131)


`s/_majorHandles/majors/`



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


insert one blank line above.


- Jie Yu


On Feb. 1, 2016, 2:20 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42586/
> ---
> 
> (Updated Feb. 1, 2016, 2:20 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4345
> https://issues.apache.org/jira/browse/MESOS-4345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This class will be responsible for keeping track of the free and allocated
> net_cls handles.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
> b4bc52114389d1c1efce2830f4292bd89bb0de7c 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
> ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 
> 
> Diff: https://reviews.apache.org/r/42586/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 43032: Set Docker labels based on TaskInfo labels.

[Abhishek Dasgupta]

> On Jan. 31, 2016, 4 p.m., haosdent huang wrote:
> > src/tests/containerizer/docker_tests.cpp, line 122
> > 
> >
> > Do you need verify this in inspect result?

Will you please tell a little bit more about this? Do you want to verify labels 
in inspect result as I found that only container name is available from the 
current inspect result.


- Abhishek


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


On Jan. 31, 2016, 3:22 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43032/
> ---
> 
> (Updated Jan. 31, 2016, 3:22 p.m.)
> 
> 
> Review request for mesos and Guangya Liu.
> 
> 
> Bugs: MESOS-4446
> https://issues.apache.org/jira/browse/MESOS-4446
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Set Docker labels based on TaskInfo labels.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 7802f23585121d41c738cc28f6bcfa5e6dc9e972 
>   src/docker/docker.cpp 4d2f1fa14f4450b8fa3401081bf52807d2e79a7e 
>   src/docker/executor.cpp 654a41db843a85d953880d5145bc95ada9ed2920 
>   src/slave/containerizer/docker.cpp 2887cb4a01febbbf276026e584ffc466289e10c9 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 645bdcf095145097d8b8c65d592c787417883145 
>   src/tests/containerizer/docker_tests.cpp 
> f0ad20e5a8252ad761e8fa42724caed31b8fae9d 
>   src/tests/mesos.hpp bdecd6828f07f051847b700a7cc9262a3f835b2b 
>   src/tests/mesos.cpp 18d0d8f8037ebc27c87bcb0f1ce9f143e7505ec8 
> 
> Diff: https://reviews.apache.org/r/43032/diff/
> 
> 
> Testing
> ---
> 
> The following test cases in docker_tests have been changed:
> DockerTest.ROOT_DOCKER_interface is updated to check labels are passed to 
> docker run command through arguments.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 43041: Update Master to send revocable resources in separate offers.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43041]

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

- Mesos ReviewBot


On Feb. 1, 2016, 10:05 a.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43041/
> ---
> 
> (Updated Feb. 1, 2016, 10:05 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, Klaus Ma, Niklas Nielsen, Qian Zhang, 
> Jian Qiu, and Vinod Kone.
> 
> 
> Bugs: MESOS-2646
> https://issues.apache.org/jira/browse/MESOS-2646
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Split regular and revocable resources into separate offers,
> so that rescinding offers with revocable resources does not
> affect offers with regular resources.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 3a7e18232323a1c051bcc97915484b1195fffe58 
>   src/master/master.cpp 98441a543611d4083b2495ee103f0ab5e2187e83 
>   src/tests/oversubscription_tests.cpp 
> 6f43103e81303015fb614653e3bfece55009d1bf 
> 
> Diff: https://reviews.apache.org/r/43041/diff/
> 
> 
> Testing
> ---
> 
> Make && Make check
>  
> (TODO) I will append the related test cases in another patch later.
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 42797: Fix expire CERN GPG key link.

[haosdent huang]

> On Feb. 1, 2016, 4:12 p.m., Benjamin Bannier wrote:
> > docs/getting-started.md, lines 87-90
> > 
> >
> > While this appears to work, it seems weird to import the repo from 
> > `linuxsoft.cern.ch`, and the key from `ftp.scientificlinux.org`. Any reason 
> > this cannot be consistent? I am mostly worried about it running out of sync 
> > again.

Yes, you concern is reasonable. But this is the best link I could find by 
google except we give up cern repo.


- haosdent


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


On Jan. 26, 2016, 2:13 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42797/
> ---
> 
> (Updated Jan. 26, 2016, 2:13 p.m.)
> 
> 
> Review request for mesos, Adam B and Timothy Chen.
> 
> 
> Bugs: MESOS-4511
> https://issues.apache.org/jira/browse/MESOS-4511
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix expire CERN GPG key link.
> 
> 
> Diffs
> -
> 
>   docs/getting-started.md 7177f9d5ae33dbabf0ed1bb1b9870873906e1a56 
> 
> Diff: https://reviews.apache.org/r/42797/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 42586: Defined the NetClsHandleMgr class.

[Guangya Liu]

> On 二月 1, 2016, 5:49 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp, line 40
> > 
> >
> > 2 blank lines above please.

Does there are any policies for this when need one blank line and when need two 
blank line?


- Guangya


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


On 二月 1, 2016, 2:20 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42586/
> ---
> 
> (Updated 二月 1, 2016, 2:20 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4345
> https://issues.apache.org/jira/browse/MESOS-4345
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This class will be responsible for keeping track of the free and allocated
> net_cls handles.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.hpp 
> b4bc52114389d1c1efce2830f4292bd89bb0de7c 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
> ddc1bf0939e5e8995e6f34fe7b8509b51704f63e 
> 
> Diff: https://reviews.apache.org/r/42586/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 43075: Fixed error message when dropping a scheduler::Call message.

[Guangya Liu]

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



I saw that there are still other places using the call.type() directly, can you 
please update those files as well? 

Such as the executor, the scheduler library etc.

- Guangya Liu


On 二月 2, 2016, 5:12 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43075/
> ---
> 
> (Updated 二月 2, 2016, 5:12 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Old output: "Dropping 2 call from framework ..."
> 
> New output: "Dropping TEARDOWN call from framework ..."
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 98441a543611d4083b2495ee103f0ab5e2187e83 
> 
> Diff: https://reviews.apache.org/r/43075/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43075: Fixed error message when dropping a scheduler::Call message.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43075]

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

- Mesos ReviewBot


On Feb. 2, 2016, 5:12 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43075/
> ---
> 
> (Updated Feb. 2, 2016, 5:12 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Old output: "Dropping 2 call from framework ..."
> 
> New output: "Dropping TEARDOWN call from framework ..."
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 98441a543611d4083b2495ee103f0ab5e2187e83 
> 
> Diff: https://reviews.apache.org/r/43075/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 42998: Added docs for shareable resources.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [42992, 42993, 42994, 42995, 42996, 42997, 42998]

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

- Mesos ReviewBot


On Feb. 2, 2016, 12:39 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42998/
> ---
> 
> (Updated Feb. 2, 2016, 12:39 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-4325
> https://issues.apache.org/jira/browse/MESOS-4325
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added docs for shareable resources.
> 
> 
> Diffs
> -
> 
>   docs/home.md dea6ec2605662dbd4b10d69b2bf8f35af50389ec 
>   docs/shareable-resources.md PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/42998/diff/
> 
> 
> Testing
> ---
> 
> Just documentation changes in this RR.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Review Request 43075: Fixed error message when dropping a scheduler::Call message.

[Neil Conway]

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Old output: "Dropping 2 call from framework ..."

New output: "Dropping TEARDOWN call from framework ..."


Diffs
-

  src/master/master.cpp 98441a543611d4083b2495ee103f0ab5e2187e83 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 43075: Fixed error message when dropping a scheduler::Call message.

[Guangya Liu]

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


Ship it!




Ship It!

- Guangya Liu


On 二月 2, 2016, 5:12 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43075/
> ---
> 
> (Updated 二月 2, 2016, 5:12 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Old output: "Dropping 2 call from framework ..."
> 
> New output: "Dropping TEARDOWN call from framework ..."
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 98441a543611d4083b2495ee103f0ab5e2187e83 
> 
> Diff: https://reviews.apache.org/r/43075/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 43072: Use full screen width for Mesos UI using Bootstraps (v3.3.6) container-fluid

[haosdent huang]

> On Feb. 2, 2016, 2:45 a.m., Mesos ReviewBot wrote:
> > Bad patch!
> > 
> > Reviews applied: [43072]
> > 
> > Failed command: ./support/apply-review.sh -n -r 43072
> > 
> > Error:
> > 2016-02-02 02:45:28 URL:https://reviews.apache.org/r/43072/diff/raw/ 
> > [226634/226634] -> "43072.patch" [1]
> > Traceback (most recent call last):
> >   File "support/apply-reviews.py", line 342, in 
> > reviewboard()
> >   File "support/apply-reviews.py", line 321, in reviewboard
> > apply_review()
> >   File "support/apply-reviews.py", line 139, in apply_review
> > commit_patch()
> >   File "support/apply-reviews.py", line 180, in commit_patch
> > data = patch_data()
> >   File "support/apply-reviews.py", line 199, in patch_data
> > return reviewboard_data()
> >   File "support/apply-reviews.py", line 252, in reviewboard_data
> > email=user.get('email'))
> > UnicodeEncodeError: 'ascii' codec can't encode character u'\xf8' in 
> > position 11: ordinal not in range(128)
> > 
> > Full log: https://builds.apache.org/job/mesos-reviewbot/11187/console

I think we could ignore this error message.


- haosdent


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


On Feb. 1, 2016, 11:36 p.m., Michael Lunøe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43072/
> ---
> 
> (Updated Feb. 1, 2016, 11:36 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Thomas Rampelberg.
> 
> 
> Bugs: MESOS-2585
> https://issues.apache.org/jira/browse/MESOS-2585
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The solution to our problem is solved by updating the Bootstrap CSS file, as 
> container-fluid was introduced later in Bootstrap.
> 
> I have updated to lastest stable release of Bootstrap v3.3.6. Looking through 
> the change log, the only thing that we are using in the Mesos UI and changed 
> is the "hide" class (deprecated), so instances of these have been replaced by 
> its successor ".hidden" class.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 6b18056 
>   src/webui/master/static/css/bootstrap-3.0.3.min.css c547283 
>   src/webui/master/static/css/bootstrap-3.3.6.min.css PRE-CREATION 
>   src/webui/master/static/framework.html 9b28820 
>   src/webui/master/static/home.html d6cde1e 
>   src/webui/master/static/index.html 25caf53 
>   src/webui/master/static/slave.html bc46885 
>   src/webui/master/static/slave_executor.html 9d582d5 
>   src/webui/master/static/slave_framework.html 96d788f 
> 
> Diff: https://reviews.apache.org/r/43072/diff/
> 
> 
> Testing
> ---
> 
> - Mobile: http://cl.ly/2R2O0m1a1G3Q/Image%202016-02-01%20at%2015.21.53.png
> - 1680 x 1050px screen: 
> http://cl.ly/0K1o112u3L2I/Image%202016-02-01%20at%2015.23.37.png
> - 2880 x 1800px screen: 
> http://cl.ly/1M0L3M2X0J2i/Image%202016-02-01%20at%2015.22.50.png
> 
> Visual test in latest Chrome, Firefox, Safari and IE. No functional changes 
> introduced.
> 
> 
> Thanks,
> 
> Michael Lunøe
> 
>

Re: Review Request 43072: Use full screen width for Mesos UI using Bootstraps (v3.3.6) container-fluid

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On Feb. 1, 2016, 11:36 p.m., Michael Lunøe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43072/
> ---
> 
> (Updated Feb. 1, 2016, 11:36 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Thomas Rampelberg.
> 
> 
> Bugs: MESOS-2585
> https://issues.apache.org/jira/browse/MESOS-2585
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The solution to our problem is solved by updating the Bootstrap CSS file, as 
> container-fluid was introduced later in Bootstrap.
> 
> I have updated to lastest stable release of Bootstrap v3.3.6. Looking through 
> the change log, the only thing that we are using in the Mesos UI and changed 
> is the "hide" class (deprecated), so instances of these have been replaced by 
> its successor ".hidden" class.
> 
> 
> Diffs
> -
> 
>   src/webui/master/static/browse.html 6b18056 
>   src/webui/master/static/css/bootstrap-3.0.3.min.css c547283 
>   src/webui/master/static/css/bootstrap-3.3.6.min.css PRE-CREATION 
>   src/webui/master/static/framework.html 9b28820 
>   src/webui/master/static/home.html d6cde1e 
>   src/webui/master/static/index.html 25caf53 
>   src/webui/master/static/slave.html bc46885 
>   src/webui/master/static/slave_executor.html 9d582d5 
>   src/webui/master/static/slave_framework.html 96d788f 
> 
> Diff: https://reviews.apache.org/r/43072/diff/
> 
> 
> Testing
> ---
> 
> - Mobile: http://cl.ly/2R2O0m1a1G3Q/Image%202016-02-01%20at%2015.21.53.png
> - 1680 x 1050px screen: 
> http://cl.ly/0K1o112u3L2I/Image%202016-02-01%20at%2015.23.37.png
> - 2880 x 1800px screen: 
> http://cl.ly/1M0L3M2X0J2i/Image%202016-02-01%20at%2015.22.50.png
> 
> Visual test in latest Chrome, Firefox, Safari and IE. No functional changes 
> introduced.
> 
> 
> Thanks,
> 
> Michael Lunøe
> 
>