Review Request 46426: Added paths helper function for docker volume checkpoint.

[Guangya Liu]

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

Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
Jie Yu.


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


Repository: mesos


Description
---

Added paths helper function for docker volume checkpoint.


Diffs
-

  src/CMakeLists.txt e4923c4d4a7a9eefcab9267fe5def7f7535e49ed 
  src/Makefile.am 5a2b4efa781752863d6751f98614fb78bece73ac 
  src/slave/containerizer/mesos/isolators/docker/volume/paths.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/docker/volume/paths.cpp PRE-CREATION 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 45270: Added spec protobuf for DockerVolumeMount.

[Guangya Liu]

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

(Updated 四月 20, 2016, 6:38 a.m.)


Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
Jie Yu.


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


Repository: mesos


Description (updated)
---

Added spec protobuf for DockerVolume.
The DockerVolume was used to check point the mount
point for each container when recover.


Diffs (updated)
-

  src/CMakeLists.txt e4923c4d4a7a9eefcab9267fe5def7f7535e49ed 
  src/Makefile.am 5a2b4efa781752863d6751f98614fb78bece73ac 
  src/slave/containerizer/mesos/isolators/docker/volume/state.proto 
PRE-CREATION 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 46418: Refactored the `os::access` function between POSIX and Windows.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46418]

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 April 19, 2016, 11:35 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46418/
> ---
> 
> (Updated April 19, 2016, 11:35 p.m.)
> 
> 
> Review request for mesos, Daniel Pravat, Alex Clemmer, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-5237
> https://issues.apache.org/jira/browse/MESOS-5237
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 33ddb06e25920096f2d16d4f372129ee2f6a8721 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> a81086860a17bd1912e55051dd7bb98a20772b51 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/access.hpp PRE-CREATION 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> f1584d0786ee0236a0e703d4aa9532e138ad49f0 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> 37ef33257830e32875500537df7af38757c6efac 
> 
> Diff: https://reviews.apache.org/r/46418/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 43588: Added allocator recovery tests in presence of quota.

[Klaus Ma]

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



ping :).

- Klaus Ma


