Re: Review Request 44758: Upgrade to clang-format-3.8 (MESOS-4906).

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44758]

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 March 16, 2016, 1:37 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44758/
> ---
> 
> (Updated March 16, 2016, 1:37 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-4906
> https://issues.apache.org/jira/browse/MESOS-4906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade to clang-format-3.8 (MESOS-4906).
> 
> 
> Diffs
> -
> 
>   docs/clang-format.md 7f1c1dfd70e1fe9bfa186df1bdda7bdcf867db04 
>   support/clang-format 499d0e749e14e50256ae649afa0ced2b04589a0e 
> 
> Diff: https://reviews.apache.org/r/44758/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 42036: Windows: Added `slave/flags.cpp` to Windows build.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On March 11, 2016, 9:51 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42036/
> ---
> 
> (Updated March 11, 2016, 9:51 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, and 
> Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added `slave/flags.cpp` to Windows build.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt e9f7c3a447aad79ab19d53879888a413a587408f 
> 
> Diff: https://reviews.apache.org/r/42036/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 44883: Fix the mis-leading URI fetcher error message (MESOS-4954).

[haosdent huang]

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


Ship it!




Ship It!

- haosdent huang


On March 16, 2016, 3:31 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44883/
> ---
> 
> (Updated March 16, 2016, 3:31 a.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Bugs: MESOS-4954
> https://issues.apache.org/jira/browse/MESOS-4954
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fix changes the URL fetcher error message from LOG(ERROR)
> to LOG(INFO) as the plugin is actually skipped if it is not
> created.
> 
> 
> Diffs
> -
> 
>   src/uri/fetcher.cpp 8645b66f6c64c76b6c02ef0b9827a7d694d5ba97 
> 
> Diff: https://reviews.apache.org/r/44883/diff/
> 
> 
> Testing
> ---
> 
> make check (Ubuntu 14.04)
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 43615: Update test suite to use the reworked MesosTest helpers.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [43613, 43614, 43629, 43630, 43615]

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 March 16, 2016, 12:21 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43615/
> ---
> 
> (Updated March 16, 2016, 12:21 a.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4633 and MESOS-4634
> https://issues.apache.org/jira/browse/MESOS-4633
> https://issues.apache.org/jira/browse/MESOS-4634
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Includes the following changes:
> 
> * Added the `` header where appropriate.
> * Added the namespace `using process::Owned;` where appropriate.
> * Generally replaced `Try` with `Try`.  
> And `Try` with `Try`.
> * Added the (now required) `MasterDetector` argument to all slaves.  Before, 
> this was fetched from the first master in `Cluster`.
> * Removed `Shutdown();` from all tests.
> * Replaced `Stop(...)` with the appropriate master/slave destruction calls.
> * Wrap various slave objects in `Owned` (i.e. containerizers, isolators, 
> launchers, etc).
> * Replace `CHECK` in tests with `ASSERT`.
> 
> 
> Diffs
> -
> 
>   src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 
>   src/tests/command_executor_tests.cpp 
> 0d2fcf6d4b8d9a925eb6748e6bd33cf279b8f7f8 
>   src/tests/container_logger_tests.cpp 
> 00f4129e46aa9268fbb66da25b34e61004fa87b2 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> 8afaa4dab3984e9866b7b223e8e2e70ef83a39dc 
>   src/tests/containerizer/external_containerizer_test.cpp 
> 8e1dbe306a088eb16cd3b9c6174b95fad5685da4 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> e72239a55724f1aeeec5362cc370c93dbeca7164 
>   src/tests/containerizer/isolator_tests.cpp 
> 342037ce0a5f8caa4e3cf1550b8f9a7cc328acf9 
>   src/tests/containerizer/memory_pressure_tests.cpp 
> 03879d99c371f296f8d9904666911b34209c114d 
>   src/tests/containerizer/mesos_containerizer_tests.cpp 
> 15f0f93d2e5c19a22f6cc4a71a7d94be4aaec2c1 
>   src/tests/containerizer/port_mapping_tests.cpp 
> a1427fd0157dee343b643f3272dba8ffea61f7b0 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> b2663a83afccbb156f10b4703f29ee0f638384bf 
>   src/tests/containerizer/runtime_isolator_tests.cpp 
> 1625614f560a4c9a644b1b9e9568199f30373ab5 
>   src/tests/credentials_tests.cpp 7edcc857e0f6f8e80e265deeec59d6349d392224 
>   src/tests/disk_quota_tests.cpp 413e562026a4fc9779f616e921ae2fa2ca51e012 
>   src/tests/exception_tests.cpp 6b71316d545e97f14a45daa14d0fd95204befd3b 
>   src/tests/executor_http_api_tests.cpp 
> 2fc0893f5f5e80a783296fb31b30abe86d92df1b 
>   src/tests/fault_tolerance_tests.cpp 
> 67f362747ac4301a74693aeaa5631b7dd866e782 
>   src/tests/gc_tests.cpp 61a8abb9581dc4602b197a88a677b19386969cbf 
>   src/tests/health_check_tests.cpp 9605859b665645654bbdb2688455cae2d692a057 
>   src/tests/hook_tests.cpp bb287c77493209e663a37e7f88d09a0459855f7d 
>   src/tests/http_fault_tolerance_tests.cpp 
> 7c7f3d90210148176e83553346100a506f263591 
>   src/tests/master_allocator_tests.cpp 
> cba7c36471f93b678d94e1da0251a28a893696b1 
>   src/tests/master_authorization_tests.cpp 
> 6a4de4709960d7ca505e99396e14a1bb51d6902d 
>   src/tests/master_contender_detector_tests.cpp 
> 255ab8119a04b55bb4f1b61dee19c4be64499376 
>   src/tests/master_quota_tests.cpp f1277f03f5b5cf735fea7ba324a27614a829cd50 
>   src/tests/master_slave_reconciliation_tests.cpp 
> d41178eb41df519073fc0890c5716bbc9fed6ad2 
>   src/tests/master_tests.cpp e8c39e775d6ca218ce74cfc6bb50c7576d73e90e 
>   src/tests/master_validation_tests.cpp 
> c9bc38ce604d2d44d6e6b1286507d1c45e5e9e25 
>   src/tests/metrics_tests.cpp 419d275e0b32817388120222bd433ee6f4835efd 
>   src/tests/monitor_tests.cpp 869c9e032817e8859a968232d4a61556a3d53d45 
>   src/tests/oversubscription_tests.cpp 
> e528476cd83b0e3f7ae8cea7d86dfabc1f66484e 
>   src/tests/partition_tests.cpp 3776a0a104582f60b9f19ea58b011485194399b9 
>   src/tests/persistent_volume_endpoints_tests.cpp 
> b5f425ab7f2b4a540418d761bea145565467bd2b 
>   src/tests/persistent_volume_tests.cpp 
> e9215de2e073025f67cdc73e8a8de38cf030671f 
>   src/tests/rate_limiting_tests.cpp 352a5e7a58b500c25c7c8a421047c4e79434e38e 
>   src/tests/reconciliation_tests.cpp e8f3f29836652d20a6ee1bb5231a15e71eb76990 
>   src/tests/registrar_zookeeper_tests.cpp 
> 3df9779ee5d076e16f6a538326693a36f986b6d0 
>   src/tests/repair_tests.cpp 

Re: Review Request 41334: Added helper functions to filter allocation slack resources.

[Klaus Ma]

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




include/mesos/resources.hpp (line 170)


I think we should also update those helper fuction according to protobuf 
change, using `throttleable` instead of ALLOCATION_SLACK/USAGE_SLACK, e.g. 
`throttleable()`, `nonThrottleable()` and `revocable()`.


- Klaus Ma


On March 15, 2016, 10:33 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41334/
> ---
> 
> (Updated March 15, 2016, 10:33 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, and Klaus Ma.
> 
> 
> Bugs: MESOS-4146
> https://issues.apache.org/jira/browse/MESOS-4146
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This helper function is used to filter out allocation slack resources.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp bb343ad852576a75615a93ef850b413bf77698e0 
>   include/mesos/v1/resources.hpp 719110fbbf39f1755460ac0b32e3893656054a4e 
>   src/common/resources.cpp 1f23dc83f7330c305a836d698f114b7eaf3d7ba1 
>   src/tests/resources_tests.cpp 6b004d64bb25112b19fc5d98b5bca874c5329e8c 
>   src/v1/resources.cpp c6f125ec317e2da537a6456f5cff2da0a48701d8 
> 
> Diff: https://reviews.apache.org/r/41334/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> GLOG_v=2 ./bin/mesos-tests.sh  --gtest_filter="RevocableResourceTest.Filter" 
> --verbose
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 41333: Added helper functions to filter usage slack resources.

[Klaus Ma]

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




include/mesos/resources.hpp (line 174)


`isThrottleable`?


- Klaus Ma


On March 15, 2016, 10:33 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41333/
> ---
> 
> (Updated March 15, 2016, 10:33 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, and Klaus Ma.
> 
> 
> Bugs: MESOS-4146
> https://issues.apache.org/jira/browse/MESOS-4146
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added helper functions to filter usage slack resources.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp bb343ad852576a75615a93ef850b413bf77698e0 
>   include/mesos/v1/resources.hpp 719110fbbf39f1755460ac0b32e3893656054a4e 
>   src/common/resources.cpp 1f23dc83f7330c305a836d698f114b7eaf3d7ba1 
>   src/tests/resources_tests.cpp 6b004d64bb25112b19fc5d98b5bca874c5329e8c 
>   src/v1/resources.cpp c6f125ec317e2da537a6456f5cff2da0a48701d8 
> 
> Diff: https://reviews.apache.org/r/41333/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 42547: Added helper function to get non usage slack resources.

[Klaus Ma]

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




include/mesos/resources.hpp (line 263)


should be `nonThrottleable`?


- Klaus Ma


On March 15, 2016, 10:32 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42547/
> ---
> 
> (Updated March 15, 2016, 10:32 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van 
> Remoortere, Joseph Wu, Klaus Ma, and Jian Qiu.
> 
> 
> Bugs: MESOS-4146
> https://issues.apache.org/jira/browse/MESOS-4146
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added helper function to get non usage slack resources.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp bb343ad852576a75615a93ef850b413bf77698e0 
>   include/mesos/v1/resources.hpp 719110fbbf39f1755460ac0b32e3893656054a4e 
>   src/common/resources.cpp 1f23dc83f7330c305a836d698f114b7eaf3d7ba1 
>   src/tests/resources_tests.cpp 6b004d64bb25112b19fc5d98b5bca874c5329e8c 
>   src/v1/resources.cpp c6f125ec317e2da537a6456f5cff2da0a48701d8 
> 
> Diff: https://reviews.apache.org/r/42547/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 44836: Update TaskId in long_lived_framework to use fixed length taskid.

[Jay Guo]

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

(Updated March 16, 2016, 2:27 a.m.)


Review request for mesos and haosdent huang.


Changes
---

rebase master


Repository: mesos


Description
---

Current WebUI sorts taskId lexically, which results in non-intuitive ordering.
This patch updates long_lived_framework to use `01`, `02` instead.


Diffs (updated)
-

  src/examples/long_lived_framework.cpp 
289a0b9dd3d1ce30f20dd9bb381126bff30c 

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


Testing
---

make check

In addition, `long_lived_framework` is ran and we confirm correct sorting of 
`TaskID` in WebUI


Thanks,

Jay Guo

Re: Review Request 44836: Update TaskId in long_lived_framework to use fixed length taskid.

[Jay Guo]

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

(Updated March 16, 2016, 2:21 a.m.)


Review request for mesos and haosdent huang.


Changes
---

remove unused `#include`


Repository: mesos


Description
---

Current WebUI sorts taskId lexically, which results in non-intuitive ordering.
This patch updates long_lived_framework to use `01`, `02` instead.


Diffs (updated)
-

  include/mesos/executor/executor.proto 
ae211194a44e0bf2fadc79e833881e45ea3eb2c2 
  include/mesos/v1/executor/executor.proto 
36a2b3f9bc3aaa524f655b9e686a6d33512e6aaa 
  src/examples/long_lived_framework.cpp 
289a0b9dd3d1ce30f20dd9bb381126bff30c 
  src/exec/exec.cpp 741786132f3a8cc43f5b9ced262429038832a946 
  src/executor/executor.cpp 87db4e02cbaa778aab0173741bfe066fdee9a48d 
  src/slave/constants.hpp 0b4e41b4eee3280515cf8a1f049fe923503066f1 
  src/slave/flags.cpp 4d10818105627738e258116647ccada374e3d7b9 

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


Testing
---

make check

In addition, `long_lived_framework` is ran and we confirm correct sorting of 
`TaskID` in WebUI


Thanks,

Jay Guo

Re: Review Request 40375: Support distinguishing revocable resources in the Resource protobuf.

[Klaus Ma]

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

(Updated March 16, 2016, 10:12 a.m.)


Review request for mesos, Guangya Liu, Artem Harutyunyan, and Joris Van 
Remoortere.


Changes
---

Update descriptions.


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


Repository: mesos


Description (updated)
---

MESOS-3888: Distinguish revocable resource to throttleable and non-throttleable.


Diffs
-

  include/mesos/mesos.proto deb9c0910a27afd67276f54b3f666a878212727b 
  include/mesos/v1/mesos.proto a981e750c24cfc48177bbc9ca56f0c3ecfae1a1b 
  src/common/resources.cpp 1f23dc83f7330c305a836d698f114b7eaf3d7ba1 
  src/tests/resources_tests.cpp 6b004d64bb25112b19fc5d98b5bca874c5329e8c 
  src/v1/resources.cpp c6f125ec317e2da537a6456f5cff2da0a48701d8 

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


Testing
---

make && make check


Thanks,

Klaus Ma

Re: Review Request 44549: Introduced a protobuf message "NetworkConfig".

[Qian Zhang]

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

(Updated March 16, 2016, 10:09 a.m.)


Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.


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


Repository: mesos


Description
---

Introduced a protobuf message "NetworkConfig".


Diffs (updated)
-

  src/CMakeLists.txt 0517dd1ae7125ac1ae85cc83b6daaca66a10b8b4 
  src/Makefile.am 9dd21b56af0500f7125b07bf535b45fe5c544aaf 
  src/slave/containerizer/mesos/isolators/network/cni/spec.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/cni/spec.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/network/cni/spec.proto PRE-CREATION 

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


Testing
---

make check


Thanks,

Qian Zhang

Re: Review Request 44881: Generize perf event parsing to match PerfStatistics filed name for "perf stat"

[fan du]

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

(Updated 三月 16, 2016, 2:07 a.m.)


Review request for mesos.


Changes
---

Make the summary more informative


Summary (updated)
-

Generize perf event parsing to match PerfStatistics filed name for "perf stat"


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


Repository: mesos


Description
---

Current design does not support event like:

SUBSYS/EVENT  <- Most notable intel_cqm/llc_occupancy/
SUSSYS:EVENT  <- All Tracepoint event

The current logic could be easily expended a bit to support
above to scenario.


Diffs
-

  include/mesos/mesos.proto 3d22ec32655dca741169e1f0e382303e061c38b7 
  include/mesos/v1/mesos.proto 31960a52061f70d80528fb8326522ae1d6f75b2c 
  src/linux/perf.cpp 1c113a2b3f57877e132bbd65e01fb2f045132128 

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


Testing
---

Before this modification, slave spew:

E0316 09:37:15.352880 116764 perf_event.cpp:408] Failed to get perf sample: 
Failed to parse perf sample: Unexpected event 'intel_cqm/llc_occupancy/' in 
perf output at line: 
0.00,Bytes,intel_cqm/llc_occupancy/,mesos/fad99e5c-5ef2-4908-83de-3b5d7c2bb71f,9795836,100.00

After this modificaiton, I could intel_cqm/llc_occupancy statistics.


Thanks,

fan du

Review Request 44881: Provide generic perf event parsing.

[fan du]

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

Review request for mesos.


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


Repository: mesos


Description
---

Current design does not support event like:

SUBSYS/EVENT  <- Most notable intel_cqm/llc_occupancy/
SUSSYS:EVENT  <- All Tracepoint event

The current logic could be easily expended a bit to support
above to scenario.


Diffs
-

  include/mesos/mesos.proto 3d22ec32655dca741169e1f0e382303e061c38b7 
  include/mesos/v1/mesos.proto 31960a52061f70d80528fb8326522ae1d6f75b2c 
  src/linux/perf.cpp 1c113a2b3f57877e132bbd65e01fb2f045132128 

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


Testing
---

Before this modification, slave spew:

E0316 09:37:15.352880 116764 perf_event.cpp:408] Failed to get perf sample: 
Failed to parse perf sample: Unexpected event 'intel_cqm/llc_occupancy/' in 
perf output at line: 
0.00,Bytes,intel_cqm/llc_occupancy/,mesos/fad99e5c-5ef2-4908-83de-3b5d7c2bb71f,9795836,100.00

After this modificaiton, I could intel_cqm/llc_occupancy statistics.


Thanks,

fan du

Re: Review Request 44549: Introduced a protobuf message "NetworkConfig".

[Qian Zhang]

> On March 15, 2016, 11:19 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/spec.proto, line 40
> > 
> >
> > Why are the position identifiers not sequential ?

Sorry, my bad, will fix it soon


> On March 15, 2016, 11:19 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/spec.proto, line 42
> > 
> >
> > You will need to store the path of the network config as well, since 
> > during the isolate/prepare phase you will need to re-read the whole config 
> > (since it might have plugin specific parameters).

We are going to introduce a new struct `NetworkConfigInfo` keep path and 
protobuf for each network configuration, please see 
https://reviews.apache.org/r/44555/ for details.


- Qian


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


On March 15, 2016, 10:05 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44549/
> ---
> 
> (Updated March 15, 2016, 10:05 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4759
> https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced a protobuf message "NetworkConfig".
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 0517dd1ae7125ac1ae85cc83b6daaca66a10b8b4 
>   src/Makefile.am 7ee5a653fc96907021f14ab28f74c0b3ed0649d9 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.proto PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44549/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 44549: Introduced a protobuf message "NetworkConfig".

[Qian Zhang]

> On March 15, 2016, 11:28 p.m., Avinash sridharan wrote:
> > src/slave/containerizer/mesos/isolators/network/cni/spec.proto, lines 45-46
> > 
> >
> > The isolator doesn't actually need the IPAM or the DNS information. 
> > This information would be used only by the plugin and the isolator doesn't 
> > have to do anything with them. So don't think keeping these fields in the 
> > `NetworkConfig` would serve any purpose?
> > 
> > We do need to store the DNS and ROUTES returned in the `RESULT` of 
> > invoking the plugin, but thoses pieces should probably go in a separate 
> > protobuf (NetworkResult maybe) ?

As we discussed in the sync up meeting, it should be OK to keep IPAM and DNS in 
`NetworkConfig`.


- Qian


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


On March 15, 2016, 10:05 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44549/
> ---
> 
> (Updated March 15, 2016, 10:05 p.m.)
> 
> 
> Review request for mesos, Avinash sridharan, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-4759
> https://issues.apache.org/jira/browse/MESOS-4759
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced a protobuf message "NetworkConfig".
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 0517dd1ae7125ac1ae85cc83b6daaca66a10b8b4 
>   src/Makefile.am 7ee5a653fc96907021f14ab28f74c0b3ed0649d9 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/network/cni/spec.proto PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44549/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 44832: Validate string when convert `Flags` to `hashmap<string, string>`.

[haosdent huang]

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

(Updated March 16, 2016, 1:50 a.m.)


Review request for mesos, Adam B, Ben Mahler, and Kevin Klues.


Changes
---

address @bmahler's comment.


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


Repository: mesos


Description
---

Validate string when convert `Flags` to `hashmap`.


Diffs (updated)
-

  src/common/parse.hpp 9535fad0d50b469fbb4dc6ac5cf5c89b40d29b47 

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


Testing
---

1. Without pass `--env=`.
```
$ ./src/mesos-execute  --master=127.0.0.1:5050 --name=test --command='echo $a'
```

Output:
```
Starting task test
Forked command at 4542
sh -c 'echo $a'
I0315 11:45:39.153188  4402 slave.cpp:3002] Handling status update TASK_RUNNING 
(UUID: 06b8f974-b6ec-4d10-b1d8-f84b5d4f728d) for task test of framework 
cf1af3e0-dd66-41e8-8c56-7ad80d7dce98-0001 from execu
tor(1)@127.0.0.1:54698
I0315 11:45:39.153964  4401 status_update_manager.cpp:320] Received status 
update TASK_RUNNING (UUID: 06b8f974-b6ec-4d10-b1d8-f84b5d4f728d) for task test 
of framework cf1af3e0-dd66-41e8-8c56-7ad80d7dce98-
0001
```

2. Pass `--env=`.
```
$ ./src/mesos-execute  --master=127.0.0.1:5050 --name=test --command='echo $a' 
--env="{\"a\": \"stdin\"}"
```

Output:
```
Registered executor on localhost
Starting task test
Forked command at 4675
sh -c 'echo $a'
stdin
I0315 11:46:34.797502  4408 slave.cpp:3002] Handling status update TASK_RUNNING 
(UUID: 16040c40-f5e4-4bf0-8690-447f2901310b) for task test of framework 
cf1af3e0-dd66-41e8-8c56-7ad80d7dce98-0003 from execu
tor(1)@127.0.0.1:57831
```

3. Pass incorrect json format in `--env=`.
```
./src/mesos-execute  --master=127.0.0.1:5050 --name=test --command='echo $a' 
--env="{\"a\": {}}"
Failed to load flag 'env': Failed to load value '{"a": {}}': The value of key 
'a' in '{"a":{}}' is not a valid string.
```


Thanks,

haosdent huang

Re: Review Request 40532: Added notion of evict resources to RunTaskMessage.

[Guangya Liu]

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

(Updated 三月 16, 2016, 1:46 a.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, 
Joseph Wu, Klaus Ma, and Jian Qiu.


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


Repository: mesos


Description
---

Added notion of evict resources to RunTaskMessage.


Diffs (updated)
-

  src/messages/messages.proto e0f1fca92d3ea8c29c095da31653c317873a934c 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 44758: Upgrade to clang-format-3.8 (MESOS-4906).

[Yong Tang]

> On March 15, 2016, 9:24 p.m., Michael Park wrote:
> > support/clang-format, lines 101-107
> > 
> >
> > We can help out a little more here:
> > 
> > The following code orders the headers:
> > (1) C standard library headers
> > (2) C++ standard library headers
> > (3) Our public headers
> > (4) Our private headers
> > 
> > ```
> > IncludeCategories:
> >   - Regex:   '^<.*.h>'
> > Priority:1
> >   - Regex:   '^<.*.hpp>'
> > Priority:3
> >   - Regex:   '^<.*>'
> > Priority:2
> >   - Regex:   '.*'
> > Priority:4
> > ```
> > 
> > __NOTE__: 3 seems to need to appear before 2, probably because it's 
> > more specific.

Thanks for the help, Michael. Just updated the review request. Let me know if 
there are any other issues.


- Yong


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


On March 16, 2016, 1:37 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44758/
> ---
> 
> (Updated March 16, 2016, 1:37 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-4906
> https://issues.apache.org/jira/browse/MESOS-4906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade to clang-format-3.8 (MESOS-4906).
> 
> 
> Diffs
> -
> 
>   docs/clang-format.md 7f1c1dfd70e1fe9bfa186df1bdda7bdcf867db04 
>   support/clang-format 499d0e749e14e50256ae649afa0ced2b04589a0e 
> 
> Diff: https://reviews.apache.org/r/44758/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 44758: Upgrade to clang-format-3.8 (MESOS-4906).

[Yong Tang]

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

(Updated March 16, 2016, 1:37 a.m.)


Review request for mesos and Michael Park.


Changes
---

Update based on Michael's suggestions.


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


Repository: mesos


Description
---

Upgrade to clang-format-3.8 (MESOS-4906).


Diffs (updated)
-

  docs/clang-format.md 7f1c1dfd70e1fe9bfa186df1bdda7bdcf867db04 
  support/clang-format 499d0e749e14e50256ae649afa0ced2b04589a0e 

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


Testing
---

make check


Thanks,

Yong Tang

Re: Review Request 40532: Added notion of evict resources to RunTaskMessage.

[Guangya Liu]

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

(Updated 三月 16, 2016, 1:24 a.m.)


Review request for mesos, Ben Mahler, Artem Harutyunyan, Joris Van Remoortere, 
Joseph Wu, Klaus Ma, and Jian Qiu.


Summary (updated)
-

Added notion of evict resources to RunTaskMessage.


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


Repository: mesos


Description (updated)
---

Added notion of evict resources to RunTaskMessage.


Diffs (updated)
-

  src/messages/messages.proto e0f1fca92d3ea8c29c095da31653c317873a934c 

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


Testing
---


Thanks,

Guangya Liu

Re: Review Request 44378: Upgrade libev to 4.22 to support PowerPC LE platform.

[Vinod Kone]

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



Can you explain what testing you have done in the "Testing done" section? 

According to MESOS-4312, we agreed to do the following tests:

-- Verify SSL, perf and docker related test cases work as expected on all 
platforms.
-- Do the validation on all platforms by "sudo make dist check" and "sudo make 
check --benchmark --gtest_filter="*Benchmark*"" and make sure the perf numbers 
are OK.
-- Run compatibility tests between scheduler <=> master <=> slave <=> executor 
where each of the components is running either the patched/upgraded version or 
not.

- Vinod Kone


On March 14, 2016, 2:01 p.m., Zhiwei Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44378/
> ---
> 
> (Updated March 14, 2016, 2:01 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Alex Clemmer, Kapil Arya, Neil Conway, 
> and Vinod Kone.
> 
> 
> Bugs: MESOS-4803
> https://issues.apache.org/jira/browse/MESOS-4803
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade libev to 4.22 to support PowerPC LE platform.
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake 6eac4dc0f7189e209e7d7232419e4de4bc0875c0 
>   3rdparty/libprocess/3rdparty/CMakeLists.txt 
> b8351ad0181d885a984580ae8de208ea0524b0e7 
>   3rdparty/libprocess/3rdparty/libev-4.15.patch 
> bbd83e6928e6caba3bc5a9739823d60923cfaa2a 
>   3rdparty/libprocess/3rdparty/libev-4.15.tar.gz 
> 4c282b573aa9331fd16197ef286faf323b6515eb 
>   3rdparty/libprocess/3rdparty/libev-4.22.patch PRE-CREATION 
>   3rdparty/libprocess/3rdparty/libev-4.22.tar.gz PRE-CREATION 
>   3rdparty/libprocess/3rdparty/versions.am 
> 98195b8eb60b2673d610d8ab7ea31103f137debf 
>   LICENSE 66a99b8a84e614cc89a22df02e3c47d01e26cd39 
>   src/python/native/ext_modules.py.in 
> eb93864733713dddad66141c6b8b6cd895f41484 
> 
> Diff: https://reviews.apache.org/r/44378/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhiwei Chen
> 
>

Re: Review Request 44874: Added support for FTS_SLNONE in rmdir.

[Jojy Varghese]

> On March 16, 2016, 12:29 a.m., Neil Conway wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp, line 222
> > 
> >
> > Why is this a `string&` and `newDirectory` is a `string`? I'd think 
> > both should probably be `string`.
> 
> Jie Yu wrote:
> Good catch, we should use 'string'. We avoid using const ref to capture 
> temp variable.

A pattern I copied from `os_tests.cpp`. Copy paste horror. I think we need to 
fix it there as well as we fix it here.


- Jojy


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


On March 15, 2016, 11:47 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44874/
> ---
> 
> (Updated March 15, 2016, 11:47 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for FTS_SLNONE in rmdir.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> cf21e7fe458626c7533e596997cab3afdabb55f4 
>   3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp 
> 291a22b5aae53b0bc32ae18b9343ceb5a638b37b 
> 
> Diff: https://reviews.apache.org/r/44874/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44743: Improved grammar in `--help` output for master and agent.

[Joris Van Remoortere]

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


Ship it!




Ship It!

- Joris Van Remoortere


On March 12, 2016, 12:48 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44743/
> ---
> 
> (Updated March 12, 2016, 12:48 a.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved grammar in `--help` output for master and agent.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md f6e84023b90e560594429826ed7163310d62b265 
>   src/master/main.cpp 7c1656bcc266f6c94cb4befad37fa813a218b2fa 
>   src/slave/flags.cpp eb470154f30634b3db439be1c122ff93d3147afe 
>   src/slave/main.cpp e3a4d13ddaeb89ba01c9b2ddfc72c37934f753eb 
> 
> Diff: https://reviews.apache.org/r/44743/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 44229: Added upgrade testing script.

[Vinod Kone]

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




support/test-upgrade.py (line 24)


Can you document what this function does and returns?



support/test-upgrade.py (line 42)


credfile is unused?



support/test-upgrade.py (line 47)


2 blank lines.



support/test-upgrade.py (line 51)


credfile is unused?



support/test-upgrade.py (line 59)


s/insteance/instance/



support/test-upgrade.py (line 62)


credfile is unused?



support/test-upgrade.py (line 63)


s/persistent-volume-framework/test-framework/ ?

actuallly we should be testing the long-lived-framework instead of 
test-framework. but that needs updating the long lived framework to abort on 
un-expected state changes (e.g., TASK_FAILED received instead of TASK_FINISHED).



support/test-upgrade.py (line 66)


2 blank lines.



support/test-upgrade.py (line 67)


s/numbers//

s/tested/built/ ?



support/test-upgrade.py (line 80)


2 blank lines.



support/test-upgrade.py (line 102)


s/tested/built/ ?



support/test-upgrade.py (lines 110 - 113)


seems unused. just kill this for now.



support/test-upgrade.py (lines 118 - 125)


The upgrade flow we want to test is

scheduler (old) <-> master (old) <-> slave (old)
scheduler (old) <-> master (new) <-> slave (old)
scheduler (old) <-> master (new) <-> slave (new)
scheduler (new) <-> master (new) <-> slave (new)



support/test-upgrade.py (lines 153 - 154)


if prev_framework.sleep(10) != 0:
  ...



support/test-upgrade.py (line 158)


Lets add a test for upgrading master first. This is what typically 
operators do.



support/test-upgrade.py (lines 174 - 175)


no need for prev_framework_result temp variable?



support/test-upgrade.py (lines 188 - 189)


no need for next_famework_result.



support/test-upgrade.py (lines 197 - 228)


not sure, why we do it this way. just kill this.


- Vinod Kone


On March 16, 2016, 12:18 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44229/
> ---
> 
> (Updated March 16, 2016, 12:18 a.m.)
> 
> 
> Review request for mesos, Niklas Nielsen and Vinod Kone.
> 
> 
> Bugs: MESOS-2372
> https://issues.apache.org/jira/browse/MESOS-2372
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added upgrade testing script.
> 
> 
> Diffs
> -
> 
>   support/test-upgrade.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44229/diff/
> 
> 
> Testing
> ---
> 
> This script is copied from @nnielsen, original patch can be found here: 
> https://reviews.apache.org/r/31645/
> 
> To test, build two different versions of Mesos in two different directories. 
> Then do:
> `support/test-upgrade.py --prev=/path/to/earlier/version 
> --next=/path/to/later/version`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44874: Added support for FTS_SLNONE in rmdir.

[Jojy Varghese]

> On March 16, 2016, 12:25 a.m., Neil Conway wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp, line 71
> > 
> >
> > "don't". Also, we should use backticks for `FTS_COMFOLLOW` for 
> > consistency.
> > 
> > Is that comment correct? ISTM that the relevant option that controls 
> > whether symbolic links are resolved is `FTS_LOGICAL` or `FTS_PHYSICAL`; 
> > `FTS_COMFOLLOW` is a special-case for the root path that is passed to 
> > `fts_open`.
> > 
> > While we're at it, the OSX and Linux versions of the manpage for 
> > `fts_open` both claim that "either FTS_LOGICAL or FTS_PHYSICAL" *must* be 
> > specified. Seems like we're specifying neither, both here and in the other 
> > call-site to `fts_open` in `src/linux/cgroups.cpp`?
> 
> Jie Yu wrote:
> Looks like we should use `FTS_PHYSICAL` in rmdir?

I made the assumption looking at some implementation source code at 
http://osxr.org:8080/glibc/source/io/fts.c?v=glibc-2.17. L147 is where 
`fts_open` calls  `fts_stat` with a check for `FTS_COMFOLLOW`. In `fts_stat`, 
L879, the check is made for `logical` or `follow`.


- Jojy


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


On March 15, 2016, 11:47 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44874/
> ---
> 
> (Updated March 15, 2016, 11:47 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for FTS_SLNONE in rmdir.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> cf21e7fe458626c7533e596997cab3afdabb55f4 
>   3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp 
> 291a22b5aae53b0bc32ae18b9343ceb5a638b37b 
> 
> Diff: https://reviews.apache.org/r/44874/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44742: Improved documentation for multiple disks.

[Joris Van Remoortere]

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


Ship it!




Ship It!

- Joris Van Remoortere


On March 12, 2016, 1:24 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44742/
> ---
> 
> (Updated March 12, 2016, 1:24 a.m.)
> 
> 
> Review request for mesos and Joris Van Remoortere.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved documentation for multiple disks.
> 
> 
> Diffs
> -
> 
>   docs/multiple-disk.md f0fca2517138a3eef41daaba9206d6d3cd3f6f30 
> 
> Diff: https://reviews.apache.org/r/44742/diff/
> 
> 
> Testing
> ---
> 
> Previewed using site-docker.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 44874: Added support for FTS_SLNONE in rmdir.

[Jie Yu]

> On March 16, 2016, 12:25 a.m., Neil Conway wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp, line 71
> > 
> >
> > "don't". Also, we should use backticks for `FTS_COMFOLLOW` for 
> > consistency.
> > 
> > Is that comment correct? ISTM that the relevant option that controls 
> > whether symbolic links are resolved is `FTS_LOGICAL` or `FTS_PHYSICAL`; 
> > `FTS_COMFOLLOW` is a special-case for the root path that is passed to 
> > `fts_open`.
> > 
> > While we're at it, the OSX and Linux versions of the manpage for 
> > `fts_open` both claim that "either FTS_LOGICAL or FTS_PHYSICAL" *must* be 
> > specified. Seems like we're specifying neither, both here and in the other 
> > call-site to `fts_open` in `src/linux/cgroups.cpp`?

Looks like we should use `FTS_PHYSICAL` in rmdir?


- Jie


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


On March 15, 2016, 11:47 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44874/
> ---
> 
> (Updated March 15, 2016, 11:47 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for FTS_SLNONE in rmdir.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> cf21e7fe458626c7533e596997cab3afdabb55f4 
>   3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp 
> 291a22b5aae53b0bc32ae18b9343ceb5a638b37b 
> 
> Diff: https://reviews.apache.org/r/44874/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44874: Added support for FTS_SLNONE in rmdir.

[Jie Yu]

> On March 16, 2016, 12:29 a.m., Neil Conway wrote:
> > 3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp, line 222
> > 
> >
> > Why is this a `string&` and `newDirectory` is a `string`? I'd think 
> > both should probably be `string`.

Good catch, we should use 'string'. We avoid using const ref to capture temp 
variable.


- Jie


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


On March 15, 2016, 11:47 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44874/
> ---
> 
> (Updated March 15, 2016, 11:47 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for FTS_SLNONE in rmdir.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> cf21e7fe458626c7533e596997cab3afdabb55f4 
>   3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp 
> 291a22b5aae53b0bc32ae18b9343ceb5a638b37b 
> 
> Diff: https://reviews.apache.org/r/44874/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44867: Fixed linux fs isolator multi containers test.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44867]

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 March 15, 2016, 9:21 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44867/
> ---
> 
> (Updated March 15, 2016, 9:21 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Bugs: MESOS-4912
> https://issues.apache.org/jira/browse/MESOS-4912
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed linux fs isolator multi containers test.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> e72239a55724f1aeeec5362cc370c93dbeca7164 
> 
> Diff: https://reviews.apache.org/r/44867/diff/
> 
> 
> Testing
> ---
> 
> make check 
> 
> sudo ./bin/mesos-test.sh
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 44313: Described "/maintanence/schedule" GET/POST in the two paragraph.

[Joris Van Remoortere]

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


Ship it!




Ship It!

- Joris Van Remoortere


On March 3, 2016, 5:45 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44313/
> ---
> 
> (Updated March 3, 2016, 5:45 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Described /maintanence/schedule GET/POST in the two paragraph.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/master/maintenance/schedule.md 
> 81c34f598ccdc74af74d2a96f02a252a239d9b21 
>   src/master/http.cpp 5e9e28e904ba0045ee27eb828f47231632a91d74 
> 
> Diff: https://reviews.apache.org/r/44313/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 44874: Added support for FTS_SLNONE in rmdir.

[Neil Conway]

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




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


"don't". Also, we should use backticks for `FTS_COMFOLLOW` for consistency.

Is that comment correct? ISTM that the relevant option that controls 
whether symbolic links are resolved is `FTS_LOGICAL` or `FTS_PHYSICAL`; 
`FTS_COMFOLLOW` is a special-case for the root path that is passed to 
`fts_open`.

While we're at it, the OSX and Linux versions of the manpage for `fts_open` 
both claim that "either FTS_LOGICAL or FTS_PHYSICAL" *must* be specified. Seems 
like we're specifying neither, both here and in the other call-site to 
`fts_open` in `src/linux/cgroups.cpp`?


- Neil Conway


On March 15, 2016, 11:47 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44874/
> ---
> 
> (Updated March 15, 2016, 11:47 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for FTS_SLNONE in rmdir.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> cf21e7fe458626c7533e596997cab3afdabb55f4 
>   3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp 
> 291a22b5aae53b0bc32ae18b9343ceb5a638b37b 
> 
> Diff: https://reviews.apache.org/r/44874/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44867: Fixed linux fs isolator multi containers test.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On March 15, 2016, 9:21 p.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44867/
> ---
> 
> (Updated March 15, 2016, 9:21 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Bugs: MESOS-4912
> https://issues.apache.org/jira/browse/MESOS-4912
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed linux fs isolator multi containers test.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/filesystem_isolator_tests.cpp 
> e72239a55724f1aeeec5362cc370c93dbeca7164 
> 
> Diff: https://reviews.apache.org/r/44867/diff/
> 
> 
> Testing
> ---
> 
> make check 
> 
> sudo ./bin/mesos-test.sh
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 43615: Update test suite to use the reworked MesosTest helpers.

[Joseph Wu]

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

(Updated March 15, 2016, 5:21 p.m.)


Review request for mesos, Bernd Mathiske and Artem Harutyunyan.


Changes
---

Rebased and added modifications to the new HttpFaultToleranceTests.


Bugs: MESOS-4633 and MESOS-4634
https://issues.apache.org/jira/browse/MESOS-4633
https://issues.apache.org/jira/browse/MESOS-4634


Repository: mesos


Description
---

Includes the following changes:

* Added the `` header where appropriate.
* Added the namespace `using process::Owned;` where appropriate.
* Generally replaced `Try` with `Try`.  
And `Try` with `Try`.
* Added the (now required) `MasterDetector` argument to all slaves.  Before, 
this was fetched from the first master in `Cluster`.
* Removed `Shutdown();` from all tests.
* Replaced `Stop(...)` with the appropriate master/slave destruction calls.
* Wrap various slave objects in `Owned` (i.e. containerizers, isolators, 
launchers, etc).
* Replace `CHECK` in tests with `ASSERT`.


Diffs (updated)
-

  src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 
  src/tests/command_executor_tests.cpp 0d2fcf6d4b8d9a925eb6748e6bd33cf279b8f7f8 
  src/tests/container_logger_tests.cpp 00f4129e46aa9268fbb66da25b34e61004fa87b2 
  src/tests/containerizer/docker_containerizer_tests.cpp 
8afaa4dab3984e9866b7b223e8e2e70ef83a39dc 
  src/tests/containerizer/external_containerizer_test.cpp 
8e1dbe306a088eb16cd3b9c6174b95fad5685da4 
  src/tests/containerizer/filesystem_isolator_tests.cpp 
e72239a55724f1aeeec5362cc370c93dbeca7164 
  src/tests/containerizer/isolator_tests.cpp 
342037ce0a5f8caa4e3cf1550b8f9a7cc328acf9 
  src/tests/containerizer/memory_pressure_tests.cpp 
03879d99c371f296f8d9904666911b34209c114d 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
15f0f93d2e5c19a22f6cc4a71a7d94be4aaec2c1 
  src/tests/containerizer/port_mapping_tests.cpp 
a1427fd0157dee343b643f3272dba8ffea61f7b0 
  src/tests/containerizer/provisioner_docker_tests.cpp 
b2663a83afccbb156f10b4703f29ee0f638384bf 
  src/tests/containerizer/runtime_isolator_tests.cpp 
1625614f560a4c9a644b1b9e9568199f30373ab5 
  src/tests/credentials_tests.cpp 7edcc857e0f6f8e80e265deeec59d6349d392224 
  src/tests/disk_quota_tests.cpp 413e562026a4fc9779f616e921ae2fa2ca51e012 
  src/tests/exception_tests.cpp 6b71316d545e97f14a45daa14d0fd95204befd3b 
  src/tests/executor_http_api_tests.cpp 
2fc0893f5f5e80a783296fb31b30abe86d92df1b 
  src/tests/fault_tolerance_tests.cpp 67f362747ac4301a74693aeaa5631b7dd866e782 
  src/tests/gc_tests.cpp 61a8abb9581dc4602b197a88a677b19386969cbf 
  src/tests/health_check_tests.cpp 9605859b665645654bbdb2688455cae2d692a057 
  src/tests/hook_tests.cpp bb287c77493209e663a37e7f88d09a0459855f7d 
  src/tests/http_fault_tolerance_tests.cpp 
7c7f3d90210148176e83553346100a506f263591 
  src/tests/master_allocator_tests.cpp cba7c36471f93b678d94e1da0251a28a893696b1 
  src/tests/master_authorization_tests.cpp 
6a4de4709960d7ca505e99396e14a1bb51d6902d 
  src/tests/master_contender_detector_tests.cpp 
255ab8119a04b55bb4f1b61dee19c4be64499376 
  src/tests/master_quota_tests.cpp f1277f03f5b5cf735fea7ba324a27614a829cd50 
  src/tests/master_slave_reconciliation_tests.cpp 
d41178eb41df519073fc0890c5716bbc9fed6ad2 
  src/tests/master_tests.cpp e8c39e775d6ca218ce74cfc6bb50c7576d73e90e 
  src/tests/master_validation_tests.cpp 
c9bc38ce604d2d44d6e6b1286507d1c45e5e9e25 
  src/tests/metrics_tests.cpp 419d275e0b32817388120222bd433ee6f4835efd 
  src/tests/monitor_tests.cpp 869c9e032817e8859a968232d4a61556a3d53d45 
  src/tests/oversubscription_tests.cpp e528476cd83b0e3f7ae8cea7d86dfabc1f66484e 
  src/tests/partition_tests.cpp 3776a0a104582f60b9f19ea58b011485194399b9 
  src/tests/persistent_volume_endpoints_tests.cpp 
b5f425ab7f2b4a540418d761bea145565467bd2b 
  src/tests/persistent_volume_tests.cpp 
e9215de2e073025f67cdc73e8a8de38cf030671f 
  src/tests/rate_limiting_tests.cpp 352a5e7a58b500c25c7c8a421047c4e79434e38e 
  src/tests/reconciliation_tests.cpp e8f3f29836652d20a6ee1bb5231a15e71eb76990 
  src/tests/registrar_zookeeper_tests.cpp 
3df9779ee5d076e16f6a538326693a36f986b6d0 
  src/tests/repair_tests.cpp 0dfb557a62f64baf2334dbc9f75ecdfb10473c2d 
  src/tests/reservation_endpoints_tests.cpp 
f95ae7a32c3809d150adf1e9e515a3b527e61699 
  src/tests/reservation_tests.cpp c88091635949037fcfc6be504043764f9af35f79 
  src/tests/role_tests.cpp f45ee816f6ab2f988fbe4efdb14c58b7538e06c8 
  src/tests/scheduler_driver_tests.cpp f6dc25d82ae5f1e77fc6ede7ff2660ed0d9ea039 
  src/tests/scheduler_event_call_tests.cpp 
8c02ceeb3ec1783cb2f63f100700508e70f586e4 
  src/tests/scheduler_http_api_tests.cpp 
dfb0f51fec67a3951e396eab28eedb0dbf9493ae 
  src/tests/slave_tests.cpp 87c5e8caa63095fb8325ae019041398d49aa1edf 
  src/tests/status_update_manager_tests.cpp 

Re: Review Request 44229: Added upgrade testing script.

[Greg Mann]

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

(Updated March 16, 2016, 12:18 a.m.)


Review request for mesos, Niklas Nielsen and Vinod Kone.


Changes
---

Added "bugs" and "people".


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


Repository: mesos


Description
---

Added upgrade testing script.


Diffs
-

  support/test-upgrade.py PRE-CREATION 

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


Testing
---

This script is copied from @nnielsen, original patch can be found here: 
https://reviews.apache.org/r/31645/

To test, build two different versions of Mesos in two different directories. 
Then do:
`support/test-upgrade.py --prev=/path/to/earlier/version 
--next=/path/to/later/version`


Thanks,

Greg Mann

Re: Review Request 44874: Added support for FTS_SLNONE in rmdir.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On March 15, 2016, 11:47 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44874/
> ---
> 
> (Updated March 15, 2016, 11:47 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support for FTS_SLNONE in rmdir.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> cf21e7fe458626c7533e596997cab3afdabb55f4 
>   3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp 
> 291a22b5aae53b0bc32ae18b9343ceb5a638b37b 
> 
> Diff: https://reviews.apache.org/r/44874/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44722: Libprocess: Add `SOL_TCP` flag for Windows.

[Joris Van Remoortere]

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


Ship it!




Ship It!

- Joris Van Remoortere


On March 11, 2016, 6:40 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44722/
> ---
> 
> (Updated March 11, 2016, 6:40 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Libprocess: Add `SOL_TCP` flag for Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/config.hpp 1e0c2a5a61c3d1a7b50057f876f88a157a5e4ed8 
> 
> Diff: https://reviews.apache.org/r/44722/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 44078: Windows: Added Socket compatibility `#define`s to windows.hpp.

[Joris Van Remoortere]

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


Ship it!




Ship It!

- Joris Van Remoortere


On March 11, 2016, 6:39 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44078/
> ---
> 
> (Updated March 11, 2016, 6:39 p.m.)
> 
> 
> Review request for mesos, Alex Naparu, Daniel Pravat, Artem Harutyunyan, 
> Joris Van Remoortere, Michael Park, M Lawindi, and Yi Sun.
> 
> 
> Bugs: MESOS-3632
> https://issues.apache.org/jira/browse/MESOS-3632
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Added Socket compatibility `#define`s to windows.hpp.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/windows.hpp 
> ee13d12fcffcd564c7ded2d2f541d7bbdf6633c1 
> 
> Diff: https://reviews.apache.org/r/44078/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 44873: Added test for rmdir with device file.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On March 15, 2016, 11:46 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44873/
> ---
> 
> (Updated March 15, 2016, 11:46 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Neil Conway.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Existing tests did not cover the case of removing directories with
> special files like device files.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp 
> 291a22b5aae53b0bc32ae18b9343ceb5a638b37b 
> 
> Diff: https://reviews.apache.org/r/44873/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44832: Validate string when convert `Flags` to `hashmap<string, string>`.

[Ben Mahler]

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




src/common/parse.hpp (lines 107 - 113)


We tend to handle errors first, and we'll try to include the open ' and 
close ' on the same log line for readability. Also, no periods at the end of 
error messages.

```
  // Convert from JSON to Hashmap.
  hashmap map;
  foreachpair (const std::string& key,
   const JSON::Value& value,
   json.get().values) {
if (!value.is()) {
  return Error(
  "The value of key '" + key + "' in '" + stringify(json.get()) + 
"'"
  " is not a valid string");
}

map[key] = value.as().value;
  }
```


- Ben Mahler


On March 15, 2016, 3:52 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44832/
> ---
> 
> (Updated March 15, 2016, 3:52 a.m.)
> 
> 
> Review request for mesos, Adam B, Ben Mahler, and Kevin Klues.
> 
> 
> Bugs: MESOS-2023
> https://issues.apache.org/jira/browse/MESOS-2023
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Validate string when convert `Flags` to `hashmap`.
> 
> 
> Diffs
> -
> 
>   src/common/parse.hpp 9535fad0d50b469fbb4dc6ac5cf5c89b40d29b47 
> 
> Diff: https://reviews.apache.org/r/44832/diff/
> 
> 
> Testing
> ---
> 
> 1. Without pass `--env=`.
> ```
> $ ./src/mesos-execute  --master=127.0.0.1:5050 --name=test --command='echo $a'
> ```
> 
> Output:
> ```
> Starting task test
> Forked command at 4542
> sh -c 'echo $a'
> I0315 11:45:39.153188  4402 slave.cpp:3002] Handling status update 
> TASK_RUNNING (UUID: 06b8f974-b6ec-4d10-b1d8-f84b5d4f728d) for task test of 
> framework cf1af3e0-dd66-41e8-8c56-7ad80d7dce98-0001 from execu
> tor(1)@127.0.0.1:54698
> I0315 11:45:39.153964  4401 status_update_manager.cpp:320] Received status 
> update TASK_RUNNING (UUID: 06b8f974-b6ec-4d10-b1d8-f84b5d4f728d) for task 
> test of framework cf1af3e0-dd66-41e8-8c56-7ad80d7dce98-
> 0001
> ```
> 
> 2. Pass `--env=`.
> ```
> $ ./src/mesos-execute  --master=127.0.0.1:5050 --name=test --command='echo 
> $a' --env="{\"a\": \"stdin\"}"
> ```
> 
> Output:
> ```
> Registered executor on localhost
> Starting task test
> Forked command at 4675
> sh -c 'echo $a'
> stdin
> I0315 11:46:34.797502  4408 slave.cpp:3002] Handling status update 
> TASK_RUNNING (UUID: 16040c40-f5e4-4bf0-8690-447f2901310b) for task test of 
> framework cf1af3e0-dd66-41e8-8c56-7ad80d7dce98-0003 from execu
> tor(1)@127.0.0.1:57831
> ```
> 
> 3. Pass incorrect json format in `--env=`.
> ```
> ./src/mesos-execute  --master=127.0.0.1:5050 --name=test --command='echo $a' 
> --env="{\"a\": {}}"
> Failed to load flag 'env': Failed to load value '{"a": {}}': The value of key 
> 'a' in '{"a":{}}' is not a valid string.
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 44733: Added fault tolerance tests for the V1 API.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On March 14, 2016, 10:14 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44733/
> ---
> 
> (Updated March 14, 2016, 10:14 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4629
> https://issues.apache.org/jira/browse/MESOS-4629
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Similar to already existing tests in `fault_tolerance_tests.cpp`,
> this change adds fault tolerance tests for HTTP schedulers/executors.
> Some tests have been omitted since they were no longer valid now due
> to the persistent streaming connection between the master and the
> scheduler in the new API.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 8abef3bd3237a6e456faa7884637207dc4d63282 
>   src/tests/http_fault_tolerance_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/44733/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Review Request 44874: Added support for FTS_SLNONE in rmdir.

[Jojy Varghese]

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

Review request for mesos, Jie Yu and Neil Conway.


Repository: mesos


Description
---

Added support for FTS_SLNONE in rmdir.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
cf21e7fe458626c7533e596997cab3afdabb55f4 
  3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp 
291a22b5aae53b0bc32ae18b9343ceb5a638b37b 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Review Request 44873: Added test for rmdir with device file.

[Jojy Varghese]

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

Review request for mesos, Jie Yu and Neil Conway.


Repository: mesos


Description
---

Existing tests did not cover the case of removing directories with
special files like device files.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/tests/os/rmdir_tests.cpp 
291a22b5aae53b0bc32ae18b9343ceb5a638b37b 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 44662: Added kill policies and shutdown grace period to the CHANGELOG.

[Ben Mahler]

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



Looks good, just some suggestions for the descriptions.


CHANGELOG (lines 5 - 7)


Similar to my other feedback, could we clarify here that the kill policy 
allows configuring the period betwen graceful kill and forceful kill. For 
command tasks, this will be the time between SIGTERM and SIGKILL. In the 
future, we may allow more policy to be specified (e.g. htting an HTTP endpoint, 
running a command, etc).



CHANGELOG (lines 9 - 12)


Also best-effort, worth mentioning that and why that it must not be relied 
on for correctness.


- Ben Mahler


On March 15, 2016, 2:28 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44662/
> ---
> 
> (Updated March 15, 2016, 2:28 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 761238c48332bcce0bff6c411225fdb4176ddca6 
> 
> Diff: https://reviews.apache.org/r/44662/diff/
> 
> 
> Testing
> ---
> 
> `make check` on Mac OS 10.10.4
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 44661: Deprecated the `docker_stop_timeout` flag.

[Ben Mahler]

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



Looking pretty good. Would be great to have a CHANGELOG update here that 
outlines the deprecation and what we're advising users to do in each version.


docs/configuration.md (lines 1133 - 1135)


We should help the users by adding a corresponding deprecation notice 
within the CHANGELOG, can you include that in this diff please?



src/docker/executor.hpp (lines 55 - 59)


Looking at this in isolation it's hard to tell why it's deprecated. Could 
you add the similar blurb to the help string? Or expand on the comment?



src/docker/executor.hpp (line 74)


Just to be clear, is the intention to remove this in 0.30? If so, can you 
say that explicitly? Or are we giving time for users to set kill policies and 
planning to remove it 6 months from 0.29.0?



src/slave/containerizer/docker.cpp (lines 960 - 962)


How about:

```
  // TODO(alexr): After the deprecation cycle (started in 0.29.0), 
update
  // this to omit the timeout. Graceful shutdown of the container is not
  // a containerizer responsibility; it is the responsibility of the 
agent
  // in co-operation with the executor. Once `destroy` is called, the
  // container should be destroyed forcefully.
```

Ditto below.



src/slave/containerizer/docker.cpp (line 966)


Hm.. I'm a bit confused about the deprecation taking place here. Could you 
outline the steps in this description? Specifically, what does the 0.28, 0.29, 
and final versions look like and what are we advising users to do in each 
version?

This information would be great to have in the CHANGELOG deprecation 
message I suggested above.



src/slave/flags.cpp (line 532)


poly?


- Ben Mahler


On March 15, 2016, 2:28 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44661/
> ---
> 
> (Updated March 15, 2016, 2:28 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Timothy Chen.
> 
> 
> Bugs: MESOS-4910
> https://issues.apache.org/jira/browse/MESOS-4910
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead, a combination of `executor_shutdown_grace_period`
> agent flag and task kill policies should be used.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 739d4ff9aeeb1ba70ce11033168d63d37b6ef56b 
>   src/docker/executor.hpp abbc419533ab40312e917931a2fc2ce78b38da41 
>   src/slave/containerizer/docker.cpp fb9188a19a5cd8211d4f36f9647ebb70de560109 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp 4d10818105627738e258116647ccada374e3d7b9 
> 
> Diff: https://reviews.apache.org/r/44661/diff/
> 
> 
> Testing
> ---
> 
> The complete chain was tested. See https://reviews.apache.org/r/44662/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 44753: Fixed runtime isolator tests out of disk issue.

[Guangya Liu]

> On 三月 15, 2016, 1:28 a.m., Guangya Liu wrote:
> > src/tests/containerizer/runtime_isolator_tests.cpp, lines 96-98
> > 
> >
> > The comments should also be updated? Also should mention that do not 
> > want use up disk due to work dir may be copied to rootfs?
> > 
> > Ditto for other `flags.docker_store_dir`
> 
> Jie Yu wrote:
> I think the comments here still apply, right?

Do we need to mention that this can also help not use up disk?


- Guangya


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


On 三月 15, 2016, 12:22 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44753/
> ---
> 
> (Updated 三月 15, 2016, 12:22 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4942
> https://issues.apache.org/jira/browse/MESOS-4942
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed runtime isolator tests out of disk issue.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 9fd5ea9e465f21624d3cdc0125a702528fe7548b 
>   src/tests/containerizer/runtime_isolator_tests.cpp 
> d8a000413fb30d7daaa77827287a2395d81b8b04 
> 
> Diff: https://reviews.apache.org/r/44753/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-test.sh
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 44837: Added force_pull_image to Image protobuf.

[Guangya Liu]

> On 三月 15, 2016, 4:42 p.m., Jie Yu wrote:
> > include/mesos/mesos.proto, line 1501
> > 
> >
> > s/force_pull_image/no_cache/
> > 
> > IMO, force pull is a mean, not a description. Please adjust the 
> > comments accordingly.
> 
> Gilbert Song wrote:
> Shouldn't we make it consensus with docker containerizer?

Yes, I have the same question as Gilbert, the docker containerizer is now using 
`force_pull_image`, shall we keep consistent? Just keep `force_pull_image` or 
use `no_cache` for both mesos containerizer and docker containerizer?


- Guangya


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


On 三月 15, 2016, 7:31 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44837/
> ---
> 
> (Updated 三月 15, 2016, 7:31 a.m.)
> 
> 
> Review request for mesos, haosdent huang and Jie Yu.
> 
> 
> Bugs: MESOS-4886
> https://issues.apache.org/jira/browse/MESOS-4886
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added force_pull_image to Image protobuf.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto deb9c0910a27afd67276f54b3f666a878212727b 
>   include/mesos/v1/mesos.proto a981e750c24cfc48177bbc9ca56f0c3ecfae1a1b 
> 
> Diff: https://reviews.apache.org/r/44837/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 44669: Added createFromModule methods to MasterContender and MasterDetector.

[Anurag Singh]

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

(Updated March 15, 2016, 11:02 p.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

The createFromModule will be used to create a MasterContender/Detector
from a module (specified using the --modules flag on the command
line).


Diffs
-

  src/master/contenders/contender.cpp PRE-CREATION 
  src/master/detectors/detector.cpp PRE-CREATION 

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


Testing (updated)
---

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


Thanks,

Anurag Singh

Re: Review Request 44546: Moved functions in promises to a common header file.

[Anurag Singh]

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

(Updated March 15, 2016, 11:02 p.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

Moved functions in promises to a common header file.


Diffs
-

  src/master/detectors/standalone.cpp PRE-CREATION 
  src/master/detectors/zookeeper.cpp 9274435802d6292b183be48f42b43999476e016e 

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


Testing (updated)
---

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


Thanks,

Anurag Singh

Re: Review Request 44289: Added support for contender and detector modules.

[Anurag Singh]

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

(Updated March 15, 2016, 11:02 p.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

Added support for contender and detector modules.


Diffs
-

  include/mesos/module/contender.hpp PRE-CREATION 
  include/mesos/module/detector.hpp PRE-CREATION 
  src/Makefile.am 7ee5a653fc96907021f14ab28f74c0b3ed0649d9 
  src/examples/test_contender_module.cpp PRE-CREATION 
  src/examples/test_detector_module.cpp PRE-CREATION 
  src/local/local.cpp f8599e7378e9a0065bbd01ad8f23f11debb30c91 
  src/master/contenders/zookeeper.cpp 9ad49ce10439fb41d78d52eaa4c1e6b9c5c7f735 
  src/master/detectors/zookeeper.cpp PRE-CREATION 
  src/module/manager.cpp 8c9aaf7cd00c904daba9994a99df9e1329831c01 
  src/tests/module.hpp 4b32f29f2ce76100433621a5cb6b8cc87c9b38f8 
  src/tests/module.cpp 8cc305c0ef606b07eea39d548d3165a2bb2b042a 

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


Testing (updated)
---

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


Thanks,

Anurag Singh

Re: Review Request 44547: Added functions in promises to the collect header.

[Anurag Singh]

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

(Updated March 15, 2016, 11:02 p.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

Added functions in promises to the collect header.


Diffs
-

  3rdparty/libprocess/include/process/collect.hpp 
5a92b72eb7668494dc832ec446a41b3d673a20cc 

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


Testing (updated)
---

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


Thanks,

Anurag Singh

Re: Review Request 44545: Separated standalone and zookeeper classes.

[Anurag Singh]

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

(Updated March 15, 2016, 11:01 p.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

Instead of keeping standalone and zookeper contender/detector class
definitions and implementations in the same file, separated them. Also
made the necessary changes in users of class headers to point to the
new locations.


Diffs
-

  src/Makefile.am 7ee5a653fc96907021f14ab28f74c0b3ed0649d9 
  src/local/local.cpp f8599e7378e9a0065bbd01ad8f23f11debb30c91 
  src/master/contender.hpp 3fd20f8e94daab349b76d8f5ecc87398a187a847 
  src/master/contender.cpp 9ad49ce10439fb41d78d52eaa4c1e6b9c5c7f735 
  src/master/contenders/contender.hpp PRE-CREATION 
  src/master/contenders/contender.cpp PRE-CREATION 
  src/master/contenders/standalone.cpp PRE-CREATION 
  src/master/detector.hpp eb5d2a90b60c629150ddf04acf00f0edca1ca723 
  src/master/detector.cpp 9274435802d6292b183be48f42b43999476e016e 
  src/master/detectors/detector.hpp PRE-CREATION 
  src/master/detectors/detector.cpp PRE-CREATION 
  src/master/detectors/standalone.cpp PRE-CREATION 
  src/master/main.cpp 7c1656bcc266f6c94cb4befad37fa813a218b2fa 
  src/master/master.hpp 124d43931a5c8a00ee0aaa604feb1761795209f2 
  src/slave/main.cpp e3a4d13ddaeb89ba01c9b2ddfc72c37934f753eb 
  src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 
  src/tests/cluster.cpp e5796d3cc17f814bec8f02dccf40515b65cfea91 
  src/tests/containerizer/external_containerizer_test.cpp 
8e1dbe306a088eb16cd3b9c6174b95fad5685da4 
  src/tests/containerizer/isolator_tests.cpp 
342037ce0a5f8caa4e3cf1550b8f9a7cc328acf9 
  src/tests/fault_tolerance_tests.cpp 349669d6aa0ead63b2ebcfcc2f769c99a6db0192 
  src/tests/gc_tests.cpp 61a8abb9581dc4602b197a88a677b19386969cbf 
  src/tests/master_allocator_tests.cpp cba7c36471f93b678d94e1da0251a28a893696b1 
  src/tests/master_authorization_tests.cpp 
6a4de4709960d7ca505e99396e14a1bb51d6902d 
  src/tests/master_contender_detector_tests.cpp 
255ab8119a04b55bb4f1b61dee19c4be64499376 
  src/tests/master_slave_reconciliation_tests.cpp 
d41178eb41df519073fc0890c5716bbc9fed6ad2 
  src/tests/master_tests.cpp e8c39e775d6ca218ce74cfc6bb50c7576d73e90e 
  src/tests/mesos.hpp 908a53a146208a6d41c0bda5208415e39e2eae05 
  src/tests/oversubscription_tests.cpp e528476cd83b0e3f7ae8cea7d86dfabc1f66484e 
  src/tests/partition_tests.cpp 3776a0a104582f60b9f19ea58b011485194399b9 
  src/tests/persistent_volume_tests.cpp 
e9215de2e073025f67cdc73e8a8de38cf030671f 
  src/tests/reconciliation_tests.cpp e8f3f29836652d20a6ee1bb5231a15e71eb76990 
  src/tests/reservation_tests.cpp c88091635949037fcfc6be504043764f9af35f79 
  src/tests/scheduler_event_call_tests.cpp 
8c02ceeb3ec1783cb2f63f100700508e70f586e4 
  src/tests/scheduler_http_api_tests.cpp 
dfb0f51fec67a3951e396eab28eedb0dbf9493ae 
  src/tests/scheduler_tests.cpp fa42fb42f2d18060a867ade547cebbdcaead07d4 
  src/tests/slave_recovery_tests.cpp bd7b94f3f1fac6705e5bf14c6f6103b540cde56c 
  src/tests/slave_tests.cpp 124e9587180f2a55e659d966d1c9060234c19457 

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


Testing (updated)
---

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


Thanks,

Anurag Singh

Re: Review Request 44544: Moved contender and detector definitions into separate directories.

[Anurag Singh]

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

(Updated March 15, 2016, 11:01 p.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

Moved contender and detector definitions into separate directories.


Diffs
-

  src/master/contenders/contender.hpp PRE-CREATION 
  src/master/contenders/contender.cpp PRE-CREATION 
  src/master/detectors/detector.hpp PRE-CREATION 
  src/master/detectors/detector.cpp PRE-CREATION 

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


Testing (updated)
---

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


Thanks,

Anurag Singh

Re: Review Request 44543: Removed unnecessary MasterContender and MasterDetector definitions.

[Anurag Singh]

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

(Updated March 15, 2016, 11:01 p.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

MasterContender and MasterDetector are now defined in
include/mesos/master/contender.hpp and detector.hpp.


Diffs
-

  src/master/contender.hpp 3fd20f8e94daab349b76d8f5ecc87398a187a847 
  src/master/detector.hpp eb5d2a90b60c629150ddf04acf00f0edca1ca723 

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


Testing (updated)
---

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


Thanks,

Anurag Singh

Re: Review Request 44287: Added MasterContender and MasterDetector abstract classes.

[Anurag Singh]

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

(Updated March 15, 2016, 11:01 p.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

MasterContender and MasterDetector abstract classes can be subclassed by
user-provided classes to allow for arbitrary contender/detector implementations.


Diffs
-

  include/mesos/master/contender.hpp PRE-CREATION 
  include/mesos/master/detector.hpp PRE-CREATION 

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


Testing (updated)
---

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


Thanks,

Anurag Singh

Re: Review Request 44288: Changed MasterDetector/Contender namespace.

[Anurag Singh]

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

(Updated March 15, 2016, 11:01 p.m.)


Review request for mesos and Benjamin Hindman.


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


Repository: mesos


Description
---

Also modified users of MasterContender and MasterDetector to use this
namespace.


Diffs
-

  include/mesos/scheduler.hpp 14c7ff964aa7b94f439d16e605380661d2279d54 
  include/mesos/v1/scheduler.hpp 765935e97b6c1686ab464a5cf1cf2dfd816f51f1 
  src/cli/resolve.cpp 257e29034abf32491511f9a4e476b6859714829d 
  src/local/local.cpp f8599e7378e9a0065bbd01ad8f23f11debb30c91 
  src/master/contender.cpp 9ad49ce10439fb41d78d52eaa4c1e6b9c5c7f735 
  src/master/detector.cpp 9274435802d6292b183be48f42b43999476e016e 
  src/master/main.cpp 7c1656bcc266f6c94cb4befad37fa813a218b2fa 
  src/master/master.hpp 124d43931a5c8a00ee0aaa604feb1761795209f2 
  src/master/master.cpp d0380db3b90a9166607445f8dd50cc63d547228e 
  src/sched/sched.cpp 525255eec808c3fe5c0e38b3d1a2086bbd4eb171 
  src/scheduler/scheduler.cpp 35f479483740baed3f7bdbf45bf1d5bba3b9febc 
  src/slave/main.cpp e3a4d13ddaeb89ba01c9b2ddfc72c37934f753eb 
  src/slave/slave.hpp 7520cc356e2b1b7f5fff15f33071a46a7b05e762 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
  src/tests/authentication_tests.cpp 85f14c3d453ca5aeffa1c915f38fe3031c2cf712 
  src/tests/cluster.hpp 99a785ab0d4ee1a1e745202d2551de58a7631a85 
  src/tests/cluster.cpp e5796d3cc17f814bec8f02dccf40515b65cfea91 
  src/tests/fault_tolerance_tests.cpp 349669d6aa0ead63b2ebcfcc2f769c99a6db0192 
  src/tests/master_allocator_tests.cpp cba7c36471f93b678d94e1da0251a28a893696b1 
  src/tests/master_authorization_tests.cpp 
6a4de4709960d7ca505e99396e14a1bb51d6902d 
  src/tests/master_contender_detector_tests.cpp 
255ab8119a04b55bb4f1b61dee19c4be64499376 
  src/tests/master_slave_reconciliation_tests.cpp 
d41178eb41df519073fc0890c5716bbc9fed6ad2 
  src/tests/master_tests.cpp e8c39e775d6ca218ce74cfc6bb50c7576d73e90e 
  src/tests/mesos.hpp 908a53a146208a6d41c0bda5208415e39e2eae05 
  src/tests/mesos.cpp 7cca4ed4753fa365b209d1d22f0df4650b19bc6a 
  src/tests/oversubscription_tests.cpp e528476cd83b0e3f7ae8cea7d86dfabc1f66484e 
  src/tests/partition_tests.cpp 3776a0a104582f60b9f19ea58b011485194399b9 
  src/tests/persistent_volume_tests.cpp 
e9215de2e073025f67cdc73e8a8de38cf030671f 
  src/tests/reconciliation_tests.cpp e8f3f29836652d20a6ee1bb5231a15e71eb76990 
  src/tests/reservation_tests.cpp c88091635949037fcfc6be504043764f9af35f79 
  src/tests/scheduler_event_call_tests.cpp 
8c02ceeb3ec1783cb2f63f100700508e70f586e4 
  src/tests/scheduler_http_api_tests.cpp 
dfb0f51fec67a3951e396eab28eedb0dbf9493ae 
  src/tests/scheduler_tests.cpp fa42fb42f2d18060a867ade547cebbdcaead07d4 
  src/tests/slave_recovery_tests.cpp bd7b94f3f1fac6705e5bf14c6f6103b540cde56c 
  src/tests/slave_tests.cpp 124e9587180f2a55e659d966d1c9060234c19457 

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


Testing (updated)
---

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


Thanks,

Anurag Singh

Re: Review Request 44834: Documented task, executor, and volume IDs reuse is discouraged.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44834]

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 March 15, 2016, 5:54 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44834/
> ---
> 
> (Updated March 15, 2016, 5:54 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-2198
> https://issues.apache.org/jira/browse/MESOS-2198
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is generally a bad idea for (at least) two reasons:
> 
> (1) Determining when the previous entity with the ID has terminated
> can be non-trivial. Several frameworks have done this incorrectly
> in the past.
> 
> (2) When reusing IDs, logs can be more difficult to read because the
> same ID can refer to different entities at different times.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 56d456acfd35fa59f394b27d62f62772eec03f6a 
>   include/mesos/v1/mesos.proto 4fba77464bb052d27c424f7721397142850b1144 
> 
> Diff: https://reviews.apache.org/r/44834/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 44708: Fixed signed / unsigned comparison in docker.cpp.

[Alexander Rukletsov]

> On March 15, 2016, 10:37 p.m., Ben Mahler wrote:
> > src/slave/constants.hpp, line 102
> > 
> >
> > We tend to use size_t for things that are a count or a size. Otherwise, 
> > we'll tend to use `unsigned int` rather than uint32_t.

Right, here I picked `uint32_t` for consistency with all other unsigned 
constants in this file.


- Alexander


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


On March 15, 2016, 2:28 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44708/
> ---
> 
> (Updated March 15, 2016, 2:28 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp 0b4e41b4eee3280515cf8a1f049fe923503066f1 
> 
> Diff: https://reviews.apache.org/r/44708/diff/
> 
> 
> Testing
> ---
> 
> The complete chain was tested. See https://reviews.apache.org/r/44662/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 44230: Added FS_DEFAULT case in rmdir.

[Jojy Varghese]

Yes thats correct. I am planning to submit a patch that will add the test. Will 
also address the symlink case.

-jojy

> On Mar 15, 2016, at 3:33 PM, Cong Wang  wrote:
> 
> This is an automatically generated e-mail. To reply, 
> visit:https://reviews.apache.org/r/44230/ 
> 
> On March 15th, 2016, 8:02 p.m. UTC, Neil Conway wrote:
> 
> Is it feasible/portable to have a test case for this change?
> On March 15th, 2016, 8:58 p.m. UTC, Cong Wang wrote:
> 
> Yes, like in our case, you can create some socket or device file and try to 
> remove the directory contains it, it would fail without this patch.
> On March 15th, 2016, 9:26 p.m. UTC, David Robinson wrote:
> 
> AFAICT tests have already been added:
> https://github.com/apache/mesos/commit/c55ca2941b0119c13764bccdebcea46119e69e4e
> None of these tests covers this bug.
> 
> - Cong
> 
> 
> On March 1st, 2016, 10:01 p.m. UTC, Jojy Varghese wrote:
> 
> Review request for mesos and Jie Yu.
> By Jojy Varghese.
> Updated March 1, 2016, 10:01 p.m.
> 
> Repository: mesos
> Description
> 
> We currently dont handle special files like device files in rmdir. This change
> adds FS_DEFAULT as one of the cases where we try to unlink a file.
> 
> Reference: http://man7.org/linux/man-pages/man3/fts.3.html
> Testing
> 
> make check.
> Diffs
> 
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> (bc420c9c10d93ddd619a9eb2c5f4db67f31d722f)
> View Diff 

Re: Review Request 44708: Fixed signed / unsigned comparison in docker.cpp.

[Ben Mahler]

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


Fix it, then Ship it!





src/slave/constants.hpp (line 102)


We tend to use size_t for things that are a count or a size. Otherwise, 
we'll tend to use `unsigned int` rather than uint32_t.


- Ben Mahler


On March 15, 2016, 2:28 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44708/
> ---
> 
> (Updated March 15, 2016, 2:28 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp 0b4e41b4eee3280515cf8a1f049fe923503066f1 
> 
> Diff: https://reviews.apache.org/r/44708/diff/
> 
> 
> Testing
> ---
> 
> The complete chain was tested. See https://reviews.apache.org/r/44662/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 44659: Updated the comment about docker executor.

[Ben Mahler]

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


Ship it!





src/docker/executor.cpp (lines 60 - 64)


Thanks, the original comment was pretty confusing, could you also reach out 
to tim to ask him if there was anything else that we should call out here?


- Ben Mahler


On March 15, 2016, 2:28 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44659/
> ---
> 
> (Updated March 15, 2016, 2:28 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 1921d4a1ce3c45b4e2f81f0ef5914d5830da6866 
> 
> Diff: https://reviews.apache.org/r/44659/diff/
> 
> 
> Testing
> ---
> 
> The complete chain was tested. See https://reviews.apache.org/r/44662/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 44230: Added FS_DEFAULT case in rmdir.

[Cong Wang]

> On March 15, 2016, 8:02 p.m., Neil Conway wrote:
> > Is it feasible/portable to have a test case for this change?
> 
> Cong Wang wrote:
> Yes, like in our case, you can create some socket or device file and try 
> to remove the directory contains it, it would fail without this patch.
> 
> David Robinson wrote:
> AFAICT tests have already been added:
> 
> 
> https://github.com/apache/mesos/commit/c55ca2941b0119c13764bccdebcea46119e69e4e

None of these tests covers this bug.


- Cong


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


On March 1, 2016, 10:01 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44230/
> ---
> 
> (Updated March 1, 2016, 10:01 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We currently dont handle special files like device files in rmdir. This change
> adds FS_DEFAULT as one of the cases where we try to unlink a file.
> 
> Reference: http://man7.org/linux/man-pages/man3/fts.3.html
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> bc420c9c10d93ddd619a9eb2c5f4db67f31d722f 
> 
> Diff: https://reviews.apache.org/r/44230/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44230: Added FS_DEFAULT case in rmdir.

[Jojy Varghese]

Would be happy to add test. Patch forthcoming. Will also address FTS_SLNONE.

-Jojy


> On Mar 15, 2016, at 2:26 PM, David Robinson  
> wrote:
> 
> 
> 
>> On March 15, 2016, 8:02 p.m., Neil Conway wrote:
>>> Is it feasible/portable to have a test case for this change?
>> 
>> Cong Wang wrote:
>>Yes, like in our case, you can create some socket or device file and try 
>> to remove the directory contains it, it would fail without this patch.
> 
> AFAICT tests have already been added:
> 
> https://github.com/apache/mesos/commit/c55ca2941b0119c13764bccdebcea46119e69e4e
> 
> 
> - David
> 
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44230/#review123733
> ---
> 
> 
> On March 1, 2016, 10:01 p.m., Jojy Varghese wrote:
>> 
>> ---
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/44230/
>> ---
>> 
>> (Updated March 1, 2016, 10:01 p.m.)
>> 
>> 
>> Review request for mesos and Jie Yu.
>> 
>> 
>> Repository: mesos
>> 
>> 
>> Description
>> ---
>> 
>> We currently dont handle special files like device files in rmdir. This 
>> change
>> adds FS_DEFAULT as one of the cases where we try to unlink a file.
>> 
>> Reference: http://man7.org/linux/man-pages/man3/fts.3.html
>> 
>> 
>> Diffs
>> -
>> 
>>  3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
>> bc420c9c10d93ddd619a9eb2c5f4db67f31d722f 
>> 
>> Diff: https://reviews.apache.org/r/44230/diff/
>> 
>> 
>> Testing
>> ---
>> 
>> make check.
>> 
>> 
>> Thanks,
>> 
>> Jojy Varghese
>> 
>> 
> 

Re: Review Request 44658: Removed unused signal escalation constant.

[Ben Mahler]

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


Ship it!




Ship It!

- Ben Mahler


On March 15, 2016, 2:28 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44658/
> ---
> 
> (Updated March 15, 2016, 2:28 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-1571
> https://issues.apache.org/jira/browse/MESOS-1571
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp 0b4e41b4eee3280515cf8a1f049fe923503066f1 
> 
> Diff: https://reviews.apache.org/r/44658/diff/
> 
> 
> Testing
> ---
> 
> The complete chain was tested. See https://reviews.apache.org/r/44662/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 44657: Used `KillPolicy` and shutdown grace period in command executor.

[Ben Mahler]

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




src/launcher/executor.cpp (lines 121 - 125)


Ditto from previous review comments, could you adjust the comment and logic 
to reflect that it's not a default anymore?



src/launcher/executor.cpp (lines 474 - 475)


Could you be more explicit about what this guarantee is? i.e. mentioning 
that the agent, when determining the executor's shtudown grace period, will 
choose it based on the kill policy.

Also, it seems surprising that killTask calls the version of shutdown here 
that doesn't take the time, why are we looking at the shutdown grace period if 
only a killTask was issued? I see why we pick the smaller if a shutdown was 
issued, but if only a killTask was issued, it seems we can ignore the shutdown 
period?



src/launcher/executor.cpp (line 498)


Hm.. it's unfortuate we returned an Option for this overload of min, it 
should have been just a T. Oh well.



src/launcher/executor.cpp (line 499)


I'd expect that the buffer is in addition to dealing with the reap 
interval, since the reap interval is not really a buffer but a time that we are 
expecting to wait. Could you add maybe an additional 1s buffer on top of the 
reap interval?

Note that we could improve the reaper to block auxiliary threads when the 
process is a child, rather than polling for children. I had a TODO for this:

```
// TODO(bmahler): This can be optimized to use a thread per pid, where
// each thread makes a blocking call to waitpid. This eliminates the
// unfortunate poll delay.
```



src/launcher/executor.cpp (line 667)


Extra `<<` on this line



src/launcher/executor.cpp (lines 931 - 937)


Hm.. how about:

```
  // Get executor shutdown grace period from the environment.
  //
  // Note that we avoided introducing a command executor flag
  // for this because the command executor crashes if it sees
  // an unknown flag. This makes it difficult to add command
  // executor flags that are unconditionally set by the agent.
```



src/launcher/executor.cpp (line 943)


Please put the trailing space on the next line, so:

```
  cerr << "Failed to parse value '" << value.get() << "'"
   << " of 'MESOS_EXECUTOR_SHUTDOWN_GRACE_PERIOD': " << 
parse.error();
```

We've found this style makes it clearer that no spaces were forgotten, if 
you think about it, it's not the previous line that should be responsible for 
adding a space, it is the line that adds more information that is responsible 
for spacing itself from the previously added information.


- Ben Mahler


On March 15, 2016, 4:04 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44657/
> ---
> 
> (Updated March 15, 2016, 4:04 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Gilbert Song.
> 
> 
> Bugs: MESOS-4909
> https://issues.apache.org/jira/browse/MESOS-4909
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The command executor determines how much time it allots the
> underlying task to clean up (effectively how long to wait for
> the task to comply to SIGTERM before sending SIGKILL) based
> on both optional task's `KillPolicy` and optional
> `shutdown_grace_period` field in `ExecutorInfo`.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 4149f084b0b234fb995df04e9d1ca704feec082e 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
> 
> Diff: https://reviews.apache.org/r/44657/diff/
> 
> 
> Testing
> ---
> 
> The complete chain was tested. See https://reviews.apache.org/r/44662/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 44837: Added force_pull_image to Image protobuf.

[Gilbert Song]

> On March 15, 2016, 9:42 a.m., Jie Yu wrote:
> > include/mesos/mesos.proto, line 1501
> > 
> >
> > s/force_pull_image/no_cache/
> > 
> > IMO, force pull is a mean, not a description. Please adjust the 
> > comments accordingly.

Shouldn't we make it consensus with docker containerizer?


- Gilbert


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


On March 15, 2016, 12:31 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44837/
> ---
> 
> (Updated March 15, 2016, 12:31 a.m.)
> 
> 
> Review request for mesos, haosdent huang and Jie Yu.
> 
> 
> Bugs: MESOS-4886
> https://issues.apache.org/jira/browse/MESOS-4886
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added force_pull_image to Image protobuf.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto deb9c0910a27afd67276f54b3f666a878212727b 
>   include/mesos/v1/mesos.proto a981e750c24cfc48177bbc9ca56f0c3ecfae1a1b 
> 
> Diff: https://reviews.apache.org/r/44837/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 44753: Fixed runtime isolator tests out of disk issue.

[Jie Yu]

> On March 15, 2016, 1:28 a.m., Guangya Liu wrote:
> > src/tests/containerizer/runtime_isolator_tests.cpp, lines 96-98
> > 
> >
> > The comments should also be updated? Also should mention that do not 
> > want use up disk due to work dir may be copied to rootfs?
> > 
> > Ditto for other `flags.docker_store_dir`

I think the comments here still apply, right?


- Jie


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


On March 15, 2016, 12:22 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44753/
> ---
> 
> (Updated March 15, 2016, 12:22 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4942
> https://issues.apache.org/jira/browse/MESOS-4942
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed runtime isolator tests out of disk issue.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 9fd5ea9e465f21624d3cdc0125a702528fe7548b 
>   src/tests/containerizer/runtime_isolator_tests.cpp 
> d8a000413fb30d7daaa77827287a2395d81b8b04 
> 
> Diff: https://reviews.apache.org/r/44753/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-test.sh
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 44554: Added agent HTTP authentication to the docs.

[Greg Mann]

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

(Updated March 15, 2016, 9:32 p.m.)


Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
Toenshoff.


Changes
---

Removed documentation of deprecated credentials format.


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


Repository: mesos


Description
---

Added agent HTTP authentication to the docs.


Diffs (updated)
-

  docs/authentication.md e7c0bf3ed331411f607e7622419f14853006a480 
  docs/configuration.md 739d4ff9aeeb1ba70ce11033168d63d37b6ef56b 
  docs/home.md fd7794f56b4a95268dbb82288d99cab19a89f5cd 

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


Testing
---

Viewed with the Mesos website container: 
https://github.com/mesosphere/mesos-website-container


Thanks,

Greg Mann

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Greg Mann]

> On March 15, 2016, 10:33 a.m., Joerg Schad wrote:
> > src/slave/flags.cpp, line 682
> > 
> >
> > As we are actively deprecated the old text based format with Mesos-228, 
> > does it make sense to only document the new json format here?
> 
> Joerg Schad wrote:
> Mesos-2281
> 
> Greg Mann wrote:
> Perhaps we should see which patch lands first, and update the other one 
> accordingly? I can put a comment on Jan's review so that he's aware of this 
> one.

After looking at Jan's patch again I see that it's just beginning the 
deprecation cycle for that change; I think it makes sense to introduce the 
`--http_credentials` flag using only the JSON format, regardless of which patch 
lands first. I altered this patch chain accordingly. Thanks Joerg!


- Greg


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


On March 15, 2016, 9:30 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44515/
> ---
> 
> (Updated March 15, 2016, 9:30 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flags for HTTP authentication.
> 
> Three command-line flags have been added to the agent to enable HTTP 
> authentication: `--authenticate_http`, `--http_credentials`, and 
> `--http_authenticators`.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp 0b4e41b4eee3280515cf8a1f049fe923503066f1 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp 4d10818105627738e258116647ccada374e3d7b9 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/mesos.cpp 7cca4ed4753fa365b209d1d22f0df4650b19bc6a 
> 
> Diff: https://reviews.apache.org/r/44515/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` was used to test on both OSX and Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 44846: Deprecated the plain text credential format.

[Greg Mann]

> On March 15, 2016, 6:53 p.m., Greg Mann wrote:
> > FYI, this review: https://reviews.apache.org/r/44515/ adds command-line 
> > credentials to the agent with the `--http_credentials` flag. Depending on 
> > which one of these lands first, the other one should be updated to remain 
> > consistent.

Nevermind, I think it makes sense to simply remove documentation of the 
deprecated format from my patch, as Joerg suggested. No extra work needed on 
your part, Jan!


- Greg


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


On March 15, 2016, 2:01 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44846/
> ---
> 
> (Updated March 15, 2016, 2:01 p.m.)
> 
> 
> Review request for mesos, Adam B and Joerg Schad.
> 
> 
> Bugs: MESOS-2281
> https://issues.apache.org/jira/browse/MESOS-2281
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Deprecated the plain text credential format.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 761238c48332bcce0bff6c411225fdb4176ddca6 
>   docs/authentication.md e7c0bf3ed331411f607e7622419f14853006a480 
>   docs/configuration.md 739d4ff9aeeb1ba70ce11033168d63d37b6ef56b 
>   docs/upgrades.md 83b839f7fb996385baaa6ef1081dc1116cd6e338 
>   src/credentials/credentials.hpp aad17c21bb0ce98907bbfa22b890b66130e081e4 
>   src/master/flags.cpp e6fea6421ea1a16b9cd78b0e42b830829b95ad61 
>   src/slave/flags.cpp 4d10818105627738e258116647ccada374e3d7b9 
> 
> Diff: https://reviews.apache.org/r/44846/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Greg Mann]

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

(Updated March 15, 2016, 9:30 p.m.)


Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
Toenshoff.


Changes
---

Removed documentation of deprecated credentials format.


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


Repository: mesos


Description
---

Added agent flags for HTTP authentication.

Three command-line flags have been added to the agent to enable HTTP 
authentication: `--authenticate_http`, `--http_credentials`, and 
`--http_authenticators`.


Diffs (updated)
-

  src/slave/constants.hpp 0b4e41b4eee3280515cf8a1f049fe923503066f1 
  src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
  src/slave/flags.cpp 4d10818105627738e258116647ccada374e3d7b9 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
  src/tests/mesos.cpp 7cca4ed4753fa365b209d1d22f0df4650b19bc6a 

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


Testing
---

`sudo make check` was used to test on both OSX and Ubuntu 14.04.


Thanks,

Greg Mann

Re: Review Request 44230: Added FS_DEFAULT case in rmdir.

[David Robinson]

> On March 15, 2016, 8:02 p.m., Neil Conway wrote:
> > Is it feasible/portable to have a test case for this change?
> 
> Cong Wang wrote:
> Yes, like in our case, you can create some socket or device file and try 
> to remove the directory contains it, it would fail without this patch.

AFAICT tests have already been added:

https://github.com/apache/mesos/commit/c55ca2941b0119c13764bccdebcea46119e69e4e


- David


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


On March 1, 2016, 10:01 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44230/
> ---
> 
> (Updated March 1, 2016, 10:01 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We currently dont handle special files like device files in rmdir. This change
> adds FS_DEFAULT as one of the cases where we try to unlink a file.
> 
> Reference: http://man7.org/linux/man-pages/man3/fts.3.html
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> bc420c9c10d93ddd619a9eb2c5f4db67f31d722f 
> 
> Diff: https://reviews.apache.org/r/44230/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44758: Upgrade to clang-format-3.8 (MESOS-4906).

[Michael Park]

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


Fix it, then Ship it!





support/clang-format (lines 24 - 35)


These do nothing unless `BreakBeforeBraces` is set to `Custom`. Our style 
matches `Mozilla`, so let's remove these.



support/clang-format (line 37)


`s/Linux/Mozilla/`



support/clang-format (lines 50 - 56)


We can help out a little more here:

The following code orders the headers:
(1) C standard library headers
(2) C++ standard library headers
(3) Our public headers
(4) Our private headers

```
IncludeCategories:
  - Regex:   '^<.*.h>'
Priority:1
  - Regex:   '^<.*.hpp>'
Priority:3
  - Regex:   '^<.*>'
Priority:2
  - Regex:   '.*'
Priority:4
```

__NOTE__: 3 seems to need to appear before 2, probably because it's more 
specific.


- Michael Park


On March 14, 2016, 1:09 a.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44758/
> ---
> 
> (Updated March 14, 2016, 1:09 a.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-4906
> https://issues.apache.org/jira/browse/MESOS-4906
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Upgrade to clang-format-3.8 (MESOS-4906).
> 
> 
> Diffs
> -
> 
>   docs/clang-format.md 7f1c1dfd70e1fe9bfa186df1bdda7bdcf867db04 
>   support/clang-format 499d0e749e14e50256ae649afa0ced2b04589a0e 
> 
> Diff: https://reviews.apache.org/r/44758/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Review Request 44867: Fixed linux fs isolator multi containers test.

[Gilbert Song]

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

Review request for mesos, Jie Yu and Timothy Chen.


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


Repository: mesos


Description
---

Fixed linux fs isolator multi containers test.


Diffs
-

  src/tests/containerizer/filesystem_isolator_tests.cpp 
e72239a55724f1aeeec5362cc370c93dbeca7164 

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


Testing
---

make check 

sudo ./bin/mesos-test.sh


Thanks,

Gilbert Song

Re: Review Request 44230: Added FS_DEFAULT case in rmdir.

[Cong Wang]

> On March 15, 2016, 8:02 p.m., Neil Conway wrote:
> > Is it feasible/portable to have a test case for this change?

Yes, like in our case, you can create some socket or device file and try to 
remove the directory contains it, it would fail without this patch.


- Cong


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


On March 1, 2016, 10:01 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44230/
> ---
> 
> (Updated March 1, 2016, 10:01 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We currently dont handle special files like device files in rmdir. This change
> adds FS_DEFAULT as one of the cases where we try to unlink a file.
> 
> Reference: http://man7.org/linux/man-pages/man3/fts.3.html
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> bc420c9c10d93ddd619a9eb2c5f4db67f31d722f 
> 
> Diff: https://reviews.apache.org/r/44230/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 43629: Especially updated tests to use the updated MesosTest helpers.

[Joseph Wu]

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

(Updated March 15, 2016, 1:48 p.m.)


Review request for mesos, Bernd Mathiske and Artem Harutyunyan.


Changes
---

Rebased and addressed a moved line in FetcherCacheTests.


Bugs: MESOS-4633 and MESOS-4634
https://issues.apache.org/jira/browse/MESOS-4633
https://issues.apache.org/jira/browse/MESOS-4634


Repository: mesos


Description
---

Continuation of https://reviews.apache.org/r/43615/ with a slightly different 
pattern.


Diffs (updated)
-

  src/tests/dynamic_weights_tests.cpp a95663f633f376954d4201b6cfbe693429be9c39 
  src/tests/fetcher_cache_tests.cpp 776c95267caff7b27cd70c2fa6149d8158c86750 
  src/tests/resource_offers_tests.cpp 0bad45dd1dabecc88fef1ab46e8ea26718070b33 
  src/tests/slave_recovery_tests.cpp bd7b94f3f1fac6705e5bf14c6f6103b540cde56c 

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


Testing
---

Tests are run at the end of this review chain.


Thanks,

Joseph Wu

Re: Review Request 44707: Added validation for task's kill policy.

[Ben Mahler]

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



Could you add a corresponding test? Having it in the header should make this 
really easy :)

- Ben Mahler


On March 15, 2016, 3:47 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44707/
> ---
> 
> (Updated March 15, 2016, 3:47 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-4909
> https://issues.apache.org/jira/browse/MESOS-4909
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/validation.hpp 29dbdf1da540db4966ff9e86037badde06e69a4b 
>   src/master/validation.cpp 820a9faee6fde09a98317854b181fe897167e2ca 
> 
> Diff: https://reviews.apache.org/r/44707/diff/
> 
> 
> Testing
> ---
> 
> The complete chain was tested. See https://reviews.apache.org/r/44662/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 44656: Introduced `KillPolicy` protobuf.

[Ben Mahler]

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



Looks great, just some suggestions for clarifying the documentation.


include/mesos/mesos.proto (lines 347 - 352)


I'm realizing I should have made this more clearly separate out the 
interpretation for command tasks.

Could you rephrase this to say that this allows a configurable grace period 
between gently and forcibly killing the task? Then we can say that for 
executor-less command-based tasks, mesos will currently perfrom the graceful 
kill via a SIGTERM to the process and the forcible kill via a SIGKILL to the 
process.

Does that seem clearer for the users?



include/mesos/mesos.proto (lines 1240 - 1241)


I'm thinking we need to clarify that the health check / kill policy are 
provided for executor-less tasks (where mesos uses a built-in executor). And 
for tasks that specify an executor, it is the executor's responsibility to 
provide the health checking and kill policy.

Does that seem clearer for the users?



include/mesos/v1/mesos.proto (lines 359 - 360)


This is missing the NOTE from the non-v1 version?


- Ben Mahler


On March 15, 2016, 3:54 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44656/
> ---
> 
> (Updated March 15, 2016, 3:54 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-4909
> https://issues.apache.org/jira/browse/MESOS-4909
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Describes a kill policy for a task. Currently does not express
> different policies (e.g. hitting HTTP endpoints). For now, this
> controls how long to wait between SIGTERM and SIGKILL.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto deb9c0910a27afd67276f54b3f666a878212727b 
>   include/mesos/v1/mesos.proto a981e750c24cfc48177bbc9ca56f0c3ecfae1a1b 
> 
> Diff: https://reviews.apache.org/r/44656/diff/
> 
> 
> Testing
> ---
> 
> The complete chain was tested. See https://reviews.apache.org/r/44662/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 44380: Change IOTest.BufferedRead to write to the temporary directory.

[Joris Van Remoortere]

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


Ship it!




Ship It!

- Joris Van Remoortere


On March 15, 2016, 3:37 p.m., Yong Tang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44380/
> ---
> 
> (Updated March 15, 2016, 3:37 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-4807
> https://issues.apache.org/jira/browse/MESOS-4807
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit changes IOTest.BufferedRead so that this specific
> test (not all IOTest) could be executed from temporary
> directories via TemporaryDirectoryTest fixture (MESOS-4807).
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/io_tests.cpp 
> 2bffc7cd9c3aa204a1d1b8eb45f0bff12f49ca62 
> 
> Diff: https://reviews.apache.org/r/44380/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Yong Tang
> 
>

Re: Review Request 44853: Added benchmark test for the allocator metrics endpoint.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44853]

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 March 15, 2016, 2:52 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44853/
> ---
> 
> (Updated March 15, 2016, 2:52 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added benchmark test for the allocator metrics endpoint.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 459e02576f6d05abbbcc83ae5cabac5c66e93f05 
> 
> Diff: https://reviews.apache.org/r/44853/diff/
> 
> 
> Testing
> ---
> 
> The benchmark uses the same parametrized setup as other 
> `HierarchicalAllocator_BENCHMARK_Tests` which already elsewhere take 
> considerable time. The reason for covering the same parameter space here was 
> the assumption that that parameter space does capture the relevant scenarios.
> 
> The benchmark shows that the time needed to obtain the metrics has a linear 
> relationship with the number of registered frameworks, roughly independent of 
> the number of slaves. With my setup, the time per framework was well below 1 
> ms after already a few frameworks.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 44655: Made `shutdown_grace_period` configurable in `ExecutorInfo`.

[Ben Mahler]

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



Looking good! Just some minor logical tweaks and some updates to the 
documentation.


include/mesos/executor/executor.proto (lines 41 - 55)


Ditto the comments I made on the v1 changes below.



include/mesos/v1/executor/executor.proto (lines 47 - 48)


The "or" here is going to lead to some confusion, which takes precendence? 
I'd suggest we just clarify that the environment variable is how they find it, 
and they can configure it with the ExecutorInfo field. Thoughts?



include/mesos/v1/executor/executor.proto (lines 53 - 55)


Now that the period within the Shutdown message was removed, we should 
probably have a TODO following this note for adding the period into the 
Shutdown message so that the agent can communicate when a shorter period has 
been alloted.



src/exec/exec.cpp (line 235)


Ditto the other comment, could we update this to reflect that it's not a 
default anymore? Perhaps the logic here should be to set if not found in 
environment.



src/exec/exec.cpp (lines 713 - 714)


Per the other suggestions, can you update this to reflect that it's not a 
default?



src/exec/exec.cpp (lines 715 - 716)


Can you confirm the environment variable setting didn't make it in 0.28.0?



src/executor/executor.cpp (lines 235 - 236)


Not sure if you grabbed these comments from my reviews, but I had been 
thinking about the environment variable only specifying the default (i.e. 
DEFAULT_MESOS_EXECUTOR_SHUTDOWN_GRACE_PERIOD rather than 
MESOS_EXECUTOR_SHUTDOWN_GRACE_PERIOD).

Since MESOS_EXECUTOR_SHUTDOWN_GRACE_PERIOD now contains the expected value, 
we should just document that it's available in both places (so long as the 
agent is new enough to be setting the environment variable). It would bge 
helpful to mention that it's in the environment so that we know the shutdown 
time before we register with the agent.

I suppose the reasoning here is that for those executors that do not set 
`ExecutorInfo.shutdown_grace_period`, they still want to know how much time 
they have. Also executors may, for some reason that escapes me, want to know 
how much time they have if they have to shutdown before they register. This 
latter use case seems a bit strange.

Anyway, could you update these comments to reflect that the environment 
variable isn't a "default" anymore?



src/executor/executor.cpp (line 703)


I'm thinking now that we should trust the value in the environment if it's 
set, rather than overwriting, since the agent could set a smaller one. This 
would only happen if, rather than rejecting too large of timeouts, we bounded 
them down implicitly. There's also the case of the agent deciding it has less 
time, but that case would likely come at the time of the SHUTDOWN Event.

Perhaps this logic should be: set if not found in environment. And we 
should document that the agent will use the ExecutorInfo to set the environment 
value.



src/slave/slave.cpp (lines 4214 - 4215)


Would you mind wrapping this a bit more cleanly? Like the one above for 
example:

```
  // If the executor specifies shutdown grace period,
  // pass it instead of the default.
```


- Ben Mahler


On March 15, 2016, 3:49 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44655/
> ---
> 
> (Updated March 15, 2016, 3:49 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Gilbert Song.
> 
> 
> Bugs: MESOS-4949
> https://issues.apache.org/jira/browse/MESOS-4949
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If `ExecutorInfo.shutdown_grace_period` is set, the executor
> driver uses it, otherwise it falls back to the environment
> variable `MESOS_EXECUTOR_SHUTDOWN_GRACE_PERIOD`.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 739d4ff9aeeb1ba70ce11033168d63d37b6ef56b 
>   include/mesos/executor/executor.proto 
> ae211194a44e0bf2fadc79e833881e45ea3eb2c2 
>   include/mesos/mesos.proto deb9c0910a27afd67276f54b3f666a878212727b 
>   include/mesos/v1/executor/executor.proto 
> 36a2b3f9bc3aaa524f655b9e686a6d33512e6aaa 
>   include/mesos/v1/mesos.proto 

Re: Review Request 44230: Added FS_DEFAULT case in rmdir.

[Neil Conway]

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



Also, seems pretty clear we should handle `FTS_SLNONE`, as the TODO suggests.

- Neil Conway


On March 1, 2016, 10:01 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44230/
> ---
> 
> (Updated March 1, 2016, 10:01 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We currently dont handle special files like device files in rmdir. This change
> adds FS_DEFAULT as one of the cases where we try to unlink a file.
> 
> Reference: http://man7.org/linux/man-pages/man3/fts.3.html
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> bc420c9c10d93ddd619a9eb2c5f4db67f31d722f 
> 
> Diff: https://reviews.apache.org/r/44230/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44230: Added FS_DEFAULT case in rmdir.

[Neil Conway]

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



Is it feasible/portable to have a test case for this change?

- Neil Conway


On March 1, 2016, 10:01 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44230/
> ---
> 
> (Updated March 1, 2016, 10:01 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We currently dont handle special files like device files in rmdir. This change
> adds FS_DEFAULT as one of the cases where we try to unlink a file.
> 
> Reference: http://man7.org/linux/man-pages/man3/fts.3.html
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/rmdir.hpp 
> bc420c9c10d93ddd619a9eb2c5f4db67f31d722f 
> 
> Diff: https://reviews.apache.org/r/44230/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 43614: Refactor MesosTest and remove cleanup logic.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On March 4, 2016, 11:43 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43614/
> ---
> 
> (Updated March 4, 2016, 11:43 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-4633 and MESOS-4634
> https://issues.apache.org/jira/browse/MESOS-4633
> https://issues.apache.org/jira/browse/MESOS-4634
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updates `StartMaster` and `StartSlave` test helpers to use the reworked 
> `cluster` helpers.  Removes all `MesosTest` cleanup logic and as well as the 
> helpers that accept a `MockExecutor` pointer.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp d36840f6e23e5823332de53061bf6852330bdf0b 
>   src/tests/mesos.cpp e0f641c6828833de13a0a233e39ff6dc3f343d5c 
> 
> Diff: https://reviews.apache.org/r/43614/diff/
> 
> 
> Testing
> ---
> 
> Tests are run at the end of this review chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 43613: Refactor cluster test helpers into self-contained objects.

[Michael Park]

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


Ship it!




I think there are a few parts that are a bit clunky, the `zookeeperURL` for 
example, but I think this is a good first iteration to ship to make our tests 
more stable.

- Michael Park


On March 14, 2016, 9:31 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43613/
> ---
> 
> (Updated March 14, 2016, 9:31 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Bernd Mathiske, Artem 
> Harutyunyan, and Michael Park.
> 
> 
> Bugs: MESOS-4633 and MESOS-4634
> https://issues.apache.org/jira/browse/MESOS-4633
> https://issues.apache.org/jira/browse/MESOS-4634
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Major rewrite of the `tests/cluster` helpers.  This strongly ties the scope 
> of test objects to the test body.
> 
> Changes the `Cluster` class into two RAII objects (`Master` and `Slave`).  
> The `Slave` object performs cleanup originally found in 
> `cluster::Slave::stop`.  `cluster::Master::start` and `cluster::Slave::start` 
> were changed to factory methods.
> 
> 
> Diffs
> -
> 
>   src/tests/cluster.hpp 99a785ab0d4ee1a1e745202d2551de58a7631a85 
>   src/tests/cluster.cpp e5796d3cc17f814bec8f02dccf40515b65cfea91 
> 
> Diff: https://reviews.apache.org/r/43613/diff/
> 
> 
> Testing
> ---
> 
> Tests are run at the end of this review chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 44657: Used `KillPolicy` and shutdown grace period in command executor.

[James DeFelice]

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




src/launcher/executor.cpp (line 931)


I don't understand this comment as written. Can this be clarified?


- James DeFelice


On March 15, 2016, 4:04 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44657/
> ---
> 
> (Updated March 15, 2016, 4:04 p.m.)
> 
> 
> Review request for mesos, Ben Mahler and Gilbert Song.
> 
> 
> Bugs: MESOS-4909
> https://issues.apache.org/jira/browse/MESOS-4909
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The command executor determines how much time it allots the
> underlying task to clean up (effectively how long to wait for
> the task to comply to SIGTERM before sending SIGKILL) based
> on both optional task's `KillPolicy` and optional
> `shutdown_grace_period` field in `ExecutorInfo`.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 4149f084b0b234fb995df04e9d1ca704feec082e 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
> 
> Diff: https://reviews.apache.org/r/44657/diff/
> 
> 
> Testing
> ---
> 
> The complete chain was tested. See https://reviews.apache.org/r/44662/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 44846: Deprecated the plain text credential format.

[Greg Mann]

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



FYI, this review: https://reviews.apache.org/r/44515/ adds command-line 
credentials to the agent with the `--http_credentials` flag. Depending on which 
one of these lands first, the other one should be updated to remain consistent.

- Greg Mann


On March 15, 2016, 2:01 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44846/
> ---
> 
> (Updated March 15, 2016, 2:01 p.m.)
> 
> 
> Review request for mesos, Adam B and Joerg Schad.
> 
> 
> Bugs: MESOS-2281
> https://issues.apache.org/jira/browse/MESOS-2281
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Deprecated the plain text credential format.
> 
> 
> Diffs
> -
> 
>   CHANGELOG 761238c48332bcce0bff6c411225fdb4176ddca6 
>   docs/authentication.md e7c0bf3ed331411f607e7622419f14853006a480 
>   docs/configuration.md 739d4ff9aeeb1ba70ce11033168d63d37b6ef56b 
>   docs/upgrades.md 83b839f7fb996385baaa6ef1081dc1116cd6e338 
>   src/credentials/credentials.hpp aad17c21bb0ce98907bbfa22b890b66130e081e4 
>   src/master/flags.cpp e6fea6421ea1a16b9cd78b0e42b830829b95ad61 
>   src/slave/flags.cpp 4d10818105627738e258116647ccada374e3d7b9 
> 
> Diff: https://reviews.apache.org/r/44846/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 44654: Fixed hard-coded executor shutdown grace period in executor library.

[Ben Mahler]

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



Looks great! Main thing is documenting why we don't assume the environment 
variable is present, and which version of the agent sets it.


src/exec/exec.cpp (line 110)


If you'd like to add the `private` qualifier, why isn't `kill` left as 
protected?



src/exec/exec.cpp (line 707)


A comment about why we don't require it to be set like the others above 
(backwards-compatibility) would be helpful for the reader here. Also would be 
good to mention that agents that are 0.29.0 (or did those patches land in 
0.28.0?) and above set this, so that we don't have to do a deeper investigation 
later if we want to figure out which agent version sets this.


- Ben Mahler


On March 15, 2016, 2:15 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44654/
> ---
> 
> (Updated March 15, 2016, 2:15 p.m.)
> 
> 
> Review request for mesos and Ben Mahler.
> 
> 
> Bugs: MESOS-4911
> https://issues.apache.org/jira/browse/MESOS-4911
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Executor shutdown grace period, which configured on the agent, is
> propagated to executors via the `MESOS_EXECUTOR_SHUTDOWN_GRACE_PERIOD`
> environment variable. The executor library uses this timeout to delay
> the hard shutdown of the related executor.
> 
> 
> Diffs
> -
> 
>   src/exec/exec.cpp 741786132f3a8cc43f5b9ced262429038832a946 
> 
> Diff: https://reviews.apache.org/r/44654/diff/
> 
> 
> Testing
> ---
> 
> The complete chain was tested. See https://reviews.apache.org/r/44662/.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 44755: Added getAppcImage for Appc provisioning tests.

[Jojy Varghese]

> On March 15, 2016, 4:56 p.m., Jie Yu wrote:
> > src/tests/containerizer/provisioner_appc_tests.cpp, lines 587-610
> > 
> >
> > Can you move this function to the top of this file and make it 'static'?
> 
> Anand Mazumdar wrote:
> Wondering why does this function needs to be a global and can't be part 
> of the test fixture i.e. `AppcImageFetcherTest`?
> 
> Jojy Varghese wrote:
> The intend is to use it in all fixtures. Eg, for integegration tests, we 
> have a separate fixture.
> 
> Anand Mazumdar wrote:
> I see. If the function has static scope, how would we use it across 
> separate fixtures i.e. in some other test file? 
> 
> This is assuming that the integration tests would be part of a separate 
> test file since we typically avoid having multiple test fixtures in a single 
> file.

Right now the integration tests are in the same file (in another patch). Not 
sure if having multiple fixtures in a test file is a no-no (eg. 
provisioner_docker_tests).


- Jojy


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


On March 15, 2016, 6:26 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44755/
> ---
> 
> (Updated March 15, 2016, 6:26 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added helper function to get an Appc image protobuf object with given
> parameters.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 069af8a0b5e031d9032fd13154cee250f279b404 
> 
> Diff: https://reviews.apache.org/r/44755/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44755: Added getAppcImage for Appc provisioning tests.

[Anand Mazumdar]

> On March 15, 2016, 4:56 p.m., Jie Yu wrote:
> > src/tests/containerizer/provisioner_appc_tests.cpp, lines 587-610
> > 
> >
> > Can you move this function to the top of this file and make it 'static'?
> 
> Anand Mazumdar wrote:
> Wondering why does this function needs to be a global and can't be part 
> of the test fixture i.e. `AppcImageFetcherTest`?
> 
> Jojy Varghese wrote:
> The intend is to use it in all fixtures. Eg, for integegration tests, we 
> have a separate fixture.

I see. If the function has static scope, how would we use it across separate 
fixtures i.e. in some other test file? 

This is assuming that the integration tests would be part of a separate test 
file since we typically avoid having multiple test fixtures in a single file.


- Anand


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


On March 15, 2016, 6:26 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44755/
> ---
> 
> (Updated March 15, 2016, 6:26 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added helper function to get an Appc image protobuf object with given
> parameters.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 069af8a0b5e031d9032fd13154cee250f279b404 
> 
> Diff: https://reviews.apache.org/r/44755/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44755: Added getAppcImage for Appc provisioning tests.

[Jojy Varghese]

> On March 15, 2016, 4:56 p.m., Jie Yu wrote:
> > src/tests/containerizer/provisioner_appc_tests.cpp, lines 587-610
> > 
> >
> > Can you move this function to the top of this file and make it 'static'?
> 
> Anand Mazumdar wrote:
> Wondering why does this function needs to be a global and can't be part 
> of the test fixture i.e. `AppcImageFetcherTest`?

The intend is to use it in all fixtures. Eg, for integegration tests, we have a 
separate fixture.


- Jojy


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


On March 12, 2016, 5:11 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44755/
> ---
> 
> (Updated March 12, 2016, 5:11 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added helper function to get an Appc image protobuf object with given
> parameters.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 069af8a0b5e031d9032fd13154cee250f279b404 
> 
> Diff: https://reviews.apache.org/r/44755/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44755: Added getAppcImage for Appc provisioning tests.

[Jojy Varghese]

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

(Updated March 15, 2016, 6:26 p.m.)


Review request for mesos and Jie Yu.


Changes
---

review addressed.


Repository: mesos


Description
---

Added helper function to get an Appc image protobuf object with given
parameters.


Diffs (updated)
-

  src/tests/containerizer/provisioner_appc_tests.cpp 
069af8a0b5e031d9032fd13154cee250f279b404 

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


Testing
---

make check.


Thanks,

Jojy Varghese

Re: Review Request 44515: Added agent flags for HTTP authentication.

[Greg Mann]

> On March 15, 2016, 10:33 a.m., Joerg Schad wrote:
> > src/slave/flags.cpp, line 682
> > 
> >
> > As we are actively deprecated the old text based format with Mesos-228, 
> > does it make sense to only document the new json format here?
> 
> Joerg Schad wrote:
> Mesos-2281

Perhaps we should see which patch lands first, and update the other one 
accordingly? I can put a comment on Jan's review so that he's aware of this one.


- Greg


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


On March 14, 2016, 4:17 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44515/
> ---
> 
> (Updated March 14, 2016, 4:17 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Joerg Schad, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-4849
> https://issues.apache.org/jira/browse/MESOS-4849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added agent flags for HTTP authentication.
> 
> Three command-line flags have been added to the agent to enable HTTP 
> authentication: `--authenticate_http`, `--http_credentials`, and 
> `--http_authenticators`.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp 4110061d576d5e20e512e84bb049b6ac910ceab0 
>   src/slave/flags.hpp feb095da4521f678c96f4cc53bdfda262d350388 
>   src/slave/flags.cpp 5bacf109bcd9c9eae1b7ec3c32095c72899ebdbd 
>   src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 
>   src/tests/mesos.cpp 7cca4ed4753fa365b209d1d22f0df4650b19bc6a 
> 
> Diff: https://reviews.apache.org/r/44515/diff/
> 
> 
> Testing
> ---
> 
> `sudo make check` was used to test on both OSX and Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

[Anand Mazumdar]

> On March 15, 2016, 5:40 p.m., Greg Mann wrote:
> > src/examples/dynamic_reservation_framework.cpp, line 42
> > 
> >
> > Instead of using boost here, perhaps we could use members of the STL 
> > like `std::stoi` and `std::to_string`?

We already have `numify` in stout that internally uses `lexical_cast`.


- Anand


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


On March 7, 2016, 9:20 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated March 7, 2016, 9:20 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, and Michael Park.
> 
> 
> Bugs: MESOS-3063
> https://issues.apache.org/jira/browse/MESOS-3063
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provide example for dynamic reservation features.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 37168: Add an example framework using dynamic reservation.

[Greg Mann]

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



Thanks Klaus, this is great! It will be awesome to have an example framework 
for dynamic reservation :-)


src/Makefile.am (line 1600)


It looks like hyphens rather than underscores here would be more consistent 
with the naming of other example frameworks: `dynamic-reservation-framework`.



src/examples/dynamic_reservation_framework.cpp (line 27)


The ACL definitions were recently moved into 
include/mesos/authorizer/acls.hpp, so this include needs to be changed.



src/examples/dynamic_reservation_framework.cpp (line 42)


Instead of using boost here, perhaps we could use members of the STL like 
`std::stoi` and `std::to_string`?



src/examples/dynamic_reservation_framework.cpp (line 56)


Is this true? It looks like we will launch multiple tasks on some slaves.



src/examples/dynamic_reservation_framework.cpp (lines 57 - 58)


s/when it's offered to framework at the first time/when offered to the 
framework for the first time/

s/tasks done/tasks are done/



src/examples/dynamic_reservation_framework.cpp (line 84)


Would you mind changing the logging messages in this file to use the glog 
library?



src/examples/dynamic_reservation_framework.cpp (line 101)


If you want, you could use `hashmap` for `states` and take advantage of the 
`contains` method here.



src/examples/dynamic_reservation_framework.cpp (line 117)


s/reserved resources re-offer/reserved resources are re-offered/



src/examples/dynamic_reservation_framework.cpp (line 130)


s/this resources/these resources/



src/examples/dynamic_reservation_framework.cpp (line 143)


s/waiting for task done/waiting for task to finish/

Also, should remove the period from the end of the logging message



src/examples/dynamic_reservation_framework.cpp (line 157)


s/tasks done/tasks are done/

s/resources unreserved/resources are unreserved/



src/examples/dynamic_reservation_framework.cpp (line 207)


s/for unreserving resources/for resources to be unreserved/

Also, remove period at end



src/examples/dynamic_reservation_framework.cpp (line 255)


s/Fail to/Failed to/

Also, remove period at end



src/examples/dynamic_reservation_framework.cpp (line 423)


This error message could be a bit more descriptive, something like: "The 
default '*' role cannot be used"

Also, the period at the end of the error message should be removed.



src/examples/dynamic_reservation_framework.cpp (line 429)


Should this comment be a TODO?



src/examples/dynamic_reservation_framework.cpp (line 441)


Now that we have implicit roles, I don't think this is necessary?


- Greg Mann


On March 7, 2016, 9:20 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37168/
> ---
> 
> (Updated March 7, 2016, 9:20 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Joerg Schad, and Michael Park.
> 
> 
> Bugs: MESOS-3063
> https://issues.apache.org/jira/browse/MESOS-3063
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Provide example for dynamic reservation features.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am a41e95ddeb838fdebf4ced953c4a29181916e261 
>   src/examples/dynamic_reservation_framework.cpp PRE-CREATION 
>   src/tests/dynamic_reservation_framework_test.sh PRE-CREATION 
>   src/tests/examples_tests.cpp 6ddac17bb2ac0330bcc09eaab975ae70e84a7695 
> 
> Diff: https://reviews.apache.org/r/37168/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 43883: Added allocator metrics for the number of offer filters per framework.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44850, 44851, 44852, 43884, 43880, 43879, 43881, 43882, 43883]

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 March 15, 2016, 2:52 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43883/
> ---
> 
> (Updated March 15, 2016, 2:52 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Ben Mahler.
> 
> 
> Bugs: MESOS-4722
> https://issues.apache.org/jira/browse/MESOS-4722
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added allocator metrics for the number of offer filters per framework.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 827f7073204fcf8575ca980a5571c8be4f5e4110 
>   src/master/allocator/mesos/hierarchical.hpp 
> 52b3a9bfbe73d24968f7ddb0672aee1e05636ab0 
>   src/master/allocator/mesos/hierarchical.cpp 
> 70291075c00a9a557529c2562dedcfc6c6c3ec32 
>   src/master/allocator/mesos/metrics.hpp 
> d04e9a11c77b6fb2d522608e3085f81f8a6657ca 
>   src/master/allocator/mesos/metrics.cpp 
> 46dd7f7b3d8d49c59b42e789fe5dcd49f1693ace 
>   src/tests/hierarchical_allocator_tests.cpp 
> 459e02576f6d05abbbcc83ae5cabac5c66e93f05 
> 
> Diff: https://reviews.apache.org/r/43883/diff/
> 
> 
> Testing
> ---
> 
> make check (OS X)
> 
> I confirmed that this does not lead to general performance regressions in the 
> allocator; this is partially expected since the added code only inserts 
> metrics in the allocator while the actual work is perform asynchronously. 
> These tests where performed with 
> `HierarchicalAllocator_BENCHMARK_Test.DeclineOffers` on an optimized build 
> under OS X using clang(trunk) as compiler.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 44755: Added getAppcImage for Appc provisioning tests.

[Anand Mazumdar]

> On March 15, 2016, 4:56 p.m., Jie Yu wrote:
> > src/tests/containerizer/provisioner_appc_tests.cpp, lines 587-610
> > 
> >
> > Can you move this function to the top of this file and make it 'static'?

Wondering why does this function needs to be a global and can't be part of the 
test fixture i.e. `AppcImageFetcherTest`?


- Anand


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


On March 12, 2016, 5:11 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44755/
> ---
> 
> (Updated March 12, 2016, 5:11 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added helper function to get an Appc image protobuf object with given
> parameters.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 069af8a0b5e031d9032fd13154cee250f279b404 
> 
> Diff: https://reviews.apache.org/r/44755/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44755: Added getAppcImage for Appc provisioning tests.

[Jie Yu]

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


Fix it, then Ship it!





src/tests/containerizer/provisioner_appc_tests.cpp (lines 587 - 610)


Can you move this function to the top of this file and make it 'static'?



src/tests/containerizer/provisioner_appc_tests.cpp (line 588)


s/imageName/name/


- Jie Yu


On March 12, 2016, 5:11 p.m., Jojy Varghese wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44755/
> ---
> 
> (Updated March 12, 2016, 5:11 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added helper function to get an Appc image protobuf object with given
> parameters.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 069af8a0b5e031d9032fd13154cee250f279b404 
> 
> Diff: https://reviews.apache.org/r/44755/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jojy Varghese
> 
>

Re: Review Request 44753: Fixed runtime isolator tests out of disk issue.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On March 15, 2016, 12:22 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44753/
> ---
> 
> (Updated March 15, 2016, 12:22 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-4942
> https://issues.apache.org/jira/browse/MESOS-4942
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed runtime isolator tests out of disk issue.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/provisioner_docker_tests.cpp 
> 9fd5ea9e465f21624d3cdc0125a702528fe7548b 
>   src/tests/containerizer/runtime_isolator_tests.cpp 
> d8a000413fb30d7daaa77827287a2395d81b8b04 
> 
> Diff: https://reviews.apache.org/r/44753/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> sudo ./bin/mesos-test.sh
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 44848: Moved defaults in configuration.md to a new line for readability.

[Greg Mann]

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



These docs were generated from the flag help strings, as printed by our flag 
renderer; at some point we might want to consider modifying the flag renderer 
to print the defaults on a new line as well.


docs/configuration.md (lines 98 - 99)


I viewed this with the Mesos website container 
(https://github.com/mesosphere/mesos-website-container) and the newlines have 
no effect, you need to explicitly include a line break with something like 
``.


- Greg Mann


On March 15, 2016, 1:22 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44848/
> ---
> 
> (Updated March 15, 2016, 1:22 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Greg Mann, and Joerg Schad.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When the document is rendered in HTML, it is easier to spot defaults
> when they are on a new line. Also, the previous line should end with
> a period like in proper English text.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 739d4ff9aeeb1ba70ce11033168d63d37b6ef56b 
> 
> Diff: https://reviews.apache.org/r/44848/diff/
> 
> 
> Testing
> ---
> 
> None: not a functional change.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 44837: Added force_pull_image to Image protobuf.

[Jie Yu]

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




include/mesos/mesos.proto (line 1501)


s/force_pull_image/no_cache/

IMO, force pull is a mean, not a description. Please adjust the comments 
accordingly.



include/mesos/v1/mesos.proto (lines 1492 - 1495)


Ditto.


- Jie Yu


On March 15, 2016, 7:31 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44837/
> ---
> 
> (Updated March 15, 2016, 7:31 a.m.)
> 
> 
> Review request for mesos, haosdent huang and Jie Yu.
> 
> 
> Bugs: MESOS-4886
> https://issues.apache.org/jira/browse/MESOS-4886
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added force_pull_image to Image protobuf.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto deb9c0910a27afd67276f54b3f666a878212727b 
>   include/mesos/v1/mesos.proto a981e750c24cfc48177bbc9ca56f0c3ecfae1a1b 
> 
> Diff: https://reviews.apache.org/r/44837/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>