Re: Review Request 47442: Windows: Enable `JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE` on job objects.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [47442]

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

Error:
2016-05-17 05:42:46 URL:https://reviews.apache.org/r/47442/diff/raw/ 
[2899/2899] -> "47442.patch" [1]
47442.patch:19: trailing whitespace.
  
47442.patch:71: trailing whitespace.
  
47442.patch:75: trailing whitespace.
  
error: patch failed: src/launcher/executor.cpp:412
error: src/launcher/executor.cpp: patch does not apply

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

- Mesos ReviewBot


On May 17, 2016, 2:27 a.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47442/
> ---
> 
> (Updated May 17, 2016, 2:27 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joris 
> Van Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Enable `JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE` on job objects.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/windows/os.hpp 
> 261768eace6ab09956f4a80e1ec5dba988d831e1 
>   src/launcher/executor.cpp fa4a89e97b0c427b4595a6adebc397aeb5bfaaa5 
> 
> Diff: https://reviews.apache.org/r/47442/diff/
> 
> 
> Testing
> ---
> 
> Windows build/test
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

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

[zhou xing]

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

(Updated 五月 17, 2016, 5:29 a.m.)


Review request for mesos, Benjamin Mahler and Greg Mann.


Changes
---

rebased the code and let length accept -1


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


Repository: mesos


Description
---

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


Diffs (updated)
-

  src/files/files.cpp e4b0ada00aabba6553810391f4015a896f8a69a5 
  src/tests/files_tests.cpp 5d6620d13babaf0bb7f9c888bb1b4fa2228b6ccd 

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


Testing
---

make
make check

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


Thanks,

zhou xing

Re: Review Request 47089: Waited for first statusUpdate before advance clock in GracePeriod test.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47089]

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 May 17, 2016, 2:14 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47089/
> ---
> 
> (Updated May 17, 2016, 2:14 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Niklas Nielsen, Timothy Chen, and 
> Jiang Yan Xu.
> 
> 
> Bugs: MESOS-1653
> https://issues.apache.org/jira/browse/MESOS-1653
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Waited for first statusUpdate before advance clock in GracePeriod test.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp 1c4a554ab07731963a4a38e3ae40b0323bf317bb 
> 
> Diff: https://reviews.apache.org/r/47089/diff/
> 
> 
> Testing
> ---
> 
> Test with
> 
> ```
> GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="HealthCheckTest.GracePeriod" 
> --verbose --gtest_break_on_failure --gtest_repeat=-1
> ```
> 
> more than 1500 iteractions in my slow machine.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 47274: Authorize what quota can be seen by GET_QUOTA_BY_ROLE.

[Zhitao Li]

> On May 16, 2016, 6:06 p.m., Alexander Rukletsov wrote:
> > src/master/master.hpp, line 1022
> > 
> >
> > Do you still need `request`?

We still need `jsonp` argument to be passed. I feel keeping request captured 
will be easier to handle in the future.


- Zhitao


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


On May 17, 2016, 5:06 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47274/
> ---
> 
> (Updated May 17, 2016, 5:06 a.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-5336
> https://issues.apache.org/jira/browse/MESOS-5336
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Authorize what quota can be seen by GET_QUOTA_BY_ROLE.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/master/quota.md 83913fac9d45ef25629222f572529493d0c66f05 
>   include/mesos/authorizer/acls.proto 
> 9adae8c2a2e1b2ee4b9068ea50fcebb0544f2e5d 
>   include/mesos/authorizer/authorizer.proto 
> 32492a59ad95df3bb673ec42321518f86c11af59 
>   src/authorizer/local/authorizer.cpp 
> e95435327bb3b6f447e814b8657bce8084535346 
>   src/master/http.cpp a0d67671c89b9794e721dc3ba012acd3bdc447e3 
>   src/master/master.hpp 3e55114ee7866e06513071e86e15608099dae052 
>   src/master/quota_handler.cpp 6590519d8440f352f5bf00fda805414a5aad725c 
>   src/tests/master_quota_tests.cpp 9ce7e2e481706c74f080d54b6f58d57874a27e64 
> 
> Diff: https://reviews.apache.org/r/47274/diff/
> 
> 
> Testing
> ---
> 
> Unit test.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 47274: Authorize what quota can be seen by GET_QUOTA_BY_ROLE.

[Zhitao Li]

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




src/master/quota_handler.cpp (lines 518 - 520)


It'll be great if we can either catch this in linter/commit hook during 
`git commit`, or publish .vimrc/etc so contributors don't need to remember this.


- Zhitao Li


On May 17, 2016, 5:06 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47274/
> ---
> 
> (Updated May 17, 2016, 5:06 a.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-5336
> https://issues.apache.org/jira/browse/MESOS-5336
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Authorize what quota can be seen by GET_QUOTA_BY_ROLE.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/master/quota.md 83913fac9d45ef25629222f572529493d0c66f05 
>   include/mesos/authorizer/acls.proto 
> 9adae8c2a2e1b2ee4b9068ea50fcebb0544f2e5d 
>   include/mesos/authorizer/authorizer.proto 
> 32492a59ad95df3bb673ec42321518f86c11af59 
>   src/authorizer/local/authorizer.cpp 
> e95435327bb3b6f447e814b8657bce8084535346 
>   src/master/http.cpp a0d67671c89b9794e721dc3ba012acd3bdc447e3 
>   src/master/master.hpp 3e55114ee7866e06513071e86e15608099dae052 
>   src/master/quota_handler.cpp 6590519d8440f352f5bf00fda805414a5aad725c 
>   src/tests/master_quota_tests.cpp 9ce7e2e481706c74f080d54b6f58d57874a27e64 
> 
> Diff: https://reviews.apache.org/r/47274/diff/
> 
> 
> Testing
> ---
> 
> Unit test.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 47274: Authorize what quota can be seen by GET_QUOTA_BY_ROLE.

[Zhitao Li]

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

(Updated May 17, 2016, 5:06 a.m.)


Review request for mesos, Adam B and Alexander Rukletsov.


Changes
---

Review comments.


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


Repository: mesos


Description
---

Authorize what quota can be seen by GET_QUOTA_BY_ROLE.


Diffs (updated)
-

  docs/endpoints/master/quota.md 83913fac9d45ef25629222f572529493d0c66f05 
  include/mesos/authorizer/acls.proto 9adae8c2a2e1b2ee4b9068ea50fcebb0544f2e5d 
  include/mesos/authorizer/authorizer.proto 
32492a59ad95df3bb673ec42321518f86c11af59 
  src/authorizer/local/authorizer.cpp e95435327bb3b6f447e814b8657bce8084535346 
  src/master/http.cpp a0d67671c89b9794e721dc3ba012acd3bdc447e3 
  src/master/master.hpp 3e55114ee7866e06513071e86e15608099dae052 
  src/master/quota_handler.cpp 6590519d8440f352f5bf00fda805414a5aad725c 
  src/tests/master_quota_tests.cpp 9ce7e2e481706c74f080d54b6f58d57874a27e64 

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


Testing
---

Unit test.


Thanks,

Zhitao Li

Re: Review Request 47367: Removed references to HTTP command executor.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47363, 47364, 47365, 47431, 47366, 47367]

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 May 17, 2016, 1:53 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47367/
> ---
> 
> (Updated May 17, 2016, 1:53 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-5302
> https://issues.apache.org/jira/browse/MESOS-5302
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 
>   src/launcher/http_command_executor.cpp 
> f78d7988e4be0ee05b75c49c9e04bab58d0916db 
>   src/slave/slave.cpp 7c870396b4d6804bfda6169d76d136e95cd839f5 
> 
> Diff: https://reviews.apache.org/r/47367/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 46498: Add runtime for Appc Spec ex: command, workingdir and environment.

[Guangya Liu]

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




include/mesos/appc/spec.proto (line 58)


Better add some comments here for Exec



src/slave/containerizer/mesos/containerizer.cpp (line 79)


This should be 
#ifdef __linux__
#include "slave/containerizer/mesos/isolators/appc/runtime.hpp"
#endif


- Guangya Liu


On May 14, 2016, 4:20 a.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46498/
> ---
> 
> (Updated May 14, 2016, 4:20 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add runtime for Appc Spec ex: command, workingdir and environment.
> 
> 
> Diffs
> -
> 
>   include/mesos/appc/spec.proto 4ef0473748c2f47aff1729c1d969185803fe72d4 
>   include/mesos/slave/isolator.proto 60a9bb637e12593a97ed1a7c510ebccd4e5a9615 
>   src/Makefile.am 447bc2ab511ad173d3d911be10992be0974f4584 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 75e5a32a3e70ec60a6800e21a621673184ea0956 
>   src/slave/containerizer/mesos/isolators/appc/runtime.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/appc/runtime.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> 33e97fc645a9b7fbc9ae47f67c1b5dacf999fce0 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 5c96e9f6603d39889e6bc807874d35d0cb3556be 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> dcbbbaf797d4467bfd0bb1ee91ee9ce843e7d546 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> 1d477ef13ddd24fd8badae0decaa4a2271ecc746 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 84fe52b6937c3b7d7628b17a2f045eec2f386b4d 
> 
> Diff: https://reviews.apache.org/r/46498/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 47436: Fixed some typos in oversubscription test and allocator.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47436]

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 May 17, 2016, 12:38 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47436/
> ---
> 
> (Updated May 17, 2016, 12:38 a.m.)
> 
> 
> Review request for mesos, Joris Van Remoortere and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed some typos in oversubscription test and allocator.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 51125bac62735879f66234765dad92a759f6724b 
>   src/tests/oversubscription_tests.cpp 
> f73f7bee34774092bc1fd48b11c0869afc60375a 
> 
> Diff: https://reviews.apache.org/r/47436/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 47442: Windows: Enable `JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE` on job objects.

[Daniel Pravat]

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

(Updated May 17, 2016, 2:27 a.m.)


Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joris 
Van Remoortere, and Michael Park.


Summary (updated)
-

Windows: Enable `JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE` on job objects.


Repository: mesos


Description (updated)
---

Windows: Enable `JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE` on job objects.


Diffs (updated)
-

  3rdparty/stout/include/stout/windows/os.hpp 
261768eace6ab09956f4a80e1ec5dba988d831e1 
  src/launcher/executor.cpp fa4a89e97b0c427b4595a6adebc397aeb5bfaaa5 

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


Testing
---

Windows build/test


Thanks,

Daniel Pravat

Re: Review Request 47089: Waited for first statusUpdate before advance clock in GracePeriod test.

[haosdent huang]

> On May 13, 2016, 8:37 p.m., Benjamin Mahler wrote:
> > src/tests/health_check_tests.cpp, line 1059
> > 
> >
> > It looks like the clock needs to be paused before the task is launched? 
> > Otherwise we may have too much time advance between launching the task and 
> > pausing the clock, no?

Hi, @bmhaler. I find the advance could not work in `mesos-health-check`, 
because it launched in a separate process. So I removed it.


- haosdent


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


On May 17, 2016, 2:14 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47089/
> ---
> 
> (Updated May 17, 2016, 2:14 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Niklas Nielsen, Timothy Chen, and 
> Jiang Yan Xu.
> 
> 
> Bugs: MESOS-1653
> https://issues.apache.org/jira/browse/MESOS-1653
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Waited for first statusUpdate before advance clock in GracePeriod test.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp 1c4a554ab07731963a4a38e3ae40b0323bf317bb 
> 
> Diff: https://reviews.apache.org/r/47089/diff/
> 
> 
> Testing
> ---
> 
> Test with
> 
> ```
> GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="HealthCheckTest.GracePeriod" 
> --verbose --gtest_break_on_failure --gtest_repeat=-1
> ```
> 
> more than 1500 iteractions in my slow machine.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 47089: Waited for first statusUpdate before advance clock in GracePeriod test.

[haosdent huang]

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

(Updated May 17, 2016, 2:14 a.m.)


Review request for mesos, Benjamin Mahler, Niklas Nielsen, Timothy Chen, and 
Jiang Yan Xu.


Changes
---

Removed Clock advance because it could not work on a subprocess.


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


Repository: mesos


Description
---

Waited for first statusUpdate before advance clock in GracePeriod test.


Diffs (updated)
-

  src/tests/health_check_tests.cpp 1c4a554ab07731963a4a38e3ae40b0323bf317bb 

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


Testing
---

Test with

```
GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="HealthCheckTest.GracePeriod" 
--verbose --gtest_break_on_failure --gtest_repeat=-1
```

more than 1500 iteractions in my slow machine.


Thanks,

haosdent huang

Review Request 47442: Windows: Enable `JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE` on job objwects.

[Daniel Pravat]

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

Review request for mesos, Alex Naparu, Artem Harutyunyan, Alex Clemmer, Joris 
Van Remoortere, and Michael Park.


Repository: mesos


Description
---

Windows: Enable `JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE` on job objwects.


Diffs
-

  3rdparty/stout/include/stout/windows/os.hpp 
261768eace6ab09956f4a80e1ec5dba988d831e1 
  src/launcher/executor.cpp fa4a89e97b0c427b4595a6adebc397aeb5bfaaa5 

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


Testing
---

Windows build/test


Thanks,

Daniel Pravat

Re: Review Request 47367: Removed references to HTTP command executor.

[Anand Mazumdar]

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

(Updated May 17, 2016, 1:53 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Make review bot run on the updated patches


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 
  src/launcher/http_command_executor.cpp 
f78d7988e4be0ee05b75c49c9e04bab58d0916db 
  src/slave/slave.cpp 7c870396b4d6804bfda6169d76d136e95cd839f5 

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


Testing
---


Thanks,

Anand Mazumdar

Re: Review Request 47089: Waited for first statusUpdate before advance clock in GracePeriod test.

[haosdent huang]

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

(Updated May 17, 2016, 1:33 a.m.)


Review request for mesos, Benjamin Mahler, Niklas Nielsen, Timothy Chen, and 
Jiang Yan Xu.


Changes
---

Address @bmahler's comments.


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


Repository: mesos


Description
---

Waited for first statusUpdate before advance clock in GracePeriod test.


Diffs (updated)
-

  src/tests/health_check_tests.cpp 1c4a554ab07731963a4a38e3ae40b0323bf317bb 

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


Testing
---

Test with

```
GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="HealthCheckTest.GracePeriod" 
--verbose --gtest_break_on_failure --gtest_repeat=-1
```

more than 1500 iteractions in my slow machine.


Thanks,

haosdent huang

Re: Review Request 47089: Waited for first statusUpdate before advance clock in GracePeriod test.

[haosdent huang]

> On May 13, 2016, 8:37 p.m., Benjamin Mahler wrote:
> > src/tests/health_check_tests.cpp, line 1015
> > 
> >
> > Why did this change?

Yes, we need this so that we could check the stdout/stderr from console if it 
still flaky next time.


- haosdent


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


On May 7, 2016, 11:46 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47089/
> ---
> 
> (Updated May 7, 2016, 11:46 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Niklas Nielsen, Timothy Chen, and 
> Jiang Yan Xu.
> 
> 
> Bugs: MESOS-1653
> https://issues.apache.org/jira/browse/MESOS-1653
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Waited for first statusUpdate before advance clock in GracePeriod test.
> 
> 
> Diffs
> -
> 
>   src/tests/health_check_tests.cpp 1c4a554ab07731963a4a38e3ae40b0323bf317bb 
> 
> Diff: https://reviews.apache.org/r/47089/diff/
> 
> 
> Testing
> ---
> 
> Test with
> 
> ```
> GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="HealthCheckTest.GracePeriod" 
> --verbose --gtest_break_on_failure --gtest_repeat=-1
> ```
> 
> more than 1500 iteractions in my slow machine.
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Review Request 47436: Fixed some typos in oversubscription test and allocator.

[Guangya Liu]

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

Review request for mesos, Joris Van Remoortere and Vinod Kone.


Repository: mesos


Description
---

Fixed some typos in oversubscription test and allocator.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
51125bac62735879f66234765dad92a759f6724b 
  src/tests/oversubscription_tests.cpp f73f7bee34774092bc1fd48b11c0869afc60375a 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 44209: Do not need to close the ifstream explicitly.

[Guangya Liu]

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

(Updated 五月 17, 2016, 12:14 a.m.)


Review request for mesos, Anand Mazumdar, Gilbert Song, Jie Yu, and Timothy 
Chen.


Summary (updated)
-

Do not need to close the ifstream explicitly.


Repository: mesos


Description (updated)
---

Do not need to close the ifstream explicitly.


Diffs (updated)
-

  src/log/tool/benchmark.cpp 7e1350675bdfe6c44e86b082bc01f692cc72b850 

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


Testing
---

make
make check


Thanks,

Guangya Liu

Re: Review Request 44209: Closed the input ifstream before return error.

[Guangya Liu]

> On 五月 16, 2016, 5:47 p.m., Anand Mazumdar wrote:
> > Why do we need to close it explicitly? `ifstream` already follows the RAII 
> > idiom i.e. things are cleaned up upon destruction. What am I missing?

Thanks Anand, got it, 
http://stackoverflow.com/questions/4802494/do-i-need-to-close-a-stdfstream , I 
think that we can remove the `input.close()` here.


- Guangya


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


On 五月 16, 2016, 5:32 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44209/
> ---
> 
> (Updated 五月 16, 2016, 5:32 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Closed the input ifstream before return error.
> 
> 
> Diffs
> -
> 
>   src/log/tool/benchmark.cpp 7e1350675bdfe6c44e86b082bc01f692cc72b850 
> 
> Diff: https://reviews.apache.org/r/44209/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 47363: Introduced a driver to v1 executor shim/adapter.

[Anand Mazumdar]

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

(Updated May 16, 2016, 11:51 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Review comments from Vinod


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


Repository: mesos


Description
---

This change adds a driver to v1 executor shim/adapter. This can
be used by the command executor/docker executor to toggle
between using the new/old API instead of duplicating the
code like we did with `http_command_executor.cpp`.


Diffs (updated)
-

  include/mesos/v1/executor.hpp adca287be3bb88c8b3298705740cb6bcbb3a09f0 
  src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 
  src/executor/v0_v1executor.hpp PRE-CREATION 
  src/executor/v0_v1executor.cpp PRE-CREATION 

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


Testing
---

make check (Later in the chain, we make the command executor use this interface)
So the existing tests should be able to test this out.


Thanks,

Anand Mazumdar

Re: Review Request 47363: Introduced a driver to v1 executor shim/adapter.

[Anand Mazumdar]

> On May 15, 2016, 12:55 a.m., Vinod Kone wrote:
> > include/mesos/v1/executor.hpp, line 38
> > 
> >
> > We don't name our interfaces as *Interface :) Is there an alternative 
> > name we could use? 
> > 
> > 
> > Also, I'm curious to know why you used inheritance instead of 
> > composition. For example, the Mesos class itself could take a parameter 
> > which tells it use V1 or V0 API.

+1, modified it to `MesosBase` as per our offline discussion. Our style guide 
was mum on the naming scheme. Hence, I picked up the recommendation for naming 
interfaces from the google style guide.


> On May 15, 2016, 12:55 a.m., Vinod Kone wrote:
> > src/executor/v0_v1executor.hpp, line 75
> > 
> >
> > Any reason why the driver is not a part of the V0ToV1AdapterProcess ?

As per our offline discussion: The `driver` takes in an argument to `this` i.e. 
the class that implements the executor interface. Hence, I kept it as part of 
the adapter interface since it implements the executor interface and not the 
process.


> On May 15, 2016, 12:55 a.m., Vinod Kone wrote:
> > src/executor/v0_v1executor.cpp, lines 66-67
> > 
> >
> > why did you need to make a copy?

These are needed for creating the `Event::Subscribed` object upon receiving 
`reregistered` callback. I added an explicit comment now.


> On May 15, 2016, 12:55 a.m., Vinod Kone wrote:
> > src/executor/v0_v1executor.cpp, lines 106-107
> > 
> >
> > hmm. this seems like a bug. we should fix the driver!

Would file a JIRA issue.


- Anand


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


On May 14, 2016, 8:39 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47363/
> ---
> 
> (Updated May 14, 2016, 8:39 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-5302
> https://issues.apache.org/jira/browse/MESOS-5302
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds a driver to v1 executor shim/adapter. This can
> be used by the command executor/docker executor to toggle
> between using the new/old API instead of duplicating the
> code like we did with `http_command_executor.cpp`.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/executor.hpp adca287be3bb88c8b3298705740cb6bcbb3a09f0 
>   src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 
>   src/executor/v0_v1executor.hpp PRE-CREATION 
>   src/executor/v0_v1executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47363/diff/
> 
> 
> Testing
> ---
> 
> make check (Later in the chain, we make the command executor use this 
> interface)
> So the existing tests should be able to test this out.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 47364: Set env variable used to toggle executor implementation.

[Anand Mazumdar]

> On May 15, 2016, 12:55 a.m., Vinod Kone wrote:
> > src/slave/containerizer/containerizer.cpp, line 364
> > 
> >
> > How about
> > 
> > MESOS_HTTP_API
> > 
> > Also, should this be only set if it's a command executor?

As per our offline discussion, would file a JIRA to do a sweep of other 
environment variables that are not used by the HTTP API but are part of the 
`executorEnvironment` method.


- Anand


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


On May 13, 2016, 10:34 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47364/
> ---
> 
> (Updated May 13, 2016, 10:34 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-5063
> https://issues.apache.org/jira/browse/MESOS-5063
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds the environment variable that is set by
> the agent allowing the command executor to toggle between
> implementations.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/containerizer.cpp 
> d0cae79834e451594d7675f00c5f7d2d2cd3a264 
> 
> Diff: https://reviews.apache.org/r/47364/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 47365: Moved code from HTTP command executor to command executor.

[Anand Mazumdar]

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

(Updated May 16, 2016, 11:50 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Updated description


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


Repository: mesos


Description (updated)
---

This is a blatant copy of `src/launcher/http_command_executor.cpp`.
This change makes reviewing code later in the chain easier.
Later in the chain, we would make this use the Shim/Adapter
to toggle between using the driver or the v1 API via an
environment variable. The existing `http_command_executor.cpp` 
would be deleted.


Diffs
-

  src/launcher/executor.cpp 7d111e668e0a139a98bdeb959997843180b40452 

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


Testing
---

make check
(this should not be committed on its own)


Thanks,

Anand Mazumdar

Review Request 47431: Renamed `HttpCommandExecutor` to `CommandExecutor`.

[Anand Mazumdar]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/launcher/executor.cpp 7d111e668e0a139a98bdeb959997843180b40452 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 47365: Moved code from HTTP command executor to command executor.

[Anand Mazumdar]

> On May 15, 2016, 12:55 a.m., Vinod Kone wrote:
> > src/launcher/executor.cpp, line 104
> > 
> >
> > s/HttpCommandExecutor/CommandExecutor/

Updated in https://reviews.apache.org/r/47431


- Anand


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


On May 13, 2016, 10:33 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47365/
> ---
> 
> (Updated May 13, 2016, 10:33 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-5302
> https://issues.apache.org/jira/browse/MESOS-5302
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change makes reviewing code later in the chain easier.
> Later in the chain, we would make this use the Shim/Adapter
> to toggle between using the driver or the v1 API via an
> environment variable. `http_command_executor.cpp` would
> be deleted.
> 
> 
> Diffs
> -
> 
>   src/launcher/executor.cpp 7d111e668e0a139a98bdeb959997843180b40452 
> 
> Diff: https://reviews.apache.org/r/47365/diff/
> 
> 
> Testing
> ---
> 
> make check
> (this should not be committed on its own)
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 47366: Made the command executor use the adapter interface.

[Anand Mazumdar]

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

(Updated May 16, 2016, 11:49 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Review comments from Vinod


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


Repository: mesos


Description
---

This change adds support to the command executor to toggle
between the driver based adapter/v1 API based on the
`MESOS_HTTP_COMMAND_EXECUTOR` environment variable
set by the agent.


Diffs (updated)
-

  src/launcher/executor.cpp 7d111e668e0a139a98bdeb959997843180b40452 

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


Testing
---

make check


Thanks,

Anand Mazumdar

Re: Review Request 47374: Experimental: Separated mesos test helpers into a separate library.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47374]

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 May 16, 2016, 8:02 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47374/
> ---
> 
> (Updated May 16, 2016, 8:02 p.m.)
> 
> 
> Review request for mesos, Adam B, Artem Harutyunyan, Kapil Arya, and Jan 
> Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gives external projects easier access to the test helpers used in
> mesos tests.  
> 
> For example, a module writer may want to write a test like 
> `src/tests/oversubscription_tests.cpp`.  To build and link against 
> this library, the module writer would mimic the build flags for tests:
> ```
> my_module_CPPFLAGS =   \
>   -I$(ZOOKEEPER)/include   \
>   -I$(ZOOKEEPER)/generated \
>   -DSOURCE_DIR=\"$(abs_top_srcdir)\"   \
>   -DBUILD_DIR=\"$(abs_top_builddir)\"  \
>   ...
> ```
> 
> 
> Diffs
> -
> 
>   src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 
> 
> Diff: https://reviews.apache.org/r/47374/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 47363: Introduced a driver to v1 executor shim/adapter.

[Alexander Rukletsov]

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




src/executor/v0_v1executor.cpp (lines 17 - 19)


Swap these two please.


- Alexander Rukletsov


On May 14, 2016, 8:39 a.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47363/
> ---
> 
> (Updated May 14, 2016, 8:39 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-5302
> https://issues.apache.org/jira/browse/MESOS-5302
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change adds a driver to v1 executor shim/adapter. This can
> be used by the command executor/docker executor to toggle
> between using the new/old API instead of duplicating the
> code like we did with `http_command_executor.cpp`.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/executor.hpp adca287be3bb88c8b3298705740cb6bcbb3a09f0 
>   src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 
>   src/executor/v0_v1executor.hpp PRE-CREATION 
>   src/executor/v0_v1executor.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47363/diff/
> 
> 
> Testing
> ---
> 
> make check (Later in the chain, we make the command executor use this 
> interface)
> So the existing tests should be able to test this out.
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 47412: Windows: Use Winsock class in slave and containerizer.

[Joris Van Remoortere]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/main.cpp (lines 20 - 22)


before the stout block please.



src/slave/main.cpp (line 32)


separate block as it's a subdirectory.


- Joris Van Remoortere


On May 16, 2016, 2:04 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47412/
> ---
> 
> (Updated May 16, 2016, 2:04 p.m.)
> 
> 
> Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Use Winsock class in slave and containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/main.cpp 
> ed0c6cd0721b9f486be056d2ae62cbccf53bc834 
>   src/slave/main.cpp fee46bafc88f8cdade868aab8c0fee79b8d2fb6d 
> 
> Diff: https://reviews.apache.org/r/47412/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 47411: Agent: Changed the names of symbols that are ambiguous on MSVC.

[Joris Van Remoortere]

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


Fix it, then Ship it!





src/docker/docker.hpp (lines 123 - 124)


Please add a small comment, and change the name.
```
const process::Subprocess::IO& _stdout = process::Subprocess::PIPE(),
const process::Subprocess::IO& _stderr = process::Subprocess::PIPE())
```



src/slave/slave.cpp (lines 486 - 487)


```
resources->filter([](const Resource& _resource) {
```


- Joris Van Remoortere


On May 16, 2016, 1:46 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47411/
> ---
> 
> (Updated May 16, 2016, 1:46 p.m.)
> 
> 
> Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Agent: Changed the names of symbols that are ambiguous on MSVC.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp da6e9a25b99079940958001128ee949cdb9b931b 
>   src/slave/slave.cpp feb9f291b4188aac1a63ab9cfae8349a048663b1 
> 
> Diff: https://reviews.apache.org/r/47411/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 47404: Stout: Implemented `HANDLE` versions of file descriptor functions.

[Joris Van Remoortere]

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


Fix it, then Ship it!





3rdparty/stout/include/stout/os/windows/fcntl.hpp (lines 72 - 73)


Is that true?



3rdparty/stout/include/stout/os/windows/read.hpp (line 26)


Can you clarify that this is a forward declaration for an os agnostic read 
in `os/read.hpp`?
Same with write.


- Joris Van Remoortere


On May 16, 2016, 3:23 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47404/
> ---
> 
> (Updated May 16, 2016, 3:23 a.m.)
> 
> 
> Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Bugs: MESOS-5386
> https://issues.apache.org/jira/browse/MESOS-5386
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout: Implemented `HANDLE` versions of file descriptor functions.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/fcntl.hpp 
> 14734317d7fb40053ee808745ac3ba8c706a7669 
>   3rdparty/stout/include/stout/os/windows/read.hpp 
> a10df7358205da9bc9d3176e27db12556bf0e6ae 
>   3rdparty/stout/include/stout/os/windows/write.hpp 
> 9f75568b88db11f9706beb4e7eb45c6e7759b779 
>   3rdparty/stout/include/stout/protobuf.hpp 
> eb4ac8cd738859a119f4f64bf52ae49a5bbef899 
> 
> Diff: https://reviews.apache.org/r/47404/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 47403: Stout: Set `_fmode` to binary in `protobuf.hpp`.

[Joris Van Remoortere]

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


Fix it, then Ship it!





3rdparty/stout/include/stout/protobuf.hpp (lines 110 - 117)


How about:
```
  const int operation_flags = O_WRONLY | O_CREAT | O_TRUNC | O_CLOEXEC;
#ifdef __WINDOWS__
// NOTE: Windows does automatic linefeed conversions in I/O on text 
files.
// We include the `_O_BINARY` flag here to avoid this.
operation_flags |= _O_BINARY;
#endif
```


- Joris Van Remoortere


On May 15, 2016, 10:28 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47403/
> ---
> 
> (Updated May 15, 2016, 10:28 p.m.)
> 
> 
> Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Bugs: MESOS-3443 and MESOS-3658
> https://issues.apache.org/jira/browse/MESOS-3443
> https://issues.apache.org/jira/browse/MESOS-3658
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In Windows, the I/O subsystem will automatically convert linefeed
> endings if the I/O is done on "text" files. For binary files, this
> effect is almost never desirable.
> 
> This commit will turn of these conversions for protobuf I/O operations.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/protobuf.hpp 
> eb4ac8cd738859a119f4f64bf52ae49a5bbef899 
> 
> Diff: https://reviews.apache.org/r/47403/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 47374: Experimental: Separated mesos test helpers into a separate library.

[Joseph Wu]

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

(Updated May 16, 2016, 1:02 p.m.)


Review request for mesos, Adam B, Artem Harutyunyan, Kapil Arya, and Jan 
Schlicht.


Changes
---

Add java and python flags.


Repository: mesos


Description
---

This gives external projects easier access to the test helpers used in
mesos tests.  

For example, a module writer may want to write a test like 
`src/tests/oversubscription_tests.cpp`.  To build and link against 
this library, the module writer would mimic the build flags for tests:
```
my_module_CPPFLAGS =   \
  -I$(ZOOKEEPER)/include   \
  -I$(ZOOKEEPER)/generated \
  -DSOURCE_DIR=\"$(abs_top_srcdir)\"   \
  -DBUILD_DIR=\"$(abs_top_builddir)\"  \
  ...
```


Diffs (updated)
-

  src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 

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


Testing
---

make check


Thanks,

Joseph Wu

Re: Review Request 47391: Stout: Removed warning in Windows implementation of `stat.hpp`.

[Joris Van Remoortere]

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


Fix it, then Ship it!





3rdparty/stout/include/stout/os/windows/stat.hpp (lines 128 - 129)


Can you add a small comment explaining that this is necessary for Windows?


- Joris Van Remoortere


On May 14, 2016, noon, Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47391/
> ---
> 
> (Updated May 14, 2016, noon)
> 
> 
> Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout: Removed warning in Windows implementation of `stat.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/stat.hpp 
> f4414dda61386e9458a6aba6014b1df799c89500 
> 
> Diff: https://reviews.apache.org/r/47391/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 47390: Stout: Removed implicit conversions in `permissions.hpp` on Windows.

[Joris Van Remoortere]

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


Ship it!




Ship It!

- Joris Van Remoortere


On May 14, 2016, 11:58 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47390/
> ---
> 
> (Updated May 14, 2016, 11:58 a.m.)
> 
> 
> Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout: Removed implicit conversions in `permissions.hpp` on Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/permissions.hpp 
> 16b5bca67bc32ae0a2aa32a13b7ff1368e11dcc3 
> 
> Diff: https://reviews.apache.org/r/47390/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 47389: Stout: Added support for correct path delimiters in Windows.

[Joris Van Remoortere]

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




3rdparty/stout/include/stout/os/constants.hpp (lines 20 - 28)


Can we have just the char versions?
I think you can add support for `stringify(char)` to resolve the case where 
you want to construct just the string from a character.

From the name `WINDOWS_SEPARATOR_CHAR` it' not clear what this means. Is 
this the path separator?
The other variable is called `DIRECTORY_SEPARATOR`, which is more intuitive.

Why can't the `char` versions be `constexpr`?



3rdparty/stout/include/stout/path.hpp (line 31)


Not sure what `char_separator` means. Can you name this differently?
Is this just the `separator`?


- Joris Van Remoortere


On May 14, 2016, 11:58 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47389/
> ---
> 
> (Updated May 14, 2016, 11:58 a.m.)
> 
> 
> Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout: Added support for correct path delimiters in Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/constants.hpp 
> 438840aaef189ef4fb457b856790efb3b6333a7d 
>   3rdparty/stout/include/stout/path.hpp 
> ef538045a8b7a1e3d8962c869317d86a85e0259f 
> 
> Diff: https://reviews.apache.org/r/47389/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 47387: Stout: Implemented `fsync.hpp` for Windows.

[Joris Van Remoortere]

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


Fix it, then Ship it!





3rdparty/stout/include/stout/os/windows/fsync.hpp (line 36)


indentation


- Joris Van Remoortere


On May 14, 2016, 11:58 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47387/
> ---
> 
> (Updated May 14, 2016, 11:58 a.m.)
> 
> 
> Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Bugs: MESOS-5382
> https://issues.apache.org/jira/browse/MESOS-5382
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout: Implemented `fsync.hpp` for Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/Makefile.am 9b39ce32c0269479066cf7991afaeed65d8ab547 
>   3rdparty/stout/include/stout/os/fsync.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/posix/fsync.hpp PRE-CREATION 
>   3rdparty/stout/include/stout/os/windows/fsync.hpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/47387/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 47388: Stout: Included missing headers in some files.

[Joris Van Remoortere]

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


Ship it!




Ship It!

- Joris Van Remoortere


On May 14, 2016, 11:58 a.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47388/
> ---
> 
> (Updated May 14, 2016, 11:58 a.m.)
> 
> 
> Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Stout: Included missing headers in some files.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/posix/fork.hpp 
> 695fe66b3c23e01a1b12c1233ab2cc14ab702981 
>   3rdparty/stout/include/stout/os/windows/killtree.hpp 
> b075d625541ed6c10192e3e98bf399b38b69cdc5 
>   3rdparty/stout/include/stout/windows.hpp 
> a7a59e78575e1456b4e14d18ac97f51dd23d794e 
> 
> Diff: https://reviews.apache.org/r/47388/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Re: Review Request 47324: Update leveldb to 1.18.

[Bing Li]

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


Ship it!




Tested on s390x, SLES12SP1.

- Bing Li


On May 14, 2016, 3:01 p.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47324/
> ---
> 
> (Updated May 14, 2016, 3:01 p.m.)
> 
> 
> Review request for mesos, Bing Li, Benjamin Mahler, Zhiwei Chen, and haosdent 
> huang.
> 
> 
> Bugs: MESOS-970
> https://issues.apache.org/jira/browse/MESOS-970
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Leveldb in modern version is required to support s390x.
> It's also required to replace default byte-wise comparator
> with varint comparator in `src/log/leveldb.cpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake 86c51edb3aa2daf6451459aaf18278f09b91b000 
>   3rdparty/leveldb-1.18.patch PRE-CREATION 
>   3rdparty/leveldb-1.18.tar.gz PRE-CREATION 
>   3rdparty/leveldb-1.4.patch b899f0141d633b1ffb2321e573395256fc893b16 
>   3rdparty/leveldb-1.4.tar.gz 2ddbc0c2e02054406ff0ea43ddc10d14979de8d8 
>   3rdparty/versions.am 7dcd6bf914de3213755ec9d4e701a190750424e9 
>   LICENSE eb39f6d69a165f59c00e8bb0ba9e15be8c958a5b 
>   src/python/native_common/ext_modules.py.in 
> 2d4a45efa224b32f80ace4542a00062c5ccb06d5 
> 
> Diff: https://reviews.apache.org/r/47324/diff/
> 
> 
> Testing
> ---
> 
> Tested on Ubuntu amd_64 need to test on PPC
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 47260: Added 'ReviveAndSuppress' test for the allocator.

[Dario Rexin]

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


Ship it!




Ship It!

- Dario Rexin


On May 12, 2016, 4:01 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47260/
> ---
> 
> (Updated May 12, 2016, 4:01 p.m.)
> 
> 
> Review request for mesos and Dario Rexin.
> 
> 
> Bugs: MESOS-5279
> https://issues.apache.org/jira/browse/MESOS-5279
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added 'ReviveAndSuppress' test for the allocator.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> e6101fdffe5e340619d79d821a2f5f891cb2dec7 
> 
> Diff: https://reviews.apache.org/r/47260/diff/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 47401: Corrected using order in authorizer.cpp.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On May 15, 2016, 9:25 p.m., Joerg Schad wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47401/
> ---
> 
> (Updated May 15, 2016, 9:25 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mesos uses case-insensitive ordering of `using`.
> 
> 
> Diffs
> -
> 
>   src/authorizer/local/authorizer.cpp 
> e95435327bb3b6f447e814b8657bce8084535346 
> 
> Diff: https://reviews.apache.org/r/47401/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joerg Schad
> 
>

Re: Review Request 45960: Added interfaces to handle and track shareable resources.

[Jiang Yan Xu]

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




include/mesos/resources.hpp (line 167)


s/isShareable/isShared/



include/mesos/resources.hpp (lines 314 - 317)


We don't need this. As dicussed offline, whether destroying a volume is 
allowed is determined by the master and not the resources abstraction.



include/mesos/resources.hpp (line 343)


s/shareable/shared/



include/mesos/resources.hpp (line 345)


I know it's mirroring the comments for nonRevocable() but it feels a little 
strange as the API returns a Resouces object and not a bool.

Just 

```
// Returns the non-shared resources.
```

would be fine.



include/mesos/resources.hpp (line 346)


s/nonShareable/nonshared/.



src/common/resources.cpp (line 370)


s/ShareInfo/SharedInfo/



src/common/resources.cpp (line 750)


I like the use of `shareability` here. ;)



src/common/resources.cpp (lines 760 - 761)


This fits in one line.



src/common/resources.cpp (line 835)


Why the extra `()`?


- Jiang Yan Xu


On April 28, 2016, 5:16 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45960/
> ---
> 
> (Updated April 28, 2016, 5:16 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4431
> https://issues.apache.org/jira/browse/MESOS-4431
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added interfaces to handle and track shareable resources.
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp a557e97c65194d4aad879fb88d8edefd1c95b8d8 
>   include/mesos/v1/resources.hpp a5ba8fec4c9c3643646308f75a4b28cefe0b3df3 
>   src/common/resources.cpp f6ff92b591c15bc8e93fd85e1896349c3a7bb968 
>   src/v1/resources.cpp 8c3f2d1c1529915a59d47fe37bb3fc7a3267079a 
> 
> Diff: https://reviews.apache.org/r/45960/diff/
> 
> 
> Testing
> ---
> 
> Tests successful.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 45605: Introduced HTB class.

[haosdent huang]

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




src/linux/routing/queueing/htb.cpp (line 119)


Only need one blank line here.


- haosdent huang


On April 1, 2016, 9:50 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45605/
> ---
> 
> (Updated April 1, 2016, 9:50 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Jie Yu.
> 
> 
> Bugs: mesos-4749
> https://issues.apache.org/jira/browse/mesos-4749
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced HTB class API, prepare for per-container bandwidth limit.
> 
> 
> Diffs
> -
> 
>   src/linux/routing/internal.hpp 8f68119819f7c79ece1a13ac1894b1802ddc8e19 
>   src/linux/routing/queueing/discipline.hpp 
> 54d6b214ef6a38fd8279f6d01e6f4e3ccfddf634 
>   src/linux/routing/queueing/htb.hpp 857646190d21387f98832f5094128505a52a0776 
>   src/linux/routing/queueing/htb.cpp faadf32bd48cc6bf968b1229789903c0d01fd75c 
>   src/linux/routing/queueing/internal.hpp 
> 768ed325f9b259e150779eb3ad74f4e5d4bcc7a2 
> 
> Diff: https://reviews.apache.org/r/45605/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 45606: Added default class to htb qdisc.

[haosdent huang]

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




src/linux/routing/queueing/htb.cpp (line 49)


I think the style prefer

```
Config()
  : rate(0),
defaultClass(Handle(TC_H_UNSPEC)) {}


- haosdent huang


On April 1, 2016, 9:51 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45606/
> ---
> 
> (Updated April 1, 2016, 9:51 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Jie Yu.
> 
> 
> Bugs: mesos-4749
> https://issues.apache.org/jira/browse/mesos-4749
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added default class to htb::create().
> 
> 
> Diffs
> -
> 
>   src/linux/routing/queueing/htb.hpp 857646190d21387f98832f5094128505a52a0776 
>   src/linux/routing/queueing/htb.cpp faadf32bd48cc6bf968b1229789903c0d01fd75c 
> 
> Diff: https://reviews.apache.org/r/45606/diff/
> 
> 
> Testing
> ---
> 
> make && make check
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 45607: Move HTB qdisc out of containers.

[haosdent huang]

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




src/linux/routing/queueing/fq_codel.hpp (line 44)


Why change to constexpr here?


- haosdent huang


On April 1, 2016, 11:05 p.m., Cong Wang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45607/
> ---
> 
> (Updated April 1, 2016, 11:05 p.m.)
> 
> 
> Review request for mesos, Ian Downes and Jie Yu.
> 
> 
> Bugs: mesos-4749
> https://issues.apache.org/jira/browse/mesos-4749
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Move HTB qdisc out of containers.
> 
> 
> Diffs
> -
> 
>   src/linux/routing/queueing/fq_codel.hpp 
> 6fcd9a21e821da476e9c16f50ed781424338022a 
>   src/linux/routing/queueing/fq_codel.cpp 
> 26860d1552839b47be2b4cbdc575c0d20e6fcdc5 
>   src/linux/routing/queueing/htb.hpp 857646190d21387f98832f5094128505a52a0776 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
> 0fe2f486eb733acf738c1c61fc44f820d7401afc 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 323c84a3d960a196d8ba87f753814e9d43a07957 
> 
> Diff: https://reviews.apache.org/r/45607/diff/
> 
> 
> Testing
> ---
> 
> Manual test with --egress_unique_flow_per_container and/or 
> --egress_rate_limit_per_container
> 
> 
> Thanks,
> 
> Cong Wang
> 
>

Re: Review Request 46498: Add runtime for Appc Spec ex: command, workingdir and environment.

[Jojy Varghese]

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




src/slave/containerizer/mesos/containerizer.cpp (line 953)


It would be readable if we separat the two `if` blocks with a blanlk line.



src/slave/containerizer/mesos/isolators/appc/runtime.hpp (line 26)


s/APPC/AppC? Please look for more occurrences.



src/slave/containerizer/mesos/isolators/appc/runtime.hpp (line 29)


s/Appc/AppC as Guangya proposed?



src/slave/containerizer/mesos/isolators/appc/runtime.hpp (lines 64 - 73)


Do these need to be member functions? Could they be functions in `cpp` file?



src/slave/containerizer/mesos/isolators/appc/runtime.cpp (line 50)


No need to wrap to newline. It fits within 80 chars.



src/slave/containerizer/mesos/isolators/appc/runtime.cpp (line 147)


blank line above.



src/slave/containerizer/mesos/isolators/appc/runtime.cpp (line 171)


blank line above.



src/slave/containerizer/mesos/isolators/appc/runtime.cpp (line 174)


blank line above



src/slave/containerizer/mesos/isolators/appc/runtime.cpp (line 246)


I think you should use `string::empty`



src/slave/containerizer/mesos/provisioner/appc/store.cpp (line 201)


Do you need this variable?



src/slave/containerizer/mesos/provisioner/appc/store.cpp (line 203)


I think you just need the base image manifest here and not all the 
manifests. Right?


- Jojy Varghese


On May 14, 2016, 4:20 a.m., Srinivas Brahmaroutu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46498/
> ---
> 
> (Updated May 14, 2016, 4:20 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Jojy Varghese.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add runtime for Appc Spec ex: command, workingdir and environment.
> 
> 
> Diffs
> -
> 
>   include/mesos/appc/spec.proto 4ef0473748c2f47aff1729c1d969185803fe72d4 
>   include/mesos/slave/isolator.proto 60a9bb637e12593a97ed1a7c510ebccd4e5a9615 
>   src/Makefile.am 447bc2ab511ad173d3d911be10992be0974f4584 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 75e5a32a3e70ec60a6800e21a621673184ea0956 
>   src/slave/containerizer/mesos/isolators/appc/runtime.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/appc/runtime.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> 33e97fc645a9b7fbc9ae47f67c1b5dacf999fce0 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 5c96e9f6603d39889e6bc807874d35d0cb3556be 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> dcbbbaf797d4467bfd0bb1ee91ee9ce843e7d546 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> 1d477ef13ddd24fd8badae0decaa4a2271ecc746 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> 84fe52b6937c3b7d7628b17a2f045eec2f386b4d 
> 
> Diff: https://reviews.apache.org/r/46498/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Srinivas Brahmaroutu
> 
>

Re: Review Request 47374: Experimental: Separated mesos test helpers into a separate library.

[Joseph Wu]

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

(Updated May 16, 2016, 11:10 a.m.)


Review request for mesos, Adam B, Artem Harutyunyan, Kapil Arya, and Jan 
Schlicht.


Changes
---

Rebase and remove dependency.


Repository: mesos


Description
---

This gives external projects easier access to the test helpers used in
mesos tests.  

For example, a module writer may want to write a test like 
`src/tests/oversubscription_tests.cpp`.  To build and link against 
this library, the module writer would mimic the build flags for tests:
```
my_module_CPPFLAGS =   \
  -I$(ZOOKEEPER)/include   \
  -I$(ZOOKEEPER)/generated \
  -DSOURCE_DIR=\"$(abs_top_srcdir)\"   \
  -DBUILD_DIR=\"$(abs_top_builddir)\"  \
  ...
```


Diffs (updated)
-

  src/Makefile.am ce5245883f3d2661812272702c0d2060513b6d88 

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


Testing
---

make check


Thanks,

Joseph Wu

Re: Review Request 47274: Authorize what quota can be seen by GET_QUOTA_BY_ROLE.

[Alexander Rukletsov]

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



Okay, almost there. We should be good after one more pass.


src/master/master.hpp (line 1022)


Do you still need `request`?



src/master/quota_handler.cpp (line 421)


Since we are adding support for authorizing GET now, we need to adjust 
wording in `QUOTA_HELP()` in "master/http.cpp" (and run 
"./support/generate-endpoint-help.py" to update the docs). It would be great to 
say that if a user GETs /quota, the results can be silently filtered.



src/master/quota_handler.cpp (lines 427 - 428)


Mind writing a concise comment on why you need this copy?



src/master/quota_handler.cpp (line 430)


Feel free to kill this blank line.



src/master/quota_handler.cpp (line 434)


Could you please restore the TODO from the previous version of the patch? 
Also your comment

// Create a list of authorization actions for each role we may return.
// TODO(alexr): Batch these actions once we have BatchRequest in authorizer.



src/master/quota_handler.cpp (line 435)


How about s/getQuotaByRoleAuthorized/authorizedRoles?



src/master/quota_handler.cpp (line 443)


Any reason you don't say `const list&`?

Also maybe s/authorized/authorizedRolesCollected ?



src/master/quota_handler.cpp (line 452)


s/authorized/authorizedRoles ?



src/master/quota_handler.cpp (line 461)


Feel free to use `auto` here.

Also, `quotaInfoIt` can be a better name.



src/master/quota_handler.cpp (line 462)


If you rename `authorized` to `authorizedRoles`, you can then rename 
`result` to `authorized`. I'd say `if (authorized)` is more descriptive.



src/master/quota_handler.cpp (lines 467 - 468)


Blank line, please.



src/master/quota_handler.cpp (line 495)


We separate implementations with two blank lines. Thanks!



src/master/quota_handler.cpp (line 504)


Now in retrospect it looks like it was a mediocre idea to use `LOG(INFO)` 
in this case. We may have to revisit our policy around log levels one day.



src/master/quota_handler.cpp (lines 516 - 517)


Let's add a blank line for consistency with other `authorize*()` functions 
in this file.



src/master/quota_handler.cpp (lines 518 - 520)


Two blanks here, please!


- Alexander Rukletsov


On May 13, 2016, 6:48 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47274/
> ---
> 
> (Updated May 13, 2016, 6:48 p.m.)
> 
> 
> Review request for mesos, Adam B and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-5336
> https://issues.apache.org/jira/browse/MESOS-5336
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Authorize what quota can be seen by GET_QUOTA_BY_ROLE.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> 9adae8c2a2e1b2ee4b9068ea50fcebb0544f2e5d 
>   include/mesos/authorizer/authorizer.proto 
> 32492a59ad95df3bb673ec42321518f86c11af59 
>   src/authorizer/local/authorizer.cpp 
> e95435327bb3b6f447e814b8657bce8084535346 
>   src/master/http.cpp a0d67671c89b9794e721dc3ba012acd3bdc447e3 
>   src/master/master.hpp 3e55114ee7866e06513071e86e15608099dae052 
>   src/master/quota_handler.cpp 6590519d8440f352f5bf00fda805414a5aad725c 
>   src/tests/master_quota_tests.cpp 9ce7e2e481706c74f080d54b6f58d57874a27e64 
> 
> Diff: https://reviews.apache.org/r/47274/diff/
> 
> 
> Testing
> ---
> 
> Unit test.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 47213: Added FlagsBase::toVector method as an alternative to stringify(flags).

[Joseph Wu]

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

(Updated May 16, 2016, 11 a.m.)


Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, and Kapil Arya.


Changes
---

Rebase on top of the 3rdparty renaming change.


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


Repository: mesos


Description
---

`stringify(flags)` will return a version of the flags that can be passed
through `/bin/sh` for execution.  This method returns the same flags,
but in a more `exec`-friendly form.


Diffs (updated)
-

  3rdparty/stout/include/stout/flags/flags.hpp 
91542f56431579be8a3da5ec6e3e58f68e7dfcbe 

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


Testing
---

make check


Thanks,

Joseph Wu

Re: Review Request 45959: Support arithmetic operations for shared resources with consumer counts.

[Jiang Yan Xu]

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



Haven't looked at tests but given the anticipated change in the new revision 
let's do that with the new revision. :)


include/mesos/resources.hpp (line 189)


According to our discussion with other reviewers the scope the `Resource_` 
is changed from "a step towards a generic C++ resource abstraction" to "an 
internal abstraction to facilitiate managing shared resources".

We can comment here:

```
// An internal abstraction to facilitiate managing shared resources.
// It allows 'Resources' to group identical shared resource objects 
// together into a single 'Resource_' object and tracked by its internal
// counters. Nonshared resource objects are not grouped.
```



include/mesos/resources.hpp (line 190)


This is supposed to be hidden in private right?



include/mesos/resources.hpp (line 195)


No space before `(None())`.



include/mesos/resources.hpp (lines 197 - 200)


Chatted offline already and this should be 1 for shared resources and None 
for nonshared resources.



include/mesos/resources.hpp (line 203)


This comment states the obvious fact.

To add more information we could say:

```
// By implicitly converting to Resource we are able to keep Resource_ logic 
// internal and expose only the protobuf object.
```



include/mesos/resources.hpp (line 211)


This indentation is a bit unusual but this fits in one line if we just do:

```
bool isShared() const { return shareCount.isSome(); }
```

`resource.has_share()` is the same as `shareCount.isSome()`, we should just 
use one.



include/mesos/resources.hpp (lines 225 - 238)


In the latest design we shouldn't need this anymore.



include/mesos/resources.hpp (lines 240 - 253)


These can be public if the class itself is private.



include/mesos/resources.hpp (lines 255 - 260)


As disucssed we should remove this.

It's hard to explain what 'initialized state' is in this context.



include/mesos/resources.hpp (line 267)


Could you s/sharedCount/sharedCount/ so it's consistent with the use of the 
word `shared` in protobuf, code and comments elsewhere.



include/mesos/resources.hpp (lines 281 - 282)


Do we need this?



include/mesos/resources.hpp (lines 303 - 304)


We talked about its use in tests: I think exposing `size_t 
Resources::count(const Resource&)` is more direct in returning the information 
about how many copies of the shared resources are in the container.

As for the concerns about leaking the value of the internal counter: I 
think this is fine because the correctness of the Resources arithmetic doesn't 
rely on the callers carefully manipulating the counters correctly or calling 
particular methods based on the result of `count()`. The `count()` method 
simply exposes information for uses other than Resources arithmetic operations 
themselves. Similar to 
[set::count()](http://en.cppreference.com/w/cpp/container/unordered_multiset/count).



include/mesos/resources.hpp (lines 306 - 309)


As discussed, we'll no longer need this.



include/mesos/resources.hpp (lines 406 - 409)


As discussed, we'll no longer need this.



include/mesos/v1/resources.hpp (lines 303 - 304)






include/mesos/v1/resources.hpp (lines 453 - 454)


When do people need to using the vector explicitly?



include/mesos/v1/resources.hpp (line 458)


s/generated runtime/generated at runtime/.



include/mesos/v1/resources.hpp (lines 484 - 491)


Should they both exist or we can just replace 

```
bool Resources::_contains(const Resource& that) const
```

with 

```
bool Resources::_contains(const Resource_& that) const
```

?



src/common/resources.cpp (line 210)

Re: Review Request 47214: Replaced subprocess flag stringification with flags.toVector().

[Joseph Wu]

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

(Updated May 16, 2016, 11:01 a.m.)


Review request for mesos, Adam B, Artem Harutyunyan, Jie Yu, and Kapil Arya.


Changes
---

Rebase on the flag alias change.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  3rdparty/libprocess/src/subprocess.cpp 
44073146118b6c6e9e730b8c901852594080a3eb 

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


Testing
---

make check


Thanks,

Joseph Wu

Re: Review Request 44209: Closed the input ifstream before return error.

[Anand Mazumdar]

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



Why do we need to close it explicitly? `ifstream` already follows the RAII 
idiom i.e. things are cleaned up upon destruction. What am I missing?

- Anand Mazumdar


On May 16, 2016, 5:32 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44209/
> ---
> 
> (Updated May 16, 2016, 5:32 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Closed the input ifstream before return error.
> 
> 
> Diffs
> -
> 
>   src/log/tool/benchmark.cpp 7e1350675bdfe6c44e86b082bc01f692cc72b850 
> 
> Diff: https://reviews.apache.org/r/44209/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 47281: Mesos::master::allocation namespace removed.

[haosdent huang]

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


Fix it, then Ship it!




Ship It!


src/local/local.cpp 


Kill this line.



src/master/allocator/mesos/hierarchical.hpp (line 46)


Add a blank line above.



src/tests/master_allocator_tests.cpp (line 58)


Kill this line



src/tests/rate_limiting_tests.cpp (line 41)


I think this line is unecessary here because `using namespace` before.



src/tests/reservation_tests.cpp (line 52)


Kill this line.


- haosdent huang


On May 14, 2016, 6:25 p.m., Jose Guilherme Vanz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47281/
> ---
> 
> (Updated May 14, 2016, 6:25 p.m.)
> 
> 
> Review request for mesos and haosdent huang.
> 
> 
> Bugs: MESOS-2516
> https://issues.apache.org/jira/browse/MESOS-2516
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To keep consistence with the rest of the codebase, the allocation
> namespace has been removed from mesos::master namespace. To not
> pollute the master namespace, sorters have been moved to the new
> mesos::master::sorter namespace
> 
> MESOS-2516
> 
> 
> Diffs
> -
> 
>   docs/allocation-module.md 651f0a232426f537f7a1f3dbd96b0f24b60d695f 
>   include/mesos/master/allocator.hpp a4743c5a31b18d96722a42d526bfb669d30e6e48 
>   include/mesos/module/allocator.hpp 5e65e69b6d62611f51e85aa9adaa283713896d4c 
>   src/examples/test_allocator_module.cpp 
> 1255a4a22a9d1a5724bef78dfc4dee498ed50fff 
>   src/local/local.hpp f4ae285edc30a0fb1c960d50dfb1a859b2eae166 
>   src/local/local.cpp 352f4239cbcb6834b57295b86a57e41ff2798497 
>   src/master/allocator/allocator.cpp 8e76a3a33efd896a21960b0092dfdee800d211f5 
>   src/master/allocator/mesos/allocator.hpp 
> 64bce0fb143b109c26f923cd97d5facb393dee9d 
>   src/master/allocator/mesos/hierarchical.hpp 
> 3f5dff72945bf81fda4e5bc9b11edfba40cced9d 
>   src/master/allocator/mesos/hierarchical.cpp 
> 51125bac62735879f66234765dad92a759f6724b 
>   src/master/allocator/mesos/metrics.hpp 
> 06fdd1633f63c005230e1cc5fafdad358a0ac254 
>   src/master/allocator/mesos/metrics.cpp 
> a36d21c297160bc1c9f43a22743fd5448d7ae5ac 
>   src/master/allocator/sorter/drf/metrics.hpp 
> 61568cb520826ab59d675824b212e0d3deb63764 
>   src/master/allocator/sorter/drf/metrics.cpp 
> c793f3216287fd40998a1d4aab016d55aa1ac151 
>   src/master/allocator/sorter/drf/sorter.hpp 
> 05d5205d29ad74e01e07c508d88b6f8371541513 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 4306973277b9d32356eed31ceabac09fb2a03e6c 
>   src/master/allocator/sorter/sorter.hpp 
> 9e04adf54f2d80541a95f0a9a49b329dc9e8f5e3 
>   src/master/main.cpp 172c0992a00700c0addf4d1faddb2d449eca736f 
>   src/master/master.hpp 380be95c0e861100680c5906b589049d56463766 
>   src/master/master.cpp 9205b3c50f655c7f5d617949320abdf47e7fb6a2 
>   src/tests/allocator.hpp 4081193abed29b99f60254e98ef9ebc981bde859 
>   src/tests/cluster.hpp c93f0526382360bf0bf2dcca7aca48ded2dc5828 
>   src/tests/cluster.cpp c03e053ffdc708792905b2d494402733928d6589 
>   src/tests/fault_tolerance_tests.cpp 
> c39a0ccf8fa124b2c230864f3ba1f7e8e05b36dc 
>   src/tests/hierarchical_allocator_tests.cpp 
> d75df737d2a62c77176cc02fc47362943c29b551 
>   src/tests/http_fault_tolerance_tests.cpp 
> baa07395b9bd588daa5438369954584787d7952a 
>   src/tests/master_allocator_tests.cpp 
> ec46913e7fed4bc8cda68ff20d04d6714ecb644d 
>   src/tests/master_authorization_tests.cpp 
> 5c221f059585d49be5848bc0b655adab6b02 
>   src/tests/master_maintenance_tests.cpp 
> 971c4473acbec5206614f46f51b06b3460595d50 
>   src/tests/master_tests.cpp 25c3559f2ac0112137c56efe423ba828fd2b2157 
>   src/tests/mesos.hpp 79bf1ff16412ce2a510a9b75ab1ac91c1c182653 
>   src/tests/mesos.cpp 849e656c8b85304cccecc9d7b9d9116884132244 
>   src/tests/partition_tests.cpp 3ec51cedcea9c7ed5cf41e9e0087fc044c85d2f1 
>   src/tests/rate_limiting_tests.cpp 5d68ac170cb5a948df1429ffbf8e5b8f02f708d9 
>   src/tests/reservation_tests.cpp 2d7fb21e2fe153c2b62dfd60bbaccb350a157391 
>   src/tests/resource_offers_tests.cpp 
> 046adaedf9121655f377f503bb30437803bf0005 
>   src/tests/scheduler_driver_tests.cpp 
> 217185780e3576faf633dd9629ae93a275fac284 
>   src/tests/scheduler_tests.cpp d9cc3fde0caeadb16164e68ed66be098fd3ada7c 
>   src/tests/slave_recovery_tests.cpp 70fdd0dae4ada1ccbdbc809c7e805d9738346b02 
>   src/tests/sorter_tests.cpp eb207a36c18198588cd8a98b3860a66c2ff7a641 
> 
> Diff: 

Re: Review Request 47362: Added a test for the SSL head-of-line blocking issue in MESOS-5340.

[Alexander Rukletsov]

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




3rdparty/libprocess/src/tests/ssl_tests.cpp (lines 730 - 731)


IIUC, this is an implementation detail, which may change in the future. 
Currently our code relies on the callback from lebevent, which is invoked then 
data is sent. We actually don't care whether the SSL handshake has completed, 
we *just* want that a pending socket does not prevent us from accepting new 
connections.

How about
```
  // We initiate a connection on which we will not send
  // any data. This may prevent the socket from entering
  // ready state and hence the future signaled.
```



3rdparty/libprocess/src/tests/ssl_tests.cpp (line 744)


If you accept the suggestion above, it makes sense to kill this sentence.



3rdparty/libprocess/src/tests/ssl_tests.cpp (lines 755 - 759)


Why do you construct the response manually instead of using `http::OK`?


- Alexander Rukletsov


On May 13, 2016, 9:46 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47362/
> ---
> 
> (Updated May 13, 2016, 9:46 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, and Neil 
> Conway.
> 
> 
> Bugs: MESOS-5340
> https://issues.apache.org/jira/browse/MESOS-5340
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test works by creating a connection on which no data is sent (the SSL 
> handshake does not complete, nor is the socket downgraded). After which, we 
> expect that an HTTP request should succeed.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/ssl_tests.cpp 
> 4d237815a03828b915e821c3af78132e2915c610 
> 
> Diff: https://reviews.apache.org/r/47362/diff/
> 
> 
> Testing
> ---
> 
> This test fails before the fix in MESOS-5340.
> 
> ```
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from SSLTest
> [ RUN  ] SSLTest.SilentSocket
> ../../../3rdparty/libprocess/src/tests/ssl_tests.cpp:752: Failure
> Failed to wait 15secs for socket
> [  FAILED  ] SSLTest.SilentSocket (15221 ms)
> [--] 1 test from SSLTest (15221 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (15222 ms total)
> [  PASSED  ] 0 tests.
> [  FAILED  ] 1 test, listed below:
> [  FAILED  ] SSLTest.SilentSocket
> ```
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 47209: Establish TCP connection after backing off.

[Cong Wang]

> On May 14, 2016, 2:01 a.m., Qian Zhang wrote:
> > src/slave/slave.cpp, line 1020
> > 
> >
> > If we do the link here, that means we still establish TCP connection 
> > with master right after a new master is detected, right? But I think we 
> > want to do the link after the initial backoff to avoid SYN flood.
> 
> Cong Wang wrote:
> Authentication code path doesn't have a backoff, so we don't want to 
> touch it in this patch.
> 
> Qian Zhang wrote:
> Thanks, so that means if there are a large number of agents in the 
> cluster and each agent has authentication enabled, when new master is 
> detected, it is still possible all those agents try to connect with master at 
> the same time?

If they use TCP, I think so. Also, there is already a TODO saying we need a 
backoff for authentication too.


- Cong


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


On May 13, 2016, 3:37 a.m., David Robinson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47209/
> ---
> 
> (Updated May 13, 2016, 3:37 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Ian Downes, and Cong Wang.
> 
> 
> Bugs: MESOS-5330
> https://issues.apache.org/jira/browse/MESOS-5330
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Establish TCP connection after backing off.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 116ea59b72950db4a7cd721b7ba5bfbb2e1c1454 
> 
> Diff: https://reviews.apache.org/r/47209/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> Also started a master and agent and captured a tcpdump to verify the 
> connection is only established _after_ the registration backoff.
> 
> 
> Thanks,
> 
> David Robinson
> 
>

Re: Review Request 47412: Windows: Use Winsock class in slave and containerizer.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [47052, 47053, 47221, 47054, 41632, 47168, 47169, 47386, 
47387, 47388, 47389, 47390, 47391, 47403, 47404, 47409, 47410, 47411, 47412]

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 May 16, 2016, 2:04 p.m., Alex Clemmer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47412/
> ---
> 
> (Updated May 16, 2016, 2:04 p.m.)
> 
> 
> Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
> Remoortere, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Use Winsock class in slave and containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/main.cpp 
> ed0c6cd0721b9f486be056d2ae62cbccf53bc834 
>   src/slave/main.cpp fee46bafc88f8cdade868aab8c0fee79b8d2fb6d 
> 
> Diff: https://reviews.apache.org/r/47412/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>

Review Request 47412: Windows: Use Winsock class in slave and containerizer.

[Alex Clemmer]

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

Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Repository: mesos


Description
---

Windows: Use Winsock class in slave and containerizer.


Diffs
-

  src/slave/containerizer/mesos/main.cpp 
ed0c6cd0721b9f486be056d2ae62cbccf53bc834 
  src/slave/main.cpp fee46bafc88f8cdade868aab8c0fee79b8d2fb6d 

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


Testing
---


Thanks,

Alex Clemmer

Review Request 47411: Agent: Changed the names of symbols that are ambiguous on MSVC.

[Alex Clemmer]

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

Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Repository: mesos


Description
---

Agent: Changed the names of symbols that are ambiguous on MSVC.


Diffs
-

  src/docker/docker.hpp da6e9a25b99079940958001128ee949cdb9b931b 
  src/slave/slave.cpp feb9f291b4188aac1a63ab9cfae8349a048663b1 

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


Testing
---


Thanks,

Alex Clemmer

Review Request 47409: Libprocess: Implemented `HANDLE` versions of file descriptor functions.

[Alex Clemmer]

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

Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


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


Repository: mesos


Description
---

Libprocess: Implemented `HANDLE` versions of file descriptor functions.


Diffs
-

  3rdparty/libprocess/include/process/io.hpp 
2f1e626c0eec7c965ddb7acbc9cfe890a621afd3 
  3rdparty/libprocess/src/io.cpp 44dee23ec9fa958f8e047cf93b87a4031638ab5e 

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


Testing
---


Thanks,

Alex Clemmer

Review Request 47410: CMake: Added build targets for Mesos executables.

[Alex Clemmer]

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

Review request for mesos, Daniel Pravat, Artem Harutyunyan, Joris Van 
Remoortere, and Michael Park.


Repository: mesos


Description
---

CMake: Added build targets for Mesos executables.


Diffs
-

  3rdparty/libprocess/cmake/Process3rdpartyConfigure.cmake 
ac75baf3c660f2bdda308f4eaa856f44d80f1ea1 

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


Testing
---


Thanks,

Alex Clemmer

Re: Review Request 47406: Rename all flag variables from keyword 'slave' to 'agent'.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [45200, 47406]

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 May 16, 2016, 7:39 a.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47406/
> ---
> 
> (Updated May 16, 2016, 7:39 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-3781
> https://issues.apache.org/jira/browse/MESOS-3781
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Include:
> --agent_reregister_timeout
> --recovery_agent_removal_limit
> --agent_removal_rate_limit
> --authenticate_agents
> --agent_ping_timeout
> --max_agent_ping_timeouts
> --max_executors_per_agent
> --agent_subsystems
> 
> 
> Diffs
> -
> 
>   src/local/local.cpp 352f4239cbcb6834b57295b86a57e41ff2798497 
>   src/master/constants.hpp e7e02a3675134273472ada968df03fa772e3b4a9 
>   src/master/flags.hpp e4cac1f8d688319c804e608b7229f458f779364a 
>   src/master/flags.cpp b97f01149b428a736d052cf7aece8ae092ca8cd4 
>   src/master/http.cpp 0245b865b78f89edd5a6c1096d664560e2d33d69 
>   src/master/main.cpp 172c0992a00700c0addf4d1faddb2d449eca736f 
>   src/master/master.cpp 9205b3c50f655c7f5d617949320abdf47e7fb6a2 
>   src/messages/messages.proto 5593881229f57a11634357f1f85d697f11f4c827 
>   src/slave/constants.cpp fb6b9f87aa1933af3ce5e8fab93496c11412d48f 
>   src/slave/containerizer/mesos/isolators/cgroups/cpushare.cpp 
> 52d3ddcf80cb0a3f9eb98c082dfb0216c34a7ffd 
>   src/slave/containerizer/mesos/isolators/cgroups/devices/gpus/nvidia.cpp 
> 703e49f86ae41c11ae350ae51107eda00d0d5b7e 
>   src/slave/containerizer/mesos/isolators/cgroups/mem.cpp 
> 680cdc7b030490e442dfcf12a981082262eb5b20 
>   src/slave/containerizer/mesos/isolators/cgroups/net_cls.cpp 
> e886f7e425c14f11d07bf9ba530f40498f54317f 
>   src/slave/containerizer/mesos/isolators/cgroups/perf_event.cpp 
> 5ef4ae5c468580352cd16e7716b9ca4c0acde659 
>   src/slave/flags.hpp 80ba2887448e91c40ae68fc2d9f0c0bee1a49f48 
>   src/slave/flags.cpp b7df8f760d0f75459f1e80e3d8e18d49a3995df8 
>   src/slave/slave.cpp bea35e5f7e22ced32514706827b4331c956ee674 
>   src/tests/authentication_tests.cpp 513a0b18a3238a064365002c03e99e3ee5de398c 
>   src/tests/cluster.cpp c03e053ffdc708792905b2d494402733928d6589 
>   src/tests/containerizer/memory_pressure_tests.cpp 
> c90dcb964d172d3da96e83f1f8721e672ca2c677 
>   src/tests/hook_tests.cpp 60d52c5849ba555f6f3070883d87aadf105f05b0 
>   src/tests/master_tests.cpp 25c3559f2ac0112137c56efe423ba828fd2b2157 
>   src/tests/mesos.cpp 849e656c8b85304cccecc9d7b9d9116884132244 
>   src/tests/partition_tests.cpp 3ec51cedcea9c7ed5cf41e9e0087fc044c85d2f1 
>   src/tests/slave_recovery_tests.cpp 70fdd0dae4ada1ccbdbc809c7e805d9738346b02 
>   src/tests/slave_tests.cpp 4b1cfbb9f8de0e60a8a2c1f48b78c62be9fbaa40 
> 
> Diff: https://reviews.apache.org/r/47406/diff/
> 
> 
> Testing
> ---
> 
> Linux:
> configure --with-network-isolator --disable-python --disable-java
> make check
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 46093: Enhanced the error message for invalid duration unit.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46093]

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 May 16, 2016, 6:18 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46093/
> ---
> 
> (Updated May 16, 2016, 6:18 a.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-5179
> https://issues.apache.org/jira/browse/MESOS-5179
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enhanced the error message for invalid duration unit.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/duration.hpp 
> aea280830731a0e8c1aacd3fb6aef27291cc6f0b 
> 
> Diff: https://reviews.apache.org/r/46093/diff/
> 
> 
> Testing
> ---
> 
> LiuGuangyas-MacBook-Pro:build gyliu$ ./bin/mesos-master.sh --offer_timeout=4cc
> Failed to load flag 'offer_timeout': Failed to load value '4cc': Unknown 
> duration unit 'cc', the supported units are 'ns', 'us', 'ms', 'secs', 'mins', 
> 'hrs', 'days' and 'weeks'
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 39848: Validated revocable resources before run task.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [39845, 39848]

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 May 16, 2016, 6:10 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39848/
> ---
> 
> (Updated May 16, 2016, 6:10 a.m.)
> 
> 
> Review request for mesos, Niklas Nielsen and Vinod Kone.
> 
> 
> Bugs: MESOS-2647
> https://issues.apache.org/jira/browse/MESOS-2647
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Validated revocable resources before run task.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp bea35e5f7e22ced32514706827b4331c956ee674 
> 
> Diff: https://reviews.apache.org/r/39848/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 47340: Modified doc file for '/containers'.

[Abhishek Dasgupta]

> On May 13, 2016, 8:15 p.m., Alexander Rukletsov wrote:
> > Could you please squash this with https://reviews.apache.org/r/47061/ to 
> > minimize the churn?

done. Please re-check.


- Abhishek


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


On May 13, 2016, 2:23 p.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47340/
> ---
> 
> (Updated May 13, 2016, 2:23 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, and Till 
> Toenshoff.
> 
> 
> Bugs: Mesos-5316
> https://issues.apache.org/jira/browse/Mesos-5316
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch modifies marked down file for generating doc for endpoint
> '/containers' to include authentication requirement.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/slave/containers.md ae6559b4fa360a012aab2cc5f00e4ba626a59256 
> 
> Diff: https://reviews.apache.org/r/47340/diff/
> 
> 
> Testing
> ---
> 
> Viewed the marked down file in the mesos documentation website generated 
> locally.
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 47061: Authenticated the agent's '/containers' endpoint.

[Abhishek Dasgupta]

> On May 13, 2016, 2:50 p.m., Alexander Rukletsov wrote:
> > src/slave/slave.cpp, line 766
> > 
> >
> > Uups, is it intended? If yes, please do it in a separate patch.

Thought again and discarded logging.


- Abhishek


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


On May 16, 2016, 7:37 a.m., Abhishek Dasgupta wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/47061/
> ---
> 
> (Updated May 16, 2016, 7:37 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-5316
> https://issues.apache.org/jira/browse/MESOS-5316
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Authenticated the agent's '/containers' endpoint.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/slave/containers.md ae6559b4fa360a012aab2cc5f00e4ba626a59256 
>   src/slave/http.cpp 6f4548d7817e239035e491c10899ede5679d282d 
>   src/slave/slave.hpp be622d31de29a242a6c71fd8dedb06aeff19142d 
>   src/slave/slave.cpp feb9f291b4188aac1a63ab9cfae8349a048663b1 
> 
> Diff: https://reviews.apache.org/r/47061/diff/
> 
> 
> Testing
> ---
> 
> make -j2 check
> 
> on ubuntu 16.04 (Intel)
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>

Re: Review Request 45200: Add deprecated flag names.

[Jay Guo]

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

(Updated May 16, 2016, 7:37 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

delete redundant tests


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


Repository: mesos


Description
---

Replace 'slave' with 'agent' in flag names. Original names are marked
as deprecated.


Diffs (updated)
-

  src/master/flags.cpp b97f01149b428a736d052cf7aece8ae092ca8cd4 
  src/slave/flags.cpp b7df8f760d0f75459f1e80e3d8e18d49a3995df8 

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


Testing
---

./configure --with-network-isolator
make check (on OSX and Ubuntu)


Thanks,

Jay Guo

Re: Review Request 47061: Authenticated the agent's '/containers' endpoint.

[Abhishek Dasgupta]

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

(Updated May 16, 2016, 7:37 a.m.)


Review request for mesos, Adam B, Alexander Rukletsov, Greg Mann, and Till 
Toenshoff.


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


Repository: mesos


Description
---

Authenticated the agent's '/containers' endpoint.


Diffs (updated)
-

  docs/endpoints/slave/containers.md ae6559b4fa360a012aab2cc5f00e4ba626a59256 
  src/slave/http.cpp 6f4548d7817e239035e491c10899ede5679d282d 
  src/slave/slave.hpp be622d31de29a242a6c71fd8dedb06aeff19142d 
  src/slave/slave.cpp feb9f291b4188aac1a63ab9cfae8349a048663b1 

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


Testing (updated)
---

make -j2 check

on ubuntu 16.04 (Intel)


Thanks,

Abhishek Dasgupta

Re: Review Request 44209: Closed the input ifstream before return error.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [44209]

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 May 16, 2016, 5:32 a.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44209/
> ---
> 
> (Updated May 16, 2016, 5:32 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Timothy Chen.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Closed the input ifstream before return error.
> 
> 
> Diffs
> -
> 
>   src/log/tool/benchmark.cpp 7e1350675bdfe6c44e86b082bc01f692cc72b850 
> 
> Diff: https://reviews.apache.org/r/44209/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 46093: Enhanced the error message for invalid duration unit.

[Guangya Liu]

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

(Updated 五月 16, 2016, 6:18 a.m.)


Review request for mesos and Alexander Rukletsov.


Changes
---

Rebase


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


Repository: mesos


Description
---

Enhanced the error message for invalid duration unit.


Diffs (updated)
-

  3rdparty/stout/include/stout/duration.hpp 
aea280830731a0e8c1aacd3fb6aef27291cc6f0b 

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


Testing
---

LiuGuangyas-MacBook-Pro:build gyliu$ ./bin/mesos-master.sh --offer_timeout=4cc
Failed to load flag 'offer_timeout': Failed to load value '4cc': Unknown 
duration unit 'cc', the supported units are 'ns', 'us', 'ms', 'secs', 'mins', 
'hrs', 'days' and 'weeks'


Thanks,

Guangya Liu

Re: Review Request 39845: Added REASON_RESOURCE_OVERSUBSCRIBED to mesos proto.

[Guangya Liu]

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

(Updated 五月 16, 2016, 6:10 a.m.)


Review request for mesos, Niklas Nielsen and Vinod Kone.


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


Repository: mesos


Description
---

Added REASON_RESOURCE_OVERSUBSCRIBED to mesos proto.


Diffs (updated)
-

  include/mesos/mesos.proto 887ffe9c5f04e25539a4c6b3d52ce5299c65e8d3 
  include/mesos/v1/mesos.proto 9e59aed20965d50ee10989ff6b75db742cf2b83b 

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


Testing
---

make
make check


Thanks,

Guangya Liu