On Feb. 23, 2016, 7:55 a.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43588/
> ---
> 
> (Updated Feb. 23, 2016, 7:55 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Klaus Ma.
> 
> 
> Bugs: MESOS-3986
> https://issues.apache.org/jira/browse/MESOS-3986
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator recovery tests in presence of quota.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 5f771f02db9bd098f3cd36730cd84bf2f5e87a33 
> 
> Diff: https://reviews.apache.org/r/43588/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 46373: Request /files/read.json with a negative length value causes error.

[zhou xing]

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

(Updated 四月 20, 2016, 5:57 a.m.)


Review request for mesos, Ben Mahler and Greg Mann.


Changes
---

rebased and updated the code according to Greg's comments


Bugs: mesos-5060
https://issues.apache.org/jira/browse/mesos-5060


Repository: mesos


Description (updated)
---

[MESOS-5060]
The patch did the following changes:
1. Fix the length logic in files.cpp.
2. Add some tests to test the /files/read.json endponit with
negative length.


Diffs (updated)
-

  src/files/files.cpp 4e916101b378b0e9032a08a3f6c73e195b2a08a1 
  src/tests/files_tests.cpp 5d6620d13babaf0bb7f9c888bb1b4fa2228b6ccd 

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


Testing
---

make
make check

request 'files/read.json' endpoint with negative offset or length argument


Thanks,

zhou xing

Re: Review Request 43561: Improve Ranges parsing to handle single values.

[Klaus Ma]

> On March 21, 2016, 10:02 p.m., Joris Van Remoortere wrote:
> > src/common/values.cpp, line 613
> > 
> >
> > @benm I wish we had support for iterating over these splicers eg:
> > `foreachtoken(temp, ",\n", [](const string& token) { ... });`
> > 
> > These helpers become rather heavy (at 3 nested) considering their 
> > utility.
> 
> Klaus Ma wrote:
> @joris, I think `foreachtoken` is also nested loop. When doing the 
> patches, there're three options to me: FSM, regex and nested loop; FSM & 
> regex seems hard to maintain, so I used nested loop.
> 
> Klaus Ma wrote:
> `foreachtoken(temp, ",\n", [](const string& token) { ... });` is a good 
> helper function; but no performance improvement. Any comments?
> 
> Klaus Ma wrote:
> @joris, I logged MESOS-5234 to trace this; I'll update patches 
> accordingly.

@joris, had a patch for `foreachtoken`; would you help to review it?

https://reviews.apache.org/r/46425/


- Klaus


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


On March 22, 2016, 9:22 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43561/
> ---
> 
> (Updated March 22, 2016, 9:22 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4627
> https://issues.apache.org/jira/browse/MESOS-4627
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improve Ranges parsing to handle single values.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 9f3c0b9930bd3dd9d84d97119419825862f531c3 
>   src/tests/resources_tests.cpp 6b004d64bb25112b19fc5d98b5bca874c5329e8c 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/43561/diff/
> 
> 
> Testing
> ---
> 
> make
> make check GTEST_FILTER=~"*"
> ./src/mesos-test
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 46425: Add helper function to simplify tokenize handling.

[Klaus Ma]

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

(Updated April 20, 2016, 1:48 p.m.)


Review request for mesos, Ben Mahler, Joris Van Remoortere, and Michael Park.


Changes
---

Correct typo.


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


Repository: mesos


Description
---

Add helper function to simplify tokenize handling.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp 
162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 
  3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 
7eed0f3d08cd52a07c46b6ad194496186ac205b7 

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


Testing
---

make && make check


Thanks,

Klaus Ma

Review Request 46425: Add helper function to simplify tokenize handling.

[Klaus Ma]

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

Review request for mesos, Ben Mahler, Joris Van Remoortere, and Michael Park.


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


Repository: mesos


Description
---

Add helper function to simplify tokenize handling.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/strings.hpp 
162bdfb6c4f5a6b108761ebccd9b77e672f6dd87 
  3rdparty/libprocess/3rdparty/stout/tests/strings_tests.cpp 
7eed0f3d08cd52a07c46b6ad194496186ac205b7 

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


Testing
---

make && make check


Thanks,

Klaus Ma

Re: Review Request 46364: Change to explicit case statements (`UNKNOWN`) instead of default.

[Vinod Kone]

> On April 19, 2016, 5:58 a.m., Vinod Kone wrote:
> > can you also update cli/execute.cpp, examples/test_http_framework.cpp, 
> > examples/long_lived_framework.cpp, master/validation.cpp, master/http.cpp, 
> > master/master.cpp, slave/http.cpp, slave/validation.cpp and slave/http.cpp?
> > 
> > make sure to grep through the code base to catch all other cases where 
> > switch over an enum uses a "default".
> 
> Yong Tang wrote:
> Thanks Vinod for the review. I just updated the listed files.
> 
> Yong Tang wrote:
> Oh I noticed there are 54 other instances of "default:" (e.g., 
> src/slave/slave.cpp). I will rework on the review request.
> 
> Yong Tang wrote:
> Hi Ben/Vinod,
> 
> I take a second look at this issue and I am not so sure about the scope. 
> Is the goal of this task to replace/remove ALL usages of `default:` in switch 
> statement, or this taks is to replace only Event/Call related ones with 
> 'UNKNOWN'? For example, I saw the usage of the following in 
> src/docker/docker.cpp:
> 
> ```
>   if (volume.has_mode()) {
> switch (volume.mode()) {
>   case Volume::RW: volumeConfig += ":rw"; break;
>   case Volume::RO: volumeConfig += ":ro"; break;
>   default: return Failure("Unsupported volume mode");
> }
>   }
> ```
> I am not sure if we should remove the "default:" line in the above or not.
> 
> Let me know which way is the preferred way and I will update the review 
> request.

Just Event/Call in this review. And perhaps AuthZ Action in another review.


- Vinod


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


On April 19, 2016, 3:19 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46364/
> ---
> 
> (Updated April 19, 2016, 3:19 p.m.)
> 
> 
> Review request for mesos, Adam B, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-5031
> https://issues.apache.org/jira/browse/MESOS-5031
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch changes from `default` statement to explicit case
> statement (`UNKNOWN`) so that complier could help catching all
> switches when new enums are introduced in the future.
> 
> This patch is related to:
> https://reviews.apache.org/r/45317/ (MESOS-5014)
> https://reviews.apache.org/r/45304/ (MESOS-5015)
> https://reviews.apache.org/r/45342/ (MESOS-5031)
> 
> 
> Diffs
> -
> 
>   src/authorizer/local/authorizer.cpp 
> c744a16041c2466d3839a37fbee2bf86887bf4e1 
>   src/cli/execute.cpp 087a73143d739f41638b8335c7e24dfcd14bc7fb 
>   src/examples/long_lived_framework.cpp 
> c696ccb6b2ace6e047f6509b291dd14be240cf70 
>   src/examples/test_http_executor.cpp 
> ceb489d43f35d24c8a7f5fbb0148529745ee357a 
>   src/examples/test_http_framework.cpp 
> 8cc3107034d46cb6a2966835f509508223c6e674 
>   src/master/http.cpp a9cb99a92ff5a783e719df5e5cfb6e8301241df9 
>   src/master/master.cpp 6dacf5fbd73771e5c31ffb0e8723cd2905dcefb3 
>   src/master/validation.cpp 0cd118ee4f89f749a063f6ba7f419d5a220dc1d4 
>   src/slave/http.cpp 3908e33ed5b233387790276f6f5d884452087d2c 
>   src/slave/validation.cpp ec1a4b6d9c55ab0c9894d5a49e290e15dee32c22 
>   src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b 
> 
> Diff: https://reviews.apache.org/r/46364/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 46168: Add subdirectory support to URI.filename field.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [46168]

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

Error:
2016-04-20 04:33:12 URL:https://reviews.apache.org/r/46168/diff/raw/ 
[545835/545835] -> "46168.patch" [1]
46168.patch:881: trailing whitespace.
# 
46168.patch:884: trailing whitespace.
# 
46168.patch:922: trailing whitespace.
# 
46168.patch:925: trailing whitespace.
# 
46168.patch:2742: trailing whitespace.

error: patch failed: 
3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp:462
error: 3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp: patch 
does not apply
error: patch failed: 
3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp:38
error: 3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp: patch 
does not apply
error: patch failed: include/mesos/zookeeper/authentication.hpp:60
error: include/mesos/zookeeper/authentication.hpp: patch does not apply
error: patch failed: src/CMakeLists.txt:40
error: src/CMakeLists.txt: patch does not apply
error: patch failed: 
src/slave/containerizer/mesos/isolators/network/cni/cni.cpp:1251
error: src/slave/containerizer/mesos/isolators/network/cni/cni.cpp: patch does 
not apply

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

- Mesos ReviewBot


On April 19, 2016, 11:05 p.m., Michael Browning wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46168/
> ---
> 
> (Updated April 19, 2016, 11:05 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Jiang Yan Xu, and Zhitao Li.
> 
> 
> Bugs: MESOS-5119
> https://issues.apache.org/jira/browse/MESOS-5119
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add subdirectory support to URI.filename field.
> 
> URI.filename allows the user to specify the name of the file that will
> be saved in the sandbox when the URI is fetched, but previously it would
> fail at fetch time if "filename" had a directory component. This change
> allows users to specify a relative path for custom filenames within the
> sandbox.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 7b3621d24a05a3b3b860e3b8c13a1531208e5e26 
>   3rdparty/libprocess/3rdparty/Makefile.am 
> 76f946dc11b66e86ce843808e371737b9e022e36 
>   3rdparty/libprocess/3rdparty/http-parser/CMakeLists.txt.template 
> 7f7105f5d55fd66f3e826c1c37c0074775bf89ea 
>   3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
> 33ddb06e25920096f2d16d4f372129ee2f6a8721 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> a81086860a17bd1912e55051dd7bb98a20772b51 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/freebsd.hpp 
> ce4e4ccd1340e2cff18f5d1b6a9236809bdc69f1 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp 
> 33825280eb1404bcd89324f8ab5949f735b2d130 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp 
> a8010925c15fccc9bcd7c5d150ccdd9c98b8bb47 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/pagesize.hpp 
> f46da5577ecf4c336ff4d490f4f36ac0d3d058a9 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/pagesize.hpp 
> f3ae69adf096d558e083615dfcf848c94e017e6e 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/pagesize.hpp 
> 6112e9781a9d42f7ec1ae0832c0c877d1915b09b 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> 52978f37a27c6db45b71fa1a1d41bb833a76e666 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> 352ecc5fed99f52ef8ffce48291be720791b8b23 
>   3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
> ac75baf3c660f2bdda308f4eaa856f44d80f1ea1 
>   3rdparty/libprocess/src/decoder.hpp 
> 2c41ce9f00c857aa320b1d2cfa3b1048c316976a 
>   3rdparty/libprocess/src/process.cpp 
> afeddec20495bb9621c3e26b0d425c9419654739 
>   3rdparty/libprocess/src/test-master.cpp 
> 5026af32c6d72d3e031ebf265680ab7bbf937435 
>   3rdparty/libprocess/src/test-slave.cpp 
> 4516bdca66de5889c3163ff7d6890a9806dc4322 
>   3rdparty/libprocess/src/tests/future_tests.cpp 
> 383d260a4c0b17e9a0b5af002bb35070e3ec0b01 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 3fce6fc41faaa7b6e8a2a957e85de3de973a51ba 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 4d237815a03828b915e821c3af78132e2915c610 
>   CHANGELOG d2e902f8295644c527964123e409be460c2a5789 
>   bin/gdb-mesos-agent.sh.in 1e02c48c27b255139ab73e233abf577e402c5401 
>   bin/lldb-mesos-agent.sh.in 480d6cec9ee8b6bb1b698961df6a555a38226a0a 
>   bin/mesos-agent-flags.sh.in  
>   bin/mesos-agent.sh.in adf79e0eb0c62236fb6095bd3d3539308dded975 
>   bin/mesos-local-flags.sh.in 5b4553a808dc9f34a15390e69b2f85e95761ec53 

Review Request 46423: Windows: Forked subprocess.cpp, added `Windows` implementation.

[Alex Clemmer]

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

Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris 
Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


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


Repository: mesos


Description
---

Windows: Forked subprocess.cpp, added `Windows` implementation.


Diffs
-

  3rdparty/libprocess/Makefile.am c51c31eb91dd7be4cff8a188942ea77b3b361d45 
  3rdparty/libprocess/include/process/subprocess.hpp 
8a3fe5526f480187441a8aee2c72636bec3e2b2d 
  3rdparty/libprocess/src/CMakeLists.txt 
a8379d323f30037bdd15e151957f885664b5e242 
  3rdparty/libprocess/src/subprocess.cpp 
bb0fcbcd0dfa455c8700247c5b4ca0473fd163c3 
  3rdparty/libprocess/src/subprocess_posix.cpp PRE-CREATION 
  3rdparty/libprocess/src/subprocess_windows.cpp PRE-CREATION 

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


Testing
---


Thanks,

Alex Clemmer

Review Request 46424: Windows: Added libprocess to build.

[Alex Clemmer]

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

Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris 
Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


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


Repository: mesos


Description
---

Windows: Added libprocess to build.


Diffs
-

  3rdparty/libprocess/CMakeLists.txt 5633c395bcb3b3ce377193c1ca1d6d9810c97852 
  3rdparty/libprocess/src/tests/CMakeLists.txt 
9558adf1e45043630b637fa6fb21dc92ec036bd4 
  3rdparty/libprocess/src/tests/main.cpp 
78858a2b84a439d8f8a60ec8bcb6ac3a308087a6 
  3rdparty/libprocess/src/tests/subprocess_tests.cpp 
727e940f12643974de4ff2734fba431b285b5de3 

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


Testing
---


Thanks,

Alex Clemmer

Review Request 46422: Stout: Added Windows support for `stopwatch.hpp`.

[Alex Clemmer]

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

Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris 
Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


Repository: mesos


Description
---

Stout: Added Windows support for `stopwatch.hpp`.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/stopwatch.hpp 
18d94afe8f337ae406c7af303ac593f4f712d225 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 46364: Change to explicit case statements (`UNKNOWN`) instead of default.

[Yong Tang]

> On April 19, 2016, 5:58 a.m., Vinod Kone wrote:
> > can you also update cli/execute.cpp, examples/test_http_framework.cpp, 
> > examples/long_lived_framework.cpp, master/validation.cpp, master/http.cpp, 
> > master/master.cpp, slave/http.cpp, slave/validation.cpp and slave/http.cpp?
> > 
> > make sure to grep through the code base to catch all other cases where 
> > switch over an enum uses a "default".
> 
> Yong Tang wrote:
> Thanks Vinod for the review. I just updated the listed files.
> 
> Yong Tang wrote:
> Oh I noticed there are 54 other instances of "default:" (e.g., 
> src/slave/slave.cpp). I will rework on the review request.

Hi Ben/Vinod,

I take a second look at this issue and I am not so sure about the scope. Is the 
goal of this task to replace/remove ALL usages of `default:` in switch 
statement, or this taks is to replace only Event/Call related ones with 
'UNKNOWN'? For example, I saw the usage of the following in 
src/docker/docker.cpp:

```
  if (volume.has_mode()) {
switch (volume.mode()) {
  case Volume::RW: volumeConfig += ":rw"; break;
  case Volume::RO: volumeConfig += ":ro"; break;
  default: return Failure("Unsupported volume mode");
}
  }
```
I am not sure if we should remove the "default:" line in the above or not.

Let me know which way is the preferred way and I will update the review request.


- Yong


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


On April 19, 2016, 3:19 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46364/
> ---
> 
> (Updated April 19, 2016, 3:19 p.m.)
> 
> 
> Review request for mesos, Adam B, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-5031
> https://issues.apache.org/jira/browse/MESOS-5031
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch changes from `default` statement to explicit case
> statement (`UNKNOWN`) so that complier could help catching all
> switches when new enums are introduced in the future.
> 
> This patch is related to:
> https://reviews.apache.org/r/45317/ (MESOS-5014)
> https://reviews.apache.org/r/45304/ (MESOS-5015)
> https://reviews.apache.org/r/45342/ (MESOS-5031)
> 
> 
> Diffs
> -
> 
>   src/authorizer/local/authorizer.cpp 
> c744a16041c2466d3839a37fbee2bf86887bf4e1 
>   src/cli/execute.cpp 087a73143d739f41638b8335c7e24dfcd14bc7fb 
>   src/examples/long_lived_framework.cpp 
> c696ccb6b2ace6e047f6509b291dd14be240cf70 
>   src/examples/test_http_executor.cpp 
> ceb489d43f35d24c8a7f5fbb0148529745ee357a 
>   src/examples/test_http_framework.cpp 
> 8cc3107034d46cb6a2966835f509508223c6e674 
>   src/master/http.cpp a9cb99a92ff5a783e719df5e5cfb6e8301241df9 
>   src/master/master.cpp 6dacf5fbd73771e5c31ffb0e8723cd2905dcefb3 
>   src/master/validation.cpp 0cd118ee4f89f749a063f6ba7f419d5a220dc1d4 
>   src/slave/http.cpp 3908e33ed5b233387790276f6f5d884452087d2c 
>   src/slave/validation.cpp ec1a4b6d9c55ab0c9894d5a49e290e15dee32c22 
>   src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b 
> 
> Diff: https://reviews.apache.org/r/46364/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45905: Added metrics to the balloon framework.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46407, 45604, 46411, 45905]

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 April 19, 2016, 9:52 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45905/
> ---
> 
> (Updated April 19, 2016, 9:52 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5174
> https://issues.apache.org/jira/browse/MESOS-5174
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds metrics to gauge the health of the framework.  This includes:
> 
> * uptime_secs = How long the framework has been running.
> * registered = If the framework is registered.
> * tasks_finished = Number of tasks finished (successfully).
> * tasks_oomed = Number of tasks that were OOM killed.
> * allowed_terminations = Number of terminal status updates which
>   are acceptable due to infrastructure reasons.
> * abnormal_terminations = Number of terminal status updates which 
>   were not `TASK_FINISHED` or `TASK_FAILED` due to OOM.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_framework.cpp 15c45612b777edaf97aea9b953439d4ad56920f3 
> 
> Diff: https://reviews.apache.org/r/45905/diff/
> 
> 
> Testing
> ---
> 
> ```
> make check
> 
> sudo bin/mesos-tests.sh --gtest_filter="*ROOT_CGROUPS_BalloonFramework"
> 
> # Also launched two instances on a cluster.
> # This one OOM's:
> ./balloon-framework --master=zk://localhost:2181/mesos --checkpoint 
> --balloon_limit=256MB --task_memory=128MB 
> --executor_uri="https://s3.amazonaws.com/url/to/balloon-executor"; 
> --executor_command="LD_LIBRARY_PATH=/path/to/libmesos && ./balloon-executor"
> 
> # This one does not OOM:
> ./balloon-framework --master=zk://localhost:2181/mesos --checkpoint 
> --balloon_limit=256MB --task_memory=256MB 
> --executor_uri="https://s3.amazonaws.com/url/to/balloon-executor"; 
> --executor_command="LD_LIBRARY_PATH=/path/to/libmesos && ./balloon-executor"
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 45014: Add /containers endpoint.

[Jay Guo]

> On April 15, 2016, 12:33 a.m., Jie Yu wrote:
> > ping?

bump


- Jay


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


On April 18, 2016, 3:38 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45014/
> ---
> 
> (Updated April 18, 2016, 3:38 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4891
> https://issues.apache.org/jira/browse/MESOS-4891
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It returns both resource statistics and container status.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 3f96f2c201597706dfc814c0a20bb983cd56905a 
>   src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1 
>   src/slave/slave.cpp de99e9eb5cc812b2e07deb749b98b4f4db363728 
>   src/tests/containerizer.hpp efc1ca87c00b04d70efd1e3f6acf4e763132d6b0 
>   src/tests/containerizer.cpp 4c7f5a26d38222daf013bd3ca9a133e12d7bf338 
>   src/tests/slave_tests.cpp ee58488b0b927c7c5833add4718941539663e6d2 
> 
> Diff: https://reviews.apache.org/r/45014/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 46370: Introduced linux capabilities API.

[Kevin Klues]

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



I added a few comments below, but in general, I feel like there are places this 
code could be greatly simplified.  Specifically, it's not obvious to me why we 
need all of the different classes you define (or maybe more about why they need 
to be part of the public interface).  Could you clarify the rational here a bit?


src/linux/capabilities.hpp (line 30)


Is this file #including itself? Does this compile?



src/linux/capabilities.hpp (lines 32 - 33)


This should all probably live in the mesos::internal::capabilities 
namespace.



src/linux/capabilities.hpp (lines 39 - 45)


These are both probably unnecessary (see comments below).



src/linux/capabilities.hpp (lines 50 - 90)


Since we should probably be embedding this in a `capabilities` namespace, 
it is redundant to call this enum `Capability`. I'd sugggest `Privilege`.  That 
way one of these can be accessed as e.g. `capabiliites::Privilege::SETGID`.

Also, as mentioned in a comment below, this should probably be declared as 
an `enum class` for better type checking.

The `COUNT` trick mentioned below should probably be applied here as well.



src/linux/capabilities.hpp (lines 94 - 99)


From my reading of: http://man7.org/linux/man-pages/man7/capabilities.7.html

this enum should probably be called `Set`.

Note, the name `Capability` at the front is unnecessary if we embed this in 
the `capabilities` namespace.

Also, it's pretty standard practice in C++ to define an `enum` as a `enum 
class` for better type checking.  As such, you can define the final element 
with a common name of `COUNT` to get at the size of the enum.

For example, you can get at the size of the enum as: 
`capabilities::Set::COUNT` instead of relying on the `const` for 
`NUMBER_OF_CAP_SETS` defined above.



src/linux/capabilities.hpp (line 178)


Didn't we discuss not making this a class, and only having get()/set() 
calls as part of the namespace?



src/linux/capabilities.hpp (line 209)


What did we decide about the `add()` pairing to this `drop()` call?



src/linux/capabilities.cpp (lines 36 - 38)


Is there not a header file you can just include here?



src/linux/capabilities.cpp (lines 124 - 125)


This should be unnecessary. See: 
https://github.com/klueska-mesosphere/mesos/blob/master/src/linux/cgroups.cpp#L2435


- Kevin Klues


On April 19, 2016, 5:02 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46370/
> ---
> 
> (Updated April 19, 2016, 5:02 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Kevin Klues.
> 
> 
> Bugs: MESOS-5051
> https://issues.apache.org/jira/browse/MESOS-5051
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces basic API for linux capabilities. This is not a
> comprehensive API but is strictly limited to the need for securing Mesos
> containers using linux capabilities.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt ca59a1824352423f4db6ef8bb41acc6fe602c041 
>   src/Makefile.am a8f68316c5d4d4c82d99363535a97fa5b1caafc5 
>   src/linux/capabilities.hpp PRE-CREATION 
>   src/linux/capabilities.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46370/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 46401: Corrected acls protobuf file in documentation and flag.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46401]

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 April 19, 2016, 6:38 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46401/
> ---
> 
> (Updated April 19, 2016, 6:38 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The acls protobuf messages have moved from
> authorization.proto to acls.proto. This patch reflects
> this change in the documentation/flag.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md cd9733002a0940f44edd4e56593a0ca3fe9f77f5 
>   src/master/flags.cpp e6a239c15f52b381a83cc359d8b5a335a5fbd0af 
> 
> Diff: https://reviews.apache.org/r/46401/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 45360: Added volume driver client for mount and unmount.

[Guangya Liu]

> On 四月 19, 2016, 10:37 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/docker/volume/driver.hpp, line 41
> > 
> >
> > Do you want to use `const Option& dvdcliPath` here? The idea is 
> > that if not specified, the client will just use 'dvdcli' in $PATH. You want 
> > to use os::which to do a check in 'create' to make sure it's there if 
> > dvdcliPath is not specified.

So you mean that we still need an agent flag to speicify the dvdcli path as an 
option?


- Guangya


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


On 四月 15, 2016, 2:43 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45360/
> ---
> 
> (Updated 四月 15, 2016, 2:43 p.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added volume driver client for mount and unmount.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 06f58c4a88e3c527df727df1efe11ed3ab77efa8 
>   src/Makefile.am ec855263d620e4723c8ba9cd056c40a3a2e9ca99 
>   src/slave/containerizer/mesos/isolators/docker/volume/driver.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45360/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45673: PoC: Docker Volume Isolator.

[Guangya Liu]

> On 四月 19, 2016, 11:40 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp, lines 
> > 108-109
> > 
> >
> > To me, this is just an optimization, isn't it? We can still go through 
> > the 'infos' list to see a volume is still begin used, right?
> > 
> > Can we not introduce this optimizaiton yet?

Yes, as the reference will only be used in `cleanup`, I will move the logic to 
`cleanup`.


> On 四月 19, 2016, 11:40 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp, line 337
> > 
> >
> > We need to serialize calls to 'client' for a given driver and a given 
> > volume name. You can use `Sequence` for each driver+name pair in 
> > DockerVolumeDriverClient to achieve that.

Yes, that's why I was using `mountInfo` in `DockerVolumeClient` `mount` API, as 
I want to keep the relationship between container path and mount point. ok, 
will try to see how `Sequence` can help.


> On 四月 19, 2016, 11:40 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/docker/volume/state.hpp, line 34
> > 
> >
> > I suggest that we only put informations that we can recover in 'Info' 
> > struct. For things like container_path, mount_point that we cannot recover, 
> > try not to put them in 'Info' struct. You can always create a parallel 
> > vector with those additional information, and pass down using parameters in 
> > the lambda.

yes, I was putting it here is because I want to make this shared between 
`DockerVolumeClient` and `Isolator`, but if I use `Sequence`, then there will 
be no need to keep this but just move this to `isolator.hpp`


- Guangya


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


On 四月 19, 2016, 4:45 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45673/
> ---
> 
> (Updated 四月 19, 2016, 4:45 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> PoC: Docker Volume Isolator.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt aabb33dfd4b985fe5774f87bb63b4ec1ec853962 
>   src/Makefile.am 5a2b4efa781752863d6751f98614fb78bece73ac 
>   src/cli/execute.cpp 087a73143d739f41638b8335c7e24dfcd14bc7fb 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1e1a36903f4377497bb72b69e4ead63675d453c0 
>   src/slave/containerizer/mesos/isolators/docker/volume/driver.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
> 786f917c0ec04b6839bbd524fa7c8de3729f9bdb 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 0ad473dc3bd45e122fba55a670e1a893e61c977a 
>   src/slave/containerizer/mesos/isolators/docker/volume/paths.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/volume/paths.cpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/volume/state.hpp 
> PRE-CREATION 
>   src/slave/slave.cpp d82dec2b10d496065013eb4ad6a35dc054b72553 
> 
> Diff: https://reviews.apache.org/r/45673/diff/
> 
> 
> Testing
> ---
> 
> state.proto
> 
> message DockerVolumeMount {
>   required string volume_driver = 1;
>   required string volume_name = 2;
> }
> 
> 
> message DockerVolumeMountList {
>   repeated DockerVolumeMount mounts = 1;
> }
> 
> 
> message DockerVolumeInfo {
>   // Docker volume mount info for volume driver and volume name.
>   required DockerVolumeMount dvm = 1;
> 
>   // Container path, this is optional as this is not needed by recover()
>   // but only for prepare() .
>   optional string container_path = 2;
> 
>   // Mount point for the container, this is optiona as this is only needed
>   // by prepare().
>   optional string mount_point = 3;
>   
>   // Volume driver mount options.
>   optional Parameters driver_options = 4;
> }
> 
> DockerVolumeMount will be checkpointed.
> 
> 
> //   /var/run/mesos/isolators/docker/volume
> //|- /
> //|  |-- DockerVolumeMountList
> //|-- /
> //|  |-- DockerVolumeMountList
> //|-/
> //|- ...
> 
> Info structure
> We assume that one container do not use multiple same volume driver and 
> volume name pair.
> struct Info
> {
>   Info (const list& _volumeInfos)
> : volumeInfos(_volumeInfos) {}
> 
>   list volumeInfos;
> };
> 
> Hashmap in isolator:
> hashmap> infos;
> hashmap volumeReference; // string is 
> volume_dri

Re: Review Request 45270: Added spec protobuf for DockerVolumeMount.

[Guangya Liu]

> On 四月 19, 2016, 5:27 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/docker/volume/state.proto, lines 
> > 32-46
> > 
> >
> > I still don't understand why we want to make it a protobuf? Can that 
> > just be a nested struct in the isolator?

OK, will move this to struct.


- Guangya


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


On 四月 19, 2016, 3:04 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45270/
> ---
> 
> (Updated 四月 19, 2016, 3:04 p.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-5013
> https://issues.apache.org/jira/browse/MESOS-5013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added spec protobuf for DockerVolumeMount.
> The DockerVolumeMount was used to check point the mount
> point for each container to recover.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt aabb33dfd4b985fe5774f87bb63b4ec1ec853962 
>   src/Makefile.am 5a2b4efa781752863d6751f98614fb78bece73ac 
>   src/slave/containerizer/mesos/isolators/docker/volume/state.proto 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45270/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45673: PoC: Docker Volume Isolator.

[Jie Yu]

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




src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp (lines 89 - 
90)


I would swap the order of these two.



src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp (lines 108 - 
109)


To me, this is just an optimization, isn't it? We can still go through the 
'infos' list to see a volume is still begin used, right?

Can we not introduce this optimizaiton yet?



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 282)


This can just be a nested struct in IsolatorProcess (no need to be a 
protobuf):
```
struct MountInfo
{
  DockerVolume volume;
  string containerPath;
};
```



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (lines 316 - 
317)


Let's try to write JSON files instead of protobuf files directly. It's 
better to be human readable.



src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp (line 332)


We need to serialize calls to 'client' for a given driver and a given 
volume name. You can use `Sequence` for each driver+name pair in 
DockerVolumeDriverClient to achieve that.



src/slave/containerizer/mesos/isolators/docker/volume/state.hpp (line 29)


Should this be nested in IsolatorProcess?



src/slave/containerizer/mesos/isolators/docker/volume/state.hpp (line 34)


I suggest that we only put informations that we can recover in 'Info' 
struct. For things like container_path, mount_point that we cannot recover, try 
not to put them in 'Info' struct. You can always create a parallel vector with 
those additional information, and pass down using parameters in the lambda.


- Jie Yu


On April 19, 2016, 4:45 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45673/
> ---
> 
> (Updated April 19, 2016, 4:45 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> PoC: Docker Volume Isolator.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt aabb33dfd4b985fe5774f87bb63b4ec1ec853962 
>   src/Makefile.am 5a2b4efa781752863d6751f98614fb78bece73ac 
>   src/cli/execute.cpp 087a73143d739f41638b8335c7e24dfcd14bc7fb 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 1e1a36903f4377497bb72b69e4ead63675d453c0 
>   src/slave/containerizer/mesos/isolators/docker/volume/driver.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
> 786f917c0ec04b6839bbd524fa7c8de3729f9bdb 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 0ad473dc3bd45e122fba55a670e1a893e61c977a 
>   src/slave/containerizer/mesos/isolators/docker/volume/paths.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/volume/paths.cpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/volume/state.hpp 
> PRE-CREATION 
>   src/slave/slave.cpp d82dec2b10d496065013eb4ad6a35dc054b72553 
> 
> Diff: https://reviews.apache.org/r/45673/diff/
> 
> 
> Testing
> ---
> 
> state.proto
> 
> message DockerVolumeMount {
>   required string volume_driver = 1;
>   required string volume_name = 2;
> }
> 
> 
> message DockerVolumeMountList {
>   repeated DockerVolumeMount mounts = 1;
> }
> 
> 
> message DockerVolumeInfo {
>   // Docker volume mount info for volume driver and volume name.
>   required DockerVolumeMount dvm = 1;
> 
>   // Container path, this is optional as this is not needed by recover()
>   // but only for prepare() .
>   optional string container_path = 2;
> 
>   // Mount point for the container, this is optiona as this is only needed
>   // by prepare().
>   optional string mount_point = 3;
>   
>   // Volume driver mount options.
>   optional Parameters driver_options = 4;
> }
> 
> DockerVolumeMount will be checkpointed.
> 
> 
> //   /var/run/mesos/isolators/docker/volume
> //|- /
> //|  |-- DockerVolumeMountList
> //|-- /
> //|  |-- DockerVolumeMountList
> //|-/
> //|- ...
> 
> Info structure
> We assume that one container do not use multiple same volume driver and 
> volume name pair.
> struct Info
> {
>   Info (const list& _volumeInfos)
> : volumeInfos(_v

Re: Review Request 45270: Added spec protobuf for DockerVolumeMount.

[Jie Yu]

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




src/slave/containerizer/mesos/isolators/docker/volume/state.proto (line 21)


s/DockerVolumeMount/DockerVolume/



src/slave/containerizer/mesos/isolators/docker/volume/state.proto (line 22)


s/volume_driver/driver/
s/volume_name/name/

Volume already in the message Type.



src/slave/containerizer/mesos/isolators/docker/volume/state.proto (line 27)


s/DockerVolumeMountList/DockerVolumes/



src/slave/containerizer/mesos/isolators/docker/volume/state.proto (line 28)


s/mounts/volumes/


- Jie Yu


On April 19, 2016, 3:04 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45270/
> ---
> 
> (Updated April 19, 2016, 3:04 p.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-5013
> https://issues.apache.org/jira/browse/MESOS-5013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added spec protobuf for DockerVolumeMount.
> The DockerVolumeMount was used to check point the mount
> point for each container to recover.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt aabb33dfd4b985fe5774f87bb63b4ec1ec853962 
>   src/Makefile.am 5a2b4efa781752863d6751f98614fb78bece73ac 
>   src/slave/containerizer/mesos/isolators/docker/volume/state.proto 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45270/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Review Request 46418: Refactored the `os::access` function between POSIX and Windows.

[Michael Park]

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

Review request for mesos, Daniel Pravat, Alex Clemmer, and Joris Van Remoortere.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
33ddb06e25920096f2d16d4f372129ee2f6a8721 
  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
a81086860a17bd1912e55051dd7bb98a20772b51 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/access.hpp PRE-CREATION 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
f1584d0786ee0236a0e703d4aa9532e138ad49f0 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
37ef33257830e32875500537df7af38757c6efac 

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


Testing
---

`make check`


Thanks,

Michael Park

Re: Review Request 46371: Added basic tests for capabilities API.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [46371, 46370, 46369]

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

Error:
2016-04-19 23:35:36 URL:https://reviews.apache.org/r/46370/diff/raw/ 
[17675/17675] -> "46370.patch" [1]
error: patch failed: src/Makefile.am:858
error: src/Makefile.am: patch does not apply

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

- Mesos ReviewBot


On April 19, 2016, 5:03 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46371/
> ---
> 
> (Updated April 19, 2016, 5:03 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added basic tests for capabilities API.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a8f68316c5d4d4c82d99363535a97fa5b1caafc5 
>   src/tests/capabilities_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46371/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 46168: Add subdirectory support to URI.filename field.

[Michael Browning]

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

(Updated April 19, 2016, 11:05 p.m.)


Review request for mesos, Bernd Mathiske, Jiang Yan Xu, and Zhitao Li.


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


Repository: mesos


Description (updated)
---

Add subdirectory support to URI.filename field.

URI.filename allows the user to specify the name of the file that will
be saved in the sandbox when the URI is fetched, but previously it would
fail at fetch time if "filename" had a directory component. This change
allows users to specify a relative path for custom filenames within the
sandbox.


Diffs
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
7b3621d24a05a3b3b860e3b8c13a1531208e5e26 
  3rdparty/libprocess/3rdparty/Makefile.am 
76f946dc11b66e86ce843808e371737b9e022e36 
  3rdparty/libprocess/3rdparty/http-parser/CMakeLists.txt.template 
7f7105f5d55fd66f3e826c1c37c0074775bf89ea 
  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
33ddb06e25920096f2d16d4f372129ee2f6a8721 
  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
a81086860a17bd1912e55051dd7bb98a20772b51 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/freebsd.hpp 
ce4e4ccd1340e2cff18f5d1b6a9236809bdc69f1 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp 
33825280eb1404bcd89324f8ab5949f735b2d130 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp 
a8010925c15fccc9bcd7c5d150ccdd9c98b8bb47 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/pagesize.hpp 
f46da5577ecf4c336ff4d490f4f36ac0d3d058a9 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/pagesize.hpp 
f3ae69adf096d558e083615dfcf848c94e017e6e 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/pagesize.hpp 
6112e9781a9d42f7ec1ae0832c0c877d1915b09b 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
52978f37a27c6db45b71fa1a1d41bb833a76e666 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
352ecc5fed99f52ef8ffce48291be720791b8b23 
  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
ac75baf3c660f2bdda308f4eaa856f44d80f1ea1 
  3rdparty/libprocess/src/decoder.hpp 2c41ce9f00c857aa320b1d2cfa3b1048c316976a 
  3rdparty/libprocess/src/process.cpp afeddec20495bb9621c3e26b0d425c9419654739 
  3rdparty/libprocess/src/test-master.cpp 
5026af32c6d72d3e031ebf265680ab7bbf937435 
  3rdparty/libprocess/src/test-slave.cpp 
4516bdca66de5889c3163ff7d6890a9806dc4322 
  3rdparty/libprocess/src/tests/future_tests.cpp 
383d260a4c0b17e9a0b5af002bb35070e3ec0b01 
  3rdparty/libprocess/src/tests/process_tests.cpp 
3fce6fc41faaa7b6e8a2a957e85de3de973a51ba 
  3rdparty/libprocess/src/tests/ssl_tests.cpp 
4d237815a03828b915e821c3af78132e2915c610 
  CHANGELOG d2e902f8295644c527964123e409be460c2a5789 
  bin/gdb-mesos-agent.sh.in 1e02c48c27b255139ab73e233abf577e402c5401 
  bin/lldb-mesos-agent.sh.in 480d6cec9ee8b6bb1b698961df6a555a38226a0a 
  bin/mesos-agent-flags.sh.in  
  bin/mesos-agent.sh.in adf79e0eb0c62236fb6095bd3d3539308dded975 
  bin/mesos-local-flags.sh.in 5b4553a808dc9f34a15390e69b2f85e95761ec53 
  bin/valgrind-mesos-agent.sh.in 08d9730e995d71236b224786ecec96f526ed033a 
  configure.ac ae91a07e2bafd97566ac6e6873990302acad14c9 
  docs/configuration.md cd9733002a0940f44edd4e56593a0ca3fe9f77f5 
  docs/endpoints/master/create-volumes.md 
5c86480b488e2722a12ab09b0a28399402410e2b 
  docs/endpoints/master/destroy-volumes.md 
f75dd52bd6b75d35aa62dca54148a41cfec36ded 
  docs/endpoints/master/reserve.md 8f38cc0f9c16c368ee3698ec0a6e7fd484110ebc 
  docs/endpoints/master/unreserve.md 8e9a696a50f1821cfb11afae68cc4fb62b6f2a2c 
  docs/fetcher.md fd6d8a78bd35c5644dceff7005dd7dfd9f5f2171 
  docs/persistent-volume.md e5f2836c8867d5221da9e4f89167850ef9dab8ec 
  docs/reservation.md a400d19aec7a48d122ba1c9c23d38d792b8dbe6f 
  docs/upgrades.md 4f0c86db4c8d51f70487c03b2e75c1a4189b67b9 
  include/mesos/hook.hpp 210ffba09f5acae34ca49b888a781f683777f9ca 
  include/mesos/log/log.hpp 9c8634987181b1345005619e6d16d738903c49fc 
  include/mesos/mesos.proto 87af4a06db8cc3889fe4d3b314206103f5ce5f2d 
  include/mesos/state/in_memory.hpp 203274242854d482b01275597b249c58e6dfe2ad 
  include/mesos/state/leveldb.hpp 6c732d38d68a3d60d28ce68a6340e8771d849c53 
  include/mesos/state/log.hpp ac0312fdb92c46bfa2a7b83e95e04fd1eaf87d03 
  include/mesos/state/protobuf.hpp cde7b771f3c787293fb909a4b982c47ee19f4057 
  include/mesos/state/state.hpp f2fddee4fa803fa0572f6194e7f5f45a56254c00 
  include/mesos/state/state.proto 7a7d68e6abc85c0ead04f5771e878d10348f44b8 
  include/mesos/state/storage.hpp 2bfa0478b0edf76d592cc9644da83d15a00bc68c 
  include/mesos/state/zookeeper.hpp 8d8c19ce778f2499d86eb84008a61f211c528a3a 
  include/mesos/v1/mesos.proto 34da0a1484dc2f71262d8b97484b8edaae35bb6c 
  include/mesos/v1/scheduler.hpp 18e7a95f8342ea155f9e339945b05810b6bd82b5 
  incl

Re: Review Request 46395: Windows: Removed `std::bind` from `process.cpp` to build on Windows.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46393, 46395]

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 April 19, 2016, 4:35 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46395/
> ---
> 
> (Updated April 19, 2016, 4:35 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-3637
> https://issues.apache.org/jira/browse/MESOS-3637
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Removed `std::bind` from `process.cpp` to build on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> d2c458ed93307f75358bb642aaf2ed8e17b2fe97 
> 
> Diff: https://reviews.apache.org/r/46395/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 46168: Add subdirectory support to URI.filename field.

[Michael Browning]

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

(Updated April 19, 2016, 11:04 p.m.)


Review request for mesos, Bernd Mathiske, Jiang Yan Xu, and Zhitao Li.


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


Repository: mesos


Description (updated)
---

Add subdirectory support to URI.filename field.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
7b3621d24a05a3b3b860e3b8c13a1531208e5e26 
  3rdparty/libprocess/3rdparty/Makefile.am 
76f946dc11b66e86ce843808e371737b9e022e36 
  3rdparty/libprocess/3rdparty/http-parser/CMakeLists.txt.template 
7f7105f5d55fd66f3e826c1c37c0074775bf89ea 
  3rdparty/libprocess/3rdparty/stout/include/Makefile.am 
33ddb06e25920096f2d16d4f372129ee2f6a8721 
  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
a81086860a17bd1912e55051dd7bb98a20772b51 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/freebsd.hpp 
ce4e4ccd1340e2cff18f5d1b6a9236809bdc69f1 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/linux.hpp 
33825280eb1404bcd89324f8ab5949f735b2d130 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/osx.hpp 
a8010925c15fccc9bcd7c5d150ccdd9c98b8bb47 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/pagesize.hpp 
f46da5577ecf4c336ff4d490f4f36ac0d3d058a9 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/pagesize.hpp 
f3ae69adf096d558e083615dfcf848c94e017e6e 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/pagesize.hpp 
6112e9781a9d42f7ec1ae0832c0c877d1915b09b 
  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
52978f37a27c6db45b71fa1a1d41bb833a76e666 
  3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
352ecc5fed99f52ef8ffce48291be720791b8b23 
  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
ac75baf3c660f2bdda308f4eaa856f44d80f1ea1 
  3rdparty/libprocess/src/decoder.hpp 2c41ce9f00c857aa320b1d2cfa3b1048c316976a 
  3rdparty/libprocess/src/process.cpp afeddec20495bb9621c3e26b0d425c9419654739 
  3rdparty/libprocess/src/test-master.cpp 
5026af32c6d72d3e031ebf265680ab7bbf937435 
  3rdparty/libprocess/src/test-slave.cpp 
4516bdca66de5889c3163ff7d6890a9806dc4322 
  3rdparty/libprocess/src/tests/future_tests.cpp 
383d260a4c0b17e9a0b5af002bb35070e3ec0b01 
  3rdparty/libprocess/src/tests/process_tests.cpp 
3fce6fc41faaa7b6e8a2a957e85de3de973a51ba 
  3rdparty/libprocess/src/tests/ssl_tests.cpp 
4d237815a03828b915e821c3af78132e2915c610 
  CHANGELOG d2e902f8295644c527964123e409be460c2a5789 
  bin/gdb-mesos-agent.sh.in 1e02c48c27b255139ab73e233abf577e402c5401 
  bin/lldb-mesos-agent.sh.in 480d6cec9ee8b6bb1b698961df6a555a38226a0a 
  bin/mesos-agent-flags.sh.in  
  bin/mesos-agent.sh.in adf79e0eb0c62236fb6095bd3d3539308dded975 
  bin/mesos-local-flags.sh.in 5b4553a808dc9f34a15390e69b2f85e95761ec53 
  bin/valgrind-mesos-agent.sh.in 08d9730e995d71236b224786ecec96f526ed033a 
  configure.ac ae91a07e2bafd97566ac6e6873990302acad14c9 
  docs/configuration.md cd9733002a0940f44edd4e56593a0ca3fe9f77f5 
  docs/endpoints/master/create-volumes.md 
5c86480b488e2722a12ab09b0a28399402410e2b 
  docs/endpoints/master/destroy-volumes.md 
f75dd52bd6b75d35aa62dca54148a41cfec36ded 
  docs/endpoints/master/reserve.md 8f38cc0f9c16c368ee3698ec0a6e7fd484110ebc 
  docs/endpoints/master/unreserve.md 8e9a696a50f1821cfb11afae68cc4fb62b6f2a2c 
  docs/fetcher.md fd6d8a78bd35c5644dceff7005dd7dfd9f5f2171 
  docs/persistent-volume.md e5f2836c8867d5221da9e4f89167850ef9dab8ec 
  docs/reservation.md a400d19aec7a48d122ba1c9c23d38d792b8dbe6f 
  docs/upgrades.md 4f0c86db4c8d51f70487c03b2e75c1a4189b67b9 
  include/mesos/hook.hpp 210ffba09f5acae34ca49b888a781f683777f9ca 
  include/mesos/log/log.hpp 9c8634987181b1345005619e6d16d738903c49fc 
  include/mesos/mesos.proto 87af4a06db8cc3889fe4d3b314206103f5ce5f2d 
  include/mesos/state/in_memory.hpp 203274242854d482b01275597b249c58e6dfe2ad 
  include/mesos/state/leveldb.hpp 6c732d38d68a3d60d28ce68a6340e8771d849c53 
  include/mesos/state/log.hpp ac0312fdb92c46bfa2a7b83e95e04fd1eaf87d03 
  include/mesos/state/protobuf.hpp cde7b771f3c787293fb909a4b982c47ee19f4057 
  include/mesos/state/state.hpp f2fddee4fa803fa0572f6194e7f5f45a56254c00 
  include/mesos/state/state.proto 7a7d68e6abc85c0ead04f5771e878d10348f44b8 
  include/mesos/state/storage.hpp 2bfa0478b0edf76d592cc9644da83d15a00bc68c 
  include/mesos/state/zookeeper.hpp 8d8c19ce778f2499d86eb84008a61f211c528a3a 
  include/mesos/v1/mesos.proto 34da0a1484dc2f71262d8b97484b8edaae35bb6c 
  include/mesos/v1/scheduler.hpp 18e7a95f8342ea155f9e339945b05810b6bd82b5 
  include/mesos/zookeeper/authentication.hpp 
1c8855a5ec60d8628887fddff73d460e9ba1e643 
  include/mesos/zookeeper/contender.hpp 
348354ae1ab0a699f5d84b0e33b708cf06341789 
  include/mesos/zookeeper/detector.hpp 5c45f72b3fa540816b4f225004d9ae92158b4ef6 
  include/mesos/zookeeper/group.hpp d5ffca

Re: Review Request 46168: Add subdirectory support to URI.filename field.

[Michael Browning]

> On April 19, 2016, 4:08 p.m., Jiang Yan Xu wrote:
> > src/launcher/fetcher.cpp, line 252
> > 
> >
> > `basename` usually specifically refers to the `the component following 
> > the final '/'`.
> > 
> > So here s/basename/outputFile/

It is a proper basename in the case where we're just truncating the URI, but I 
agree that outputFile is more general given the expanded use here.


> On April 19, 2016, 4:08 p.m., Jiang Yan Xu wrote:
> > docs/fetcher.md, line 89
> > 
> >
> > I feel that "filename" is a bit odd when it can be a path. Many 
> > utilities simply call this a file, or output file to be more generic and 
> > flexible (when they do take a path).
> > 
> > e.g.,
> > ```
> > wget --output-document file
> > url --output file
> > tar --file file
> > gcc -o outfile
> > clang -o output-file
> > ```
> > 
> > I think `output_file` sounds good. (Notice the snake_casing in proto). 
> > What do you think?
> > 
> > We need to change docs and code (including CHANGELOG) elsewhere too but 
> > luckily the API is not released yet.
> > 
> > In CHANGELOG we can still group things under [MESOS-4735], just 
> > s/'filename'/'output_file'/.

output_file sounds good to me.


> On April 19, 2016, 4:08 p.m., Jiang Yan Xu wrote:
> > src/slave/containerizer/fetcher.cpp, lines 150-153
> > 
> >
> > What does this check against?
> > 
> > Path(filename).basename() never returns an Error().

Ah, got it confused with Fetcher::basename, which can return an Error.


- Michael


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


On April 14, 2016, 12:06 a.m., Michael Browning wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46168/
> ---
> 
> (Updated April 14, 2016, 12:06 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Jiang Yan Xu, and Zhitao Li.
> 
> 
> Bugs: MESOS-5119
> https://issues.apache.org/jira/browse/MESOS-5119
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add subdirectory support to URI.filename field.
> 
> URI.filename allows the user to specify the name of the file that will
> be saved in the sandbox when the URI is fetched, but previously it would
> fail at fetch time if "filename" had a directory component. This change
> allows users to specify a relative path for custom filenames within the
> sandbox.
> 
> 
> Diffs
> -
> 
>   docs/fetcher.md fd6d8a78bd35c5644dceff7005dd7dfd9f5f2171 
>   src/launcher/fetcher.cpp 47583eeaed53b3e7ed4db26fee7cdd2fae5e0c9d 
>   src/slave/containerizer/fetcher.cpp 
> d5910ad570371ba54580be5bb94344a1de38d1f9 
>   src/tests/fetcher_cache_tests.cpp 9ffcd2375f1203bd3d7c5d0cc898e955d5cb124e 
>   src/tests/fetcher_tests.cpp 23a8dc5f4402c5613744753284aabbe3d09bf797 
> 
> Diff: https://reviews.apache.org/r/46168/diff/
> 
> 
> Testing
> ---
> 
> Three tests were added. In fetcher_tests.cpp, CustomFilenameSubdirectory 
> tests that the fetcher creates a file in a specified subdirectory in the 
> sandbox, and AbsoluteCustomSubdirectoryFails tests that a custom filename 
> with an absolute path is rejected. In fetcher_cache_tests.cpp, 
> CachedCustomFilenameWithSubdirectory tests that the same behavior holds when 
> the URI is fetched from the cache.
> 
> 
> Thanks,
> 
> Michael Browning
> 
>

Re: Review Request 46013: Stout: Implemented `os::processes` on Windows.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On April 18, 2016, 7:09 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46013/
> ---
> 
> (Updated April 18, 2016, 7:09 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-4471
> https://issues.apache.org/jira/browse/MESOS-4471
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout: Implemented `os::processes` on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 79e30ca04c6d23f92e3a2f80fbe38ae63fde3520 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> edaa76a5322d0bf60b7172405aa754b5aca95458 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows/os.hpp 
> c48106e5905e3be0faeba7177ef534766089faff 
> 
> Diff: https://reviews.apache.org/r/46013/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 45360: Added volume driver client for mount and unmount.

[Jie Yu]

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




src/Makefile.am (line 687)


Please align the tailing ``



src/slave/containerizer/mesos/isolators/docker/volume/driver.hpp (line 41)


Do you want to use `const Option& dvdcliPath` here? The idea is 
that if not specified, the client will just use 'dvdcli' in $PATH. You want to 
use os::which to do a check in 'create' to make sure it's there if dvdcliPath 
is not specified.



src/slave/containerizer/mesos/isolators/docker/volume/driver.hpp (line 43)


Are you using virtual because of testing?

If not, let's try not to use virtual for now. Ditto for 'mount' and 
'unmount'. Let's just use concrete class with concrete implementation for now.

If it's for testing, then i think that makse sense as you want to use a 
MockDockerVolumeDriverClient.



src/slave/containerizer/mesos/isolators/docker/volume/driver.hpp (line 51)


Is the idea here that we use 'None' to express the intention that we don't 
want to call 'Create' implicitly? If yes, please document this in the comments.

Also, please remove 'const' for now as it's not clear it'll be const or not.



src/slave/containerizer/mesos/isolators/docker/volume/driver.hpp (line 58)


Ditto on removing 'const'.



src/slave/containerizer/mesos/isolators/docker/volume/driver.hpp (line 60)


Let's make it 'private' for now.



src/slave/containerizer/mesos/isolators/docker/volume/driver.hpp (line 68)


Why 'static'? If it's static, can that be a file local helper in driver.cpp?



src/slave/containerizer/mesos/isolators/docker/volume/driver.hpp (lines 72 - 75)


Ditto.



src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp (lines 58 - 63)


Can you use the argv version of 'subprocess' to avoid the need for escaping 
special chars.
```
vector argv = {
  dvdcli,
  "mount",
  "--volumedriver=" + driver,
  "--volumename=" + name,
};

...
```



src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp (lines 83 - 87)


You need to call io::read() before the process terminates. Otherwise, if 
the pipe is full, the child process will block.

```
return await(
s->status(),
s->out().get(),
s->err().get())
  .then([](const tuple<
  Future>,
  Future,
  Future>& t) -> Future {
Future> status = std::get<0>(t);
if (!status.isReady()) {
  ...
}
...
  });
```

Please follow the similar logic here:

https://github.com/apache/mesos/blob/master/src/slave/containerizer/mesos/isolators/network/cni/cni.cpp#L807


- Jie Yu


On April 15, 2016, 2:43 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45360/
> ---
> 
> (Updated April 15, 2016, 2:43 p.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added volume driver client for mount and unmount.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 06f58c4a88e3c527df727df1efe11ed3ab77efa8 
>   src/Makefile.am ec855263d620e4723c8ba9cd056c40a3a2e9ca99 
>   src/slave/containerizer/mesos/isolators/docker/volume/driver.hpp 
> PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45360/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46293: Windows: [3/3] Updated `sendfile` test.

[Michael Park]

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




3rdparty/libprocess/3rdparty/stout/tests/os/sendfile_tests.cpp (line 63)


Shouldn't this be `Try`? Here and below.



3rdparty/libprocess/3rdparty/stout/tests/os/sendfile_tests.cpp (line 65)


Please use `ASSERT_TRUE` instead.



3rdparty/libprocess/3rdparty/stout/tests/os/sendfile_tests.cpp (lines 66 - 68)


This fits in one line.



3rdparty/libprocess/3rdparty/stout/tests/os/sendfile_tests.cpp (line 83)


We should be looking at `result.error().code`. Here and below.


- Michael Park


On April 16, 2016, 12:20 a.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46293/
> ---
> 
> (Updated April 16, 2016, 12:20 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: [3/3] Updated `sendfile` test.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/tests/os/sendfile_tests.cpp 
> 5b1543443b83b35d0f3bf1ca5884845fb8d34e7a 
> 
> Diff: https://reviews.apache.org/r/46293/diff/
> 
> 
> Testing
> ---
> 
> OSX: make check
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 46285: Windows: [2/3] `sendfile` used with the typed error state of `Try`.

[Michael Park]

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




3rdparty/libprocess/src/poll_socket.cpp (line 135)


This is already part of master. It looks like this patch needs to be 
rebased.



3rdparty/libprocess/src/poll_socket.cpp (lines 204 - 220)


How about we clean this up a little bit:

```cpp
if (!length.isError()) {
  CHECK(length.get() >= 0);
  if (length.get() == 0) {
// Socket closed.
VLOG(1) << "Socket closed while sending";
  }
  return length.get();
}

if (net::is_restartable_error(length.error().code)) {
  // Interrupted, try again now.
  continue;
} else if (net::is_retryable_error(length.error().code)) {
  // Might block, try again later.
  return io::poll(s, io::WRITE)
.then(lambda::bind(&internal::socket_send_file, s, fd, offset, size));
} else {
  // Socket error.
  VLOG(1) << length.error().message;
  return Failure(length.error());
}
```


- Michael Park


On April 16, 2016, 12:20 a.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46285/
> ---
> 
> (Updated April 16, 2016, 12:20 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: [2/3] `sendfile` used with the typed error state of `Try`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/poll_socket.cpp 
> cb2878565a112017b190b4ff83dc65a876ea45f9 
> 
> Diff: https://reviews.apache.org/r/46285/diff/
> 
> 
> Testing
> ---
> 
> OSX: make check
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 43985: Windows: [1/3] Implemented `sendfile`.

[Michael Park]

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




3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/sendfile.hpp (lines 
33 - 34)


(1) `s/On error, On error,/On error,/`
(2) Remove the extra space in `//  Try`. i.e., `// Try`



3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/sendfile.hpp (lines 
39 - 40)


```
inline Try sendfile(
int s, int fd, off_t offset, size_t length)
{
```



3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/sendfile.hpp (line 45)


`s/send/sent/`



3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/sendfile.hpp (line 46)


(1) `s/return return/return/`
(2) Based on other syscalls, it seems like we should just do `return 
SocketError();`. e.g., `close.hpp`, `chroot.hpp`, `fnctl.hpp`, etc. Here and 
below.



3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp (lines 16 - 17)


When we fork to have separate implementation for `POSIX` and `Windows`, we 
should keep them completely separate. That is, even though `` 
is used in both implementations, they should include them individually.

For example, starting from 
`3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/sendfile.hpp` we 
can't find where `ErrnoError` came from.



3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp (lines 19 - 24)


Let's consolidate this with the `process::network::SocketError` in 
`3rdparty/libprocess/include/process/network.hpp`.

For now, I think `stout/error.hpp` is the best place to put it.



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/sendfile.hpp (lines 
16 - 17)


Add a newline in between.



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/sendfile.hpp (line 
23)


Backquotes around `Try`.



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/sendfile.hpp (lines 
24 - 25)


```
inline Try sendfile(
int s, int fd, off_t offset, size_t length)
{
```



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/sendfile.hpp (line 
27)


`s/ will/, it will/`



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/sendfile.hpp (lines 
28 - 30)


Please use the correct C++ casts here for the casts to `HANDLE`, `LONG`.



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/sendfile.hpp (line 
30)


`hight_Part`? did we mean `high_part`? Just `high` seems fine too.



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/sendfile.hpp (line 
31)


It looks like the second parameter of `SetFilePointer` is `LONG`, not 
`DWORD`. Why do we cast to `DWORD`? Also, please use C++ cast.



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/sendfile.hpp (line 
33)


Should return `SocketError` here (I know they're the same, but still).



3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/sendfile.hpp (line 
39)


Should return `SocketError` here (I know they're the same, but still).


- Michael Park


On April 16, 2016, 1:10 a.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43985/
> ---
> 
> (Updated April 16, 2016, 1:10 a.m.)
> 
> 
> Review request for mesos, Artem Harutyunyan, Alex Clemmer, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: [1/3] Implemented `sendfile`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/sendfile.hpp 
> 293f82f6730551491504721e0af28e9537540db1 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/sendfile.hpp 
> f3197679a9c22b37acae003262e5c6d21e110f56 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/sendfile.hpp 
> 750ca6749cb028703ed2fb5dec2aac6c5dabea0d 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> a7f855dc9d0a87fe3b6d1611e7ae22e4d7cd7b6d 
> 
> Diff: https

Re: Review Request 45604: Updated the balloon framework and executor.

[Joseph Wu]

> On April 12, 2016, 3:46 p.m., Vinod Kone wrote:
> > src/examples/balloon_executor.cpp, lines 143-153
> > 
> >
> > Why the change here?
> 
> Joseph Wu wrote:
> I couple reasons:
> 
> * I needed to put the "ballooning" logic into a separate thread, so that 
> it doesn't block the scheduler driver.  If the scheduler driver is blocked, 
> then we have no way of sending a `TASK_FINISHED` status update before 
> `driver->stop()`.
> * Seemed reasonable to move all the task logic into the task thread, 
> after the above change.
> * The comment was then encompassed by the task thread's comment.
> 
> Vinod Kone wrote:
> I see. Wish you had done this change in a separate review though.

Pulled into: https://reviews.apache.org/r/46407/


> On April 12, 2016, 3:46 p.m., Vinod Kone wrote:
> > src/examples/balloon_framework.cpp, lines 250-271
> > 
> >
> > why the explicit listing of terminal states rather than what we had 
> > before? that is future proof if we add new terminal states.
> 
> Joseph Wu wrote:
> The `protobuf::isTerminalState` helper is held in an internal header, so 
> it doesn't exactly belong in example code (which, I believe, should be 
> compilable outside of the mesos build chain.)
> 
> Do you think we should move that helper (and possibly others) into public 
> headers?
> 
> Vinod Kone wrote:
> Again, should've been done in a separate review.

Pulled into: https://reviews.apache.org/r/46411/


- Joseph


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


On April 19, 2016, 2:51 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45604/
> ---
> 
> (Updated April 19, 2016, 2:51 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5174
> https://issues.apache.org/jira/browse/MESOS-5174
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gives the example `balloon-framework` enough options to run
> outside of the build environment.
> 
> This also updates:
> 
> * The style of the framework code.
> * Adds an option for restricting the number of resources per task
>   (otherwise, it will eat up an entire node).
> * Adds an option for persisting the framework and launching one task
>   after another.
> * Adds filters for declined offers.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_framework.cpp 15c45612b777edaf97aea9b953439d4ad56920f3 
>   src/tests/balloon_framework_test.sh 
> a242f6cb9ca1850e5fef90e0938f41044bdaddbf 
> 
> Diff: https://reviews.apache.org/r/45604/diff/
> 
> 
> Testing
> ---
> 
> ```
> make check 
> 
> sudo bin/mesos-tests.sh --gtest_filter="*ROOT_CGROUPS_BalloonFramework"
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 45604: Updated the balloon framework and executor.

[Joseph Wu]

> On April 15, 2016, 2:13 p.m., Vinod Kone wrote:
> > src/examples/balloon_framework.cpp, lines 117-118
> > 
> >
> > why would they be incomplete?

There will be several metrics that count the number of tasks run (successfully 
or not) over time.  If the framework exits after one task, these counters will 
be meaningless (always 0).  I guess this is sort of implied, so I'll remove 
this line.


> On April 15, 2016, 2:13 p.m., Vinod Kone wrote:
> > src/examples/balloon_framework.cpp, lines 67-69
> > 
> >
> > Just change the type of  task_memory_usage_limit from Option to 
> > Bytes; no need for this validation then.

Counter-intuitively, specifying `Option` with a lambda is how you specify a 
required argument.  A flag without an `Option<>` must have a default value, and 
is therefore an optional flag.  :)


> On April 15, 2016, 2:13 p.m., Vinod Kone wrote:
> > src/examples/balloon_framework.cpp, lines 85-91
> > 
> >
> > Ditto. Change `task_memory` to Bytes instead of Option.

Dropping for the same reason as above


- Joseph


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


On April 19, 2016, 2:51 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45604/
> ---
> 
> (Updated April 19, 2016, 2:51 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-5174
> https://issues.apache.org/jira/browse/MESOS-5174
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gives the example `balloon-framework` enough options to run
> outside of the build environment.
> 
> This also updates:
> 
> * The style of the framework code.
> * Adds an option for restricting the number of resources per task
>   (otherwise, it will eat up an entire node).
> * Adds an option for persisting the framework and launching one task
>   after another.
> * Adds filters for declined offers.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_framework.cpp 15c45612b777edaf97aea9b953439d4ad56920f3 
>   src/tests/balloon_framework_test.sh 
> a242f6cb9ca1850e5fef90e0938f41044bdaddbf 
> 
> Diff: https://reviews.apache.org/r/45604/diff/
> 
> 
> Testing
> ---
> 
> ```
> make check 
> 
> sudo bin/mesos-tests.sh --gtest_filter="*ROOT_CGROUPS_BalloonFramework"
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 45905: Added metrics to the balloon framework.

[Joseph Wu]

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

(Updated April 19, 2016, 2:52 p.m.)


Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and Vinod 
Kone.


Changes
---

Update/rebase based on changes in previous reviews.


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


Repository: mesos


Description
---

Adds metrics to gauge the health of the framework.  This includes:

* uptime_secs = How long the framework has been running.
* registered = If the framework is registered.
* tasks_finished = Number of tasks finished (successfully).
* tasks_oomed = Number of tasks that were OOM killed.
* allowed_terminations = Number of terminal status updates which
  are acceptable due to infrastructure reasons.
* abnormal_terminations = Number of terminal status updates which 
  were not `TASK_FINISHED` or `TASK_FAILED` due to OOM.


Diffs (updated)
-

  src/examples/balloon_framework.cpp 15c45612b777edaf97aea9b953439d4ad56920f3 

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


Testing
---

```
make check

sudo bin/mesos-tests.sh --gtest_filter="*ROOT_CGROUPS_BalloonFramework"

# Also launched two instances on a cluster.
# This one OOM's:
./balloon-framework --master=zk://localhost:2181/mesos --checkpoint 
--balloon_limit=256MB --task_memory=128MB 
--executor_uri="https://s3.amazonaws.com/url/to/balloon-executor"; 
--executor_command="LD_LIBRARY_PATH=/path/to/libmesos && ./balloon-executor"

# This one does not OOM:
./balloon-framework --master=zk://localhost:2181/mesos --checkpoint 
--balloon_limit=256MB --task_memory=256MB 
--executor_uri="https://s3.amazonaws.com/url/to/balloon-executor"; 
--executor_command="LD_LIBRARY_PATH=/path/to/libmesos && ./balloon-executor"
```


Thanks,

Joseph Wu

Re: Review Request 45604: Updated the balloon framework and executor.

[Joseph Wu]

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

(Updated April 19, 2016, 2:51 p.m.)


Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and Vinod 
Kone.


Changes
---

Split up a few parts (as requested in comments) into separate reviews (before 
and after).


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


Repository: mesos


Description (updated)
---

This gives the example `balloon-framework` enough options to run
outside of the build environment.

This also updates:

* The style of the framework code.
* Adds an option for restricting the number of resources per task
  (otherwise, it will eat up an entire node).
* Adds an option for persisting the framework and launching one task
  after another.
* Adds filters for declined offers.


Diffs (updated)
-

  src/examples/balloon_framework.cpp 15c45612b777edaf97aea9b953439d4ad56920f3 
  src/tests/balloon_framework_test.sh a242f6cb9ca1850e5fef90e0938f41044bdaddbf 

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


Testing
---

```
make check 

sudo bin/mesos-tests.sh --gtest_filter="*ROOT_CGROUPS_BalloonFramework"
```


Thanks,

Joseph Wu

Review Request 46411: Removed private header from the balloon framework.

[Joseph Wu]

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

Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and Vinod 
Kone.


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


Repository: mesos


Description
---

Changes the `statusUpdate` handler to explicitly enumerate terminal 
states. Previously, the handler used an internal utility method, which
would not be available if this framework were compiled outside of Mesos.


Diffs
-

  src/examples/balloon_framework.cpp 15c45612b777edaf97aea9b953439d4ad56920f3 

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


Testing
---

make check


Thanks,

Joseph Wu

Review Request 46407: Updated balloon executor.

[Joseph Wu]

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

Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and Vinod 
Kone.


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


Repository: mesos


Description
---

Fixes the logic in the example `balloon-executor` to work in cases not
exercised by the `ROOT_CGROUPS_BalloonFramework` test.

* The "task" logic was moved into a separate thread.  This fixes the 
  case where the balloon executor does not exceed the memory limit.
  (i.e. by Unblocking the driver's thread while sending status updates.)
* Changed log messages to use glog.
* Added logic to prevent multiple task launches with the same executor.


Diffs
-

  src/examples/balloon_executor.cpp 108ebd9afec5b2d592ffbe5c150a9271f1899f2c 

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


Testing
---

See next review in this chain.


Thanks,

Joseph Wu

Re: Review Request 46392: Windows:[PLACEHOLDER] Implemented fcntl nonblock and other items.

[Mesos ReviewBot]

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



Bad review!

Reviews applied: []

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

- Mesos ReviewBot


On April 19, 2016, 4:35 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46392/
> ---
> 
> (Updated April 19, 2016, 4:35 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit introduces temporary versions of 2 important functions:
> `os::nonblock` and `os::cloexec`. We put them here in a placeholder
> commit so that reviewers can make progress on subprocess. In the
> immediate term, the plan is to figure out on a callsite-by-callsite
> basis how to work around the functionality of `os::cloexec`. When we
> collect more data, we will be in a better position to offer a way
> forward here.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
> 79e30ca04c6d23f92e3a2f80fbe38ae63fde3520 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/fcntl.hpp 
> 14734317d7fb40053ee808745ac3ba8c706a7669 
> 
> Diff: https://reviews.apache.org/r/46392/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 46373: Request /files/read.json with a negative length value causes error.

[Greg Mann]

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




src/files/files.cpp (line 380)


Rather than logging this and setting the length to page size here, I think 
we would want to return a BadRequest response.



src/files/files.cpp (line 426)


Negative offsets can be useful as well; for example, I could use {offset = 
-1024; length = 1024} to read the last 1024 bytes of the file.


- Greg Mann


On April 19, 2016, 8:07 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46373/
> ---
> 
> (Updated April 19, 2016, 8:07 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Greg Mann.
> 
> 
> Bugs: mesos-5060
> https://issues.apache.org/jira/browse/mesos-5060
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> [MESOS-5060]
> The patch did the following changes:
> 1. Fix the offset and length logic in files.cpp.
> 2. Add some tests to test the /files/read.json endponit with
> negative offset or length.
> 
> 
> Diffs
> -
> 
>   src/files/files.cpp 4e916101b378b0e9032a08a3f6c73e195b2a08a1 
>   src/tests/files_tests.cpp 5d6620d13babaf0bb7f9c888bb1b4fa2228b6ccd 
> 
> Diff: https://reviews.apache.org/r/46373/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> request 'files/read.json' endpoint with negative offset or length argument
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 46391: Clarified several agent log messages.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46391]

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 April 19, 2016, 4:16 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46391/
> ---
> 
> (Updated April 19, 2016, 4:16 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When diagnosing problems, it can be useful to distinguish
> between executors that are "terminating" vs. "terminated".
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp d82dec2b10d496065013eb4ad6a35dc054b72553 
> 
> Diff: https://reviews.apache.org/r/46391/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 46249: Hook and module process operation before main process initialize.

[Joseph Wu]

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


Ship it!




LGTM.


src/master/main.cpp (lines 242 - 249)


NOTE: This is technically not necessary for modules, since 
`ModuleManager::load` does not execute any dynamically loaded code.

It's still nice to have, since it makes sense to do `process::initialize` 
immediately after loading flags.


- Joseph Wu


On April 18, 2016, 7:52 p.m., Andy Pang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46249/
> ---
> 
> (Updated April 18, 2016, 7:52 p.m.)
> 
> 
> Review request for mesos, Adam B, Kapil Arya, Joseph Wu, Niklas Nielsen, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If we in hook/module manager use libprocess library, 
> it will auto use process initialize fucntion, 
> as the process::initialize will only call once,
> then in the main fucnton process initialize  
> process::initialize("master") will not be called. 
> So the HTTP request process "delegate" will be wrong, 
> http request will not be response.
> 
> 
> Diffs
> -
> 
>   src/master/main.cpp 7bbc982 
> 
> Diff: https://reviews.apache.org/r/46249/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Andy Pang
> 
>

Re: Review Request 46375: Updated MACHINE_UP_HELP's comments.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On April 19, 2016, 12:51 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46375/
> ---
> 
> (Updated April 19, 2016, 12:51 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated MACHINE_UP_HELP's comments.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp a9cb99a92ff5a783e719df5e5cfb6e8301241df9 
> 
> Diff: https://reviews.apache.org/r/46375/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 46364: Change to explicit case statements (`UNKNOWN`) instead of default.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46364]

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 April 19, 2016, 3:19 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46364/
> ---
> 
> (Updated April 19, 2016, 3:19 p.m.)
> 
> 
> Review request for mesos, Adam B, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-5031
> https://issues.apache.org/jira/browse/MESOS-5031
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch changes from `default` statement to explicit case
> statement (`UNKNOWN`) so that complier could help catching all
> switches when new enums are introduced in the future.
> 
> This patch is related to:
> https://reviews.apache.org/r/45317/ (MESOS-5014)
> https://reviews.apache.org/r/45304/ (MESOS-5015)
> https://reviews.apache.org/r/45342/ (MESOS-5031)
> 
> 
> Diffs
> -
> 
>   src/authorizer/local/authorizer.cpp 
> c744a16041c2466d3839a37fbee2bf86887bf4e1 
>   src/cli/execute.cpp 087a73143d739f41638b8335c7e24dfcd14bc7fb 
>   src/examples/long_lived_framework.cpp 
> c696ccb6b2ace6e047f6509b291dd14be240cf70 
>   src/examples/test_http_executor.cpp 
> ceb489d43f35d24c8a7f5fbb0148529745ee357a 
>   src/examples/test_http_framework.cpp 
> 8cc3107034d46cb6a2966835f509508223c6e674 
>   src/master/http.cpp a9cb99a92ff5a783e719df5e5cfb6e8301241df9 
>   src/master/master.cpp 6dacf5fbd73771e5c31ffb0e8723cd2905dcefb3 
>   src/master/validation.cpp 0cd118ee4f89f749a063f6ba7f419d5a220dc1d4 
>   src/slave/http.cpp 3908e33ed5b233387790276f6f5d884452087d2c 
>   src/slave/validation.cpp ec1a4b6d9c55ab0c9894d5a49e290e15dee32c22 
>   src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b 
> 
> Diff: https://reviews.apache.org/r/46364/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 45373: Ignored the DOCKER_VOLUME volume source.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On April 19, 2016, 2:52 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45373/
> ---
> 
> (Updated April 19, 2016, 2:52 p.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-5013
> https://issues.apache.org/jira/browse/MESOS-5013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ignored the DOCKER_VOLUME volume source.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> 9fc7c48f99155750fd3c18c7c102507e2726362b 
> 
> Diff: https://reviews.apache.org/r/45373/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46308: Moved LogProcess declaration to log/log.h.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On April 17, 2016, 4:47 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46308/
> ---
> 
> (Updated April 17, 2016, 4:47 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5171
> https://issues.apache.org/jira/browse/MESOS-5171
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved LogProcess declaration to log/log.h.
> 
> 
> Diffs
> -
> 
>   include/mesos/log/log.hpp 0f61777dd04e6d7212ac218c4cde3cd7f95d2896 
>   src/Makefile.am ec855263d620e4723c8ba9cd056c40a3a2e9ca99 
>   src/log/log.hpp PRE-CREATION 
>   src/log/log.cpp a37676068dae14b1adc61ef75e2742c16e7a6e42 
> 
> Diff: https://reviews.apache.org/r/46308/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 46388: Fixed typo in test setup error message.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46388]

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 April 19, 2016, 2:23 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46388/
> ---
> 
> (Updated April 19, 2016, 2:23 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed typo in test setup error message.
> 
> 
> Diffs
> -
> 
>   src/tests/environment.cpp 45ed8f27873e77ac5d0852a198a9474199bcdfc3 
> 
> Diff: https://reviews.apache.org/r/46388/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 46401: Corrected acls protobuf file in documentation and flag.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On April 19, 2016, 6:38 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46401/
> ---
> 
> (Updated April 19, 2016, 6:38 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The acls protobuf messages have moved from
> authorization.proto to acls.proto. This patch reflects
> this change in the documentation/flag.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md cd9733002a0940f44edd4e56593a0ca3fe9f77f5 
>   src/master/flags.cpp e6a239c15f52b381a83cc359d8b5a335a5fbd0af 
> 
> Diff: https://reviews.apache.org/r/46401/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Review Request 46401: Corrected acls protobuf file in documentation and flag.

[Joerg Schad]

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

Review request for mesos, Adam B, Alexander Rojas, and Greg Mann.


Repository: mesos


Description
---

The acls protobuf messages have moved from
authorization.proto to acls.proto. This patch reflects
this change in the documentation/flag.


Diffs
-

  docs/configuration.md cd9733002a0940f44edd4e56593a0ca3fe9f77f5 
  src/master/flags.cpp e6a239c15f52b381a83cc359d8b5a335a5fbd0af 

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


Testing
---


Thanks,

Joerg Schad

Re: Review Request 46139: Add positive tests for /weights endpoint.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46139]

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 April 19, 2016, 1:42 p.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46139/
> ---
> 
> (Updated April 19, 2016, 1:42 p.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-4316
> https://issues.apache.org/jira/browse/MESOS-4316
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add positive tests for /weights endpoint.
> 
> 
> Diffs
> -
> 
>   src/tests/dynamic_weights_tests.cpp 
> f89b89dd2553220161e28ec4784eb0bbdfdb65fe 
>   src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b 
> 
> Diff: https://reviews.apache.org/r/46139/diff/
> 
> 
> Testing
> ---
> 
> make && make check.
> 
> Yongs-MacBook-Pro:build yqwyq$ ./src/mesos-tests 
> --gtest_filter=DynamicWeightsTest.*
> [==] Running 13 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 13 tests from DynamicWeightsTest
> [ RUN  ] DynamicWeightsTest.PutInvalidRequest
> [   OK ] DynamicWeightsTest.PutInvalidRequest (89 ms)
> [ RUN  ] DynamicWeightsTest.ZeroWeight
> [   OK ] DynamicWeightsTest.ZeroWeight (39 ms)
> [ RUN  ] DynamicWeightsTest.NegativeWeight
> [   OK ] DynamicWeightsTest.NegativeWeight (46 ms)
> [ RUN  ] DynamicWeightsTest.NonNumericWeight
> [   OK ] DynamicWeightsTest.NonNumericWeight (39 ms)
> [ RUN  ] DynamicWeightsTest.MissingRole
> [   OK ] DynamicWeightsTest.MissingRole (37 ms)
> [ RUN  ] DynamicWeightsTest.UnknownRole
> [   OK ] DynamicWeightsTest.UnknownRole (32 ms)
> [ RUN  ] DynamicWeightsTest.UpdateWeightsWithExplictRoles
> [   OK ] DynamicWeightsTest.UpdateWeightsWithExplictRoles (44 ms)
> [ RUN  ] DynamicWeightsTest.UnauthenticatedUpdateWeightRequest
> [   OK ] DynamicWeightsTest.UnauthenticatedUpdateWeightRequest (40 ms)
> [ RUN  ] DynamicWeightsTest.UnauthenticatedQueryWeightRequest
> [   OK ] DynamicWeightsTest.UnauthenticatedQueryWeightRequest (35 ms)
> [ RUN  ] DynamicWeightsTest.AuthorizedWeightUpdateRequest
> [   OK ] DynamicWeightsTest.AuthorizedWeightUpdateRequest (42 ms)
> [ RUN  ] DynamicWeightsTest.AuthorizedUpdateWeightRequestWithoutPrincipal
> [   OK ] DynamicWeightsTest.AuthorizedUpdateWeightRequestWithoutPrincipal 
> (41 ms)
> [ RUN  ] DynamicWeightsTest.UnauthorizedWeightUpdateRequest
> [   OK ] DynamicWeightsTest.UnauthorizedWeightUpdateRequest (40 ms)
> [ RUN  ] DynamicWeightsTest.RecoveredWeightsFromRegistry
> [   OK ] DynamicWeightsTest.RecoveredWeightsFromRegistry (135 ms)
> [--] 13 tests from DynamicWeightsTest (659 ms total)
> 
> [--] Global test environment tear-down
> [==] 13 tests from 1 test case ran. (669 ms total)
> [  PASSED  ] 13 tests.
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 45270: Added spec protobuf for DockerVolumeMount.

[Jie Yu]

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




src/CMakeLists.txt (line 55)


s/SPEC/STATE/?



src/CMakeLists.txt (line 88)


Ditto.



src/slave/containerizer/mesos/isolators/docker/volume/state.proto (line 27)


s/DockerVolumeMountList/DockerVolumeMounts/



src/slave/containerizer/mesos/isolators/docker/volume/state.proto (lines 32 - 
46)


I still don't understand why we want to make it a protobuf? Can that just 
be a nested struct in the isolator?


- Jie Yu


On April 19, 2016, 3:04 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45270/
> ---
> 
> (Updated April 19, 2016, 3:04 p.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-5013
> https://issues.apache.org/jira/browse/MESOS-5013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added spec protobuf for DockerVolumeMount.
> The DockerVolumeMount was used to check point the mount
> point for each container to recover.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt aabb33dfd4b985fe5774f87bb63b4ec1ec853962 
>   src/Makefile.am 5a2b4efa781752863d6751f98614fb78bece73ac 
>   src/slave/containerizer/mesos/isolators/docker/volume/state.proto 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/45270/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46245: Renamed Docker and CNI BASE_NAME to meaningful name.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On April 19, 2016, 2:48 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46245/
> ---
> 
> (Updated April 19, 2016, 2:48 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Renamed Docker and CNI BASE_NAME to meaningful name.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt aabb33dfd4b985fe5774f87bb63b4ec1ec853962 
> 
> Diff: https://reviews.apache.org/r/46245/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 34646: Redirect to the leader master when current master is not a leader.

[haosdent huang]

> On April 19, 2016, 3:37 p.m., Neil Conway wrote:
> > Can we update the docs to describe this behavior? e.g., add a note to 
> > https://mesos.apache.org/documentation/latest/endpoints/ describing the 
> > redirect behavior -- I suppose it is worth adding a note to every master 
> > endpoint's doc page describing the redirect behavior, as well as a note to 
> > the "Master Endpoints" section of the top-level endpoints doc. Might also 
> > be worth adding a note to the "Operational Guide" or "High Availability" 
> > pages. We also need to update the docs for the "master/redirect/" endpoint.
> > 
> > This change requires one or more test cases. For example: (a) normal 
> > redirect case (b) return ServiceUnavailable when there is no leading master 
> > (c) avoiding redirect loop for `/redirect` endpoint.

Thank you very much for your review. Let me update.


- haosdent


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


On April 19, 2016, 1:55 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34646/
> ---
> 
> (Updated April 19, 2016, 1:55 a.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, Cody Maloney, Ian Downes, 
> Neil Conway, and Vinod Kone.
> 
> 
> Bugs: MESOS-1865
> https://issues.apache.org/jira/browse/MESOS-1865
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In this changes, we redirect to the leader master in those http
> endpoints which depend on master elected status if current master is
> not a leader.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp a9cb99a92ff5a783e719df5e5cfb6e8301241df9 
> 
> Diff: https://reviews.apache.org/r/34646/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> when current master is not a leader, it would redirect to the leader master.
> 
> ```
> $ curl -i http://master1:5050/master/tasks.json
> HTTP/1.1 307 Temporary Redirect
> Date: Mon, 01 Jun 2015 06:30:08 GMT
> Location: http://master2:5050//master/tasks.json
> Content-Length: 0
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 45326: Implemented os::which().

[Jie Yu]

> On April 19, 2016, 3:03 p.m., haosdent huang wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp, line 975
> > 
> >
> > Do we need test the priority here? When a binary exists in different 
> > folders at the same time.
> 
> Guangya Liu wrote:
> I think that as long as the binary can be found should be good enough, 
> why need care the priority here?
> 
> haosdent huang wrote:
> Let me describe it in another way. Suppose `PATH` is `/usr/bin:/bin`, and 
> `bar` exists in both `/usr/bin/bar` and `/bin/bar`. Are we we always 
> guarantee that take the first one in `os::which()`? Or add comment about the 
> priority?
> 
> Guangya Liu wrote:
> Yes, I was following linux system, always using the first. Does adding 
> some comments here make sense?
> 
> root@mesos002:~/src/mesos/m2/mesos# which ls
> /usr/local/bin/ls
> root@mesos002:~/src/mesos/m2/mesos# env | grep PATH
> 
> PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/usr/local/go/bin:/root/go-tools/bin
> GOPATH=/root/go-tools
> root@mesos002:~/src/mesos/m2/mesos# ls /usr/local/bin/ls
> /usr/local/bin/ls
> root@mesos002:~/src/mesos/m2/mesos# ls /bin/ls
> /bin/ls
> 
> haosdent huang wrote:
> Seems add a comment would be better.

I'll add a TODO. We need to test file without 'x' permission as well.


- Jie


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


On April 19, 2016, 2:56 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45326/
> ---
> 
> (Updated April 19, 2016, 2:56 p.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-4576
> https://issues.apache.org/jira/browse/MESOS-4576
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented os::which().
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> dfe4eab2d812ef807c7416ac205573d55383c4ee 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> 9bd34c7508cd813c5de18028956f6a740997c266 
> 
> Diff: https://reviews.apache.org/r/45326/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> [ RUN  ] OsTest.Which
> [   OK ] OsTest.Which (0 ms)
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45326: Implemented os::which().

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On April 19, 2016, 2:56 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45326/
> ---
> 
> (Updated April 19, 2016, 2:56 p.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-4576
> https://issues.apache.org/jira/browse/MESOS-4576
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented os::which().
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> dfe4eab2d812ef807c7416ac205573d55383c4ee 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> 9bd34c7508cd813c5de18028956f6a740997c266 
> 
> Diff: https://reviews.apache.org/r/45326/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> [ RUN  ] OsTest.Which
> [   OK ] OsTest.Which (0 ms)
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46331: Fixed the issue that command executor can not join CNI network.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On April 19, 2016, 8:21 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46331/
> ---
> 
> (Updated April 19, 2016, 8:21 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Jie Yu.
> 
> 
> Bugs: MESOS-5225
> https://issues.apache.org/jira/browse/MESOS-5225
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the issue that command executor can not join CNI network.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 6e3fe118fb63d1ce7d5b01a6ac32f8f3a1369cfb 
> 
> Diff: https://reviews.apache.org/r/46331/diff/
> 
> 
> Testing
> ---
> 
> 1. Start master
> sudo ./bin/mesos-master.sh --work_dir=/tmp
> 
> 2. Start agent
> sudo ./bin/mesos-slave.sh --master=192.168.122.171:5050 
> --containerizers=mesos --image_providers=docker 
> --isolation=filesystem/linux,docker/runtime,network/cni 
> --network_cni_config_dir=/opt/cni/net_configs 
> --network_cni_plugins_dir=/opt/cni/plugins
> 
> 3. Launch a command task with mesos-execute, and it will join a CNI network 
> net1.
> sudo src/mesos-execute --master=192.168.122.171:5050 --name=test 
> --docker_image=library/busybox --networks=net1 --command="sleep 10" 
> --shell=true
> I0418 21:34:57.248507 24604 scheduler.cpp:177] Version: 0.29.0
> Subscribed with ID 'c992158d-e625-4359-97fe-6320172fd957-0016'
> Submitted task 'test' to agent 'eeb0be14-77cb-462d-b088-657745453c83-S0'
> Received status update TASK_RUNNING for task 'test'
>   source: SOURCE_EXECUTOR
> Received status update TASK_FINISHED for task 'test'
>   message: 'Command exited with status 0'
>   source: SOURCE_EXECUTOR
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 46323: Propagated KillPolicy in kill task from scheduler to executor.

[Alexander Rukletsov]

> On April 19, 2016, 1:09 a.m., Ben Mahler wrote:
> > src/internal/evolve.hpp, line 46
> > 
> >
> > Why did you choose to inject it here? Seems better closer to TaskInfo?

Because in "mesos.proto" `KillPolicy` is defined between `FrameworkInfo` and 
`ExecutorInfo`. Do you think we should move it closer to `TaskInfo` in *both* 
"mesos.proto" and "evolve.hpp"?


> On April 19, 2016, 1:09 a.m., Ben Mahler wrote:
> > src/master/master.cpp, lines 4060-4061
> > 
> >
> > Since it seems straightforward, why not just handle the field instead 
> > of introducing this NOTE? It seems nice to add the straightforward support 
> > here and decide on the old API later (if at all).

We can do it, but I don't understand the purpose (and I'm reluctant to add any 
code if there is no purpose). The scheduler driver uses `Call` message instead 
of internal `KillTaskMessage` since Mesos 0.24. The only use case that comes to 
my mind is pure bindings that do not use the scheduler driver and send 
`KillTaskMessage` directly. Do we want to add support for this case?


> On April 19, 2016, 1:09 a.m., Ben Mahler wrote:
> > src/slave/slave.hpp, line 149
> > 
> >
> > Converting an optional field into an Option is not yet supported, 
> > unless I'm missing something?
> > 
> > See: https://issues.apache.org/jira/browse/MESOS-2638
> > 
> > Note that I added the current workaround to that ticket description, 
> > you can take the entire message in the handler to check the optionality.

Hm, I see. I've noticed a similar pattern for 
https://github.com/apache/mesos/blob/45d5fc379ff59c537ffc9ddfdf33c845af1e381f/src/slave/slave.cpp#L683
 and thought we support that.


- Alexander


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


On April 18, 2016, 12:44 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46323/
> ---
> 
> (Updated April 18, 2016, 12:44 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-4908
> https://issues.apache.org/jira/browse/MESOS-4908
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/internal/evolve.hpp cff6b657a6d69ff9e7f7dedc965122a03a200da1 
>   src/internal/evolve.cpp 4ca2abacd75523de8fb4ffe69a9edd8627f512af 
>   src/master/master.cpp 6dacf5fbd73771e5c31ffb0e8723cd2905dcefb3 
>   src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1 
>   src/slave/slave.cpp d82dec2b10d496065013eb4ad6a35dc054b72553 
>   src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b 
>   src/tests/mesos.cpp b5937af7713e1ee2af475518b3e968b2defa8beb 
>   src/tests/slave_tests.cpp ee58488b0b927c7c5833add4718941539663e6d2 
> 
> Diff: https://reviews.apache.org/r/46323/diff/
> 
> 
> Testing
> ---
> 
> The whole chain is tested in https://reviews.apache.org/r/46325/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Review Request 46371: Added basic tests for capabilities API.

[Jojy Varghese]

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Added basic tests for capabilities API.


Diffs
-

  src/Makefile.am a8f68316c5d4d4c82d99363535a97fa5b1caafc5 
  src/tests/capabilities_tests.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 45326: Implemented os::which().

[haosdent huang]

> On April 19, 2016, 3:03 p.m., haosdent huang wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp, line 976
> > 
> >
> > Should `EXPECT_SOME` here?
> 
> Guangya Liu wrote:
> Can you please show more detail for why use `EXPECT_SOME` here?
> 
> haosdent huang wrote:
> `ASSERT` would abort the test while `EXPECT` would continue to run 
> following code. I think we usually use `ASSERT` when something block the 
> test. e.g. `ASSERT_SOME(master);`. And usually use `EXPECT` at those thing we 
> want to test in the test cases. e.g. `EXPECT_CALL(...`.
> 
> Guangya Liu wrote:
> Yes, but I was following this in test case, 
> https://github.com/apache/mesos/blob/master/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp#L964

Got it, let me drop this.


> On April 19, 2016, 3:03 p.m., haosdent huang wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp, line 975
> > 
> >
> > Do we need test the priority here? When a binary exists in different 
> > folders at the same time.
> 
> Guangya Liu wrote:
> I think that as long as the binary can be found should be good enough, 
> why need care the priority here?
> 
> haosdent huang wrote:
> Let me describe it in another way. Suppose `PATH` is `/usr/bin:/bin`, and 
> `bar` exists in both `/usr/bin/bar` and `/bin/bar`. Are we we always 
> guarantee that take the first one in `os::which()`? Or add comment about the 
> priority?
> 
> Guangya Liu wrote:
> Yes, I was following linux system, always using the first. Does adding 
> some comments here make sense?
> 
> root@mesos002:~/src/mesos/m2/mesos# which ls
> /usr/local/bin/ls
> root@mesos002:~/src/mesos/m2/mesos# env | grep PATH
> 
> PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/usr/local/go/bin:/root/go-tools/bin
> GOPATH=/root/go-tools
> root@mesos002:~/src/mesos/m2/mesos# ls /usr/local/bin/ls
> /usr/local/bin/ls
> root@mesos002:~/src/mesos/m2/mesos# ls /bin/ls
> /bin/ls

Seems add a comment would be better.


- haosdent


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


On April 19, 2016, 2:56 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45326/
> ---
> 
> (Updated April 19, 2016, 2:56 p.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-4576
> https://issues.apache.org/jira/browse/MESOS-4576
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented os::which().
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> dfe4eab2d812ef807c7416ac205573d55383c4ee 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> 9bd34c7508cd813c5de18028956f6a740997c266 
> 
> Diff: https://reviews.apache.org/r/45326/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> [ RUN  ] OsTest.Which
> [   OK ] OsTest.Which (0 ms)
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Review Request 46370: Introduced linux capabilities API.

[Jojy Varghese]

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

Review request for mesos, Jie Yu and Kevin Klues.


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


Repository: mesos


Description
---

This change introduces basic API for linux capabilities. This is not a
comprehensive API but is strictly limited to the need for securing Mesos
containers using linux capabilities.


Diffs
-

  src/CMakeLists.txt ca59a1824352423f4db6ef8bb41acc6fe602c041 
  src/Makefile.am a8f68316c5d4d4c82d99363535a97fa5b1caafc5 
  src/linux/capabilities.hpp PRE-CREATION 
  src/linux/capabilities.cpp PRE-CREATION 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 45326: Implemented os::which().

[Guangya Liu]

> On 四月 19, 2016, 3:03 p.m., haosdent huang wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp, line 975
> > 
> >
> > Do we need test the priority here? When a binary exists in different 
> > folders at the same time.
> 
> Guangya Liu wrote:
> I think that as long as the binary can be found should be good enough, 
> why need care the priority here?
> 
> haosdent huang wrote:
> Let me describe it in another way. Suppose `PATH` is `/usr/bin:/bin`, and 
> `bar` exists in both `/usr/bin/bar` and `/bin/bar`. Are we we always 
> guarantee that take the first one in `os::which()`? Or add comment about the 
> priority?

Yes, I was following linux system, always using the first. Does adding some 
comments here make sense?

root@mesos002:~/src/mesos/m2/mesos# which ls
/usr/local/bin/ls
root@mesos002:~/src/mesos/m2/mesos# env | grep PATH
PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/usr/local/go/bin:/root/go-tools/bin
GOPATH=/root/go-tools
root@mesos002:~/src/mesos/m2/mesos# ls /usr/local/bin/ls
/usr/local/bin/ls
root@mesos002:~/src/mesos/m2/mesos# ls /bin/ls
/bin/ls


> On 四月 19, 2016, 3:03 p.m., haosdent huang wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp, line 976
> > 
> >
> > Should `EXPECT_SOME` here?
> 
> Guangya Liu wrote:
> Can you please show more detail for why use `EXPECT_SOME` here?
> 
> haosdent huang wrote:
> `ASSERT` would abort the test while `EXPECT` would continue to run 
> following code. I think we usually use `ASSERT` when something block the 
> test. e.g. `ASSERT_SOME(master);`. And usually use `EXPECT` at those thing we 
> want to test in the test cases. e.g. `EXPECT_CALL(...`.

Yes, but I was following this in test case, 
https://github.com/apache/mesos/blob/master/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp#L964


- Guangya


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


On 四月 19, 2016, 2:56 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45326/
> ---
> 
> (Updated 四月 19, 2016, 2:56 p.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-4576
> https://issues.apache.org/jira/browse/MESOS-4576
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented os::which().
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> dfe4eab2d812ef807c7416ac205573d55383c4ee 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> 9bd34c7508cd813c5de18028956f6a740997c266 
> 
> Diff: https://reviews.apache.org/r/45326/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> [ RUN  ] OsTest.Which
> [   OK ] OsTest.Which (0 ms)
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45673: PoC: Docker Volume Isolator.

[Guangya Liu]

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

(Updated 四月 19, 2016, 4:45 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Repository: mesos


Description
---

PoC: Docker Volume Isolator.


Diffs (updated)
-

  src/CMakeLists.txt aabb33dfd4b985fe5774f87bb63b4ec1ec853962 
  src/Makefile.am 5a2b4efa781752863d6751f98614fb78bece73ac 
  src/cli/execute.cpp 087a73143d739f41638b8335c7e24dfcd14bc7fb 
  src/slave/containerizer/mesos/containerizer.cpp 
1e1a36903f4377497bb72b69e4ead63675d453c0 
  src/slave/containerizer/mesos/isolators/docker/volume/driver.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
786f917c0ec04b6839bbd524fa7c8de3729f9bdb 
  src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
0ad473dc3bd45e122fba55a670e1a893e61c977a 
  src/slave/containerizer/mesos/isolators/docker/volume/paths.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/docker/volume/paths.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/docker/volume/state.hpp PRE-CREATION 
  src/slave/slave.cpp d82dec2b10d496065013eb4ad6a35dc054b72553 

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


Testing (updated)
---

state.proto

message DockerVolumeMount {
  required string volume_driver = 1;
  required string volume_name = 2;
}


message DockerVolumeMountList {
  repeated DockerVolumeMount mounts = 1;
}


message DockerVolumeInfo {
  // Docker volume mount info for volume driver and volume name.
  required DockerVolumeMount dvm = 1;

  // Container path, this is optional as this is not needed by recover()
  // but only for prepare() .
  optional string container_path = 2;

  // Mount point for the container, this is optiona as this is only needed
  // by prepare().
  optional string mount_point = 3;
  
  // Volume driver mount options.
  optional Parameters driver_options = 4;
}

DockerVolumeMount will be checkpointed.


//   /var/run/mesos/isolators/docker/volume
//|- /
//|  |-- DockerVolumeMountList
//|-- /
//|  |-- DockerVolumeMountList
//|-/
//|- ...

Info structure
We assume that one container do not use multiple same volume driver and volume 
name pair.
struct Info
{
  Info (const list& _volumeInfos)
: volumeInfos(_volumeInfos) {}

  list volumeInfos;
};

Hashmap in isolator:
hashmap> infos;
hashmap volumeReference; // string is volume_driver::volume_name, 
e.g. convoy:dvd1

Cleanup()
When cleanup, check the container’s mount point reference counter, if it is 
greater than 1, do not remove the mount point; If it is 1, unmount the mount 
point.
driver.cpp::mount
We need to transfer the whole mount info to mount() as parameter to make sure 
that the container path can get its own related mount point. If the mount() 
only return a string, then we cannot get the relationship of container path and 
mount point.


Thanks,

Guangya Liu

Re: Review Request 46331: Fixed the issue that command executor can not join CNI network.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46331]

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 April 19, 2016, 8:21 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46331/
> ---
> 
> (Updated April 19, 2016, 8:21 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Jie Yu.
> 
> 
> Bugs: MESOS-5225
> https://issues.apache.org/jira/browse/MESOS-5225
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the issue that command executor can not join CNI network.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 6e3fe118fb63d1ce7d5b01a6ac32f8f3a1369cfb 
> 
> Diff: https://reviews.apache.org/r/46331/diff/
> 
> 
> Testing
> ---
> 
> 1. Start master
> sudo ./bin/mesos-master.sh --work_dir=/tmp
> 
> 2. Start agent
> sudo ./bin/mesos-slave.sh --master=192.168.122.171:5050 
> --containerizers=mesos --image_providers=docker 
> --isolation=filesystem/linux,docker/runtime,network/cni 
> --network_cni_config_dir=/opt/cni/net_configs 
> --network_cni_plugins_dir=/opt/cni/plugins
> 
> 3. Launch a command task with mesos-execute, and it will join a CNI network 
> net1.
> sudo src/mesos-execute --master=192.168.122.171:5050 --name=test 
> --docker_image=library/busybox --networks=net1 --command="sleep 10" 
> --shell=true
> I0418 21:34:57.248507 24604 scheduler.cpp:177] Version: 0.29.0
> Subscribed with ID 'c992158d-e625-4359-97fe-6320172fd957-0016'
> Submitted task 'test' to agent 'eeb0be14-77cb-462d-b088-657745453c83-S0'
> Received status update TASK_RUNNING for task 'test'
>   source: SOURCE_EXECUTOR
> Received status update TASK_FINISHED for task 'test'
>   message: 'Command exited with status 0'
>   source: SOURCE_EXECUTOR
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Review Request 46395: Windows: Removed `std::bind` from `process.cpp` to build on Windows.

[Alex Clemmer]

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

Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris 
Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


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


Repository: mesos


Description
---

Windows: Removed `std::bind` from `process.cpp` to build on Windows.


Diffs
-

  3rdparty/libprocess/src/process.cpp d2c458ed93307f75358bb642aaf2ed8e17b2fe97 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 46255: Added a realm parameter to 'process::initialize' (Mesos).

[Greg Mann]

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

(Updated April 19, 2016, 4:35 p.m.)


Review request for mesos, Adam B and Alexander Rojas.


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


Repository: mesos


Description
---

In order to enable authentication on libprocess-level
HTTP endpoints, this patch adds code to the master and
agent's main.cpp file which makes use of the new
`authenticationRealm` argument to `process::initialize`
which allows the authentication realm of such endpoints
to be set when libprocess is initialized. The argument is
added to libprocess initialization in the tests as well.


Diffs
-

  src/master/main.cpp ea7f0fc87c8912309a4679105dde5d8d5bb9ead6 
  src/slave/main.cpp 70df4f384b09a1fb078cd893efe52a5e3b116f48 
  src/tests/main.cpp 142585096493a334ac9ac0df511ae0fc10798040 
  src/tests/mesos.hpp 20370a277d55efeea8daae7ea5e2f6575b5a2d62 

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


Testing
---

`sudo make check` on OSX.


Thanks,

Greg Mann

Review Request 46393: Windows: Added Windows support for `stout/os/shell.hpp`.

[Alex Clemmer]

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

Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, Joris 
Van Remoortere, Michael Park, M Lawindi, and Yi Sun.


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


Repository: mesos


Description
---

Windows: Added Windows support for `stout/os/shell.hpp`.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/shell.hpp 
18f038fa405b45daa5b3b46e4caee0904d7fdd1c 

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


Testing
---


Thanks,

Alex Clemmer

Review Request 46392: Windows:[PLACEHOLDER] Implemented fcntl nonblock and other items.

[Alex Clemmer]

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

Review request for mesos.


Repository: mesos


Description
---

This commit introduces temporary versions of 2 important functions:
`os::nonblock` and `os::cloexec`. We put them here in a placeholder
commit so that reviewers can make progress on subprocess. In the
immediate term, the plan is to figure out on a callsite-by-callsite
basis how to work around the functionality of `os::cloexec`. When we
collect more data, we will be in a better position to offer a way
forward here.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os.hpp 
79e30ca04c6d23f92e3a2f80fbe38ae63fde3520 
  3rdparty/libprocess/3rdparty/stout/include/stout/os/windows/fcntl.hpp 
14734317d7fb40053ee808745ac3ba8c706a7669 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 46254: Added a realm parameter to `process::initialize` (libprocess).

[Greg Mann]

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

(Updated April 19, 2016, 4:35 p.m.)


Review request for mesos, Adam B and Alexander Rojas.


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


Repository: mesos


Description
---

In order to enable authentication on libprocess-level
HTTP endpoints, this patch adds a parameter to
process::initialize which allows the authentication
realm of such endpoints to be set when libprocess is
initialized.


Diffs
-

  3rdparty/libprocess/include/process/gtest.hpp 
3e0887257e1484813b3547170a5a1b9bb89de0d2 
  3rdparty/libprocess/include/process/process.hpp 
77e96957ca556cf9a7e2bf427d6762572fe48c51 
  3rdparty/libprocess/src/process.cpp 5e9dcfdc52f3a8223bc43af149b8e1f5dbdf5b0a 
  3rdparty/libprocess/src/tests/main.cpp 
78858a2b84a439d8f8a60ec8bcb6ac3a308087a6 

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


Testing
---

`sudo make check` on OSX.


Thanks,

Greg Mann

Re: Review Request 46331: Fixed the issue that command executor can not join CNI network.

[Avinash sridharan]

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


Fix it, then Ship it!




Ship It!


src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 1388)


s/, and this/. This
s/by command executor/by the command executor



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (line 1389)


s/with host filesystem first and
it will change to the container filesystem itself when launching task.

/with rootfs of host filesystem and will later pivot to the rootfs of the 
container filesystem, when launching the task.


- Avinash sridharan


On April 19, 2016, 8:21 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46331/
> ---
> 
> (Updated April 19, 2016, 8:21 a.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Jie Yu.
> 
> 
> Bugs: MESOS-5225
> https://issues.apache.org/jira/browse/MESOS-5225
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the issue that command executor can not join CNI network.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 6e3fe118fb63d1ce7d5b01a6ac32f8f3a1369cfb 
> 
> Diff: https://reviews.apache.org/r/46331/diff/
> 
> 
> Testing
> ---
> 
> 1. Start master
> sudo ./bin/mesos-master.sh --work_dir=/tmp
> 
> 2. Start agent
> sudo ./bin/mesos-slave.sh --master=192.168.122.171:5050 
> --containerizers=mesos --image_providers=docker 
> --isolation=filesystem/linux,docker/runtime,network/cni 
> --network_cni_config_dir=/opt/cni/net_configs 
> --network_cni_plugins_dir=/opt/cni/plugins
> 
> 3. Launch a command task with mesos-execute, and it will join a CNI network 
> net1.
> sudo src/mesos-execute --master=192.168.122.171:5050 --name=test 
> --docker_image=library/busybox --networks=net1 --command="sleep 10" 
> --shell=true
> I0418 21:34:57.248507 24604 scheduler.cpp:177] Version: 0.29.0
> Subscribed with ID 'c992158d-e625-4359-97fe-6320172fd957-0016'
> Submitted task 'test' to agent 'eeb0be14-77cb-462d-b088-657745453c83-S0'
> Received status update TASK_RUNNING for task 'test'
>   source: SOURCE_EXECUTOR
> Received status update TASK_FINISHED for task 'test'
>   message: 'Command exited with status 0'
>   source: SOURCE_EXECUTOR
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Review Request 46391: Clarified several agent log messages.

[Neil Conway]

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

Review request for mesos, Anand Mazumdar and Vinod Kone.


Repository: mesos


Description
---

When diagnosing problems, it can be useful to distinguish
between executors that are "terminating" vs. "terminated".


Diffs
-

  src/slave/slave.cpp d82dec2b10d496065013eb4ad6a35dc054b72553 

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 45326: Implemented os::which().

[haosdent huang]

> On April 19, 2016, 3:03 p.m., haosdent huang wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp, line 976
> > 
> >
> > Should `EXPECT_SOME` here?
> 
> Guangya Liu wrote:
> Can you please show more detail for why use `EXPECT_SOME` here?

`ASSERT` would abort the test while `EXPECT` would continue to run following 
code. I think we usually use `ASSERT` when something block the test. e.g. 
`ASSERT_SOME(master);`. And usually use `EXPECT` at those thing we want to test 
in the test cases. e.g. `EXPECT_CALL(...`.


> On April 19, 2016, 3:03 p.m., haosdent huang wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp, line 975
> > 
> >
> > Do we need test the priority here? When a binary exists in different 
> > folders at the same time.
> 
> Guangya Liu wrote:
> I think that as long as the binary can be found should be good enough, 
> why need care the priority here?

Let me describe it in another way. Suppose `PATH` is `/usr/bin:/bin`, and `bar` 
exists in both `/usr/bin/bar` and `/bin/bar`. Are we we always guarantee that 
take the first one in `os::which()`? Or add comment about the priority?


- haosdent


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


On April 19, 2016, 2:56 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45326/
> ---
> 
> (Updated April 19, 2016, 2:56 p.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-4576
> https://issues.apache.org/jira/browse/MESOS-4576
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented os::which().
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> dfe4eab2d812ef807c7416ac205573d55383c4ee 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> 9bd34c7508cd813c5de18028956f6a740997c266 
> 
> Diff: https://reviews.apache.org/r/45326/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> [ RUN  ] OsTest.Which
> [   OK ] OsTest.Which (0 ms)
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46168: Add subdirectory support to URI.filename field.

[Jiang Yan Xu]

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




docs/fetcher.md (line 89)


I feel that "filename" is a bit odd when it can be a path. Many utilities 
simply call this a file, or output file to be more generic and flexible (when 
they do take a path).

e.g.,
```
wget --output-document file
url --output file
tar --file file
gcc -o outfile
clang -o output-file
```

I think `output_file` sounds good. (Notice the snake_casing in proto). What 
do you think?

We need to change docs and code (including CHANGELOG) elsewhere too but 
luckily the API is not released yet.

In CHANGELOG we can still group things under [MESOS-4735], just 
s/'filename'/'output_file'/.



src/launcher/fetcher.cpp (line 252)


`basename` usually specifically refers to the `the component following the 
final '/'`.

So here s/basename/outputFile/



src/launcher/fetcher.cpp (lines 253 - 268)


To avoid setting a Try with a "" when Try explicity doesn't have a default 
contructor, here we can keep the conditional operator but separately mkdir if 
needed.

```
// Prepare output directory if necessary.
if (uri.has_filename()) {
  string dirname = Path(uri.filename()).dirname();
  if (dirname != ".") {
Try result =
  os::mkdir(path::join(sandboxDirectory, uriDirname), true);

if (result.isError()) {
  return Error("Unable to create subdirectory in sandbox");
}
  }
}

Try outputFile = uri.has_filename()
  ? uri.filename()
  : Fetcher::basename(uri.value());
```



src/launcher/fetcher.cpp (line 255)


s/uriDirname/dirname/



src/launcher/fetcher.cpp (line 258)


2 space indentation here.



src/launcher/fetcher.cpp (line 261)


Here let's add the `dirname` to the error mesasge.



src/launcher/fetcher.cpp (line 308)


Same as above.

It sucks that a lot of code is duplicated here but the newly added code is 
only a part of it. Let's add a TODO to "refactor common logic in fetchFromCache 
and fetchBypassingCache into a helper".



src/slave/containerizer/fetcher.cpp (lines 150 - 153)


What does this check against?

Path(filename).basename() never returns an Error().



src/slave/containerizer/fetcher.cpp (lines 155 - 158)


`filename` is not guaranteed to be at least one character right?

Plus this doesn't prevent users from specifying something like 
`subdir/../../../../`. 

For the latter issue ideally there should be a method `Path::canonical` 
that resolves it but it's OK if we punt on it for now. Leave a TODO here for it 
here.

Also s/validateFilename/validateOutputFile/



src/tests/fetcher_tests.cpp (lines 105 - 106)


Nothing wrong with this but the source doesn't have to reside in a subdir. 
:)

We can remove this so there's no confusion.


- Jiang Yan Xu


On April 13, 2016, 5:06 p.m., Michael Browning wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46168/
> ---
> 
> (Updated April 13, 2016, 5:06 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Jiang Yan Xu, and Zhitao Li.
> 
> 
> Bugs: MESOS-5119
> https://issues.apache.org/jira/browse/MESOS-5119
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add subdirectory support to URI.filename field.
> 
> URI.filename allows the user to specify the name of the file that will
> be saved in the sandbox when the URI is fetched, but previously it would
> fail at fetch time if "filename" had a directory component. This change
> allows users to specify a relative path for custom filenames within the
> sandbox.
> 
> 
> Diffs
> -
> 
>   docs/fetcher.md fd6d8a78bd35c5644dceff7005dd7dfd9f5f2171 
>   src/launcher/fetcher.cpp 47583eeaed53b3e7ed4db26fee7cdd2fae5e0c9d 
>   src/slave/containerizer/fetcher.cpp 
> d5910ad570371ba54580be5bb94344a1de38d1f9 
>   src/tests/fetcher_cache_tests.cpp 9ffcd2375f1203bd3d7c5d0cc898e955d5cb124e 
>   src/tests/fetcher_tests.cpp 23a8dc5f4402c5613744753284aabbe3d09bf797 
> 
> Diff: https://reviews.apache.org/r/46168/diff/
> 
> 
> Testing
> ---

Re: Review Request 45326: Implemented os::which().

[Guangya Liu]

> On 四月 19, 2016, 3:03 p.m., haosdent huang wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp, line 976
> > 
> >
> > Should `EXPECT_SOME` here?

Can you please show more detail for why use `EXPECT_SOME` here?


> On 四月 19, 2016, 3:03 p.m., haosdent huang wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp, line 975
> > 
> >
> > Do we need test the priority here? When a binary exists in different 
> > folders at the same time.

I think that as long as the binary can be found should be good enough, why need 
care the priority here?


- Guangya


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


On 四月 19, 2016, 2:56 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45326/
> ---
> 
> (Updated 四月 19, 2016, 2:56 p.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-4576
> https://issues.apache.org/jira/browse/MESOS-4576
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented os::which().
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> dfe4eab2d812ef807c7416ac205573d55383c4ee 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> 9bd34c7508cd813c5de18028956f6a740997c266 
> 
> Diff: https://reviews.apache.org/r/45326/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> [ RUN  ] OsTest.Which
> [   OK ] OsTest.Which (0 ms)
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 34646: Redirect to the leader master when current master is not a leader.

[Neil Conway]

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



Can we update the docs to describe this behavior? e.g., add a note to 
https://mesos.apache.org/documentation/latest/endpoints/ describing the 
redirect behavior -- I suppose it is worth adding a note to every master 
endpoint's doc page describing the redirect behavior, as well as a note to the 
"Master Endpoints" section of the top-level endpoints doc. Might also be worth 
adding a note to the "Operational Guide" or "High Availability" pages. We also 
need to update the docs for the "master/redirect/" endpoint.

This change requires one or more test cases. For example: (a) normal redirect 
case (b) return ServiceUnavailable when there is no leading master (c) avoiding 
redirect loop for `/redirect` endpoint.


src/master/http.cpp (line 345)


"a leader" => "the leader", here and below.



src/master/http.cpp (line 1017)


`"Redirecting request for " << request.url << " to the leading master " << 
hostname.get()`


- Neil Conway


On April 19, 2016, 1:55 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34646/
> ---
> 
> (Updated April 19, 2016, 1:55 a.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, Cody Maloney, Ian Downes, 
> Neil Conway, and Vinod Kone.
> 
> 
> Bugs: MESOS-1865
> https://issues.apache.org/jira/browse/MESOS-1865
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In this changes, we redirect to the leader master in those http
> endpoints which depend on master elected status if current master is
> not a leader.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp a9cb99a92ff5a783e719df5e5cfb6e8301241df9 
> 
> Diff: https://reviews.apache.org/r/34646/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> when current master is not a leader, it would redirect to the leader master.
> 
> ```
> $ curl -i http://master1:5050/master/tasks.json
> HTTP/1.1 307 Temporary Redirect
> Date: Mon, 01 Jun 2015 06:30:08 GMT
> Location: http://master2:5050//master/tasks.json
> Content-Length: 0
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 45673: PoC: Docker Volume Isolator.

[Guangya Liu]

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

(Updated 四月 19, 2016, 3:28 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Repository: mesos


Description (updated)
---

PoC: Docker Volume Isolator.


Diffs (updated)
-

  src/CMakeLists.txt 06f58c4a88e3c527df727df1efe11ed3ab77efa8 
  src/Makefile.am ec855263d620e4723c8ba9cd056c40a3a2e9ca99 
  src/cli/execute.cpp f70d9e1c4badb7d4342e90ce4d4f8114f27a7dff 
  src/slave/containerizer/mesos/containerizer.cpp 
1e1a36903f4377497bb72b69e4ead63675d453c0 
  src/slave/containerizer/mesos/isolators/docker/volume/driver.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/docker/volume/driver.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
786f917c0ec04b6839bbd524fa7c8de3729f9bdb 
  src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
0ad473dc3bd45e122fba55a670e1a893e61c977a 
  src/slave/containerizer/mesos/isolators/docker/volume/paths.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/docker/volume/paths.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/docker/volume/state.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/docker/volume/state.proto 
PRE-CREATION 
  src/slave/slave.cpp de99e9eb5cc812b2e07deb749b98b4f4db363728 

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


Testing
---

Some issues want to discuss:
1) One container can have multiple volumes, and the prepare() will generate all 
of the mount point for all of the volumes in one container. Each volume driver 
and volume name need to map to one mount point, so in driver.cpp::mount(), if 
only adding volume_driver, volume_name, options as paramter, then once the 
mount point was generated, in prepare(), we will not able to  know the 
relationship of volume_driver, volume_name pair and mount point. So it is 
better transfer a structure including volume_driver, volume_name and mount 
point to driver.cpp::mount() so that we can keep the relationship of  
volume_driver, volume_name and mount point
2) Reference counter when cleanup. My thinking is that we can use volume driver 
and volume name generate an volume ID, if one volume driver and volume name 
pair was used by more than one container, then the reference couter of the 
volume ID will be greater than 1 and we should not unmounts for such case, the 
unmount was only called when the volume ID reference count was 1.
3) I was using “multihasmap” for infos in volume isolator, the reason is 
because one container can have multiple volumes. If do not want to use 
“multihashmap”, then I may need to update Info structure as:

Info (const hashmap 
mountPoint>>& _dockerVolumeMountInfo
  const Option& _containerPath = None(),
  const Option>& _driverOptions = None())
: dockerVolumeMountInfo (_dockerVolumeMountInfo),
  containerPath(_containerPath),
  driverOptions(_driverOptions){}

  // The driver.cpp::mount() will fill in the value of mountPoint for each 
DockerVolumeMount (driver and name)
  hashmap mountPoint>> 
dockerVolumeMountInfo;

  Option containerPath;

  Option> driverOptions;
};


Thanks,

Guangya Liu

Re: Review Request 46364: Change to explicit case statements (`UNKNOWN`) instead of default.

[Yong Tang]

> On April 19, 2016, 5:58 a.m., Vinod Kone wrote:
> > can you also update cli/execute.cpp, examples/test_http_framework.cpp, 
> > examples/long_lived_framework.cpp, master/validation.cpp, master/http.cpp, 
> > master/master.cpp, slave/http.cpp, slave/validation.cpp and slave/http.cpp?
> > 
> > make sure to grep through the code base to catch all other cases where 
> > switch over an enum uses a "default".
> 
> Yong Tang wrote:
> Thanks Vinod for the review. I just updated the listed files.

Oh I noticed there are 54 other instances of "default:" (e.g., 
src/slave/slave.cpp). I will rework on the review request.


- Yong


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


On April 19, 2016, 3:19 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46364/
> ---
> 
> (Updated April 19, 2016, 3:19 p.m.)
> 
> 
> Review request for mesos, Adam B, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-5031
> https://issues.apache.org/jira/browse/MESOS-5031
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch changes from `default` statement to explicit case
> statement (`UNKNOWN`) so that complier could help catching all
> switches when new enums are introduced in the future.
> 
> This patch is related to:
> https://reviews.apache.org/r/45317/ (MESOS-5014)
> https://reviews.apache.org/r/45304/ (MESOS-5015)
> https://reviews.apache.org/r/45342/ (MESOS-5031)
> 
> 
> Diffs
> -
> 
>   src/authorizer/local/authorizer.cpp 
> c744a16041c2466d3839a37fbee2bf86887bf4e1 
>   src/cli/execute.cpp 087a73143d739f41638b8335c7e24dfcd14bc7fb 
>   src/examples/long_lived_framework.cpp 
> c696ccb6b2ace6e047f6509b291dd14be240cf70 
>   src/examples/test_http_executor.cpp 
> ceb489d43f35d24c8a7f5fbb0148529745ee357a 
>   src/examples/test_http_framework.cpp 
> 8cc3107034d46cb6a2966835f509508223c6e674 
>   src/master/http.cpp a9cb99a92ff5a783e719df5e5cfb6e8301241df9 
>   src/master/master.cpp 6dacf5fbd73771e5c31ffb0e8723cd2905dcefb3 
>   src/master/validation.cpp 0cd118ee4f89f749a063f6ba7f419d5a220dc1d4 
>   src/slave/http.cpp 3908e33ed5b233387790276f6f5d884452087d2c 
>   src/slave/validation.cpp ec1a4b6d9c55ab0c9894d5a49e290e15dee32c22 
>   src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b 
> 
> Diff: https://reviews.apache.org/r/46364/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 46364: Change to explicit case statements (`UNKNOWN`) instead of default.

[Yong Tang]

> On April 19, 2016, 5:58 a.m., Vinod Kone wrote:
> > can you also update cli/execute.cpp, examples/test_http_framework.cpp, 
> > examples/long_lived_framework.cpp, master/validation.cpp, master/http.cpp, 
> > master/master.cpp, slave/http.cpp, slave/validation.cpp and slave/http.cpp?
> > 
> > make sure to grep through the code base to catch all other cases where 
> > switch over an enum uses a "default".

Thanks Vinod for the review. I just updated the listed files.


- Yong


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


On April 19, 2016, 3:19 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46364/
> ---
> 
> (Updated April 19, 2016, 3:19 p.m.)
> 
> 
> Review request for mesos, Adam B, Ben Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-5031
> https://issues.apache.org/jira/browse/MESOS-5031
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch changes from `default` statement to explicit case
> statement (`UNKNOWN`) so that complier could help catching all
> switches when new enums are introduced in the future.
> 
> This patch is related to:
> https://reviews.apache.org/r/45317/ (MESOS-5014)
> https://reviews.apache.org/r/45304/ (MESOS-5015)
> https://reviews.apache.org/r/45342/ (MESOS-5031)
> 
> 
> Diffs
> -
> 
>   src/authorizer/local/authorizer.cpp 
> c744a16041c2466d3839a37fbee2bf86887bf4e1 
>   src/cli/execute.cpp 087a73143d739f41638b8335c7e24dfcd14bc7fb 
>   src/examples/long_lived_framework.cpp 
> c696ccb6b2ace6e047f6509b291dd14be240cf70 
>   src/examples/test_http_executor.cpp 
> ceb489d43f35d24c8a7f5fbb0148529745ee357a 
>   src/examples/test_http_framework.cpp 
> 8cc3107034d46cb6a2966835f509508223c6e674 
>   src/master/http.cpp a9cb99a92ff5a783e719df5e5cfb6e8301241df9 
>   src/master/master.cpp 6dacf5fbd73771e5c31ffb0e8723cd2905dcefb3 
>   src/master/validation.cpp 0cd118ee4f89f749a063f6ba7f419d5a220dc1d4 
>   src/slave/http.cpp 3908e33ed5b233387790276f6f5d884452087d2c 
>   src/slave/validation.cpp ec1a4b6d9c55ab0c9894d5a49e290e15dee32c22 
>   src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b 
> 
> Diff: https://reviews.apache.org/r/46364/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 46364: Change to explicit case statements (`UNKNOWN`) instead of default.

[Yong Tang]

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

(Updated April 19, 2016, 3:19 p.m.)


Review request for mesos, Adam B, Ben Mahler, and Vinod Kone.


Changes
---

Replace UNREACHABLE with LOG(WARNING) based on review from Vinod.


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


Repository: mesos


Description
---

This patch changes from `default` statement to explicit case
statement (`UNKNOWN`) so that complier could help catching all
switches when new enums are introduced in the future.

This patch is related to:
https://reviews.apache.org/r/45317/ (MESOS-5014)
https://reviews.apache.org/r/45304/ (MESOS-5015)
https://reviews.apache.org/r/45342/ (MESOS-5031)


Diffs (updated)
-

  src/authorizer/local/authorizer.cpp c744a16041c2466d3839a37fbee2bf86887bf4e1 
  src/cli/execute.cpp 087a73143d739f41638b8335c7e24dfcd14bc7fb 
  src/examples/long_lived_framework.cpp 
c696ccb6b2ace6e047f6509b291dd14be240cf70 
  src/examples/test_http_executor.cpp ceb489d43f35d24c8a7f5fbb0148529745ee357a 
  src/examples/test_http_framework.cpp 8cc3107034d46cb6a2966835f509508223c6e674 
  src/master/http.cpp a9cb99a92ff5a783e719df5e5cfb6e8301241df9 
  src/master/master.cpp 6dacf5fbd73771e5c31ffb0e8723cd2905dcefb3 
  src/master/validation.cpp 0cd118ee4f89f749a063f6ba7f419d5a220dc1d4 
  src/slave/http.cpp 3908e33ed5b233387790276f6f5d884452087d2c 
  src/slave/validation.cpp ec1a4b6d9c55ab0c9894d5a49e290e15dee32c22 
  src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b 

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


Testing
---

make check (Ubuntu 14.04)


Thanks,

Yong Tang

Re: Review Request 46142: Added Criteo to Powered by Mesos page.

[Timothy Chen]

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


Ship it!




Ship It!

- Timothy Chen


On April 19, 2016, 12:13 p.m., Pierre Cheynier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46142/
> ---
> 
> (Updated April 19, 2016, 12:13 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added Criteo to Powered by Mesos page.
> 
> 
> Diffs
> -
> 
>   docs/powered-by-mesos.md f0c3bba107a5c539c91070296a8bbfb58ef43927 
> 
> Diff: https://reviews.apache.org/r/46142/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Pierre Cheynier
> 
>

Re: Review Request 46322: Added KillPolicy to scheduler and executor Kill protobuf messages.

[Alexander Rukletsov]

> On April 19, 2016, 12:33 a.m., Ben Mahler wrote:
> > CHANGELOG, lines 61-63
> > 
> >
> > I was initially surprised to see MESOS-4908 repeated here, but I 
> > suppose the intent was to list all non-deprecation API changes here? If so, 
> > we're not doing that already (e.g. MESOS-4909 and MESOS-4949 in 0.29.0 for 
> > example).
> > 
> > It seems the current approach is that 'new features' may include some 
> > API changes, but these aren't repeated in 'Additional API' changes. While 
> > it would be great to have a clearer approach, can you follow the existing 
> > approach?

Yes, that was the intent, but you right, it is inconsistent to what we do. Will 
drop these lines in favor of longer description above.


> On April 19, 2016, 12:33 a.m., Ben Mahler wrote:
> > CHANGELOG, line 62
> > 
> >
> > no need for the "'s" here:
> > 
> > s/scheduler's/scheduler/
> > s/executor's/executor/

Thanks! I'll drop this since I'll remove these lines altogether as per comment 
above.


> On April 19, 2016, 12:33 a.m., Ben Mahler wrote:
> > include/mesos/executor/executor.proto, lines 90-91
> > 
> >
> > All of the .proto comments introduced here would benefit from also 
> > mentioning that the kill policy overrides any previously specified 
> > kill.kill_policy. Currently it only seems clear that the kill.kill_policy 
> > overrides task_info.kill_policy. 
> > 
> > Perhaps for now, we just explicitly state that the grace period may be 
> > "overridden" (or "adjusted"?) in order to give more or less time to a 
> > graceful kill that is in progress.

Good point, will adjust the comment.


- Alexander


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


On April 18, 2016, 12:43 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46322/
> ---
> 
> (Updated April 18, 2016, 12:43 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-4908
> https://issues.apache.org/jira/browse/MESOS-4908
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A framework may want to override the `KillPolicy` set in `TaskInfo`
> when killing a task, for example to forcefully kill a task which is
> already being killed.
> 
> 
> Diffs
> -
> 
>   CHANGELOG d2e902f8295644c527964123e409be460c2a5789 
>   include/mesos/executor/executor.proto 
> 338b3638f986244122c2d39c9aca7905c12008ce 
>   include/mesos/scheduler/scheduler.proto 
> 078c6550f24a3d8ac675251168434130fc3eeef3 
>   include/mesos/v1/executor/executor.proto 
> 4552fb5d3f9d53affd8fad0abf122fce548973b7 
>   include/mesos/v1/scheduler/scheduler.proto 
> 8ed9e19a9e5aa19a518b708b0e0d9cfdc038cd11 
>   src/messages/messages.proto e0f1fca92d3ea8c29c095da31653c317873a934c 
> 
> Diff: https://reviews.apache.org/r/46322/diff/
> 
> 
> Testing
> ---
> 
> The whole chain is tested in https://reviews.apache.org/r/46325/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 45270: Added spec protobuf for DockerVolumeMount.

[Guangya Liu]

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

(Updated 四月 19, 2016, 3:04 p.m.)


Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
Jie Yu.


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


Repository: mesos


Description (updated)
---

Added spec protobuf for DockerVolumeMount.
The DockerVolumeMount was used to check point the mount
point for each container to recover.


Diffs (updated)
-

  src/CMakeLists.txt aabb33dfd4b985fe5774f87bb63b4ec1ec853962 
  src/Makefile.am 5a2b4efa781752863d6751f98614fb78bece73ac 
  src/slave/containerizer/mesos/isolators/docker/volume/state.proto 
PRE-CREATION 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 45326: Implemented os::which().

[haosdent huang]

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


Fix it, then Ship it!




Ship It!


3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp (line 467)


Should we return error here? I mean use Result.



3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp (line 975)


Do we need test the priority here? When a binary exists in different 
folders at the same time.



3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp (line 976)


Should `EXPECT_SOME` here?


- haosdent huang


On April 19, 2016, 2:56 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45326/
> ---
> 
> (Updated April 19, 2016, 2:56 p.m.)
> 
> 
> Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
> Jie Yu.
> 
> 
> Bugs: MESOS-4576
> https://issues.apache.org/jira/browse/MESOS-4576
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented os::which().
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
> dfe4eab2d812ef807c7416ac205573d55383c4ee 
>   3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
> 9bd34c7508cd813c5de18028956f6a740997c266 
> 
> Diff: https://reviews.apache.org/r/45326/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> [ RUN  ] OsTest.Which
> [   OK ] OsTest.Which (0 ms)
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 45996: Fixed memory leak of `gc` in `finalize()` in libprocess.

[Neil Conway]

> On April 14, 2016, 7:10 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/src/process.cpp, lines 489-490
> > 
> >
> > This is now effectively managed by the `ProcessManager`, so you could 
> > bring it into the `ProcessManager` 's scope (which would still be global).
> 
> Neil Conway wrote:
> This makes sense (and I would love to eliminate all the global state in 
> libprocess), but AFAICS it won't be easy to implement: if we make `gc` a 
> field of `ProcessManager`, we'd ideally like to initialize and spawn the GC 
> process in `ProcessManager`'s constructor. But that isn't possible, because 
> we can't safely call `spawn` at the time we are constructing the 
> ProcessManager. We could still move `gc` to be a field of `ProcessManager` 
> but spawn it separately, but I'm not sure if that is actually a net 
> improvement at that point.
> 
> Joseph Wu wrote:
> It should be safe to call `ProcessManager::spawn` rather than 
> `process::spawn` for `gc`.  (Unless there's some dependency on the socket 
> `__s__` for new processes...)

Sure, but it still seems a bit fragile to me. I'd prefer to take on the removal 
of all the libprocess global state as a separate project.


- Neil


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


On April 11, 2016, 1:41 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45996/
> ---
> 
> (Updated April 11, 2016, 1:41 a.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Bugs: MESOS-5144
> https://issues.apache.org/jira/browse/MESOS-5144
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed memory leak of `gc` in `finalize()` in libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/gc.hpp 
> 799468ebe49f2a49d325f40ffd8acea727abf74c 
>   3rdparty/libprocess/src/process.cpp 
> 5e9dcfdc52f3a8223bc43af149b8e1f5dbdf5b0a 
> 
> Diff: https://reviews.apache.org/r/45996/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 45326: Implemented os::which().

[Guangya Liu]

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

(Updated 四月 19, 2016, 2:56 p.m.)


Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
Jie Yu.


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


Repository: mesos


Description
---

Implemented os::which().


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/stout/include/stout/posix/os.hpp 
dfe4eab2d812ef807c7416ac205573d55383c4ee 
  3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
9bd34c7508cd813c5de18028956f6a740997c266 

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


Testing
---

make
make check

[ RUN  ] OsTest.Which
[   OK ] OsTest.Which (0 ms)


Thanks,

Guangya Liu

Re: Review Request 45373: Ignored the DOCKER_VOLUME volume source.

[Guangya Liu]

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

(Updated 四月 19, 2016, 2:52 p.m.)


Review request for mesos, David vonThenen, Gilbert Song, haosdent huang, and 
Jie Yu.


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


Repository: mesos


Description
---

Ignored the DOCKER_VOLUME volume source.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
9fc7c48f99155750fd3c18c7c102507e2726362b 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 46307: Ignored subsequent status update in HealthStatusChange tests.

[haosdent huang]

> On April 19, 2016, 2:35 p.m., Neil Conway wrote:
> > This patch does not solve the flakiness for me: failed once after 2 
> > iterations, then again after 77 iterations. Verbose test log here: 
> > https://gist.github.com/neilconway/e6134b4717ee022e7fc32a1f95619fa9
> 
> haosdent huang wrote:
> Thank you very much for your test! I saw you use `vagrant@archlinux`, may 
> you share your vagrantfile to me? So that I could try to reproduce in my 
> local.

```
I0420 00:33:13.497138 15400 http.cpp:313] HTTP GET for /master/state from 
10.0.2.15:44478
Received task health update, healthy: true
I0420 00:33:13.502598 15400 slave.cpp:3201] Handling status update TASK_RUNNING 
(UUID: e19c76cc-096a-4398-b616-afb628b8e5b8) for task 1 in health state healthy 
of framework 7cf5923c-3d03-4ed6-826a-efa97f54e765- from 
executor(1)@10.0.2.15:37107
I0420 00:33:13.504456 15400 status_update_manager.cpp:320] Received status 
update TASK_RUNNING (UUID: e19c76cc-096a-4398-b616-afb628b8e5b8) for task 1 in 
health state healthy of framework 7cf5923c-3d03-4ed6-826a-efa97f54e765-
I0420 00:33:13.505009 15400 slave.cpp:3599] Forwarding the update TASK_RUNNING 
(UUID: e19c76cc-096a-4398-b616-afb628b8e5b8) for task 1 in health state healthy 
of framework 7cf5923c-3d03-4ed6-826a-efa97f54e765- to master@10.0.2.15:41408
I0420 00:33:13.505167 15400 slave.cpp:3509] Sending acknowledgement for status 
update TASK_RUNNING (UUID: e19c76cc-096a-4398-b616-afb628b8e5b8) for task 1 in 
health state healthy of framework 7cf5923c-3d03-4ed6-826a-efa97f54e765- to 
executor(1)@10.0.2.15:37107
I0420 00:33:13.505524 15400 master.cpp:5069] Status update TASK_RUNNING (UUID: 
e19c76cc-096a-4398-b616-afb628b8e5b8) for task 1 in health state healthy of 
framework 7cf5923c-3d03-4ed6-826a-efa97f54e765- from agent 
7cf5923c-3d03-4ed6-826a-efa97f54e765-S0 at slave(76)@10.0.2.15:41408 
(archlinux.vagrant.vm)
I0420 00:33:13.505602 15400 master.cpp:5117] Forwarding status update 
TASK_RUNNING (UUID: e19c76cc-096a-4398-b616-afb628b8e5b8) for task 1 in health 
state healthy of framework 7cf5923c-3d03-4ed6-826a-efa97f54e765-
I0420 00:33:13.505738 15400 master.cpp:6725] Updating the state of task 1 of 
framework 7cf5923c-3d03-4ed6-826a-efa97f54e765- (latest state: 
TASK_RUNNING, status update state: TASK_RUNNING)
I0420 00:33:13.505985 15400 master.cpp:4224] Processing ACKNOWLEDGE call 
e19c76cc-096a-4398-b616-afb628b8e5b8 for task 1 of framework 
7cf5923c-3d03-4ed6-826a-efa97f54e765- (default) at 
scheduler-5bd5e446-a017-45d9-8193-be7d23002487@10.0.2.15:41408 on agent 
7cf5923c-3d03-4ed6-826a-efa97f54e765-S0
I0420 00:33:13.506142 15400 status_update_manager.cpp:392] Received status 
update acknowledgement (UUID: e19c76cc-096a-4398-b616-afb628b8e5b8) for task 1 
of framework 7cf5923c-3d03-4ed6-826a-efa97f54e765-
rm: cannot remove '/tmp/1NKfr1': No such file or directory
I0420 00:33:13.508203 15400 http.cpp:178] HTTP GET for /slave(76)/state from 
10.0.2.15:44482
../../mesos/src/tests/health_check_tests.cpp:647: Failure
Value of: (find).get()
  Actual: 16-byte object <05-00 00-00 00-00 00-00 90-C4 2D-03 00-00 00-00>
Expected: false
Which is: false
*** Aborted at 1461076393 (unix time) try "date -d @1461076393" if you are 
using GNU date ***
PC: @  0x1899ba0 testing::UnitTest::AddTestPartResult()
*** SIGSEGV (@0x0) received by PID 15381 (TID 0x7f0aa958a7c0) from PID 0; stack 
trace: ***

```
It looks like get `true` here. Let me try how to fix this.


- haosdent


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


On April 17, 2016, 5:15 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46307/
> ---
> 
> (Updated April 17, 2016, 5:15 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, Greg Mann, Neil 
> Conway, and Timothy Chen.
> 
> 
> Bugs: MESOS-1802
> https://issues.apache.org/jira/browse/MESOS-1802
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We need to ignore subsequent status updates in HealthStatusChange
> tests. In our test cases, we set `consecutive_failures` to 3 in
> HealthCheck message definition. But the counter for
> `consecutiveFailures` in `mesos-health-check` would be reset to 0
> after a success check. It is possible to continue to receive status
> updates before we stop the driver.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp 1c4a554ab07731963a4a38e3ae40b0323bf317bb 
> 
> Diff: https://reviews.apache.org/r/46307/diff/
> 
> 
> Testing
> ---
> 
> # I still could not reproduce the problem in old code after repeatedly tests. 
> So seems no way to verify whether my assumption is corr

Re: Review Request 46314: Added upstream patch fixing signed/unsigned comparison for protobuf.

[Benjamin Bannier]

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

(Updated April 19, 2016, 4:46 p.m.)


Review request for mesos, Zhiwei Chen and Vinod Kone.


Changes
---

Made the commit message wrappederer.


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


Repository: mesos


Description (updated)
---

This adds the upstream patch `717f807` which is contained in
`protobuf-3.0.0-alpha-1`.


Diffs
-

  3rdparty/libprocess/3rdparty/CMakeLists.txt 
7b3621d24a05a3b3b860e3b8c13a1531208e5e26 
  3rdparty/libprocess/3rdparty/protobuf-2.6.1.patch PRE-CREATION 

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


Testing
---

Building with GCC6 w/o this patch leads to a hard failure to a comparison 
between a signed and unsigned types; with this patch the build succeeds.


Thanks,

Benjamin Bannier

Re: Review Request 46245: Renamed Docker and CNI BASE_NAME to meaningful name.

[Guangya Liu]

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

(Updated 四月 19, 2016, 2:48 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Repository: mesos


Description
---

Renamed Docker and CNI BASE_NAME to meaningful name.


Diffs (updated)
-

  src/CMakeLists.txt aabb33dfd4b985fe5774f87bb63b4ec1ec853962 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 46307: Ignored subsequent status update in HealthStatusChange tests.

[haosdent huang]

> On April 19, 2016, 2:35 p.m., Neil Conway wrote:
> > This patch does not solve the flakiness for me: failed once after 2 
> > iterations, then again after 77 iterations. Verbose test log here: 
> > https://gist.github.com/neilconway/e6134b4717ee022e7fc32a1f95619fa9

Thank you very much for your test! I saw you use `vagrant@archlinux`, may you 
share your vagrantfile to me? So that I could try to reproduce in my local.


- haosdent


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


On April 17, 2016, 5:15 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46307/
> ---
> 
> (Updated April 17, 2016, 5:15 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, Greg Mann, Neil 
> Conway, and Timothy Chen.
> 
> 
> Bugs: MESOS-1802
> https://issues.apache.org/jira/browse/MESOS-1802
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We need to ignore subsequent status updates in HealthStatusChange
> tests. In our test cases, we set `consecutive_failures` to 3 in
> HealthCheck message definition. But the counter for
> `consecutiveFailures` in `mesos-health-check` would be reset to 0
> after a success check. It is possible to continue to receive status
> updates before we stop the driver.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp 1c4a554ab07731963a4a38e3ae40b0323bf317bb 
> 
> Diff: https://reviews.apache.org/r/46307/diff/
> 
> 
> Testing
> ---
> 
> # I still could not reproduce the problem in old code after repeatedly tests. 
> So seems no way to verify whether my assumption is correct or not.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 46314: Added upstream patch fixing signed/unsigned comparison for protobuf.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [46314]

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

Error:
2016-04-19 14:42:22 URL:https://reviews.apache.org/r/46314/diff/raw/ 
[1890/1890] -> "46314.patch" [1]
No files to lint

Error: No line in the commit message summary may exceed 72 characters.

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

- Mesos ReviewBot


On April 19, 2016, 9:32 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46314/
> ---
> 
> (Updated April 19, 2016, 9:32 a.m.)
> 
> 
> Review request for mesos, Zhiwei Chen and Vinod Kone.
> 
> 
> Bugs: MESOS-4678
> https://issues.apache.org/jira/browse/MESOS-4678
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds the upstream patch `717f807` which is contained in 
> `protobuf-3.0.0-alpha-1`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> 7b3621d24a05a3b3b860e3b8c13a1531208e5e26 
>   3rdparty/libprocess/3rdparty/protobuf-2.6.1.patch PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46314/diff/
> 
> 
> Testing
> ---
> 
> Building with GCC6 w/o this patch leads to a hard failure to a comparison 
> between a signed and unsigned types; with this patch the build succeeds.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 46307: Ignored subsequent status update in HealthStatusChange tests.

[Neil Conway]

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



This patch does not solve the flakiness for me: failed once after 2 iterations, 
then again after 77 iterations. Verbose test log here: 
https://gist.github.com/neilconway/e6134b4717ee022e7fc32a1f95619fa9

- Neil Conway


On April 17, 2016, 5:15 p.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46307/
> ---
> 
> (Updated April 17, 2016, 5:15 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, Greg Mann, Neil 
> Conway, and Timothy Chen.
> 
> 
> Bugs: MESOS-1802
> https://issues.apache.org/jira/browse/MESOS-1802
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We need to ignore subsequent status updates in HealthStatusChange
> tests. In our test cases, we set `consecutive_failures` to 3 in
> HealthCheck message definition. But the counter for
> `consecutiveFailures` in `mesos-health-check` would be reset to 0
> after a success check. It is possible to continue to receive status
> updates before we stop the driver.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp 1c4a554ab07731963a4a38e3ae40b0323bf317bb 
> 
> Diff: https://reviews.apache.org/r/46307/diff/
> 
> 
> Testing
> ---
> 
> # I still could not reproduce the problem in old code after repeatedly tests. 
> So seems no way to verify whether my assumption is correct or not.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 46388: Fixed typo in test setup error message.

[Neil Conway]

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

Review request for mesos and Jie Yu.


Repository: mesos


Description
---

Fixed typo in test setup error message.


Diffs
-

  src/tests/environment.cpp 45ed8f27873e77ac5d0852a198a9474199bcdfc3 

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


Testing
---


Thanks,

Neil Conway

Re: Review Request 46373: Request /files/read.json with a negative length value causes error.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46373]

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 April 19, 2016, 8:07 a.m., zhou xing wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46373/
> ---
> 
> (Updated April 19, 2016, 8:07 a.m.)
> 
> 
> Review request for mesos, Ben Mahler and Greg Mann.
> 
> 
> Bugs: mesos-5060
> https://issues.apache.org/jira/browse/mesos-5060
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> [MESOS-5060]
> The patch did the following changes:
> 1. Fix the offset and length logic in files.cpp.
> 2. Add some tests to test the /files/read.json endponit with
> negative offset or length.
> 
> 
> Diffs
> -
> 
>   src/files/files.cpp 4e916101b378b0e9032a08a3f6c73e195b2a08a1 
>   src/tests/files_tests.cpp 5d6620d13babaf0bb7f9c888bb1b4fa2228b6ccd 
> 
> Diff: https://reviews.apache.org/r/46373/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> request 'files/read.json' endpoint with negative offset or length argument
> 
> 
> Thanks,
> 
> zhou xing
> 
>

Re: Review Request 46139: Add positive tests for /weights endpoint.

[Yongqiao Wang]

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

(Updated April 19, 2016, 1:42 p.m.)


Review request for mesos and Adam B.


Changes
---

Address the comments of Adam.


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


Repository: mesos


Description
---

Add positive tests for /weights endpoint.


Diffs (updated)
-

  src/tests/dynamic_weights_tests.cpp f89b89dd2553220161e28ec4784eb0bbdfdb65fe 
  src/tests/mesos.hpp e4b63d41d883807ac39846799468b80e88c84e0b 

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


Testing
---

make && make check.

Yongs-MacBook-Pro:build yqwyq$ ./src/mesos-tests 
--gtest_filter=DynamicWeightsTest.*
[==] Running 13 tests from 1 test case.
[--] Global test environment set-up.
[--] 13 tests from DynamicWeightsTest
[ RUN  ] DynamicWeightsTest.PutInvalidRequest
[   OK ] DynamicWeightsTest.PutInvalidRequest (89 ms)
[ RUN  ] DynamicWeightsTest.ZeroWeight
[   OK ] DynamicWeightsTest.ZeroWeight (39 ms)
[ RUN  ] DynamicWeightsTest.NegativeWeight
[   OK ] DynamicWeightsTest.NegativeWeight (46 ms)
[ RUN  ] DynamicWeightsTest.NonNumericWeight
[   OK ] DynamicWeightsTest.NonNumericWeight (39 ms)
[ RUN  ] DynamicWeightsTest.MissingRole
[   OK ] DynamicWeightsTest.MissingRole (37 ms)
[ RUN  ] DynamicWeightsTest.UnknownRole
[   OK ] DynamicWeightsTest.UnknownRole (32 ms)
[ RUN  ] DynamicWeightsTest.UpdateWeightsWithExplictRoles
[   OK ] DynamicWeightsTest.UpdateWeightsWithExplictRoles (44 ms)
[ RUN  ] DynamicWeightsTest.UnauthenticatedUpdateWeightRequest
[   OK ] DynamicWeightsTest.UnauthenticatedUpdateWeightRequest (40 ms)
[ RUN  ] DynamicWeightsTest.UnauthenticatedQueryWeightRequest
[   OK ] DynamicWeightsTest.UnauthenticatedQueryWeightRequest (35 ms)
[ RUN  ] DynamicWeightsTest.AuthorizedWeightUpdateRequest
[   OK ] DynamicWeightsTest.AuthorizedWeightUpdateRequest (42 ms)
[ RUN  ] DynamicWeightsTest.AuthorizedUpdateWeightRequestWithoutPrincipal
[   OK ] DynamicWeightsTest.AuthorizedUpdateWeightRequestWithoutPrincipal 
(41 ms)
[ RUN  ] DynamicWeightsTest.UnauthorizedWeightUpdateRequest
[   OK ] DynamicWeightsTest.UnauthorizedWeightUpdateRequest (40 ms)
[ RUN  ] DynamicWeightsTest.RecoveredWeightsFromRegistry
[   OK ] DynamicWeightsTest.RecoveredWeightsFromRegistry (135 ms)
[--] 13 tests from DynamicWeightsTest (659 ms total)

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


Thanks,

Yongqiao Wang

Re: Review Request 46331: Fixed the issue that command executor can not join CNI network.

[Qian Zhang]

> On April 19, 2016, 8:56 a.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/cni.hpp, lines 119-125
> > 
> >
> > I would suggest that we don't save that in Info. This is because 'Info' 
> > struct will be used for recovery as well, and we don't have a way to know 
> > if an orphan container is using command executor or not.
> > 
> > In fact, this is also true for the above 'rootfs'. We should have just 
> > stored an Option.
> > 
> > Can you instead just store an Option in Info?

I had the same concern. However, I think `rootfs` is only used when we isolate 
container, it will not be used when we do the cleanup. So I think we do not 
need to restore it as part of `Info` during recovery since we only need to 
recover the information used to do the cleanup, right?


- Qian


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


On April 19, 2016, 4:21 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46331/
> ---
> 
> (Updated April 19, 2016, 4:21 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan and Jie Yu.
> 
> 
> Bugs: MESOS-5225
> https://issues.apache.org/jira/browse/MESOS-5225
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the issue that command executor can not join CNI network.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 6e3fe118fb63d1ce7d5b01a6ac32f8f3a1369cfb 
> 
> Diff: https://reviews.apache.org/r/46331/diff/
> 
> 
> Testing
> ---
> 
> 1. Start master
> sudo ./bin/mesos-master.sh --work_dir=/tmp
> 
> 2. Start agent
> sudo ./bin/mesos-slave.sh --master=192.168.122.171:5050 
> --containerizers=mesos --image_providers=docker 
> --isolation=filesystem/linux,docker/runtime,network/cni 
> --network_cni_config_dir=/opt/cni/net_configs 
> --network_cni_plugins_dir=/opt/cni/plugins
> 
> 3. Launch a command task with mesos-execute, and it will join a CNI network 
> net1.
> sudo src/mesos-execute --master=192.168.122.171:5050 --name=test 
> --docker_image=library/busybox --networks=net1 --command="sleep 10" 
> --shell=true
> I0418 21:34:57.248507 24604 scheduler.cpp:177] Version: 0.29.0
> Subscribed with ID 'c992158d-e625-4359-97fe-6320172fd957-0016'
> Submitted task 'test' to agent 'eeb0be14-77cb-462d-b088-657745453c83-S0'
> Received status update TASK_RUNNING for task 'test'
>   source: SOURCE_EXECUTOR
> Received status update TASK_FINISHED for task 'test'
>   message: 'Command exited with status 0'
>   source: SOURCE_EXECUTOR
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 43561: Improve Ranges parsing to handle single values.

[Klaus Ma]

> On March 21, 2016, 10:02 p.m., Joris Van Remoortere wrote:
> > src/common/values.cpp, line 613
> > 
> >
> > @benm I wish we had support for iterating over these splicers eg:
> > `foreachtoken(temp, ",\n", [](const string& token) { ... });`
> > 
> > These helpers become rather heavy (at 3 nested) considering their 
> > utility.
> 
> Klaus Ma wrote:
> @joris, I think `foreachtoken` is also nested loop. When doing the 
> patches, there're three options to me: FSM, regex and nested loop; FSM & 
> regex seems hard to maintain, so I used nested loop.
> 
> Klaus Ma wrote:
> `foreachtoken(temp, ",\n", [](const string& token) { ... });` is a good 
> helper function; but no performance improvement. Any comments?

@joris, I logged MESOS-5234 to trace this; I'll update patches accordingly.


- Klaus


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


On March 22, 2016, 9:22 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43561/
> ---
> 
> (Updated March 22, 2016, 9:22 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, and Joris Van 
> Remoortere.
> 
> 
> Bugs: MESOS-4627
> https://issues.apache.org/jira/browse/MESOS-4627
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improve Ranges parsing to handle single values.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp 9f3c0b9930bd3dd9d84d97119419825862f531c3 
>   src/tests/resources_tests.cpp 6b004d64bb25112b19fc5d98b5bca874c5329e8c 
>   src/tests/values_tests.cpp 929861549e3155c33966896f817f9bf9e6d14354 
> 
> Diff: https://reviews.apache.org/r/43561/diff/
> 
> 
> Testing
> ---
> 
> make
> make check GTEST_FILTER=~"*"
> ./src/mesos-test
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 46139: Add positive tests for /weights endpoint.

[Yongqiao Wang]

> On April 14, 2016, 11:09 a.m., Adam B wrote:
> > src/tests/dynamic_weights_tests.cpp, lines 168-179
> > 
> >
> > Why are role2 and role1 in reverse order? Is this always the case? Why 
> > wouldn't it be forward-alphabetical order?

It cannot ensure the order, because weights are stored in a hashmap. I have 
changed the check logic.


- Yongqiao


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


On April 13, 2016, 6:10 a.m., Yongqiao Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46139/
> ---
> 
> (Updated April 13, 2016, 6:10 a.m.)
> 
> 
> Review request for mesos and Adam B.
> 
> 
> Bugs: MESOS-4316
> https://issues.apache.org/jira/browse/MESOS-4316
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add positive tests for /weights endpoint.
> 
> 
> Diffs
> -
> 
>   src/tests/dynamic_weights_tests.cpp 
> f89b89dd2553220161e28ec4784eb0bbdfdb65fe 
> 
> Diff: https://reviews.apache.org/r/46139/diff/
> 
> 
> Testing
> ---
> 
> make && make check.
> 
> Yongs-MacBook-Pro:build yqwyq$ ./src/mesos-tests 
> --gtest_filter=DynamicWeightsTest.*
> [==] Running 13 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 13 tests from DynamicWeightsTest
> [ RUN  ] DynamicWeightsTest.PutInvalidRequest
> [   OK ] DynamicWeightsTest.PutInvalidRequest (89 ms)
> [ RUN  ] DynamicWeightsTest.ZeroWeight
> [   OK ] DynamicWeightsTest.ZeroWeight (39 ms)
> [ RUN  ] DynamicWeightsTest.NegativeWeight
> [   OK ] DynamicWeightsTest.NegativeWeight (46 ms)
> [ RUN  ] DynamicWeightsTest.NonNumericWeight
> [   OK ] DynamicWeightsTest.NonNumericWeight (39 ms)
> [ RUN  ] DynamicWeightsTest.MissingRole
> [   OK ] DynamicWeightsTest.MissingRole (37 ms)
> [ RUN  ] DynamicWeightsTest.UnknownRole
> [   OK ] DynamicWeightsTest.UnknownRole (32 ms)
> [ RUN  ] DynamicWeightsTest.UpdateWeightsWithExplictRoles
> [   OK ] DynamicWeightsTest.UpdateWeightsWithExplictRoles (44 ms)
> [ RUN  ] DynamicWeightsTest.UnauthenticatedUpdateWeightRequest
> [   OK ] DynamicWeightsTest.UnauthenticatedUpdateWeightRequest (40 ms)
> [ RUN  ] DynamicWeightsTest.UnauthenticatedQueryWeightRequest
> [   OK ] DynamicWeightsTest.UnauthenticatedQueryWeightRequest (35 ms)
> [ RUN  ] DynamicWeightsTest.AuthorizedWeightUpdateRequest
> [   OK ] DynamicWeightsTest.AuthorizedWeightUpdateRequest (42 ms)
> [ RUN  ] DynamicWeightsTest.AuthorizedUpdateWeightRequestWithoutPrincipal
> [   OK ] DynamicWeightsTest.AuthorizedUpdateWeightRequestWithoutPrincipal 
> (41 ms)
> [ RUN  ] DynamicWeightsTest.UnauthorizedWeightUpdateRequest
> [   OK ] DynamicWeightsTest.UnauthorizedWeightUpdateRequest (40 ms)
> [ RUN  ] DynamicWeightsTest.RecoveredWeightsFromRegistry
> [   OK ] DynamicWeightsTest.RecoveredWeightsFromRegistry (135 ms)
> [--] 13 tests from DynamicWeightsTest (659 ms total)
> 
> [--] Global test environment tear-down
> [==] 13 tests from 1 test case ran. (669 ms total)
> [  PASSED  ] 13 tests.
> 
> 
> Thanks,
> 
> Yongqiao Wang
> 
>

Re: Review Request 46375: Updated MACHINE_UP_HELP's comments.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46375]

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 April 19, 2016, 7:51 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46375/
> ---
> 
> (Updated April 19, 2016, 7:51 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated MACHINE_UP_HELP's comments.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp a9cb99a92ff5a783e719df5e5cfb6e8301241df9 
> 
> Diff: https://reviews.apache.org/r/46375/diff/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>