Re: Review Request 53009: Add support for task labels to example no_executor_framework.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [53009]

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

- Mesos ReviewBot


On Oct. 19, 2016, 1:07 a.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53009/
> ---
> 
> (Updated Oct. 19, 2016, 1:07 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Ilya Pronin.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add support for task labels to example no_executor_framework.
> 
> 
> Diffs
> -
> 
>   src/examples/no_executor_framework.cpp 
> e82ae9a9ea23c7d181b071f0e3f5071b3856d8a4 
> 
> Diff: https://reviews.apache.org/r/53009/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 52254: Updated aufs mount with `rw` and `ro+wh` options.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Sept. 26, 2016, 7:26 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52254/
> ---
> 
> (Updated Sept. 26, 2016, 7:26 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-6002
> https://issues.apache.org/jira/browse/MESOS-6002
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated aufs mount with `rw` and `ro+wh` options.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/backends/aufs.cpp 
> c69dc425987661cd84ac882b2dc90cc8d7ca1d45 
> 
> Diff: https://reviews.apache.org/r/52254/diff/
> 
> 
> Testing
> ---
> 
> make check on Ubuntu 14.04.
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 53009: Add support for task labels to example no_executor_framework.

[Jie Yu]

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




src/examples/no_executor_framework.cpp (line 315)


Can you use Option here and rely on json to protobuf parsing 
instead?


- Jie Yu


On Oct. 19, 2016, 1:07 a.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53009/
> ---
> 
> (Updated Oct. 19, 2016, 1:07 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Ilya Pronin.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add support for task labels to example no_executor_framework.
> 
> 
> Diffs
> -
> 
>   src/examples/no_executor_framework.cpp 
> e82ae9a9ea23c7d181b071f0e3f5071b3856d8a4 
> 
> Diff: https://reviews.apache.org/r/53009/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 52997: Improve Socket::connect error message.

[Michael Park]

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




3rdparty/libprocess/src/poll_socket.cpp (lines 133 - 141)


Do we need to set these here...? If I understand correctly they already 
hold the correct values, and we would be overwriting it with an incorrect one.

Assuming we want `SocketError` on line 140 to build an error message based 
on the corresponding error code, `getsockopt` automatically sets the `errno` 
and `WSALastError` correctly.

Linux:

> On success, zero is returned for the standard options.  On error, -1 is 
returned, and errno is set appropriately.

Windows:

> If no error occurs, getsockopt returns zero. Otherwise, a value of 
SOCKET_ERROR is returned, and a specific error code can be retrieved by calling 
WSAGetLastError.


- Michael Park


On Oct. 18, 2016, 10:43 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52997/
> ---
> 
> (Updated Oct. 18, 2016, 10:43 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-6412
> https://issues.apache.org/jira/browse/MESOS-6412
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rather than returning a fixed generic error when connect(2) fails,
> report the actual socket error as well as the address we were trying to
> connect to.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/poll_socket.cpp 
> d9ab3fbbf385c39c0e43ee92ff06609ae8d47e2d 
> 
> Diff: https://reviews.apache.org/r/52997/diff/
> 
> 
> Testing
> ---
> 
> `make check` on a host whose hostname doesn't match its IP address.
> 
> ```
> [jpeach@jpeach libprocess]$ ./libprocess-tests 
> --gtest_filter=HTTPTest.Endpoints
> Note: Google Test filter = HTTPTest.Endpoints
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from HTTPTest
> [ RUN  ] HTTPTest.Endpoints
> ../../../3rdparty/libprocess/src/tests/http_tests.cpp:141: Failure
> (socket.connect(http.process->self().address)).failure(): Failed to connect 
> to 17.203.52.49:39241: Connection refused
> [  FAILED  ] HTTPTest.Endpoints (4 ms)
> [--] 1 test from HTTPTest (4 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (5 ms total)
> [  PASSED  ] 0 tests.
> [  FAILED  ] 1 test, listed below:
> [  FAILED  ] HTTPTest.Endpoints
> 
>  1 FAILED TEST
> ```
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 53009: Add support for task labels to example no_executor_framework.

[Ilya Pronin]

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


Ship it!




LGTM


src/examples/no_executor_framework.cpp (lines 127 - 137)


Since currently task labels are simply copied into `TaskInfo` they can be 
baked into `Labels` outside of `NoExecutorScheduler`. But here this is more a 
style preference, feel free to drop.


- Ilya Pronin


On Oct. 19, 2016, 2:07 a.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53009/
> ---
> 
> (Updated Oct. 19, 2016, 2:07 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Ilya Pronin.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add support for task labels to example no_executor_framework.
> 
> 
> Diffs
> -
> 
>   src/examples/no_executor_framework.cpp 
> e82ae9a9ea23c7d181b071f0e3f5071b3856d8a4 
> 
> Diff: https://reviews.apache.org/r/53009/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 52976: Split mesos test helpers into 'internal' and 'v1' namespaces.

[Joris Van Remoortere]

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

(Updated Oct. 19, 2016, 4:50 a.m.)


Review request for mesos, Anand Mazumdar and Jie Yu.


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


Repository: mesos


Description
---

Split mesos test helpers into 'internal' and 'v1' namespaces.


Diffs
-

  src/tests/api_tests.cpp f0bafd1d5207a687ca0489b05ed1acdc2a6d875d 
  src/tests/containerizer.hpp 940c4146f4e854a6b1b9ccaba5687e76d5723cba 
  src/tests/containerizer.cpp 27c29b40f3241807fb22b27f0d9ca64b3e602a8f 
  src/tests/default_executor_tests.cpp 92e6b9f5fb80811c94632de3bb20c8e6d2e895ff 
  src/tests/executor_http_api_tests.cpp 
a9f1a7b0498acd541c6f58ad1388da49c9951e22 
  src/tests/fault_tolerance_tests.cpp 5a9944cf459ab688907d95bbda09f464b37efd1e 
  src/tests/http_fault_tolerance_tests.cpp 
57ef562058f8abf9256e2ab8a4a85b36b5a7add4 
  src/tests/master_contender_detector_tests.cpp 
2a7d713f74c907235f82d83eaf46630046645faf 
  src/tests/master_maintenance_tests.cpp 
6917272f2de7a09bf4de7e932994655f4e54d3da 
  src/tests/master_tests.cpp 88cf1e612ad8186ef2cea161b3a52b0df9517305 
  src/tests/mesos.hpp 9309b5a985c0d7136a2ee5aa1598b4fee6194816 
  src/tests/scheduler_http_api_tests.cpp 
6390f2eb53d8bb97b98e64ec6e0f795abc3f3c7f 
  src/tests/scheduler_tests.cpp b0ea0bbcce9d847285fda40f778caaf721804457 
  src/tests/slave_recovery_tests.cpp 703948f7a6861a4401ee45ce9cae2644106083f3 
  src/tests/slave_tests.cpp 93b81d3e5b90d8036409e943f598c865fe335bcc 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 52976: Split mesos test helpers into 'internal' and 'v1' namespaces.

[Joris Van Remoortere]

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

(Updated Oct. 19, 2016, 4:49 a.m.)


Review request for mesos, Anand Mazumdar and Jie Yu.


Changes
---

Fixed issues.


Repository: mesos


Description
---

Split mesos test helpers into 'internal' and 'v1' namespaces.


Diffs (updated)
-

  src/tests/api_tests.cpp f0bafd1d5207a687ca0489b05ed1acdc2a6d875d 
  src/tests/containerizer.hpp 940c4146f4e854a6b1b9ccaba5687e76d5723cba 
  src/tests/containerizer.cpp 27c29b40f3241807fb22b27f0d9ca64b3e602a8f 
  src/tests/default_executor_tests.cpp 92e6b9f5fb80811c94632de3bb20c8e6d2e895ff 
  src/tests/executor_http_api_tests.cpp 
a9f1a7b0498acd541c6f58ad1388da49c9951e22 
  src/tests/fault_tolerance_tests.cpp 5a9944cf459ab688907d95bbda09f464b37efd1e 
  src/tests/http_fault_tolerance_tests.cpp 
57ef562058f8abf9256e2ab8a4a85b36b5a7add4 
  src/tests/master_contender_detector_tests.cpp 
2a7d713f74c907235f82d83eaf46630046645faf 
  src/tests/master_maintenance_tests.cpp 
6917272f2de7a09bf4de7e932994655f4e54d3da 
  src/tests/master_tests.cpp 88cf1e612ad8186ef2cea161b3a52b0df9517305 
  src/tests/mesos.hpp 9309b5a985c0d7136a2ee5aa1598b4fee6194816 
  src/tests/scheduler_http_api_tests.cpp 
6390f2eb53d8bb97b98e64ec6e0f795abc3f3c7f 
  src/tests/scheduler_tests.cpp b0ea0bbcce9d847285fda40f778caaf721804457 
  src/tests/slave_recovery_tests.cpp 703948f7a6861a4401ee45ce9cae2644106083f3 
  src/tests/slave_tests.cpp 93b81d3e5b90d8036409e943f598c865fe335bcc 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 52976: Split mesos test helpers into 'internal' and 'v1' namespaces.

[Joris Van Remoortere]

> On Oct. 18, 2016, 10:09 p.m., Jie Yu wrote:
> > src/tests/mesos.hpp, line 500
> > 
> >
> > Can you do
> > ```
> > #define EXECUTOR_EXECUTOR_INFO createExecutorInfo("default", "exit 1");
> > ```

Will do these changes in a follow up review.


- Joris


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


On Oct. 18, 2016, 8:13 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52976/
> ---
> 
> (Updated Oct. 18, 2016, 8:13 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Split mesos test helpers into 'internal' and 'v1' namespaces.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp f0bafd1d5207a687ca0489b05ed1acdc2a6d875d 
>   src/tests/containerizer.hpp 940c4146f4e854a6b1b9ccaba5687e76d5723cba 
>   src/tests/containerizer.cpp 27c29b40f3241807fb22b27f0d9ca64b3e602a8f 
>   src/tests/executor_http_api_tests.cpp 
> a9f1a7b0498acd541c6f58ad1388da49c9951e22 
>   src/tests/fault_tolerance_tests.cpp 
> 5a9944cf459ab688907d95bbda09f464b37efd1e 
>   src/tests/http_fault_tolerance_tests.cpp 
> 57ef562058f8abf9256e2ab8a4a85b36b5a7add4 
>   src/tests/master_contender_detector_tests.cpp 
> 2a7d713f74c907235f82d83eaf46630046645faf 
>   src/tests/master_maintenance_tests.cpp 
> 6917272f2de7a09bf4de7e932994655f4e54d3da 
>   src/tests/master_tests.cpp 88cf1e612ad8186ef2cea161b3a52b0df9517305 
>   src/tests/mesos.hpp 9309b5a985c0d7136a2ee5aa1598b4fee6194816 
>   src/tests/scheduler_http_api_tests.cpp 
> 6390f2eb53d8bb97b98e64ec6e0f795abc3f3c7f 
>   src/tests/scheduler_tests.cpp b0ea0bbcce9d847285fda40f778caaf721804457 
>   src/tests/slave_tests.cpp 93b81d3e5b90d8036409e943f598c865fe335bcc 
> 
> Diff: https://reviews.apache.org/r/52976/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Review Request 53012: Fixed usage of 'evolve' in master http endpoints.

[Joris Van Remoortere]

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

Review request for mesos, Anand Mazumdar, haosdent huang, and Jie Yu.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/internal/evolve.hpp a38842a726f0e2634ae74b91f83b15ab1e656a47 
  src/internal/evolve.cpp b4a95771974ef11fda896d04a700c3d3d4fa024c 
  src/master/http.cpp bb9c87327dfe2161a6f1fd4cded72aa9a5ffaf66 

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


Testing
---


Thanks,

Joris Van Remoortere

Review Request 53013: Removed extra 'evolve' implementation from 'api_tests.cpp'.

[Joris Van Remoortere]

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

Review request for mesos, Anand Mazumdar, haosdent huang, and Jie Yu.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/internal/evolve.hpp a38842a726f0e2634ae74b91f83b15ab1e656a47 
  src/internal/evolve.cpp b4a95771974ef11fda896d04a700c3d3d4fa024c 
  src/tests/api_tests.cpp f0bafd1d5207a687ca0489b05ed1acdc2a6d875d 

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


Testing
---


Thanks,

Joris Van Remoortere

Review Request 53014: Removed unused tests helper macro 'DEFAULT_CONTAINER_ID'.

[Joris Van Remoortere]

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

Review request for mesos, Anand Mazumdar and Jie Yu.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/tests/mesos.hpp 9309b5a985c0d7136a2ee5aa1598b4fee6194816 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 53005: Updated release guide.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [53005]

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

- Mesos ReviewBot


On Oct. 18, 2016, 10:47 p.m., Vinod Kone wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53005/
> ---
> 
> (Updated Oct. 18, 2016, 10:47 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added information about release branches among other things.
> 
> 
> Diffs
> -
> 
>   docs/release-guide.md c5a12c5a9f36e07334b59edf0788359b42a3125f 
> 
> Diff: https://reviews.apache.org/r/53005/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Vinod Kone
> 
>

Re: Review Request 52997: Improve Socket::connect error message.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52997]

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

- Mesos ReviewBot


On Oct. 18, 2016, 10:43 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52997/
> ---
> 
> (Updated Oct. 18, 2016, 10:43 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-6412
> https://issues.apache.org/jira/browse/MESOS-6412
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rather than returning a fixed generic error when connect(2) fails,
> report the actual socket error as well as the address we were trying to
> connect to.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/poll_socket.cpp 
> d9ab3fbbf385c39c0e43ee92ff06609ae8d47e2d 
> 
> Diff: https://reviews.apache.org/r/52997/diff/
> 
> 
> Testing
> ---
> 
> `make check` on a host whose hostname doesn't match its IP address.
> 
> ```
> [jpeach@jpeach libprocess]$ ./libprocess-tests 
> --gtest_filter=HTTPTest.Endpoints
> Note: Google Test filter = HTTPTest.Endpoints
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from HTTPTest
> [ RUN  ] HTTPTest.Endpoints
> ../../../3rdparty/libprocess/src/tests/http_tests.cpp:141: Failure
> (socket.connect(http.process->self().address)).failure(): Failed to connect 
> to 17.203.52.49:39241: Connection refused
> [  FAILED  ] HTTPTest.Endpoints (4 ms)
> [--] 1 test from HTTPTest (4 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (5 ms total)
> [  PASSED  ] 0 tests.
> [  FAILED  ] 1 test, listed below:
> [  FAILED  ] HTTPTest.Endpoints
> 
>  1 FAILED TEST
> ```
> 
> 
> Thanks,
> 
> James Peach
> 
>

Review Request 53009: Add support for task labels to example no_executor_framework.

[Ian Downes]

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

Review request for mesos, Benjamin Mahler and Ilya Pronin.


Repository: mesos


Description
---

Add support for task labels to example no_executor_framework.


Diffs
-

  src/examples/no_executor_framework.cpp 
e82ae9a9ea23c7d181b071f0e3f5071b3856d8a4 

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


Testing
---


Thanks,

Ian Downes

Re: Review Request 52723: Changed agent to send TASK_DROPPED during reconciliation.

[Neil Conway]

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

(Updated Oct. 19, 2016, 12:40 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Fix comment typo per review.


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


Repository: mesos


Description
---

If a framework attempts to launch a task but the launch message is
dropped after it reaches the master but before it reaches the slave, the
failed launch will be detected during master <-> agent reconciliation
when the agent re-registers. Previously, the agent would generate a
TASK_LOST status update for such dropped tasks; now it will generate
TASK_DROPPED if the framework is partition-aware.

Note that we'll only send TASK_DROPPED if the agent is running a
sufficiently recent version of Mesos (>= 1.1.0). That means that in a
mixed cluster where the master has been upgraded to Mesos 1.1 but some
of the agents have not been, a partition-aware framework might still see
TASK_LOST in this situation.


Diffs (updated)
-

  src/messages/messages.proto 7d65be1418864333d0c4213e2e0df0374f9ec115 
  src/slave/slave.cpp 6bd9b49c3bbdb973a0d03552ae8fe55b33371083 
  src/tests/master_slave_reconciliation_tests.cpp 
2983c1b074c2d4179e95e619083f5dd4e9ac6730 
  src/tests/slave_recovery_tests.cpp 703948f7a6861a4401ee45ce9cae2644106083f3 

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 52723: Changed agent to send TASK_DROPPED during reconciliation.

[Neil Conway]

> On Oct. 18, 2016, 11:15 p.m., Vinod Kone wrote:
> > src/tests/slave_recovery_tests.cpp, line 3067
> > 
> >
> > Curious why you didn't write a new test for partition aware framework 
> > like you did above?

I was trying to strike a balance between good test coverage but not duplicating 
every existing test that uses `TASK_LOST`; in this case I thought the existing 
tests were good enough we didn't need a copy, but I'm happy to make a copy if 
you'd like.


- Neil


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


On Oct. 13, 2016, 3:10 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52723/
> ---
> 
> (Updated Oct. 13, 2016, 3:10 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6332
> https://issues.apache.org/jira/browse/MESOS-6332
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If a framework attempts to launch a task but the launch message is
> dropped after it reaches the master but before it reaches the slave, the
> failed launch will be detected during master <-> agent reconciliation
> when the agent re-registers. Previously, the agent would generate a
> TASK_LOST status update for such dropped tasks; now it will generate
> TASK_DROPPED if the framework is partition-aware.
> 
> Note that we'll only send TASK_DROPPED if the agent is running a
> sufficiently recent version of Mesos (>= 1.1.0). That means that in a
> mixed cluster where the master has been upgraded to Mesos 1.1 but some
> of the agents have not been, a partition-aware framework might still see
> TASK_LOST in this situation.
> 
> 
> Diffs
> -
> 
>   src/messages/messages.proto 7d65be1418864333d0c4213e2e0df0374f9ec115 
>   src/slave/slave.cpp 6bd9b49c3bbdb973a0d03552ae8fe55b33371083 
>   src/tests/master_slave_reconciliation_tests.cpp 
> 2983c1b074c2d4179e95e619083f5dd4e9ac6730 
>   src/tests/slave_recovery_tests.cpp 703948f7a6861a4401ee45ce9cae2644106083f3 
> 
> Diff: https://reviews.apache.org/r/52723/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 52720: Clarified a comment.

[Neil Conway]

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

(Updated Oct. 19, 2016, 12:37 a.m.)


Review request for mesos and Vinod Kone.


Changes
---

Fix typo per review.


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


Repository: mesos


Description
---

Clarified a comment.


Diffs (updated)
-

  src/master/master.cpp 3c6b18ead44cd5f2978093f5415e974cfcbfa714 

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 53008: Added a nested container test for 3 levels of nesting.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Oct. 18, 2016, 5:13 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53008/
> ---
> 
> (Updated Oct. 18, 2016, 5:13 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Gilbert Song, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a nested container test for 3 levels of nesting.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
> c690b41f3852de035e5cc6378e7fc44d2c9a6fb9 
> 
> Diff: https://reviews.apache.org/r/53008/diff/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 53008: Added a nested container test for 3 levels of nesting.

[Jie Yu]

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

Review request for mesos, Benjamin Hindman, Gilbert Song, and Kevin Klues.


Repository: mesos


Description
---

Added a nested container test for 3 levels of nesting.


Diffs
-

  src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
c690b41f3852de035e5cc6378e7fc44d2c9a6fb9 

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


Testing
---

sudo make check


Thanks,

Jie Yu

Re: Review Request 52976: Split mesos test helpers into 'internal' and 'v1' namespaces.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52976]

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

- Mesos ReviewBot


On Oct. 18, 2016, 8:13 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52976/
> ---
> 
> (Updated Oct. 18, 2016, 8:13 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Split mesos test helpers into 'internal' and 'v1' namespaces.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp f0bafd1d5207a687ca0489b05ed1acdc2a6d875d 
>   src/tests/containerizer.hpp 940c4146f4e854a6b1b9ccaba5687e76d5723cba 
>   src/tests/containerizer.cpp 27c29b40f3241807fb22b27f0d9ca64b3e602a8f 
>   src/tests/executor_http_api_tests.cpp 
> a9f1a7b0498acd541c6f58ad1388da49c9951e22 
>   src/tests/fault_tolerance_tests.cpp 
> 5a9944cf459ab688907d95bbda09f464b37efd1e 
>   src/tests/http_fault_tolerance_tests.cpp 
> 57ef562058f8abf9256e2ab8a4a85b36b5a7add4 
>   src/tests/master_contender_detector_tests.cpp 
> 2a7d713f74c907235f82d83eaf46630046645faf 
>   src/tests/master_maintenance_tests.cpp 
> 6917272f2de7a09bf4de7e932994655f4e54d3da 
>   src/tests/master_tests.cpp 88cf1e612ad8186ef2cea161b3a52b0df9517305 
>   src/tests/mesos.hpp 9309b5a985c0d7136a2ee5aa1598b4fee6194816 
>   src/tests/scheduler_http_api_tests.cpp 
> 6390f2eb53d8bb97b98e64ec6e0f795abc3f3c7f 
>   src/tests/scheduler_tests.cpp b0ea0bbcce9d847285fda40f778caaf721804457 
>   src/tests/slave_tests.cpp 93b81d3e5b90d8036409e943f598c865fe335bcc 
> 
> Diff: https://reviews.apache.org/r/52976/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 52910: Added a constants.hpp for provisioner.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Oct. 15, 2016, 1:39 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52910/
> ---
> 
> (Updated Oct. 15, 2016, 1:39 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a constants.hpp for provisioner.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 3bcc0f2dfc2c4f71841bd6d161f39e0e919fc0d7 
>   src/slave/containerizer/mesos/provisioner/backend.cpp 
> 9c258ed2cad82c2c1b1f707acf527239a1f01f95 
>   src/slave/containerizer/mesos/provisioner/constants.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/provisioner/docker/paths.cpp 
> dea37566cbe0313f84f2bac27e29752a980b4394 
>   src/slave/flags.cpp 87d9e4632321134192bb0a67f1b91db7d89f539b 
>   src/tests/containerizer/provisioner_appc_tests.cpp 
> a999fc7991da805dfbcdf5659fcfb762aee5b2b9 
>   src/tests/containerizer/provisioner_backend_tests.cpp 
> 661977b61c0711c450922a64ce0e2895385bf78d 
> 
> Diff: https://reviews.apache.org/r/52910/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 52682: Cleaned up a few style issues in the capabilities isolator tests.

[Jie Yu]

> On Oct. 17, 2016, 10:02 p.m., Jie Yu wrote:
> > src/tests/containerizer/linux_capabilities_isolator_tests.cpp, line 100
> > 
> >
> > const UsaImage&
> 
> Benjamin Bannier wrote:
> I believe we always pass fundamental types like this enum by value. This 
> one here is almost certainly not bigger than a pointer. Can we just drop 
> this, or would you like me to collect a list of examples in the code base?

IC, sounds good.


- Jie


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


On Oct. 18, 2016, 8:48 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52682/
> ---
> 
> (Updated Oct. 18, 2016, 8:48 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We remove an unused using declaration, and replace several bool
> parameters with more readable enum values.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/linux_capabilities_isolator_tests.cpp 
> edb46659324c4c6345606cfa4c19f4fce05c59fe 
> 
> Diff: https://reviews.apache.org/r/52682/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 52746: Changed agent to send TASK_DROPPED for task launch failures.

[Vinod Kone]

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


Fix it, then Ship it!




Wonder if we could use a helper that returns LOST or DROPPED state based on 
Frameworkinfo instead of duplicating that code everywhere. Not sure what to 
call it though.


src/slave/slave.cpp (line 3243)


TODO at the end sounds weird.

Lately we have been putting blank lines in front of TODOs for better 
formatting.

```
// update for...
//
// TODO(vinod): Consider...
//..
//..
//
// TODO(vinod): Use foreachvalue...

```



src/slave/slave.cpp (line 3558)


new line.



src/slave/slave.cpp (line 3561)


new line.


- Vinod Kone


On Oct. 12, 2016, 7:35 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52746/
> ---
> 
> (Updated Oct. 12, 2016, 7:35 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6332
> https://issues.apache.org/jira/browse/MESOS-6332
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If the agent cannot launch a task due to a variety of possible error
> conditions, we now send TASK_DROPPED to partition-aware frameworks
> rather than TASK_LOST.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 6bd9b49c3bbdb973a0d03552ae8fe55b33371083 
> 
> Diff: https://reviews.apache.org/r/52746/diff/
> 
> 
> Testing
> ---
> 
> `make check`.
> 
> Note that none of these code paths appear to have unit tests, so I couldn't 
> update the unit tests to ensure that `TASK_DROPPED` is sent appropriately. We 
> should add unit tests for at least some of these situations, if feasible -- 
> but I'll defer that for now.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 52740: Refactored some code into a separate function.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Oct. 12, 2016, 7:34 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52740/
> ---
> 
> (Updated Oct. 12, 2016, 7:34 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6332
> https://issues.apache.org/jira/browse/MESOS-6332
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored some code into a separate function.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp d8c5873bb7380b8449f8f344e85689519c467251 
>   src/slave/slave.cpp 6bd9b49c3bbdb973a0d03552ae8fe55b33371083 
> 
> Diff: https://reviews.apache.org/r/52740/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 52723: Changed agent to send TASK_DROPPED during reconciliation.

[Vinod Kone]

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


Fix it, then Ship it!





src/tests/master_slave_reconciliation_tests.cpp (line 326)


TASK_DROPPED



src/tests/slave_recovery_tests.cpp (line 3067)


Curious why you didn't write a new test for partition aware framework like 
you did above?


- Vinod Kone


On Oct. 13, 2016, 3:10 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52723/
> ---
> 
> (Updated Oct. 13, 2016, 3:10 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6332
> https://issues.apache.org/jira/browse/MESOS-6332
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If a framework attempts to launch a task but the launch message is
> dropped after it reaches the master but before it reaches the slave, the
> failed launch will be detected during master <-> agent reconciliation
> when the agent re-registers. Previously, the agent would generate a
> TASK_LOST status update for such dropped tasks; now it will generate
> TASK_DROPPED if the framework is partition-aware.
> 
> Note that we'll only send TASK_DROPPED if the agent is running a
> sufficiently recent version of Mesos (>= 1.1.0). That means that in a
> mixed cluster where the master has been upgraded to Mesos 1.1 but some
> of the agents have not been, a partition-aware framework might still see
> TASK_LOST in this situation.
> 
> 
> Diffs
> -
> 
>   src/messages/messages.proto 7d65be1418864333d0c4213e2e0df0374f9ec115 
>   src/slave/slave.cpp 6bd9b49c3bbdb973a0d03552ae8fe55b33371083 
>   src/tests/master_slave_reconciliation_tests.cpp 
> 2983c1b074c2d4179e95e619083f5dd4e9ac6730 
>   src/tests/slave_recovery_tests.cpp 703948f7a6861a4401ee45ce9cae2644106083f3 
> 
> Diff: https://reviews.apache.org/r/52723/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 52722: Changed master to add `FrameworkInfo` to agent reconcilation.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Oct. 11, 2016, 1:04 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52722/
> ---
> 
> (Updated Oct. 11, 2016, 1:04 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6332
> https://issues.apache.org/jira/browse/MESOS-6332
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When an agent re-registers with a master that already knows about that
> agent, the master needs to reconcile its view of the state of the agent
> with the agent's current state. For any task that the master thinks
> should be on the agent but isn't included in the ReregisterSlaveMessage,
> the master does a reconcilation with the agent to find the task's
> current state.
> 
> This commit changes adds the `FrameworkInfo` for any possibly missing
> tasks to the master -> agent reconciliation message. This is useful
> because the agent can consult the `FrameworkInfo` during reconciliation:
> this will shortly be used to make agent reconciliation behave
> differently for partition-aware frameworks.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp ad8993ae43e2508a3777f4062acaca1177ac77f2 
>   src/messages/messages.proto 7d65be1418864333d0c4213e2e0df0374f9ec115 
> 
> Diff: https://reviews.apache.org/r/52722/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 52721: Fixed typo in log message.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Oct. 11, 2016, 1:03 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52721/
> ---
> 
> (Updated Oct. 11, 2016, 1:03 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6332
> https://issues.apache.org/jira/browse/MESOS-6332
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed typo in log message.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp ad8993ae43e2508a3777f4062acaca1177ac77f2 
> 
> Diff: https://reviews.apache.org/r/52721/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 52720: Clarified a comment.

[Vinod Kone]

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


Fix it, then Ship it!





src/master/master.cpp (line 6793)


s/She/The/


- Vinod Kone


On Oct. 11, 2016, 1:03 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52720/
> ---
> 
> (Updated Oct. 11, 2016, 1:03 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6332
> https://issues.apache.org/jira/browse/MESOS-6332
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Clarified a comment.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp ad8993ae43e2508a3777f4062acaca1177ac77f2 
> 
> Diff: https://reviews.apache.org/r/52720/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Review Request 53005: Updated release guide.

[Vinod Kone]

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

Review request for mesos and Till Toenshoff.


Repository: mesos


Description
---

Added information about release branches among other things.


Diffs
-

  docs/release-guide.md c5a12c5a9f36e07334b59edf0788359b42a3125f 

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


Testing
---


Thanks,

Vinod Kone

Re: Review Request 52997: Improve Socket::connect error message.

[James Peach]

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

(Updated Oct. 18, 2016, 10:43 p.m.)


Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

Rather than returning a fixed generic error when connect(2) fails,
report the actual socket error as well as the address we were trying to
connect to.


Diffs (updated)
-

  3rdparty/libprocess/src/poll_socket.cpp 
d9ab3fbbf385c39c0e43ee92ff06609ae8d47e2d 

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


Testing
---

`make check` on a host whose hostname doesn't match its IP address.

```
[jpeach@jpeach libprocess]$ ./libprocess-tests --gtest_filter=HTTPTest.Endpoints
Note: Google Test filter = HTTPTest.Endpoints
[==] Running 1 test from 1 test case.
[--] Global test environment set-up.
[--] 1 test from HTTPTest
[ RUN  ] HTTPTest.Endpoints
../../../3rdparty/libprocess/src/tests/http_tests.cpp:141: Failure
(socket.connect(http.process->self().address)).failure(): Failed to connect to 
17.203.52.49:39241: Connection refused
[  FAILED  ] HTTPTest.Endpoints (4 ms)
[--] 1 test from HTTPTest (4 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (5 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] HTTPTest.Endpoints

 1 FAILED TEST
```


Thanks,

James Peach

Re: Review Request 52997: Improve Socket::connect error message.

[James Peach]

> On Oct. 18, 2016, 9:41 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/src/poll_socket.cpp, line 114
> > 
> >
> > Missing reference.
> 
> James Peach wrote:
> I wanted a copy (at least the lambda needs to take a copy), since 
> ``connect`` takes a reference to the address.
> 
> Joris Van Remoortere wrote:
> It should be a copy by default unless you `cref` it right?
> I think you can make the parameter a `const Address& to`.
> Mpark do you have an opinion?

Ok, `std::decay` says it removes the reference, so there's an implicit copy 
into the lambda here.


On Oct. 18, 2016, 9:41 p.m., James Peach wrote:
> > Can you also follow up with a review to change the logging level in 
> > libprocess for failing to close the socket if we fail to connect?
> > Maybe it should be `VLOG(1)`? Thoughts?
> 
> James Peach wrote:
> I actually removed the `VLOG` thinking that it would be better for the 
> caller to log the error since it can provide context.
> 
> Joris Van Remoortere wrote:
> I meant these 2:
> 
> https://github.com/apache/mesos/blob/fd1b7bdfb2aa8a6f3dffab9ce8114c1a3c183e23/3rdparty/libprocess/src/process.cpp#L2071
> 
> https://github.com/apache/mesos/blob/fd1b7bdfb2aa8a6f3dffab9ce8114c1a3c183e23/3rdparty/libprocess/src/process.cpp#L2154

Oh I see. I haven't really looked at that code, though I recall seeing spurious 
errors from it in the past. In most cases I see through that code path a socket 
that is not connected would not be added to the socket manager. Maybe reconnect 
has a way though.


- James


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


On Oct. 18, 2016, 9:37 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52997/
> ---
> 
> (Updated Oct. 18, 2016, 9:37 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-6412
> https://issues.apache.org/jira/browse/MESOS-6412
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rather than returning a fixed generic error when connect(2) fails,
> report the actual socket error as well as the address we were trying to
> connect to.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/poll_socket.cpp 
> d9ab3fbbf385c39c0e43ee92ff06609ae8d47e2d 
> 
> Diff: https://reviews.apache.org/r/52997/diff/
> 
> 
> Testing
> ---
> 
> `make check` on a host whose hostname doesn't match its IP address.
> 
> ```
> [jpeach@jpeach libprocess]$ ./libprocess-tests 
> --gtest_filter=HTTPTest.Endpoints
> Note: Google Test filter = HTTPTest.Endpoints
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from HTTPTest
> [ RUN  ] HTTPTest.Endpoints
> ../../../3rdparty/libprocess/src/tests/http_tests.cpp:141: Failure
> (socket.connect(http.process->self().address)).failure(): Failed to connect 
> to 17.203.52.49:39241: Connection refused
> [  FAILED  ] HTTPTest.Endpoints (4 ms)
> [--] 1 test from HTTPTest (4 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (5 ms total)
> [  PASSED  ] 0 tests.
> [  FAILED  ] 1 test, listed below:
> [  FAILED  ] HTTPTest.Endpoints
> 
>  1 FAILED TEST
> ```
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 52600: Enable multiple field based authorization in the authorizer interface.

[Till Toenshoff]

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



Seems you got a lot more within this review than planned :)

- Till Toenshoff


On Oct. 17, 2016, 10:12 a.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52600/
> ---
> 
> (Updated Oct. 17, 2016, 10:12 a.m.)
> 
> 
> Review request for mesos, Adam B, Kapil Arya, and Till Toenshoff.
> 
> 
> Bugs: MESOS-6401
> https://issues.apache.org/jira/browse/MESOS-6401
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Standarize the way in which authorization is perfomed by enabling
> all actions suffixed with `_WITH_ROLE` and `_WITH_PRINCIPAL` to
> perform authorization using whole protobuf messages instead of
> forcing a single field from the message.
> 
> While this change is still in the deprecation cycle, the original
> value is also set.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 71319891a451bd1d565210dcce2fb61fc69e1f61 
>   3rdparty/libprocess/configure.ac 1644035d7fe1a45c798b13dbfb3c66f74466f2a2 
>   3rdparty/libprocess/include/process/deferred.hpp 
> ee902a6c8de48f17f7401ee7b3814d92eabd65f8 
>   3rdparty/libprocess/src/openssl.cpp 
> 8e4e0490a84c46e8f4842cf567f55a3c988ad445 
>   3rdparty/libprocess/src/process.cpp 
> 18a8e206f6f297157d246a94f374311be67cd782 
>   3rdparty/libprocess/src/tests/CMakeLists.txt 
> 66597fd3021bc7c3c53cb0f38416edc85ace5a9b 
>   3rdparty/libprocess/src/tests/future_tests.cpp 
> 7c411c7be1849119fe0b070622dbe4488fa11b7a 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 533104c93dd1eaf67bf3752163d2e0cad090078d 
>   3rdparty/libprocess/src/tests/limiter_tests.cpp 
> b80b1da214f97b50aa7b61b79bbf683fd01116aa 
>   3rdparty/libprocess/src/tests/metrics_tests.cpp 
> d7fdb06060b273e16be27a263b5ee268842aa25c 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> a4af54aa78c162bc0ecbe2f25796d8c9b12e0f31 
>   3rdparty/libprocess/src/tests/reap_tests.cpp 
> 30518dee6c2fb904a607c7a457a5ec7366aab818 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> 483ac1c5540d54921de1c1488ccb5e63b17ac481 
>   3rdparty/libprocess/src/tests/time_tests.cpp 
> 08ddb56f1789f400b8cd072c53e885c759f13ddc 
>   3rdparty/stout/Makefile.am 4e10ae25a6c8a2d01d7b98b04ccf06a216611938 
>   3rdparty/stout/README.md 693e3a3e76300f7fa6c6f6518497edf44cc9f6a2 
>   3rdparty/stout/configure.ac cbb0fdb0e1584ace7bbd6bf496f0f55b9846ce0b 
>   3rdparty/stout/include/Makefile.am 1eb9c146d3eebacaea32e0f724d145f0c1dd676f 
>   3rdparty/stout/include/stout/flags/flags.hpp 
> 794a598169684c8474184b47de533498f3b234e0 
>   3rdparty/stout/include/stout/gtest.hpp 
> b2f75b6c706df9de68edbac86a1e2dec32a574ed 
>   3rdparty/stout/include/stout/mac.hpp 
> 7ed5f38ae4f93e267406254cb2acff201faffe45 
>   3rdparty/stout/include/stout/os.hpp 
> 96e8621b198a3ec4cce78e0a6ff5f271eda05ff1 
>   3rdparty/stout/include/stout/os/linux.hpp 
> bf12e0b0577810c64cc8276ff0d987524327ffcd 
>   3rdparty/stout/include/stout/os/posix/killtree.hpp 
> f47f0d85f88bf69b5a9ae74e32d4f41c9f4a985a 
>   3rdparty/stout/include/stout/os/posix/shell.hpp 
> 70a9184e85ba322f1cd4ce5d12b963cd4b3ad500 
>   3rdparty/stout/include/stout/os/raw/environment.hpp 
> b3e82ac8071b41748aeb098b7d5fcc210a1d3c43 
>   3rdparty/stout/include/stout/os/wait.hpp 
> 5f6483c0d754dd1c1f5fdcfc542a0cc05c0a8df5 
>   3rdparty/stout/include/stout/os/windows/rmdir.hpp 
> 4437484c068e9ef046e0be14683c97db447f2da1 
>   3rdparty/stout/include/stout/os/windows/shell.hpp 
> 17e3d564564abebf1d558b7a7a277aef3c87e5ae 
>   3rdparty/stout/include/stout/windows/os.hpp 
> 7ca0b5dc9793369ea142684e3614e8f33cac64b6 
>   3rdparty/stout/tests/CMakeLists.txt 
> e52aa62479a345bbcee70047109ebd80b8f9813d 
>   3rdparty/stout/tests/flags_tests.cpp 
> 201b16a6c040e32f8a14fa7a2520c86997c363ad 
>   3rdparty/stout/tests/ip_tests.cpp b5a206fe5fdcdadaa383698f505d87ddb2c6806d 
>   3rdparty/stout/tests/mac_tests.cpp d4560e75ae4e03d1765c599298e88842381690b9 
>   3rdparty/stout/tests/os/process_tests.cpp 
> 4cb3b5fab389492bdc1258a27e821e60aef19dc8 
>   3rdparty/stout/tests/os/rmdir_tests.cpp 
> 9aa4059d589e84f3c377163b1d6d2a278d4130b6 
>   3rdparty/stout/tests/os_tests.cpp 0b7ee0723b6a608d6f110fa8ac16e0fd7b75ddea 
>   CHANGELOG 2e0da96d055c7a4c228726c628d94f3414b2be86 
>   configure.ac 015255ec1876a51b9eb2cf488b375af90f73c722 
>   docs/app-framework-development-guide.md 
> de92c7570cd0d0ac8639ce50a79e5158844ac53c 
>   docs/configuration.md c83a58eb6884c8d8c37880a745e04cf0b789ebdc 
>   docs/contributors.yaml aa49acd188fada6659626d6cb9224c0dfc671f0f 
>   docs/design-docs.md 7fafd09c7178559fedfa037faee0debfc29a927c 
>   docs/docker-containerizer.md bab84dc2b0ce104b3ec59aaf0ef800b418a6517c 
>   

Re: Review Request 52976: Split mesos test helpers into 'internal' and 'v1' namespaces.

[Jie Yu]

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




src/tests/api_tests.cpp (line 838)


Can you use `using mesos::internal::evolve` in the beginning?



src/tests/executor_http_api_tests.cpp (line 362)


Can we get rid of `mesos::` here?



src/tests/fault_tolerance_tests.cpp (line 720)


Can you use `using 
mesos::internal::scheduler::DEFAULT_REGISTRATION_BACKOFF_FACTOR` in the 
begining?



src/tests/master_contender_detector_tests.cpp (lines 94 - 96)


Can you do `using mesos::internal::protobuf::createMasterInfo`?



src/tests/mesos.hpp (lines 338 - 339)


Can you add a TODO here to eventually clean this up?



src/tests/mesos.hpp (lines 450 - 459)


Remove this as no one is using thsi.



src/tests/mesos.hpp (line 489)


Can you do
```
#define EXECUTOR_EXECUTOR_INFO createExecutorInfo("default", "exit 1");
```



src/tests/mesos.hpp (line 502)


Let's remove this one as no one is using this.



src/tests/mesos.hpp (line 509)


Why static? Should be inline as this is header?



src/tests/mesos.hpp (line 623)


I don't get this.



src/tests/mesos.hpp (line 1898)


2 lines apart?



src/tests/mesos.hpp (lines 1973 - 1975)


Do you want to change that?


- Jie Yu


On Oct. 18, 2016, 8:13 p.m., Joris Van Remoortere wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52976/
> ---
> 
> (Updated Oct. 18, 2016, 8:13 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Split mesos test helpers into 'internal' and 'v1' namespaces.
> 
> 
> Diffs
> -
> 
>   src/tests/api_tests.cpp f0bafd1d5207a687ca0489b05ed1acdc2a6d875d 
>   src/tests/containerizer.hpp 940c4146f4e854a6b1b9ccaba5687e76d5723cba 
>   src/tests/containerizer.cpp 27c29b40f3241807fb22b27f0d9ca64b3e602a8f 
>   src/tests/executor_http_api_tests.cpp 
> a9f1a7b0498acd541c6f58ad1388da49c9951e22 
>   src/tests/fault_tolerance_tests.cpp 
> 5a9944cf459ab688907d95bbda09f464b37efd1e 
>   src/tests/http_fault_tolerance_tests.cpp 
> 57ef562058f8abf9256e2ab8a4a85b36b5a7add4 
>   src/tests/master_contender_detector_tests.cpp 
> 2a7d713f74c907235f82d83eaf46630046645faf 
>   src/tests/master_maintenance_tests.cpp 
> 6917272f2de7a09bf4de7e932994655f4e54d3da 
>   src/tests/master_tests.cpp 88cf1e612ad8186ef2cea161b3a52b0df9517305 
>   src/tests/mesos.hpp 9309b5a985c0d7136a2ee5aa1598b4fee6194816 
>   src/tests/scheduler_http_api_tests.cpp 
> 6390f2eb53d8bb97b98e64ec6e0f795abc3f3c7f 
>   src/tests/scheduler_tests.cpp b0ea0bbcce9d847285fda40f778caaf721804457 
>   src/tests/slave_tests.cpp 93b81d3e5b90d8036409e943f598c865fe335bcc 
> 
> Diff: https://reviews.apache.org/r/52976/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Joris Van Remoortere
> 
>

Re: Review Request 52997: Improve Socket::connect error message.

[Joris Van Remoortere]

On Oct. 18, 2016, 9:41 p.m., James Peach wrote:
> > Can you also follow up with a review to change the logging level in 
> > libprocess for failing to close the socket if we fail to connect?
> > Maybe it should be `VLOG(1)`? Thoughts?
> 
> James Peach wrote:
> I actually removed the `VLOG` thinking that it would be better for the 
> caller to log the error since it can provide context.

I meant these 2:
https://github.com/apache/mesos/blob/fd1b7bdfb2aa8a6f3dffab9ce8114c1a3c183e23/3rdparty/libprocess/src/process.cpp#L2071
https://github.com/apache/mesos/blob/fd1b7bdfb2aa8a6f3dffab9ce8114c1a3c183e23/3rdparty/libprocess/src/process.cpp#L2154


- Joris


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


On Oct. 18, 2016, 9:37 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52997/
> ---
> 
> (Updated Oct. 18, 2016, 9:37 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-6412
> https://issues.apache.org/jira/browse/MESOS-6412
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rather than returning a fixed generic error when connect(2) fails,
> report the actual socket error as well as the address we were trying to
> connect to.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/poll_socket.cpp 
> d9ab3fbbf385c39c0e43ee92ff06609ae8d47e2d 
> 
> Diff: https://reviews.apache.org/r/52997/diff/
> 
> 
> Testing
> ---
> 
> `make check` on a host whose hostname doesn't match its IP address.
> 
> ```
> [jpeach@jpeach libprocess]$ ./libprocess-tests 
> --gtest_filter=HTTPTest.Endpoints
> Note: Google Test filter = HTTPTest.Endpoints
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from HTTPTest
> [ RUN  ] HTTPTest.Endpoints
> ../../../3rdparty/libprocess/src/tests/http_tests.cpp:141: Failure
> (socket.connect(http.process->self().address)).failure(): Failed to connect 
> to 17.203.52.49:39241: Connection refused
> [  FAILED  ] HTTPTest.Endpoints (4 ms)
> [--] 1 test from HTTPTest (4 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (5 ms total)
> [  PASSED  ] 0 tests.
> [  FAILED  ] 1 test, listed below:
> [  FAILED  ] HTTPTest.Endpoints
> 
>  1 FAILED TEST
> ```
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 52997: Improve Socket::connect error message.

[Joris Van Remoortere]

> On Oct. 18, 2016, 9:41 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/src/poll_socket.cpp, line 114
> > 
> >
> > Missing reference.
> 
> James Peach wrote:
> I wanted a copy (at least the lambda needs to take a copy), since 
> ``connect`` takes a reference to the address.

It should be a copy by default unless you `cref` it right?
I think you can make the parameter a `const Address& to`.
Mpark do you have an opinion?


- Joris


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


On Oct. 18, 2016, 9:37 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52997/
> ---
> 
> (Updated Oct. 18, 2016, 9:37 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-6412
> https://issues.apache.org/jira/browse/MESOS-6412
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rather than returning a fixed generic error when connect(2) fails,
> report the actual socket error as well as the address we were trying to
> connect to.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/poll_socket.cpp 
> d9ab3fbbf385c39c0e43ee92ff06609ae8d47e2d 
> 
> Diff: https://reviews.apache.org/r/52997/diff/
> 
> 
> Testing
> ---
> 
> `make check` on a host whose hostname doesn't match its IP address.
> 
> ```
> [jpeach@jpeach libprocess]$ ./libprocess-tests 
> --gtest_filter=HTTPTest.Endpoints
> Note: Google Test filter = HTTPTest.Endpoints
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from HTTPTest
> [ RUN  ] HTTPTest.Endpoints
> ../../../3rdparty/libprocess/src/tests/http_tests.cpp:141: Failure
> (socket.connect(http.process->self().address)).failure(): Failed to connect 
> to 17.203.52.49:39241: Connection refused
> [  FAILED  ] HTTPTest.Endpoints (4 ms)
> [--] 1 test from HTTPTest (4 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (5 ms total)
> [  PASSED  ] 0 tests.
> [  FAILED  ] 1 test, listed below:
> [  FAILED  ] HTTPTest.Endpoints
> 
>  1 FAILED TEST
> ```
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 50380: Added new benchmark test for port resources.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52975, 50551, 52769, 51033, 50380]

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

- Mesos ReviewBot


On Oct. 18, 2016, 3:02 p.m., Guangya Liu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50380/
> ---
> 
> (Updated Oct. 18, 2016, 3:02 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Klaus Ma, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5898
> https://issues.apache.org/jira/browse/MESOS-5898
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The current ports benchmark test is always using same port range,
> with port, the formula for `+` is `a+a+a+a+...+a==a`; for `-`,
> it will be `a-a=0` and `0-a=0`.
> 
> The fix is adding a new benchmark test for ports resources, using
> a initial and different range ports resources. This fix also
> removed the ports resources benchmark test from original resources
> benchmark test as we introduced a new test case for ports resources.
> 
> 
> Diffs
> -
> 
>   src/tests/resources_tests.cpp 6a12783c26f359dda835b4866b299a8fcfb3f972 
> 
> Diff: https://reviews.apache.org/r/50380/diff/
> 
> 
> Testing
> ---
> 
> make
> make check
> 
> ```
>  ./bin/mesos-tests.sh --benchmark 
> --gtest_filter="Resources_Ports_BENCHMARK_Test.Arithmetic"
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from Resources_Ports_BENCHMARK_Test
> [ RUN  ] Resources_Ports_BENCHMARK_Test.Arithmetic
> Took 5.738039secs to perform 3000 'total -= r' operations on ports with 
> 'total' as ports(*):[1-3000] and 'r' as 'ports(*):[x-x]' where 'x' is from 1 
> to 3000
> Took 345315us to perform 3000 'total += r' operations on ports with 'total' 
> as ports(*):[1-3000] and 'r' as 'ports(*):[x-x]' where 'x' is from 1 to to 
> 3000
> Took 6.098373secs to perform 3000 'total = total - r' operations on ports 
> with 'total' as ports(*):[1-3000] and 'r' as 'ports(*):[x-x]' where 'x' is 
> from 1 to to 3000
> Took 715101us to perform 3000 'total = total + r' operations on ports with 
> 'total' as ports(*):[1-3000] and 'r' as 'ports(*):[x-x]' where 'x' is from 1 
> to to 3000
> [   OK ] Resources_Ports_BENCHMARK_Test.Arithmetic (12926 ms)
> [--] 1 test from Resources_Ports_BENCHMARK_Test (12926 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (12940 ms total)
> [  PASSED  ] 1 test.
> ```
> 
> 
> Thanks,
> 
> Guangya Liu
> 
>

Re: Review Request 52997: Improve Socket::connect error message.

[James Peach]

> On Oct. 18, 2016, 9:41 p.m., Joris Van Remoortere wrote:
> > 3rdparty/libprocess/src/poll_socket.cpp, line 114
> > 
> >
> > Missing reference.

I wanted a copy (at least the lambda needs to take a copy), since ``connect`` 
takes a reference to the address.


On Oct. 18, 2016, 9:41 p.m., James Peach wrote:
> > Can you also follow up with a review to change the logging level in 
> > libprocess for failing to close the socket if we fail to connect?
> > Maybe it should be `VLOG(1)`? Thoughts?

I actually removed the `VLOG` thinking that it would be better for the caller 
to log the error since it can provide context.


- James


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


On Oct. 18, 2016, 9:37 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52997/
> ---
> 
> (Updated Oct. 18, 2016, 9:37 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-6412
> https://issues.apache.org/jira/browse/MESOS-6412
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rather than returning a fixed generic error when connect(2) fails,
> report the actual socket error as well as the address we were trying to
> connect to.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/poll_socket.cpp 
> d9ab3fbbf385c39c0e43ee92ff06609ae8d47e2d 
> 
> Diff: https://reviews.apache.org/r/52997/diff/
> 
> 
> Testing
> ---
> 
> `make check` on a host whose hostname doesn't match its IP address.
> 
> ```
> [jpeach@jpeach libprocess]$ ./libprocess-tests 
> --gtest_filter=HTTPTest.Endpoints
> Note: Google Test filter = HTTPTest.Endpoints
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from HTTPTest
> [ RUN  ] HTTPTest.Endpoints
> ../../../3rdparty/libprocess/src/tests/http_tests.cpp:141: Failure
> (socket.connect(http.process->self().address)).failure(): Failed to connect 
> to 17.203.52.49:39241: Connection refused
> [  FAILED  ] HTTPTest.Endpoints (4 ms)
> [--] 1 test from HTTPTest (4 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (5 ms total)
> [  PASSED  ] 0 tests.
> [  FAILED  ] 1 test, listed below:
> [  FAILED  ] HTTPTest.Endpoints
> 
>  1 FAILED TEST
> ```
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 52997: Improve Socket::connect error message.

[Joris Van Remoortere]

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




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


Missing reference.


Can you also follow up with a review to change the logging level in libprocess 
for failing to close the socket if we fail to connect?
Maybe it should be `VLOG(1)`? Thoughts?

- Joris Van Remoortere


On Oct. 18, 2016, 9:37 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52997/
> ---
> 
> (Updated Oct. 18, 2016, 9:37 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-6412
> https://issues.apache.org/jira/browse/MESOS-6412
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rather than returning a fixed generic error when connect(2) fails,
> report the actual socket error as well as the address we were trying to
> connect to.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/poll_socket.cpp 
> d9ab3fbbf385c39c0e43ee92ff06609ae8d47e2d 
> 
> Diff: https://reviews.apache.org/r/52997/diff/
> 
> 
> Testing
> ---
> 
> `make check` on a host whose hostname doesn't match its IP address.
> 
> ```
> [jpeach@jpeach libprocess]$ ./libprocess-tests 
> --gtest_filter=HTTPTest.Endpoints
> Note: Google Test filter = HTTPTest.Endpoints
> [==] Running 1 test from 1 test case.
> [--] Global test environment set-up.
> [--] 1 test from HTTPTest
> [ RUN  ] HTTPTest.Endpoints
> ../../../3rdparty/libprocess/src/tests/http_tests.cpp:141: Failure
> (socket.connect(http.process->self().address)).failure(): Failed to connect 
> to 17.203.52.49:39241: Connection refused
> [  FAILED  ] HTTPTest.Endpoints (4 ms)
> [--] 1 test from HTTPTest (4 ms total)
> 
> [--] Global test environment tear-down
> [==] 1 test from 1 test case ran. (5 ms total)
> [  PASSED  ] 0 tests.
> [  FAILED  ] 1 test, listed below:
> [  FAILED  ] HTTPTest.Endpoints
> 
>  1 FAILED TEST
> ```
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 52997: Improve Socket::connect error message.

[James Peach]

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

(Updated Oct. 18, 2016, 9:37 p.m.)


Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

Rather than returning a fixed generic error when connect(2) fails,
report the actual socket error as well as the address we were trying to
connect to.


Diffs
-

  3rdparty/libprocess/src/poll_socket.cpp 
d9ab3fbbf385c39c0e43ee92ff06609ae8d47e2d 

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


Testing (updated)
---

`make check` on a host whose hostname doesn't match its IP address.

```
[jpeach@jpeach libprocess]$ ./libprocess-tests --gtest_filter=HTTPTest.Endpoints
Note: Google Test filter = HTTPTest.Endpoints
[==] Running 1 test from 1 test case.
[--] Global test environment set-up.
[--] 1 test from HTTPTest
[ RUN  ] HTTPTest.Endpoints
../../../3rdparty/libprocess/src/tests/http_tests.cpp:141: Failure
(socket.connect(http.process->self().address)).failure(): Failed to connect to 
17.203.52.49:39241: Connection refused
[  FAILED  ] HTTPTest.Endpoints (4 ms)
[--] 1 test from HTTPTest (4 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (5 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] HTTPTest.Endpoints

 1 FAILED TEST
```


Thanks,

James Peach

Review Request 52997: Improve Socket::connect error message.

[James Peach]

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

Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

Rather than returning a fixed generic error when connect(2) fails,
report the actual socket error as well as the address we were trying to
connect to.


Diffs
-

  3rdparty/libprocess/src/poll_socket.cpp 
d9ab3fbbf385c39c0e43ee92ff06609ae8d47e2d 

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


Testing
---

``make check`` on a host whose hostname doesn't match its IP address.

```
[jpeach@jpeach libprocess]$ ./libprocess-tests --gtest_filter=HTTPTest.Endpoints
Note: Google Test filter = HTTPTest.Endpoints
[==] Running 1 test from 1 test case.
[--] Global test environment set-up.
[--] 1 test from HTTPTest
[ RUN  ] HTTPTest.Endpoints
../../../3rdparty/libprocess/src/tests/http_tests.cpp:141: Failure
(socket.connect(http.process->self().address)).failure(): Failed to connect to 
17.203.52.49:39241: Connection refused
[  FAILED  ] HTTPTest.Endpoints (4 ms)
[--] 1 test from HTTPTest (4 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (5 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] HTTPTest.Endpoints

 1 FAILED TEST
 ```


Thanks,

James Peach

Re: Review Request 52992: Updated configure.ac in preparation of 1.2.0.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On Oct. 18, 2016, 6:45 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52992/
> ---
> 
> (Updated Oct. 18, 2016, 6:45 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   configure.ac 015255ec1876a51b9eb2cf488b375af90f73c722 
> 
> Diff: https://reviews.apache.org/r/52992/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 52659: Changed master to send TASK_DROPPED for task launch errors.

[Neil Conway]

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

(Updated Oct. 18, 2016, 9:09 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Remove no-longer-needed test variant.


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


Repository: mesos


Description
---

When a task launch fails due to a transient error (e.g., insufficient
available resources at an agent), the master sends a TASK_LOST update to
the framework. For PARTITION_AWARE frameworks, we now send TASK_DROPPED
instead.


Diffs (updated)
-

  src/master/master.cpp 3c6b18ead44cd5f2978093f5415e974cfcbfa714 
  src/tests/master_authorization_tests.cpp 
a4623d15c246651fd1038fdedf16321b1d5f273f 
  src/tests/master_tests.cpp 88cf1e612ad8186ef2cea161b3a52b0df9517305 

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 52719: Renamed a function for clarity.

[Neil Conway]

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

(Updated Oct. 18, 2016, 9:07 p.m.)


Review request for mesos and Vinod Kone.


Changes
---

Fix description per review.


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


Repository: mesos


Description (updated)
---

`Master::reconcile(Framework*, const scheduler::Call::Reconcile&)` and
`Master::reconcile(Slave*, const vector&, const
vector& tasks)` are only loosely related. Per discussion on the
development list, using overloading to distinguish these two functions
is confusing. Hence, rename the latter to `reconcileKnownSlave`.


Diffs (updated)
-

  src/master/master.hpp 881f0d6127c4d6a7fccb1677fb3129310777f737 
  src/master/master.cpp 3c6b18ead44cd5f2978093f5415e974cfcbfa714 

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 52991: Updated CHANGELOG in preparation of 1.2.0.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On Oct. 18, 2016, 6:45 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52991/
> ---
> 
> (Updated Oct. 18, 2016, 6:45 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   CHANGELOG b40f9c8106ac726cdb01bfe079a40e77ee7c8a2e 
> 
> Diff: https://reviews.apache.org/r/52991/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 52719: Renamed a function for clarity.

[Vinod Kone]

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


Ship it!




s/reconcileKnownTasks/reconcileKnownSlave/ in the description

- Vinod Kone


On Oct. 11, 2016, 1:03 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52719/
> ---
> 
> (Updated Oct. 11, 2016, 1:03 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6332
> https://issues.apache.org/jira/browse/MESOS-6332
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `Master::reconcile(Framework*, const scheduler::Call::Reconcile&)` and
> `Master::reconcile(Slave*, const vector&, const
> vector& tasks)` are only loosely related. Per discussion on the
> development list, using overloading to distinguish these two functions
> is confusing. Hence, rename the latter to `reconcileKnownTasks`.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 43518b9bf1bfaa54e26acc7f2e70c4161c667a84 
>   src/master/master.cpp ad8993ae43e2508a3777f4062acaca1177ac77f2 
> 
> Diff: https://reviews.apache.org/r/52719/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 52980: Updated os::mkdir() to take the mode as an input.

[Anindya Sinha]

> On Oct. 18, 2016, 7:58 p.m., Alex Clemmer wrote:
> > 3rdparty/stout/include/stout/os/mkdir.hpp, line 43
> > 
> >
> > Permissions is a bit of a subtle issue in Mesos, which we've largely 
> > punted on. I'm a bit worried that adding a pluggable `mode` here will cause 
> > us to add code that only works on Unix. How do you see this API evolving to 
> > accommodate both?
> 
> Alex Clemmer wrote:
> Also, sorry, I should have been more clear: I mean that permissions _on 
> Windows_ is a subtle issue in Mesos. My worry is rooted in the fact that this 
> API also has to support Windows.

So, er either extend `mkdir()` to include a `mode` (which as you indicated 
would need a change in the windows api also), OR call `chmod()` recursively 
after the call to the existing `mkdir()` [ie. without `mode`]. Currently, linux 
implementation in stout does not have a recursive `chmod()` which needs to be 
added, and I think windows just punts on `chmod()` calls.


- Anindya


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


On Oct. 18, 2016, 7:43 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52980/
> ---
> 
> (Updated Oct. 18, 2016, 7:43 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4893
> https://issues.apache.org/jira/browse/MESOS-4893
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mode still defaults to 755, but we now enable the caller to specify
> a different mode for creation of directories.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/mkdir.hpp 
> fe86864c8b480993c8f052f39b2fd3ece23798da 
> 
> Diff: https://reviews.apache.org/r/52980/diff/
> 
> 
> Testing
> ---
> 
> Tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 52693: Changed master to send TASK_UNKNOWN during reconciliation.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Oct. 18, 2016, 1:14 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52693/
> ---
> 
> (Updated Oct. 18, 2016, 1:14 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-6330
> https://issues.apache.org/jira/browse/MESOS-6330
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, the master would send TASK_LOST in response to explicit
> reconciliation requests for (a) unknown tasks at registered slaves and
> (b) tasks at unknown slaves (neither registered nor unreachable). The
> master will now send TASK_UNKNOWN for these situations if the framework
> has the PARTITION_AWARE capability.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 3c6b18ead44cd5f2978093f5415e974cfcbfa714 
>   src/tests/partition_tests.cpp 12fe8593ff17c35d540f944c428cf7f33b7dcbb3 
>   src/tests/reconciliation_tests.cpp 1412090299df388456f04ed58a1d384ce3ff550a 
> 
> Diff: https://reviews.apache.org/r/52693/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 52980: Updated os::mkdir() to take the mode as an input.

[Anindya Sinha]

> On Oct. 18, 2016, 7:58 p.m., Alex Clemmer wrote:
> > 3rdparty/stout/include/stout/os/mkdir.hpp, line 43
> > 
> >
> > Permissions is a bit of a subtle issue in Mesos, which we've largely 
> > punted on. I'm a bit worried that adding a pluggable `mode` here will cause 
> > us to add code that only works on Unix. How do you see this API evolving to 
> > accommodate both?
> 
> Alex Clemmer wrote:
> Also, sorry, I should have been more clear: I mean that permissions _on 
> Windows_ is a subtle issue in Mesos. My worry is rooted in the fact that this 
> API also has to support Windows.
> 
> Anindya Sinha wrote:
> So, er either extend `mkdir()` to include a `mode` (which as you 
> indicated would need a change in the windows api also), OR call `chmod()` 
> recursively after the call to the existing `mkdir()` [ie. without `mode`]. 
> Currently, linux implementation in stout does not have a recursive `chmod()` 
> which needs to be added, and I think windows just punts on `chmod()` calls.

s/er/we. So to restate my comments once again:

So, we either extend `mkdir()` to include a `mode` (which as you indicated 
would need a change in the windows api also), OR call `chmod()` recursively 
after the call to the existing `mkdir()` [ie. without `mode`]. Currently, linux 
implementation in stout does not have a recursive `chmod()` which needs to be 
added, and I think windows just punts on chmod() calls.


- Anindya


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


On Oct. 18, 2016, 7:43 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52980/
> ---
> 
> (Updated Oct. 18, 2016, 7:43 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4893
> https://issues.apache.org/jira/browse/MESOS-4893
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mode still defaults to 755, but we now enable the caller to specify
> a different mode for creation of directories.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/mkdir.hpp 
> fe86864c8b480993c8f052f39b2fd3ece23798da 
> 
> Diff: https://reviews.apache.org/r/52980/diff/
> 
> 
> Testing
> ---
> 
> Tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 52969: Removed redundant test.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Oct. 18, 2016, 1:13 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52969/
> ---
> 
> (Updated Oct. 18, 2016, 1:13 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `MasterAuthorizationTest.SlaveDisconnectedLost` was identical to
> `MasterAuthorizationTest.SlaveRemovedLost`.
> 
> 
> Diffs
> -
> 
>   src/tests/master_authorization_tests.cpp 
> a4623d15c246651fd1038fdedf16321b1d5f273f 
> 
> Diff: https://reviews.apache.org/r/52969/diff/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 52906: Simplify the comparison logic for `ExecutorInfo`.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Oct. 15, 2016, 5:06 a.m., haosdent huang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52906/
> ---
> 
> (Updated Oct. 15, 2016, 5:06 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-6397
> https://issues.apache.org/jira/browse/MESOS-6397
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is a follow up fix of https://reviews.apache.org/r/52817/.
> 
> 
> Diffs
> -
> 
>   src/common/type_utils.cpp c6cf4f1d4acd040d21434221e5469ca911404a39 
> 
> Diff: https://reviews.apache.org/r/52906/diff/
> 
> 
> Testing
> ---
> 
> ```
> sudo GLOG_v=1 ./bin/mesos-tests.sh --gtest_filter="*TaskUsesExecutor*" 
> --verbose
> sudo GLOG_v=1 ./bin/mesos-tests.sh 
> --gtest_filter="*TaskGroupValidationTest.TaskUsesDifferentExecutor*" --verbose
> ```
> 
> 
> Thanks,
> 
> haosdent huang
> 
>

Re: Review Request 52780: Added input and output functions for v1::CapabilityInfo.

[Jie Yu]

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


Ship it!




Made some adjustment while commit.

- Jie Yu


On Oct. 12, 2016, 12:49 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52780/
> ---
> 
> (Updated Oct. 12, 2016, 12:49 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added input and output functions for v1::CapabilityInfo.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/mesos.hpp f17145857355d7b85d70b6452bf1f89bf54edbac 
>   src/common/parse.hpp 62a333cc1f287439c761aa6f0c7f0bf7ade4a818 
>   src/v1/mesos.cpp 0f88abaf49117e844eeb3654edabeeba3862a0eb 
> 
> Diff: https://reviews.apache.org/r/52780/diff/
> 
> 
> Testing
> ---
> 
> * confirmed identically configured unversioned and `v1` `CapabilityInfo`s 
> produce identical output,
> * used and tested implicitly as part of https://reviews.apache.org/r/52741/
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 52976: Split mesos test helpers into 'internal' and 'v1' namespaces.

[Joris Van Remoortere]

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

(Updated Oct. 18, 2016, 8:13 p.m.)


Review request for mesos, Anand Mazumdar and Jie Yu.


Changes
---

Fixed whitespace style issue.


Repository: mesos


Description (updated)
---

Split mesos test helpers into 'internal' and 'v1' namespaces.


Diffs (updated)
-

  src/tests/api_tests.cpp f0bafd1d5207a687ca0489b05ed1acdc2a6d875d 
  src/tests/containerizer.hpp 940c4146f4e854a6b1b9ccaba5687e76d5723cba 
  src/tests/containerizer.cpp 27c29b40f3241807fb22b27f0d9ca64b3e602a8f 
  src/tests/executor_http_api_tests.cpp 
a9f1a7b0498acd541c6f58ad1388da49c9951e22 
  src/tests/fault_tolerance_tests.cpp 5a9944cf459ab688907d95bbda09f464b37efd1e 
  src/tests/http_fault_tolerance_tests.cpp 
57ef562058f8abf9256e2ab8a4a85b36b5a7add4 
  src/tests/master_contender_detector_tests.cpp 
2a7d713f74c907235f82d83eaf46630046645faf 
  src/tests/master_maintenance_tests.cpp 
6917272f2de7a09bf4de7e932994655f4e54d3da 
  src/tests/master_tests.cpp 88cf1e612ad8186ef2cea161b3a52b0df9517305 
  src/tests/mesos.hpp 9309b5a985c0d7136a2ee5aa1598b4fee6194816 
  src/tests/scheduler_http_api_tests.cpp 
6390f2eb53d8bb97b98e64ec6e0f795abc3f3c7f 
  src/tests/scheduler_tests.cpp b0ea0bbcce9d847285fda40f778caaf721804457 
  src/tests/slave_tests.cpp 93b81d3e5b90d8036409e943f598c865fe335bcc 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 52976: Split mesos test helpers into 'internal' and 'v1' namespaces.

[Joris Van Remoortere]

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

(Updated Oct. 18, 2016, 8:04 p.m.)


Review request for mesos, Anand Mazumdar and Jie Yu.


Changes
---

Updated.


Summary (updated)
-

Split mesos test helpers into 'internal' and 'v1' namespaces.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/tests/api_tests.cpp f0bafd1d5207a687ca0489b05ed1acdc2a6d875d 
  src/tests/containerizer.hpp 940c4146f4e854a6b1b9ccaba5687e76d5723cba 
  src/tests/containerizer.cpp 27c29b40f3241807fb22b27f0d9ca64b3e602a8f 
  src/tests/executor_http_api_tests.cpp 
a9f1a7b0498acd541c6f58ad1388da49c9951e22 
  src/tests/fault_tolerance_tests.cpp 5a9944cf459ab688907d95bbda09f464b37efd1e 
  src/tests/http_fault_tolerance_tests.cpp 
57ef562058f8abf9256e2ab8a4a85b36b5a7add4 
  src/tests/master_contender_detector_tests.cpp 
2a7d713f74c907235f82d83eaf46630046645faf 
  src/tests/master_maintenance_tests.cpp 
6917272f2de7a09bf4de7e932994655f4e54d3da 
  src/tests/master_tests.cpp 88cf1e612ad8186ef2cea161b3a52b0df9517305 
  src/tests/mesos.hpp 9309b5a985c0d7136a2ee5aa1598b4fee6194816 
  src/tests/scheduler_http_api_tests.cpp 
6390f2eb53d8bb97b98e64ec6e0f795abc3f3c7f 
  src/tests/scheduler_tests.cpp b0ea0bbcce9d847285fda40f778caaf721804457 
  src/tests/slave_tests.cpp 93b81d3e5b90d8036409e943f598c865fe335bcc 

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


Testing
---


Thanks,

Joris Van Remoortere

Re: Review Request 52980: Updated os::mkdir() to take the mode as an input.

[Alex Clemmer]

> On Oct. 18, 2016, 7:58 p.m., Alex Clemmer wrote:
> > 3rdparty/stout/include/stout/os/mkdir.hpp, line 43
> > 
> >
> > Permissions is a bit of a subtle issue in Mesos, which we've largely 
> > punted on. I'm a bit worried that adding a pluggable `mode` here will cause 
> > us to add code that only works on Unix. How do you see this API evolving to 
> > accommodate both?

Also, sorry, I should have been more clear: I mean that permissions _on 
Windows_ is a subtle issue in Mesos. My worry is rooted in the fact that this 
API also has to support Windows.


- Alex


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


On Oct. 18, 2016, 7:43 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52980/
> ---
> 
> (Updated Oct. 18, 2016, 7:43 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4893
> https://issues.apache.org/jira/browse/MESOS-4893
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mode still defaults to 755, but we now enable the caller to specify
> a different mode for creation of directories.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/mkdir.hpp 
> fe86864c8b480993c8f052f39b2fd3ece23798da 
> 
> Diff: https://reviews.apache.org/r/52980/diff/
> 
> 
> Testing
> ---
> 
> Tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 52980: Updated os::mkdir() to take the mode as an input.

[Alex Clemmer]

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




3rdparty/stout/include/stout/os/mkdir.hpp (line 43)


Permissions is a bit of a subtle issue in Mesos, which we've largely punted 
on. I'm a bit worried that adding a pluggable `mode` here will cause us to add 
code that only works on Unix. How do you see this API evolving to accommodate 
both?


- Alex Clemmer


On Oct. 18, 2016, 7:43 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52980/
> ---
> 
> (Updated Oct. 18, 2016, 7:43 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4893
> https://issues.apache.org/jira/browse/MESOS-4893
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mode still defaults to 755, but we now enable the caller to specify
> a different mode for creation of directories.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/mkdir.hpp 
> fe86864c8b480993c8f052f39b2fd3ece23798da 
> 
> Diff: https://reviews.apache.org/r/52980/diff/
> 
> 
> Testing
> ---
> 
> Tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 52988: Fixed a typo in a comment.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52988]

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

- Mesos ReviewBot


On Oct. 18, 2016, 2:20 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52988/
> ---
> 
> (Updated Oct. 18, 2016, 2:20 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a typo in a comment.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 0017d3d9d46433b391703025f611ce437bbc7ebe 
>   include/mesos/v1/mesos.proto def33ef5e446576c86da0498e8a24e2e2de17918 
> 
> Diff: https://reviews.apache.org/r/52988/diff/
> 
> 
> Testing
> ---
> 
> Visual inspection.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 52953: Added container 'exec' command to the CLI.

[Kevin Klues]

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

(Updated Oct. 18, 2016, 7:01 p.m.)


Review request for mesos and Joseph Wu.


Changes
---

Updated to factor out the function of getting the proper mount namespace path.


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


Repository: mesos


Description
---

Added container 'exec' command to the CLI.


Diffs (updated)
-

  src/cli_new/lib/mesos/plugins/container/main.py PRE-CREATION 
  src/cli_new/lib/mesos/plugins/container/tests.py PRE-CREATION 
  src/cli_new/lib/mesos/tests/base.py PRE-CREATION 
  src/cli_new/lib/mesos/util.py 87d2a65e78f04209566c1434b489b941d570ee01 
  src/cli_new/pip-requirements.txt 9cc8d096ac353f4ed2d4318f2d0e439a6edf 

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


Testing
---

$ cd src/cli_new
$ ./bootstrap
$ source activate

(mesos-cli) $ sudo bin/mesos-cli-tests


Thanks,

Kevin Klues

Review Request 52992: Updated configure.ac in preparation of 1.2.0.

[Till Toenshoff]

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

Review request for mesos and Alexander Rukletsov.


Repository: mesos


Description
---

see summary.


Diffs
-

  configure.ac 015255ec1876a51b9eb2cf488b375af90f73c722 

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


Testing
---


Thanks,

Till Toenshoff

Review Request 52991: Updated CHANGELOG in preparation of 1.2.0.

[Till Toenshoff]

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

Review request for mesos and Alexander Rukletsov.


Repository: mesos


Description
---

see summary.


Diffs
-

  CHANGELOG b40f9c8106ac726cdb01bfe079a40e77ee7c8a2e 

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


Testing
---


Thanks,

Till Toenshoff

Re: Review Request 52941: Updated CLI pylint configuration to allow 0 public methods.

[Joseph Wu]

> On Oct. 17, 2016, 6 p.m., Joseph Wu wrote:
> > src/cli_new/pylint.config, line 15
> > 
> >
> > Is this used anywhere?  I did not encounter a linting error for this in 
> > your chain.
> 
> Kevin Klues wrote:
> I know I encountered it at some point, but maybe refactoring removed it. 
> Anyway, again, I personally think this seems like a reasonable one to 
> disable, but I guess I can discard it until it comes up again.
> 
> In general I haven't been actively searching for a list of warnings to 
> disable, but when one crops up and it seems reasonable to disable globally, 
> that's when I've been adding these in there.  Seems we may disagree on what 
> should be disabled globally though...
> 
> Should I drop this one too?

This one is reasonable to disable.  I'd say that `struct`-like classes are more 
easily documented than things like named-tuples or plain old dictionaries.


- Joseph


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


On Oct. 17, 2016, 1:11 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52941/
> ---
> 
> (Updated Oct. 17, 2016, 1:11 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-5676
> https://issues.apache.org/jira/browse/MESOS-5676
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated CLI pylint configuration to allow 0 public methods.
> 
> 
> Diffs
> -
> 
>   src/cli_new/pylint.config c398220db063d249e6c62cf7e8cd6757e7860630 
> 
> Diff: https://reviews.apache.org/r/52941/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 52940: Updated CLI pylint configuration to allow up to 30 local variables.

[Joseph Wu]

> On Oct. 17, 2016, 6 p.m., Joseph Wu wrote:
> > src/cli_new/pylint.config, line 14
> > 
> >
> > Looks like `__enter_namespaces` in https://reviews.apache.org/r/52953/ 
> > is over the default limit of 15 by one variable...
> > 
> > I'd say that function needs some helpers, rather than silencing the 
> > warning.
> 
> Kevin Klues wrote:
> Again I didn't just add this patch to suppress the warning (though that's 
> what triggered it). I think having such a small limit on the number of local 
> variables could prove burdensome given our pattern for "try/except' around al 
> most every call we make. That said, I agree that `__enter_namespaces` could 
> be broken up a little bit, so I'll address that in the relevant patch.
> 
> Are you suggesting I discard this one too then?

I think 15 is a fairly reasonable limit, even with all the exception catching.  
I'd prefer to keep the warning unless it obviously degrades the code quality.  
(In which case, we'd bump the limit or so.)


- Joseph


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


On Oct. 17, 2016, 1:11 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52940/
> ---
> 
> (Updated Oct. 17, 2016, 1:11 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-5676
> https://issues.apache.org/jira/browse/MESOS-5676
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated CLI pylint configuration to allow up to 30 local variables.
> 
> 
> Diffs
> -
> 
>   src/cli_new/pylint.config c398220db063d249e6c62cf7e8cd6757e7860630 
> 
> Diff: https://reviews.apache.org/r/52940/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 52647: Fix new sign comparison errors in libprocess produced by hardened flags

[Aaron Wood]

> On Oct. 18, 2016, 3:14 a.m., James Peach wrote:
> > 3rdparty/libprocess/src/tests/io_tests.cpp, line 284
> > 
> >
> > Can you just make ``length`` type ``ssize_t``?

`length` is `ssize_t` (set on line 235)


- Aaron


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


On Oct. 14, 2016, 3:14 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52647/
> ---
> 
> (Updated Oct. 14, 2016, 3:14 p.m.)
> 
> 
> Review request for mesos, Michael Park and Neil Conway.
> 
> 
> Bugs: MESOS-6239
> https://issues.apache.org/jira/browse/MESOS-6239
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The hardening flags produced many new sign comparison errors in libprocess 
> that need to be fixed for Mesos to compile/run.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/decoder.hpp c79296b 
>   3rdparty/libprocess/src/encoder.hpp 005d1cc 
>   3rdparty/libprocess/src/process.cpp 18a8e20 
>   3rdparty/libprocess/src/tests/decoder_tests.cpp 4535614 
>   3rdparty/libprocess/src/tests/encoder_tests.cpp 9e57375 
>   3rdparty/libprocess/src/tests/http_tests.cpp 8d6c8c4 
>   3rdparty/libprocess/src/tests/io_tests.cpp b85c79f 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp c8350cf 
> 
> Diff: https://reviews.apache.org/r/52647/diff/
> 
> 
> Testing
> ---
> 
> Made sure compilation, tests, and benchmarks worked with both gcc and clang. 
> Ran `make && make check && make bench`.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 52939: Updated CLI pylint configuration to allow 2 character variable names.

[Joseph Wu]

> On Oct. 17, 2016, 6 p.m., Joseph Wu wrote:
> > src/cli_new/pylint.config, line 13
> > 
> >
> > I see a single 2 character variable name `io` here: 
> > https://reviews.apache.org/r/52953/diff/1#0 -- Line 306
> > I think a longer variable name is reasonable there.
> > 
> > `ip` can be added to the `good-names`.
> 
> Kevin Klues wrote:
> I didn't add this just to remove the warning (though that's what 
> triggered it). I added it because I figured allowing 2 character variable 
> names was probably a good thing in general. 
> 
> Are you suggesting that we drop the patch in favor of disallowing 2 
> character variable names in general?

Yeah, I'm in favor of keeping the warning.

We prefer longer/descriptive names in C++ and I'd say the same thing applies 
here.


- Joseph


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


On Oct. 17, 2016, 1:11 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52939/
> ---
> 
> (Updated Oct. 17, 2016, 1:11 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-5676
> https://issues.apache.org/jira/browse/MESOS-5676
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated CLI pylint configuration to allow 2 character variable names.
> 
> 
> Diffs
> -
> 
>   src/cli_new/pylint.config c398220db063d249e6c62cf7e8cd6757e7860630 
> 
> Diff: https://reviews.apache.org/r/52939/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 51052: Make mesos-docker-execute understand cgroups_enable_cfs.

[Zhitao Li]

> On Oct. 18, 2016, 4:49 a.m., Jie Yu wrote:
> > src/docker/docker.hpp, line 51
> > 
> >
> > Having this as a global knob sounds really weird to me. I know you want 
> > to work around the Docker::run GMock issue. But this is too weird to me. 
> > That means we cannot have per docker container cfs control (i.e., some 
> > container uses cfs while some doesn't)
> 
> haosdent huang wrote:
> `cgroups_enable_cfs` is a global flag in agent, we also enable cfs for 
> all mesos containers. So I think the behaviour in this patch is as same as 
> mesos containerizer as well?

Capturing offline message, I plan to refactor the arguments for `docker::run` 
into a `DockerRunOptions` struct first, then add `cpu-quota` into it to support 
my case.


- Zhitao


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


On Oct. 12, 2016, 5:02 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51052/
> ---
> 
> (Updated Oct. 12, 2016, 5:02 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-6134
> https://issues.apache.org/jira/browse/MESOS-6134
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fixes cpu quota for command executor (which runs outside
> of the docker container) by ensuing --cpu-quota flag to docker
> run.
> 
> Note: we have to add the boolean variable to `Docker` class
> because `Docker::run()` has reached the maximum argument length
> GMOCK can support.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
>   src/docker/docker.cpp 50fda393a42afefc70790a26b44911e4cf17185e 
>   src/docker/executor.hpp a49ec1b4045116741af6af08578791fb0440ad8f 
>   src/docker/executor.cpp ab3f0473fdc9105d1c425f0dbe7b81c566d541e8 
>   src/slave/containerizer/docker.cpp d71386089bf7677872bcb1bd36e07da9263dcf0d 
>   src/tests/mock_docker.cpp 6a0e613bde6889864a37ffd7ec0b454e5fe4df1c 
> 
> Diff: https://reviews.apache.org/r/51052/diff/
> 
> 
> Testing
> ---
> 
> I am now able to make docker containers launched through mesos-execute have a 
> cpu quota.
> 
> Also making sure `make check` still works on mac os for the linux only flag.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 52953: Added container 'exec' command to the CLI.

[Kevin Klues]

> On Oct. 18, 2016, 1:01 a.m., Joseph Wu wrote:
> > src/cli_new/lib/mesos/plugins/container/main.py, line 30
> > 
> >
> > Hit the same linter-using-old-virtualenv error when applying this patch 
> > as described in:
> > https://reviews.apache.org/r/51008/#comment12

See comment in that one.


- Kevin


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


On Oct. 17, 2016, 8:12 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52953/
> ---
> 
> (Updated Oct. 17, 2016, 8:12 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-5676
> https://issues.apache.org/jira/browse/MESOS-5676
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added container 'exec' command to the CLI.
> 
> 
> Diffs
> -
> 
>   src/cli_new/lib/mesos/plugins/container/main.py PRE-CREATION 
>   src/cli_new/lib/mesos/plugins/container/tests.py PRE-CREATION 
>   src/cli_new/lib/mesos/tests/base.py PRE-CREATION 
>   src/cli_new/lib/mesos/util.py 87d2a65e78f04209566c1434b489b941d570ee01 
>   src/cli_new/pip-requirements.txt 9cc8d096ac353f4ed2d4318f2d0e439a6edf 
> 
> Diff: https://reviews.apache.org/r/52953/diff/
> 
> 
> Testing
> ---
> 
> $ cd src/cli_new
> $ ./bootstrap
> $ source activate
> 
> (mesos-cli) $ sudo bin/mesos-cli-tests
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 51008: Added infrastructure for unit tests in the new python-based CLI.

[Kevin Klues]

> On Oct. 18, 2016, 1:01 a.m., Joseph Wu wrote:
> > src/cli_new/bin/tests.py, lines 23-25
> > 
> >
> > This actually failed to lint after applying:
> > ```
> > Checking 1 Python file
> > * Module tests
> > E: 23, 0: Unable to import 'colour_runner.runner' (import-error)
> > E: 25, 0: Unable to import 'termcolor' (import-error)
> > Total errors found: 2
> > ```
> > 
> > Reason being that I had a pre-existing virtualenv, which did not have 
> > these modules installed.
> > 
> > Doing `rm -rf src/cli_new/.virtualenv/` allows the linter to pass.

We should modify the linter to see if there were any changes to 
`pip-requirements.txt` and re-run `./bootstrap` if so.


- Kevin


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


On Oct. 17, 2016, 8:11 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51008/
> ---
> 
> (Updated Oct. 17, 2016, 8:11 p.m.)
> 
> 
> Review request for mesos, Haris Choudhary and Joseph Wu.
> 
> 
> Bugs: MESOS-6032
> https://issues.apache.org/jira/browse/MESOS-6032
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added infrastructure for unit tests in the new python-based CLI.
> 
> 
> Diffs
> -
> 
>   src/cli_new/bin/mesos-cli-tests PRE-CREATION 
>   src/cli_new/bin/tests.py PRE-CREATION 
>   src/cli_new/pip-requirements.txt 9cc8d096ac353f4ed2d4318f2d0e439a6edf 
> 
> Diff: https://reviews.apache.org/r/51008/diff/
> 
> 
> Testing
> ---
> 
> $ cd src/cli_new
> $ ./bootstrap
> $ source activate
> 
> (mesos-cli) $ mesos-cli-tests
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 52941: Updated CLI pylint configuration to allow 0 public methods.

[Kevin Klues]

> On Oct. 18, 2016, 1 a.m., Joseph Wu wrote:
> > src/cli_new/pylint.config, line 15
> > 
> >
> > Is this used anywhere?  I did not encounter a linting error for this in 
> > your chain.

I know I encountered it at some point, but maybe refactoring removed it. 
Anyway, again, I personally think this seems like a reasonable one to disable, 
but I guess I can discard it until it comes up again.

In general I haven't been actively searching for a list of warnings to disable, 
but when one crops up and it seems reasonable to disable globally, that's when 
I've been adding these in there.  Seems we may disagree on what should be 
disabled globally though...

Should I drop this one too?


- Kevin


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


On Oct. 17, 2016, 8:11 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52941/
> ---
> 
> (Updated Oct. 17, 2016, 8:11 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-5676
> https://issues.apache.org/jira/browse/MESOS-5676
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated CLI pylint configuration to allow 0 public methods.
> 
> 
> Diffs
> -
> 
>   src/cli_new/pylint.config c398220db063d249e6c62cf7e8cd6757e7860630 
> 
> Diff: https://reviews.apache.org/r/52941/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 52940: Updated CLI pylint configuration to allow up to 30 local variables.

[Kevin Klues]

> On Oct. 18, 2016, 1 a.m., Joseph Wu wrote:
> > src/cli_new/pylint.config, line 14
> > 
> >
> > Looks like `__enter_namespaces` in https://reviews.apache.org/r/52953/ 
> > is over the default limit of 15 by one variable...
> > 
> > I'd say that function needs some helpers, rather than silencing the 
> > warning.

Again I didn't just add this patch to suppress the warning (though that's what 
triggered it). I think having such a small limit on the number of local 
variables could prove burdensome given our pattern for "try/except' around al 
most every call we make. That said, I agree that `__enter_namespaces` could be 
broken up a little bit, so I'll address that in the relevant patch.

Are you suggesting I discard this one too then?


- Kevin


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


On Oct. 17, 2016, 8:11 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52940/
> ---
> 
> (Updated Oct. 17, 2016, 8:11 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-5676
> https://issues.apache.org/jira/browse/MESOS-5676
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated CLI pylint configuration to allow up to 30 local variables.
> 
> 
> Diffs
> -
> 
>   src/cli_new/pylint.config c398220db063d249e6c62cf7e8cd6757e7860630 
> 
> Diff: https://reviews.apache.org/r/52940/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 52939: Updated CLI pylint configuration to allow 2 character variable names.

[Kevin Klues]

> On Oct. 18, 2016, 1 a.m., Joseph Wu wrote:
> > src/cli_new/pylint.config, line 13
> > 
> >
> > I see a single 2 character variable name `io` here: 
> > https://reviews.apache.org/r/52953/diff/1#0 -- Line 306
> > I think a longer variable name is reasonable there.
> > 
> > `ip` can be added to the `good-names`.

I didn't add this just to remove the warning (though that's what triggered it). 
I added it because I figured allowing 2 character variable names was probably a 
good thing in general. 

Are you suggesting that we drop the patch in favor of disallowing 2 character 
variable names in general?


- Kevin


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


On Oct. 17, 2016, 8:11 p.m., Kevin Klues wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52939/
> ---
> 
> (Updated Oct. 17, 2016, 8:11 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-5676
> https://issues.apache.org/jira/browse/MESOS-5676
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated CLI pylint configuration to allow 2 character variable names.
> 
> 
> Diffs
> -
> 
>   src/cli_new/pylint.config c398220db063d249e6c62cf7e8cd6757e7860630 
> 
> Diff: https://reviews.apache.org/r/52939/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>

Re: Review Request 52967: Improved documentation for shared persistent volumes.

[Neil Conway]

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

(Updated Oct. 18, 2016, 4:15 p.m.)


Review request for mesos and Jiang Yan Xu.


Changes
---

Try to address review comments.


Repository: mesos


Description
---

Improved documentation for shared persistent volumes.


Diffs (updated)
-

  docs/home.md 1c6b191bd194a9100ce1ad4bf5aff62a20ed8f41 
  docs/persistent-volume.md 5dfbbf1fb08cdf47f97ea6fb286e21ab26235d62 
  docs/shared-resources.md 29e43389c530f816118a32dcd7c2f1b82b0af431 

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


Testing
---

Previewed with site-docker.


Thanks,

Neil Conway

Re: Review Request 52967: Improved documentation for shared persistent volumes.

[Neil Conway]

> On Oct. 18, 2016, 7:19 a.m., Jiang Yan Xu wrote:
> > docs/home.md, line 54
> > 
> >
> > s/tasks/tasks across containers/ 
> > 
> > as multiple tasks sharing an executor today can already access the same 
> > non-shared persistent volume.

I changed this to say "between tasks managed by different executors on the same 
agent". Right now we mostly talk about "tasks" and "executors", so starting to 
talk about containers seems a bit inconsistent unless we're going to adopt that 
terminology more broadly (which might not be a bad idea).


- Neil


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


On Oct. 18, 2016, 12:50 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52967/
> ---
> 
> (Updated Oct. 18, 2016, 12:50 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved documentation for shared persistent volumes.
> 
> 
> Diffs
> -
> 
>   docs/home.md 1c6b191bd194a9100ce1ad4bf5aff62a20ed8f41 
>   docs/persistent-volume.md 5dfbbf1fb08cdf47f97ea6fb286e21ab26235d62 
>   docs/shared-resources.md 29e43389c530f816118a32dcd7c2f1b82b0af431 
> 
> Diff: https://reviews.apache.org/r/52967/diff/
> 
> 
> Testing
> ---
> 
> Previewed with site-docker.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 51027: Track allocation candidates to bound allocator.

[Jacob Janco]

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

(Updated Oct. 18, 2016, 4:14 p.m.)


Review request for mesos, Benjamin Mahler, Guangya Liu, James Peach, Klaus Ma, 
and Jiang Yan Xu.


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


Repository: mesos


Description
---

- Triggered allocations dispatch allocate() only
  if there is no pending allocation in the queue.
- Allocation candidates are accumulated and only
  cleared when enqueued allocations are processed.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
2c31471ee0f5d6836393bf87ff9ecfd8df835013 
  src/master/allocator/mesos/hierarchical.cpp 
c8f9492ee1b69e125a1e841116d22a578a9b524e 

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


Testing
---

make check

note: check without filters depends on https://reviews.apache.org/r/51028 and 
https://reviews.apache.org/r/52534


Thanks,

Jacob Janco

Re: Review Request 52647: Fix new sign comparison errors in libprocess produced by hardened flags

[Aaron Wood]

> On Oct. 18, 2016, 3:14 a.m., James Peach wrote:
> > 3rdparty/libprocess/src/encoder.hpp, line 291
> > 
> >
> > It's not obvious to me that this is the right change since the 
> > surrounding code tries to deal with ``off_t``. Can you explain this some 
> > more?

In the `backup` method on line 276 it's being compared with a `size_t` as well 
as in the `remaining` method on line 283. But in `next` on line 267 `size` 
which is a `size_t` is being casted to an `off_t` and then assigned to `index`. 

The other encoders above in that file seem to be working with their own private 
`index` as a `size_t` so even though it looks like it's being used in different 
ways in `FileEncoder` I thought it should be a `size_t`.

Since this specific class is for files maybe we should keep it as `off_t` and 
just cast the things used for comparisons to `off_t`.


- Aaron


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


On Oct. 14, 2016, 3:14 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52647/
> ---
> 
> (Updated Oct. 14, 2016, 3:14 p.m.)
> 
> 
> Review request for mesos, Michael Park and Neil Conway.
> 
> 
> Bugs: MESOS-6239
> https://issues.apache.org/jira/browse/MESOS-6239
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The hardening flags produced many new sign comparison errors in libprocess 
> that need to be fixed for Mesos to compile/run.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/decoder.hpp c79296b 
>   3rdparty/libprocess/src/encoder.hpp 005d1cc 
>   3rdparty/libprocess/src/process.cpp 18a8e20 
>   3rdparty/libprocess/src/tests/decoder_tests.cpp 4535614 
>   3rdparty/libprocess/src/tests/encoder_tests.cpp 9e57375 
>   3rdparty/libprocess/src/tests/http_tests.cpp 8d6c8c4 
>   3rdparty/libprocess/src/tests/io_tests.cpp b85c79f 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp c8350cf 
> 
> Diff: https://reviews.apache.org/r/52647/diff/
> 
> 
> Testing
> ---
> 
> Made sure compilation, tests, and benchmarks worked with both gcc and clang. 
> Ran `make && make check && make bench`.
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>

Re: Review Request 52986: Updated endpoint-help in preparation of 1.1.0.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52985, 52986]

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

- Mesos ReviewBot


On Oct. 18, 2016, 3:14 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52986/
> ---
> 
> (Updated Oct. 18, 2016, 3:14 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/files/browse.json.md 
> 55ee41b1d61f46a11931a9e9d9b1e4f65f0d80ce 
>   docs/endpoints/files/browse.md de8632e95373e2719bc9a035a82ed51f1c9b0437 
>   docs/endpoints/files/debug.json.md a4dc0de5f343a799bdba983b4e75ee106b0bf64b 
>   docs/endpoints/files/debug.md 9361ffecef4a1cab1138981c5ce302561505cfda 
>   docs/endpoints/files/download.json.md 
> 88dd72cbd97845fe8e9a3bdd374502b91d7712bd 
>   docs/endpoints/files/download.md 3a3876010e0f5c55826b20c5176b6f82f3249deb 
>   docs/endpoints/files/read.json.md 6697ab1e251c22db8a7b79926d737fad9514249d 
>   docs/endpoints/files/read.md 21ea6d8a3b9a2bd7cfc214653048a9f74bbac91c 
>   docs/endpoints/master/api/v1.md 19c6142844ee9167f532496873524508d335a59d 
>   docs/endpoints/master/api/v1/scheduler.md 
> 39cd457efd144ce316e8c13c916000cac8e51ad7 
>   docs/endpoints/master/create-volumes.md 
> f6ec9384f7f2bfb8056e54532ac3417a1f15b1d1 
>   docs/endpoints/master/destroy-volumes.md 
> 054e816371f26c91ccc0d9687f377e80641e42a8 
>   docs/endpoints/master/flags.md 850b6f86ddd92af512932aec5af9bdc600cf92d8 
>   docs/endpoints/master/frameworks.md 
> 1aed723152528be0e19d79520daac5f470cb49da 
>   docs/endpoints/master/reserve.md 9bb04ed0de5b9304301fffd51b39ec07102825cd 
>   docs/endpoints/master/state-summary.md 
> 4eb517ea0f6b5dbc2e7d7c406d54e741cf50e0e5 
>   docs/endpoints/master/state.json.md 
> c5852bd01dd5f72a9a11991a033d8c97b111ad21 
>   docs/endpoints/master/state.md 8e0650f84ae13ef7952971ffb9732ec379cffd40 
>   docs/endpoints/master/tasks.json.md 
> 5d2c0e6eb53fa5e04a973eb786cd16bfad38736f 
>   docs/endpoints/master/tasks.md c7df686443e20d255b03fc31a999b9b24b5f3a0a 
>   docs/endpoints/master/teardown.md 4c14f594c3713304d5adf59e56e9e8af2b05ebee 
>   docs/endpoints/master/unreserve.md 5cce4288231a64d7ee7713f2b44487618e7cc0a7 
>   docs/endpoints/slave/api/v1.md c867d795104e053e5aae819df3deea68269228bf 
>   docs/endpoints/slave/flags.md 3ea38c160670fc302a42ae98d7ebe0679b5bdc92 
>   docs/endpoints/slave/state.json.md ac8536986ceb6532f71b4fe1c62b1b83390b03e7 
>   docs/endpoints/slave/state.md c7b61d736582f50b850fed252e558bbae6dad801 
> 
> Diff: https://reviews.apache.org/r/52986/diff/
> 
> 
> Testing
> ---
> 
> this was automatically generated using `support/generate-endpoint-help.py`.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 52783: Added documentation for mesos-containerizer Linux capabilities support.

[Avinash sridharan]

> On Oct. 18, 2016, 4:55 a.m., Avinash sridharan wrote:
> > docs/configuration.md, line 1003
> > 
> >
> > s/the agent/that the agent/
> 
> Benjamin Bannier wrote:
> Not a native speaker but to me this seems to just make the sentence more 
> wordy (and one should probably use `which` instead of `that`). Maybe a native 
> speaker could chime in how this should be phrased.

Valid point. Maybe @Neil or @BenM can chime in?


> On Oct. 18, 2016, 4:55 a.m., Avinash sridharan wrote:
> > docs/linux_capabilities.md, line 3
> > 
> >
> > s/described/describes
> > 
> > to add support for `Linux Capabilities`
> 
> Benjamin Bannier wrote:
> I do not understand your second request. How should the sentence read 
> after applying you suggestion?

I was suggesting:

This document describes the `linux/capabilities` isolator. The isolator adds 
support for controlling `Linux Capabilities` of containers launched using the 
`MesosContainerizer`.

(Re-worded my suggestion a bit actually).


- Avinash


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


On Oct. 18, 2016, 7:51 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52783/
> ---
> 
> (Updated Oct. 18, 2016, 7:51 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6376
> https://issues.apache.org/jira/browse/MESOS-6376
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for mesos-containerizer Linux capabilities support.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md c83a58eb6884c8d8c37880a745e04cf0b789ebdc 
>   docs/linux_capabilities.md PRE-CREATION 
>   docs/mesos-containerizer.md 76544625838845fe2817347fea483a60ab250f99 
> 
> Diff: https://reviews.apache.org/r/52783/diff/
> 
> 
> Testing
> ---
> 
> Checked with local renderer.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 52985: Updated CHANGELOG in preparation for RC1 of 1.1.0.

[Till Toenshoff]

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

(Updated Oct. 18, 2016, 3:24 p.m.)


Review request for mesos and Alexander Rukletsov.


Changes
---

Addressed Alex's comments - thanks for reviewing!


Repository: mesos


Description (updated)
---

Updated CHANGELOG in preparation for RC1 of 1.1.0.


Diffs (updated)
-

  CHANGELOG b40f9c8106ac726cdb01bfe079a40e77ee7c8a2e 

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


Testing
---


Thanks,

Till Toenshoff

Re: Review Request 52986: Updated endpoint-help in preparation of 1.1.0.

[Alexander Rukletsov]

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


Ship it!




I assume this is autoimatically generated *without* manual editing. Could you 
please say it explicitly in the testing done or description section?

- Alexander Rukletsov


On Oct. 18, 2016, 12:15 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52986/
> ---
> 
> (Updated Oct. 18, 2016, 12:15 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> see summary.
> 
> 
> Diffs
> -
> 
>   docs/endpoints/files/browse.json.md 
> 55ee41b1d61f46a11931a9e9d9b1e4f65f0d80ce 
>   docs/endpoints/files/browse.md de8632e95373e2719bc9a035a82ed51f1c9b0437 
>   docs/endpoints/files/debug.json.md a4dc0de5f343a799bdba983b4e75ee106b0bf64b 
>   docs/endpoints/files/debug.md 9361ffecef4a1cab1138981c5ce302561505cfda 
>   docs/endpoints/files/download.json.md 
> 88dd72cbd97845fe8e9a3bdd374502b91d7712bd 
>   docs/endpoints/files/download.md 3a3876010e0f5c55826b20c5176b6f82f3249deb 
>   docs/endpoints/files/read.json.md 6697ab1e251c22db8a7b79926d737fad9514249d 
>   docs/endpoints/files/read.md 21ea6d8a3b9a2bd7cfc214653048a9f74bbac91c 
>   docs/endpoints/master/api/v1.md 19c6142844ee9167f532496873524508d335a59d 
>   docs/endpoints/master/api/v1/scheduler.md 
> 39cd457efd144ce316e8c13c916000cac8e51ad7 
>   docs/endpoints/master/create-volumes.md 
> f6ec9384f7f2bfb8056e54532ac3417a1f15b1d1 
>   docs/endpoints/master/destroy-volumes.md 
> 054e816371f26c91ccc0d9687f377e80641e42a8 
>   docs/endpoints/master/flags.md 850b6f86ddd92af512932aec5af9bdc600cf92d8 
>   docs/endpoints/master/frameworks.md 
> 1aed723152528be0e19d79520daac5f470cb49da 
>   docs/endpoints/master/reserve.md 9bb04ed0de5b9304301fffd51b39ec07102825cd 
>   docs/endpoints/master/state-summary.md 
> 4eb517ea0f6b5dbc2e7d7c406d54e741cf50e0e5 
>   docs/endpoints/master/state.json.md 
> c5852bd01dd5f72a9a11991a033d8c97b111ad21 
>   docs/endpoints/master/state.md 8e0650f84ae13ef7952971ffb9732ec379cffd40 
>   docs/endpoints/master/tasks.json.md 
> 5d2c0e6eb53fa5e04a973eb786cd16bfad38736f 
>   docs/endpoints/master/tasks.md c7df686443e20d255b03fc31a999b9b24b5f3a0a 
>   docs/endpoints/master/teardown.md 4c14f594c3713304d5adf59e56e9e8af2b05ebee 
>   docs/endpoints/master/unreserve.md 5cce4288231a64d7ee7713f2b44487618e7cc0a7 
>   docs/endpoints/slave/api/v1.md c867d795104e053e5aae819df3deea68269228bf 
>   docs/endpoints/slave/flags.md 3ea38c160670fc302a42ae98d7ebe0679b5bdc92 
>   docs/endpoints/slave/state.json.md ac8536986ceb6532f71b4fe1c62b1b83390b03e7 
>   docs/endpoints/slave/state.md c7b61d736582f50b850fed252e558bbae6dad801 
> 
> Diff: https://reviews.apache.org/r/52986/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 52986: Updated endpoint-help in preparation of 1.1.0.

[Till Toenshoff]

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

(Updated Oct. 18, 2016, 3:14 p.m.)


Review request for mesos and Alexander Rukletsov.


Repository: mesos


Description
---

see summary.


Diffs
-

  docs/endpoints/files/browse.json.md 55ee41b1d61f46a11931a9e9d9b1e4f65f0d80ce 
  docs/endpoints/files/browse.md de8632e95373e2719bc9a035a82ed51f1c9b0437 
  docs/endpoints/files/debug.json.md a4dc0de5f343a799bdba983b4e75ee106b0bf64b 
  docs/endpoints/files/debug.md 9361ffecef4a1cab1138981c5ce302561505cfda 
  docs/endpoints/files/download.json.md 
88dd72cbd97845fe8e9a3bdd374502b91d7712bd 
  docs/endpoints/files/download.md 3a3876010e0f5c55826b20c5176b6f82f3249deb 
  docs/endpoints/files/read.json.md 6697ab1e251c22db8a7b79926d737fad9514249d 
  docs/endpoints/files/read.md 21ea6d8a3b9a2bd7cfc214653048a9f74bbac91c 
  docs/endpoints/master/api/v1.md 19c6142844ee9167f532496873524508d335a59d 
  docs/endpoints/master/api/v1/scheduler.md 
39cd457efd144ce316e8c13c916000cac8e51ad7 
  docs/endpoints/master/create-volumes.md 
f6ec9384f7f2bfb8056e54532ac3417a1f15b1d1 
  docs/endpoints/master/destroy-volumes.md 
054e816371f26c91ccc0d9687f377e80641e42a8 
  docs/endpoints/master/flags.md 850b6f86ddd92af512932aec5af9bdc600cf92d8 
  docs/endpoints/master/frameworks.md 1aed723152528be0e19d79520daac5f470cb49da 
  docs/endpoints/master/reserve.md 9bb04ed0de5b9304301fffd51b39ec07102825cd 
  docs/endpoints/master/state-summary.md 
4eb517ea0f6b5dbc2e7d7c406d54e741cf50e0e5 
  docs/endpoints/master/state.json.md c5852bd01dd5f72a9a11991a033d8c97b111ad21 
  docs/endpoints/master/state.md 8e0650f84ae13ef7952971ffb9732ec379cffd40 
  docs/endpoints/master/tasks.json.md 5d2c0e6eb53fa5e04a973eb786cd16bfad38736f 
  docs/endpoints/master/tasks.md c7df686443e20d255b03fc31a999b9b24b5f3a0a 
  docs/endpoints/master/teardown.md 4c14f594c3713304d5adf59e56e9e8af2b05ebee 
  docs/endpoints/master/unreserve.md 5cce4288231a64d7ee7713f2b44487618e7cc0a7 
  docs/endpoints/slave/api/v1.md c867d795104e053e5aae819df3deea68269228bf 
  docs/endpoints/slave/flags.md 3ea38c160670fc302a42ae98d7ebe0679b5bdc92 
  docs/endpoints/slave/state.json.md ac8536986ceb6532f71b4fe1c62b1b83390b03e7 
  docs/endpoints/slave/state.md c7b61d736582f50b850fed252e558bbae6dad801 

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


Testing (updated)
---

this was automatically generated using `support/generate-endpoint-help.py`.


Thanks,

Till Toenshoff

Re: Review Request 52985: Updated CHANGELOG in preparation for RC1 of 1.1.0.

[Alexander Rukletsov]

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


Fix it, then Ship it!





CHANGELOG (lines 1 - 2)


You probably want to remove `(WIP)` and some dashes accordingly.



CHANGELOG (line 95)


2 spaces here please. I know we are not entirely consistent, but looks like 
2 spaces happened more often : )


- Alexander Rukletsov


On Oct. 18, 2016, 12:08 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52985/
> ---
> 
> (Updated Oct. 18, 2016, 12:08 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds the JIRA release-notes to the CHANGELOG. Fixed typos and spelling 
> mistakes,
> adds periods where missing.
> 
> 
> Diffs
> -
> 
>   CHANGELOG b40f9c8106ac726cdb01bfe079a40e77ee7c8a2e 
> 
> Diff: https://reviews.apache.org/r/52985/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 50380: Added new benchmark test for port resources.

[Guangya Liu]

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

(Updated 十月 18, 2016, 3:02 p.m.)


Review request for mesos, Benjamin Mahler, Klaus Ma, and Jiang Yan Xu.


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


Repository: mesos


Description (updated)
---

The current ports benchmark test is always using same port range,
with port, the formula for `+` is `a+a+a+a+...+a==a`; for `-`,
it will be `a-a=0` and `0-a=0`.

The fix is adding a new benchmark test for ports resources, using
a initial and different range ports resources. This fix also
removed the ports resources benchmark test from original resources
benchmark test as we introduced a new test case for ports resources.


Diffs (updated)
-

  src/tests/resources_tests.cpp 6a12783c26f359dda835b4866b299a8fcfb3f972 

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


Testing (updated)
---

make
make check

```
 ./bin/mesos-tests.sh --benchmark 
--gtest_filter="Resources_Ports_BENCHMARK_Test.Arithmetic"
[==] Running 1 test from 1 test case.
[--] Global test environment set-up.
[--] 1 test from Resources_Ports_BENCHMARK_Test
[ RUN  ] Resources_Ports_BENCHMARK_Test.Arithmetic
Took 5.738039secs to perform 3000 'total -= r' operations on ports with 'total' 
as ports(*):[1-3000] and 'r' as 'ports(*):[x-x]' where 'x' is from 1 to 3000
Took 345315us to perform 3000 'total += r' operations on ports with 'total' as 
ports(*):[1-3000] and 'r' as 'ports(*):[x-x]' where 'x' is from 1 to to 3000
Took 6.098373secs to perform 3000 'total = total - r' operations on ports with 
'total' as ports(*):[1-3000] and 'r' as 'ports(*):[x-x]' where 'x' is from 1 to 
to 3000
Took 715101us to perform 3000 'total = total + r' operations on ports with 
'total' as ports(*):[1-3000] and 'r' as 'ports(*):[x-x]' where 'x' is from 1 to 
to 3000
[   OK ] Resources_Ports_BENCHMARK_Test.Arithmetic (12926 ms)
[--] 1 test from Resources_Ports_BENCHMARK_Test (12926 ms total)

[--] Global test environment tear-down
[==] 1 test from 1 test case ran. (12940 ms total)
[  PASSED  ] 1 test.
```


Thanks,

Guangya Liu

Review Request 52988: Fixed a typo in a comment.

[Neil Conway]

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

Review request for mesos and Alexander Rukletsov.


Repository: mesos


Description
---

Fixed a typo in a comment.


Diffs
-

  include/mesos/mesos.proto 0017d3d9d46433b391703025f611ce437bbc7ebe 
  include/mesos/v1/mesos.proto def33ef5e446576c86da0498e8a24e2e2de17918 

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


Testing
---

Visual inspection.


Thanks,

Neil Conway

Re: Review Request 51052: Make mesos-docker-execute understand cgroups_enable_cfs.

[haosdent huang]

> On Oct. 18, 2016, 4:49 a.m., Jie Yu wrote:
> > src/docker/docker.hpp, line 51
> > 
> >
> > Having this as a global knob sounds really weird to me. I know you want 
> > to work around the Docker::run GMock issue. But this is too weird to me. 
> > That means we cannot have per docker container cfs control (i.e., some 
> > container uses cfs while some doesn't)

`cgroups_enable_cfs` is a global flag in agent, we also enable cfs for all 
mesos containers. So I think the behaviour in this patch is as same as mesos 
containerizer as well?


- haosdent


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


On Oct. 12, 2016, 5:02 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51052/
> ---
> 
> (Updated Oct. 12, 2016, 5:02 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-6134
> https://issues.apache.org/jira/browse/MESOS-6134
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This fixes cpu quota for command executor (which runs outside
> of the docker container) by ensuing --cpu-quota flag to docker
> run.
> 
> Note: we have to add the boolean variable to `Docker` class
> because `Docker::run()` has reached the maximum argument length
> GMOCK can support.
> 
> 
> Diffs
> -
> 
>   src/docker/docker.hpp 9093371afc8ea792ba94f61c6875703e547ea6b0 
>   src/docker/docker.cpp 50fda393a42afefc70790a26b44911e4cf17185e 
>   src/docker/executor.hpp a49ec1b4045116741af6af08578791fb0440ad8f 
>   src/docker/executor.cpp ab3f0473fdc9105d1c425f0dbe7b81c566d541e8 
>   src/slave/containerizer/docker.cpp d71386089bf7677872bcb1bd36e07da9263dcf0d 
>   src/tests/mock_docker.cpp 6a0e613bde6889864a37ffd7ec0b454e5fe4df1c 
> 
> Diff: https://reviews.apache.org/r/51052/diff/
> 
> 
> Testing
> ---
> 
> I am now able to make docker containers launched through mesos-execute have a 
> cpu quota.
> 
> Also making sure `make check` still works on mac os for the linux only flag.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 52682: Cleaned up a few style issues in the capabilities isolator tests.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52682]

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

- Mesos ReviewBot


On Oct. 18, 2016, 8:48 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52682/
> ---
> 
> (Updated Oct. 18, 2016, 8:48 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We remove an unused using declaration, and replace several bool
> parameters with more readable enum values.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/linux_capabilities_isolator_tests.cpp 
> edb46659324c4c6345606cfa4c19f4fce05c59fe 
> 
> Diff: https://reviews.apache.org/r/52682/diff/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 51785: [HBASE-16608] Merge for the segments in the compaction pipeline and simplifying the user interface for in-memory flush

[Anoop Sam John]

> On Oct. 16, 2016, 10:43 p.m., Anoop Sam John wrote:
> > hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HeapMemStoreLAB.java,
> >  line 179
> > 
> >
> > Able to get what u r trying to do here.  Seems very complex now.
> > So can we take a diff path?
> > We have to merge N segments into one here means we have to merge N 
> > MSLABs referred by these segments.  What we do as per this patch is make a 
> > new HeapMSLAB object and add all pooled chunk from other MSLABs int the Q 
> > in new one.  And other stuff of refer next etc.
> > 
> > Can we make a new MSLAB impl? This will act as a collection of N 
> > HeapMSLABs. Now we have to impl 4 methods
> > incScannerCount and decScannerCount based on an openScannerCount in the 
> > new class.
> > close() method like in HeapMSLAB and has to call close() on each of the 
> > N MSLABs it wrap
> > This is an ImmutableSegment so no one is going to call copyCellInto(). 
> > Make this impl in this new class throw IllegalStateException or so.  Will 
> > that look good?
> 
> Michael Stack wrote:
> No copying? That sounds good.
> 
> Anoop Sam John wrote:
> So this fix and so this comment is applicable to merge op alone..  There 
> is still an open point which Me/Ram are not agreeing around this.. That is 
> whether to do this in mem merge.  This is required to limit the #segments it 
> pipeline and to get bigger sized segment at tail of the pipeline. As we do 
> flush of the tail of the segment.  But our point is let us flush all segments 
> (I mean flush to disk)..  In case of in mem compaction (This action is by def 
> OFF and as per data usage one can enable) it is worth to flush only tail.  In 
> this normal use case let us flush all segments and so there is only flatten 
> of segments comes in and so no bug as this. There is absolutely no gain we 
> get because of delaying of flush of the other segments in the pipeline ( I 
> mean in merge kind of usages)..   So I suggest let us discuss on that before 
> going to work on this. Else it wl b waste of time.
> 
> Anastasia Braginsky wrote:
> I am referencing in this answer the Anoop's suggestion to create a new 
> MSLAB implementation that act as a collection of N old HeapMSLABs. Please pay 
> attention that old scans (opened on N old HeapMSLABs) are not aware of 
> existing of the new MSLAB. In addition the close() of the new MSLAB doesn't 
> automatically means old MSLABs need to be closed, as there might be still 
> ongoing scans. Thus I do not think this idea works (at least not as it is 
> presented now).
> 
> Anastasia Braginsky wrote:
> Anoop, I do not understand about what exactly you disagree. Are you 
> saying that merging of the segments is waste of time? Do you prefer to do a 
> merge upon the flush to disk? Aa we have seen on stress benchmarks this 
> behavior delays flushes and blocks writes which is undesirable. Please 
> explain yourself clearer or we can talk about if we have a meeting this 
> Wednesday.
> 
> Anoop Sam John wrote:
> Reply
> First abt the COllection of N HeapMSLAB wont work.
> You say problem with old scans which were working on old HeapMSLABs and 
> they dont know abt the new MSLAB.  There wont be an issue.  We will really 
> close an MSLAB (I mean return chunks to pool) when there is a close call 
> happened on it AND the scanner count is zero.  So in the case u mentioned, as 
> there are old scan, the scanner count on these MSLABs are >0. Remember we are 
> not going to call close() on these.  We just have a new Collection based 
> MSLAB impl which point to N old MSLABs.  So the old scanners still use the 
> old MSLABs and once done they decr the counter.
> Now on the new one there might be new scan reqs.  So we have scanner 
> count in new COllection based MSLAB impl.  When we are at a point to close 
> this new MSLAB, we will call close on it. It will call close() on all old 
> MSLABs. (If its scanner count is zero or else the close on old MSLABs will be 
> delayed till a decr op makes the count to 0)  Now again within the old MSLAB 
> there will be normal close flow.. Iff its scanner count is 0, it will really 
> get closed.  So in a rare case where the new MSLAB is made and it is at a 
> point of getting closed and still a very old scanner still uses old MSLAB, 
> still no issue.. The one old MSLAB will get really closed only when the long 
> pending scan ends.
> 
> Second abt disagreement of merge
> Ya. I feel that there should NOT be an in memory merge op at all.  We 
> need to have the compaction op any way.  Only in case of compaction op mode, 
> the on disk flush will flush only tail of the pipeline.  In other case (def 
> case) there will be in memory flushes (flatten op) and we keep adding 
> segments to pipeline and here when a flush to disk req comes, we will flush 
> all segments not just tail of 

Review Request 52986: Updated endpoint-help in preparation of 1.1.0.

[Till Toenshoff]

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

Review request for mesos and Alexander Rukletsov.


Repository: mesos


Description
---

see summary.


Diffs
-

  docs/endpoints/files/browse.json.md 55ee41b1d61f46a11931a9e9d9b1e4f65f0d80ce 
  docs/endpoints/files/browse.md de8632e95373e2719bc9a035a82ed51f1c9b0437 
  docs/endpoints/files/debug.json.md a4dc0de5f343a799bdba983b4e75ee106b0bf64b 
  docs/endpoints/files/debug.md 9361ffecef4a1cab1138981c5ce302561505cfda 
  docs/endpoints/files/download.json.md 
88dd72cbd97845fe8e9a3bdd374502b91d7712bd 
  docs/endpoints/files/download.md 3a3876010e0f5c55826b20c5176b6f82f3249deb 
  docs/endpoints/files/read.json.md 6697ab1e251c22db8a7b79926d737fad9514249d 
  docs/endpoints/files/read.md 21ea6d8a3b9a2bd7cfc214653048a9f74bbac91c 
  docs/endpoints/master/api/v1.md 19c6142844ee9167f532496873524508d335a59d 
  docs/endpoints/master/api/v1/scheduler.md 
39cd457efd144ce316e8c13c916000cac8e51ad7 
  docs/endpoints/master/create-volumes.md 
f6ec9384f7f2bfb8056e54532ac3417a1f15b1d1 
  docs/endpoints/master/destroy-volumes.md 
054e816371f26c91ccc0d9687f377e80641e42a8 
  docs/endpoints/master/flags.md 850b6f86ddd92af512932aec5af9bdc600cf92d8 
  docs/endpoints/master/frameworks.md 1aed723152528be0e19d79520daac5f470cb49da 
  docs/endpoints/master/reserve.md 9bb04ed0de5b9304301fffd51b39ec07102825cd 
  docs/endpoints/master/state-summary.md 
4eb517ea0f6b5dbc2e7d7c406d54e741cf50e0e5 
  docs/endpoints/master/state.json.md c5852bd01dd5f72a9a11991a033d8c97b111ad21 
  docs/endpoints/master/state.md 8e0650f84ae13ef7952971ffb9732ec379cffd40 
  docs/endpoints/master/tasks.json.md 5d2c0e6eb53fa5e04a973eb786cd16bfad38736f 
  docs/endpoints/master/tasks.md c7df686443e20d255b03fc31a999b9b24b5f3a0a 
  docs/endpoints/master/teardown.md 4c14f594c3713304d5adf59e56e9e8af2b05ebee 
  docs/endpoints/master/unreserve.md 5cce4288231a64d7ee7713f2b44487618e7cc0a7 
  docs/endpoints/slave/api/v1.md c867d795104e053e5aae819df3deea68269228bf 
  docs/endpoints/slave/flags.md 3ea38c160670fc302a42ae98d7ebe0679b5bdc92 
  docs/endpoints/slave/state.json.md ac8536986ceb6532f71b4fe1c62b1b83390b03e7 
  docs/endpoints/slave/state.md c7b61d736582f50b850fed252e558bbae6dad801 

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


Testing
---


Thanks,

Till Toenshoff

Review Request 52985: Updated CHANGELOG in preparation for RC1 of 1.1.0.

[Till Toenshoff]

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

Review request for mesos and Alexander Rukletsov.


Repository: mesos


Description
---

Adds the JIRA release-notes to the CHANGELOG. Fixed typos and spelling mistakes,
adds periods where missing.


Diffs
-

  CHANGELOG b40f9c8106ac726cdb01bfe079a40e77ee7c8a2e 

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


Testing
---


Thanks,

Till Toenshoff

Re: Review Request 46230: Updated docs to reflect user in persistent volumes.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [46227, 52980, 46228, 46229, 46230]

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

- Mesos ReviewBot


On Oct. 18, 2016, 7:46 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46230/
> ---
> 
> (Updated Oct. 18, 2016, 7:46 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-4893
> https://issues.apache.org/jira/browse/MESOS-4893
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated docs to reflect user in persistent volumes.
> 
> 
> Diffs
> -
> 
>   docs/persistent-volume.md 5dfbbf1fb08cdf47f97ea6fb286e21ab26235d62 
> 
> Diff: https://reviews.apache.org/r/46230/diff/
> 
> 
> Testing
> ---
> 
> Documentation change only. All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 51785: [HBASE-16608] Merge for the segments in the compaction pipeline and simplifying the user interface for in-memory flush

[Anoop Sam John]

> On Oct. 16, 2016, 10:43 p.m., Anoop Sam John wrote:
> > hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HeapMemStoreLAB.java,
> >  line 179
> > 
> >
> > Able to get what u r trying to do here.  Seems very complex now.
> > So can we take a diff path?
> > We have to merge N segments into one here means we have to merge N 
> > MSLABs referred by these segments.  What we do as per this patch is make a 
> > new HeapMSLAB object and add all pooled chunk from other MSLABs int the Q 
> > in new one.  And other stuff of refer next etc.
> > 
> > Can we make a new MSLAB impl? This will act as a collection of N 
> > HeapMSLABs. Now we have to impl 4 methods
> > incScannerCount and decScannerCount based on an openScannerCount in the 
> > new class.
> > close() method like in HeapMSLAB and has to call close() on each of the 
> > N MSLABs it wrap
> > This is an ImmutableSegment so no one is going to call copyCellInto(). 
> > Make this impl in this new class throw IllegalStateException or so.  Will 
> > that look good?
> 
> Michael Stack wrote:
> No copying? That sounds good.
> 
> Anoop Sam John wrote:
> So this fix and so this comment is applicable to merge op alone..  There 
> is still an open point which Me/Ram are not agreeing around this.. That is 
> whether to do this in mem merge.  This is required to limit the #segments it 
> pipeline and to get bigger sized segment at tail of the pipeline. As we do 
> flush of the tail of the segment.  But our point is let us flush all segments 
> (I mean flush to disk)..  In case of in mem compaction (This action is by def 
> OFF and as per data usage one can enable) it is worth to flush only tail.  In 
> this normal use case let us flush all segments and so there is only flatten 
> of segments comes in and so no bug as this. There is absolutely no gain we 
> get because of delaying of flush of the other segments in the pipeline ( I 
> mean in merge kind of usages)..   So I suggest let us discuss on that before 
> going to work on this. Else it wl b waste of time.
> 
> Anastasia Braginsky wrote:
> I am referencing in this answer the Anoop's suggestion to create a new 
> MSLAB implementation that act as a collection of N old HeapMSLABs. Please pay 
> attention that old scans (opened on N old HeapMSLABs) are not aware of 
> existing of the new MSLAB. In addition the close() of the new MSLAB doesn't 
> automatically means old MSLABs need to be closed, as there might be still 
> ongoing scans. Thus I do not think this idea works (at least not as it is 
> presented now).
> 
> Anastasia Braginsky wrote:
> Anoop, I do not understand about what exactly you disagree. Are you 
> saying that merging of the segments is waste of time? Do you prefer to do a 
> merge upon the flush to disk? Aa we have seen on stress benchmarks this 
> behavior delays flushes and blocks writes which is undesirable. Please 
> explain yourself clearer or we can talk about if we have a meeting this 
> Wednesday.
> 
> Anoop Sam John wrote:
> Reply
> First abt the COllection of N HeapMSLAB wont work.
> You say problem with old scans which were working on old HeapMSLABs and 
> they dont know abt the new MSLAB.  There wont be an issue.  We will really 
> close an MSLAB (I mean return chunks to pool) when there is a close call 
> happened on it AND the scanner count is zero.  So in the case u mentioned, as 
> there are old scan, the scanner count on these MSLABs are >0. Remember we are 
> not going to call close() on these.  We just have a new Collection based 
> MSLAB impl which point to N old MSLABs.  So the old scanners still use the 
> old MSLABs and once done they decr the counter.
> Now on the new one there might be new scan reqs.  So we have scanner 
> count in new COllection based MSLAB impl.  When we are at a point to close 
> this new MSLAB, we will call close on it. It will call close() on all old 
> MSLABs. (If its scanner count is zero or else the close on old MSLABs will be 
> delayed till a decr op makes the count to 0)  Now again within the old MSLAB 
> there will be normal close flow.. Iff its scanner count is 0, it will really 
> get closed.  So in a rare case where the new MSLAB is made and it is at a 
> point of getting closed and still a very old scanner still uses old MSLAB, 
> still no issue.. The one old MSLAB will get really closed only when the long 
> pending scan ends.
> 
> Second abt disagreement of merge
> Ya. I feel that there should NOT be an in memory merge op at all.  We 
> need to have the compaction op any way.  Only in case of compaction op mode, 
> the on disk flush will flush only tail of the pipeline.  In other case (def 
> case) there will be in memory flushes (flatten op) and we keep adding 
> segments to pipeline and here when a flush to disk req comes, we will flush 
> all segments not just tail of 

Re: Review Request 51785: [HBASE-16608] Merge for the segments in the compaction pipeline and simplifying the user interface for in-memory flush

[Edward Bortnikov]

> On Oct. 16, 2016, 5:13 p.m., Anoop Sam John wrote:
> > hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HeapMemStoreLAB.java,
> >  line 179
> > 
> >
> > Able to get what u r trying to do here.  Seems very complex now.
> > So can we take a diff path?
> > We have to merge N segments into one here means we have to merge N 
> > MSLABs referred by these segments.  What we do as per this patch is make a 
> > new HeapMSLAB object and add all pooled chunk from other MSLABs int the Q 
> > in new one.  And other stuff of refer next etc.
> > 
> > Can we make a new MSLAB impl? This will act as a collection of N 
> > HeapMSLABs. Now we have to impl 4 methods
> > incScannerCount and decScannerCount based on an openScannerCount in the 
> > new class.
> > close() method like in HeapMSLAB and has to call close() on each of the 
> > N MSLABs it wrap
> > This is an ImmutableSegment so no one is going to call copyCellInto(). 
> > Make this impl in this new class throw IllegalStateException or so.  Will 
> > that look good?
> 
> Michael Stack wrote:
> No copying? That sounds good.
> 
> Anoop Sam John wrote:
> So this fix and so this comment is applicable to merge op alone..  There 
> is still an open point which Me/Ram are not agreeing around this.. That is 
> whether to do this in mem merge.  This is required to limit the #segments it 
> pipeline and to get bigger sized segment at tail of the pipeline. As we do 
> flush of the tail of the segment.  But our point is let us flush all segments 
> (I mean flush to disk)..  In case of in mem compaction (This action is by def 
> OFF and as per data usage one can enable) it is worth to flush only tail.  In 
> this normal use case let us flush all segments and so there is only flatten 
> of segments comes in and so no bug as this. There is absolutely no gain we 
> get because of delaying of flush of the other segments in the pipeline ( I 
> mean in merge kind of usages)..   So I suggest let us discuss on that before 
> going to work on this. Else it wl b waste of time.
> 
> Anastasia Braginsky wrote:
> I am referencing in this answer the Anoop's suggestion to create a new 
> MSLAB implementation that act as a collection of N old HeapMSLABs. Please pay 
> attention that old scans (opened on N old HeapMSLABs) are not aware of 
> existing of the new MSLAB. In addition the close() of the new MSLAB doesn't 
> automatically means old MSLABs need to be closed, as there might be still 
> ongoing scans. Thus I do not think this idea works (at least not as it is 
> presented now).
> 
> Anastasia Braginsky wrote:
> Anoop, I do not understand about what exactly you disagree. Are you 
> saying that merging of the segments is waste of time? Do you prefer to do a 
> merge upon the flush to disk? Aa we have seen on stress benchmarks this 
> behavior delays flushes and blocks writes which is undesirable. Please 
> explain yourself clearer or we can talk about if we have a meeting this 
> Wednesday.
> 
> Anoop Sam John wrote:
> Reply
> First abt the COllection of N HeapMSLAB wont work.
> You say problem with old scans which were working on old HeapMSLABs and 
> they dont know abt the new MSLAB.  There wont be an issue.  We will really 
> close an MSLAB (I mean return chunks to pool) when there is a close call 
> happened on it AND the scanner count is zero.  So in the case u mentioned, as 
> there are old scan, the scanner count on these MSLABs are >0. Remember we are 
> not going to call close() on these.  We just have a new Collection based 
> MSLAB impl which point to N old MSLABs.  So the old scanners still use the 
> old MSLABs and once done they decr the counter.
> Now on the new one there might be new scan reqs.  So we have scanner 
> count in new COllection based MSLAB impl.  When we are at a point to close 
> this new MSLAB, we will call close on it. It will call close() on all old 
> MSLABs. (If its scanner count is zero or else the close on old MSLABs will be 
> delayed till a decr op makes the count to 0)  Now again within the old MSLAB 
> there will be normal close flow.. Iff its scanner count is 0, it will really 
> get closed.  So in a rare case where the new MSLAB is made and it is at a 
> point of getting closed and still a very old scanner still uses old MSLAB, 
> still no issue.. The one old MSLAB will get really closed only when the long 
> pending scan ends.
> 
> Second abt disagreement of merge
> Ya. I feel that there should NOT be an in memory merge op at all.  We 
> need to have the compaction op any way.  Only in case of compaction op mode, 
> the on disk flush will flush only tail of the pipeline.  In other case (def 
> case) there will be in memory flushes (flatten op) and we keep adding 
> segments to pipeline and here when a flush to disk req comes, we will flush 
> all segments not just tail of 

Re: Review Request 52741: Added capabilities support to mesos-execute.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52780, 52741]

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

- Mesos ReviewBot


On Oct. 18, 2016, 7:07 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52741/
> ---
> 
> (Updated Oct. 18, 2016, 7:07 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-5303
> https://issues.apache.org/jira/browse/MESOS-5303
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces a flags `capabilities` to mesos-execute which
> can be used to specify the capabilities a task should be able to
> aquire.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp 94ac463110d7b83ee633fe32a6b66b0a8e181ba2 
> 
> Diff: https://reviews.apache.org/r/52741/diff/
> 
> 
> Testing
> ---
> 
> * `make check`, including `ROOT` tests,
> * tested as root with a `ping` task with an agent where both the task and the 
> agent have different or no capabilities configured.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 46824: Fully qualified addresses of Flag members in add calls.

[Benjamin Bannier]

> On Oct. 18, 2016, 11:22 a.m., Michael Park wrote:
> > I'll just make the adjustments below when I commit it. Please let me know 
> > if you're against any of them.

Thanks for the careful review. Your suggested adjustments are needed, +1.


- Benjamin


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


On Oct. 18, 2016, 10:39 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46824/
> ---
> 
> (Updated Oct. 18, 2016, 10:39 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Michael Park.
> 
> 
> Bugs: MESOS-3335
> https://issues.apache.org/jira/browse/MESOS-3335
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While right now we can technically `add` variables to `Flags` classes
> which are not members, the in order to have correct copy semantics for
> `Flags` only member variables should be used.
> 
> Here we changed all instances to a full pointer-to-member syntax in
> the current code.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp 94ac463110d7b83ee633fe32a6b66b0a8e181ba2 
>   src/cli/resolve.cpp 3a12f123e0969382a79d045c15f372e2f5eea02e 
>   src/docker/executor.hpp 495fad5b1afbea829e7b87103d68dfa672a458cf 
>   src/examples/balloon_framework.cpp 50add6a16297383beefd312de212381b8f14fa8a 
>   src/examples/disk_full_framework.cpp 
> 78f13d34f183628372f925f450f76f250a5d81d1 
>   src/examples/dynamic_reservation_framework.cpp 
> c9a68637ea2dfbb0a9367f14358b5e5de46f3331 
>   src/examples/load_generator_framework.cpp 
> 81c8c237dedd27615fe34ad629978a7359ee1efd 
>   src/examples/long_lived_framework.cpp 
> bfb253efbe80f52147123abdd804877c175e6f5e 
>   src/examples/no_executor_framework.cpp 
> 466854263cab96e764a3aedca5e59e54bdc6f500 
>   src/examples/persistent_volume_framework.cpp 
> a2a6944a8eed4fd72cc22e0628e38ac7f1a6260b 
>   src/examples/test_framework.cpp a9106404bd5f63ac7b3f76c7368dcf5b02128a9d 
>   src/examples/test_http_framework.cpp 
> d6e248cd1e7776b5e63764f1d1740d3e29dcfaa4 
>   src/launcher/executor.cpp dec1e07f8f55019b74bbc911a588083160202989 
>   src/local/main.cpp 578b65efac1dd8ec201bfcc85de353ca6b867148 
>   src/master/main.cpp 9d2fd92dbc2fc5af166edec907adbdb541eb0dd3 
>   src/slave/container_loggers/lib_logrotate.hpp 
> f91db3edc3d0cc93fedda906d175cbcca0c00c12 
>   src/slave/container_loggers/logrotate.hpp 
> f906a167f8897385af5f54e1e77cb790121a 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> c87e6715a3f00a6ed2f6da03bd7492f9e77ffc21 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 20fb6ab35ffa85917c6705d79f22f0d54a416353 
>   src/slave/containerizer/mesos/launch.cpp 
> 8a30ff8bd6f9263d68a4344b79f2374a2ae53c04 
>   src/slave/containerizer/mesos/mount.cpp 
> 4b90666d9c75dadd70e6b690ca17fc5699825442 
>   src/slave/main.cpp 219914d594ace009201d9caf8209ca3dd4eaa1a5 
>   src/tests/active_user_test_helper.cpp 
> 80bd0acff47689be95a42bad3cb867e0faa68554 
>   src/tests/containerizer/capabilities_test_helper.cpp 
> 2d7be6ddaf4a5e687e928e4651df68ea9b234202 
> 
> Diff: https://reviews.apache.org/r/46824/diff/
> 
> 
> Testing
> ---
> 
> Tested as part of the review chain ending in 
> https://reviews.apache.org/r/52388/ on various Linux configurations in 
> internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 46824: Fully qualified addresses of Flag members in add calls.

[Michael Park]

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


Fix it, then Ship it!




I'll just make the adjustments below when I commit it. Please let me know if 
you're against any of them.


src/examples/test_http_framework.cpp (lines 385 - 386)


`&::Flags::role` seems unnecessary. Let's just do `::role`



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (lines 1583 - 1586)


Seems like you missed this one.
I'll commit this as:
```
add(::hostname, "hostname", "Hostname of the container");
```



src/slave/main.cpp (lines 102 - 135)


Similar to above. `&::Flags::ip` seems unnecessary. Let's just do 
`::ip`


- Michael Park


On Oct. 18, 2016, 8:39 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46824/
> ---
> 
> (Updated Oct. 18, 2016, 8:39 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Michael Park.
> 
> 
> Bugs: MESOS-3335
> https://issues.apache.org/jira/browse/MESOS-3335
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While right now we can technically `add` variables to `Flags` classes
> which are not members, the in order to have correct copy semantics for
> `Flags` only member variables should be used.
> 
> Here we changed all instances to a full pointer-to-member syntax in
> the current code.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp 94ac463110d7b83ee633fe32a6b66b0a8e181ba2 
>   src/cli/resolve.cpp 3a12f123e0969382a79d045c15f372e2f5eea02e 
>   src/docker/executor.hpp 495fad5b1afbea829e7b87103d68dfa672a458cf 
>   src/examples/balloon_framework.cpp 50add6a16297383beefd312de212381b8f14fa8a 
>   src/examples/disk_full_framework.cpp 
> 78f13d34f183628372f925f450f76f250a5d81d1 
>   src/examples/dynamic_reservation_framework.cpp 
> c9a68637ea2dfbb0a9367f14358b5e5de46f3331 
>   src/examples/load_generator_framework.cpp 
> 81c8c237dedd27615fe34ad629978a7359ee1efd 
>   src/examples/long_lived_framework.cpp 
> bfb253efbe80f52147123abdd804877c175e6f5e 
>   src/examples/no_executor_framework.cpp 
> 466854263cab96e764a3aedca5e59e54bdc6f500 
>   src/examples/persistent_volume_framework.cpp 
> a2a6944a8eed4fd72cc22e0628e38ac7f1a6260b 
>   src/examples/test_framework.cpp a9106404bd5f63ac7b3f76c7368dcf5b02128a9d 
>   src/examples/test_http_framework.cpp 
> d6e248cd1e7776b5e63764f1d1740d3e29dcfaa4 
>   src/launcher/executor.cpp dec1e07f8f55019b74bbc911a588083160202989 
>   src/local/main.cpp 578b65efac1dd8ec201bfcc85de353ca6b867148 
>   src/master/main.cpp 9d2fd92dbc2fc5af166edec907adbdb541eb0dd3 
>   src/slave/container_loggers/lib_logrotate.hpp 
> f91db3edc3d0cc93fedda906d175cbcca0c00c12 
>   src/slave/container_loggers/logrotate.hpp 
> f906a167f8897385af5f54e1e77cb790121a 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> c87e6715a3f00a6ed2f6da03bd7492f9e77ffc21 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 20fb6ab35ffa85917c6705d79f22f0d54a416353 
>   src/slave/containerizer/mesos/launch.cpp 
> 8a30ff8bd6f9263d68a4344b79f2374a2ae53c04 
>   src/slave/containerizer/mesos/mount.cpp 
> 4b90666d9c75dadd70e6b690ca17fc5699825442 
>   src/slave/main.cpp 219914d594ace009201d9caf8209ca3dd4eaa1a5 
>   src/tests/active_user_test_helper.cpp 
> 80bd0acff47689be95a42bad3cb867e0faa68554 
>   src/tests/containerizer/capabilities_test_helper.cpp 
> 2d7be6ddaf4a5e687e928e4651df68ea9b234202 
> 
> Diff: https://reviews.apache.org/r/46824/diff/
> 
> 
> Testing
> ---
> 
> Tested as part of the review chain ending in 
> https://reviews.apache.org/r/52388/ on various Linux configurations in 
> internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 52682: Cleaned up a few style issues in the capabilities isolator tests.

[Benjamin Bannier]

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

(Updated Oct. 18, 2016, 10:48 a.m.)


Review request for mesos and Jie Yu.


Changes
---

Renamed some test parameters.


Repository: mesos


Description
---

We remove an unused using declaration, and replace several bool
parameters with more readable enum values.


Diffs (updated)
-

  src/tests/containerizer/linux_capabilities_isolator_tests.cpp 
edb46659324c4c6345606cfa4c19f4fce05c59fe 

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


Testing
---

make check


Thanks,

Benjamin Bannier

Re: Review Request 46824: Fully qualified addresses of Flag members in add calls.

[Benjamin Bannier]

> On Oct. 18, 2016, 9:54 a.m., Michael Park wrote:
> > src/local/main.cpp, lines 48-63
> > 
> >
> > Is there a reason why we don't just add this directly to `local::Flags`?

`local::Flags` is used in some other places as well. Adding `port` or `ip` 
flags to it would surface these flags for all users, and I believe this is not 
what we want. Dropping this issue.


> On Oct. 18, 2016, 9:54 a.m., Michael Park wrote:
> > src/master/main.cpp, lines 123-208
> > 
> >
> > Similar to above, why don't we just add these to `master::Flags`?

`master::Flags` is also reused in many places. Dropping this issue.


> On Oct. 18, 2016, 9:54 a.m., Michael Park wrote:
> > src/slave/main.cpp, lines 97-151
> > 
> >
> > One more. Can we just add these to `slave::Flags`?

`slave::Flags` is also reused in many places. Dropping this issue.


- Benjamin


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


On Oct. 18, 2016, 10:39 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46824/
> ---
> 
> (Updated Oct. 18, 2016, 10:39 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Michael Park.
> 
> 
> Bugs: MESOS-3335
> https://issues.apache.org/jira/browse/MESOS-3335
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While right now we can technically `add` variables to `Flags` classes
> which are not members, the in order to have correct copy semantics for
> `Flags` only member variables should be used.
> 
> Here we changed all instances to a full pointer-to-member syntax in
> the current code.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp 94ac463110d7b83ee633fe32a6b66b0a8e181ba2 
>   src/cli/resolve.cpp 3a12f123e0969382a79d045c15f372e2f5eea02e 
>   src/docker/executor.hpp 495fad5b1afbea829e7b87103d68dfa672a458cf 
>   src/examples/balloon_framework.cpp 50add6a16297383beefd312de212381b8f14fa8a 
>   src/examples/disk_full_framework.cpp 
> 78f13d34f183628372f925f450f76f250a5d81d1 
>   src/examples/dynamic_reservation_framework.cpp 
> c9a68637ea2dfbb0a9367f14358b5e5de46f3331 
>   src/examples/load_generator_framework.cpp 
> 81c8c237dedd27615fe34ad629978a7359ee1efd 
>   src/examples/long_lived_framework.cpp 
> bfb253efbe80f52147123abdd804877c175e6f5e 
>   src/examples/no_executor_framework.cpp 
> 466854263cab96e764a3aedca5e59e54bdc6f500 
>   src/examples/persistent_volume_framework.cpp 
> a2a6944a8eed4fd72cc22e0628e38ac7f1a6260b 
>   src/examples/test_framework.cpp a9106404bd5f63ac7b3f76c7368dcf5b02128a9d 
>   src/examples/test_http_framework.cpp 
> d6e248cd1e7776b5e63764f1d1740d3e29dcfaa4 
>   src/launcher/executor.cpp dec1e07f8f55019b74bbc911a588083160202989 
>   src/local/main.cpp 578b65efac1dd8ec201bfcc85de353ca6b867148 
>   src/master/main.cpp 9d2fd92dbc2fc5af166edec907adbdb541eb0dd3 
>   src/slave/container_loggers/lib_logrotate.hpp 
> f91db3edc3d0cc93fedda906d175cbcca0c00c12 
>   src/slave/container_loggers/logrotate.hpp 
> f906a167f8897385af5f54e1e77cb790121a 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> c87e6715a3f00a6ed2f6da03bd7492f9e77ffc21 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 20fb6ab35ffa85917c6705d79f22f0d54a416353 
>   src/slave/containerizer/mesos/launch.cpp 
> 8a30ff8bd6f9263d68a4344b79f2374a2ae53c04 
>   src/slave/containerizer/mesos/mount.cpp 
> 4b90666d9c75dadd70e6b690ca17fc5699825442 
>   src/slave/main.cpp 219914d594ace009201d9caf8209ca3dd4eaa1a5 
>   src/tests/active_user_test_helper.cpp 
> 80bd0acff47689be95a42bad3cb867e0faa68554 
>   src/tests/containerizer/capabilities_test_helper.cpp 
> 2d7be6ddaf4a5e687e928e4651df68ea9b234202 
> 
> Diff: https://reviews.apache.org/r/46824/diff/
> 
> 
> Testing
> ---
> 
> Tested as part of the review chain ending in 
> https://reviews.apache.org/r/52388/ on various Linux configurations in 
> internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 46824: Fully qualified addresses of Flag members in add calls.

[Benjamin Bannier]

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

(Updated Oct. 18, 2016, 10:39 a.m.)


Review request for mesos, Alexander Rukletsov and Michael Park.


Changes
---

Addressed mpark's comments.


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


Repository: mesos


Description
---

While right now we can technically `add` variables to `Flags` classes
which are not members, the in order to have correct copy semantics for
`Flags` only member variables should be used.

Here we changed all instances to a full pointer-to-member syntax in
the current code.


Diffs (updated)
-

  src/cli/execute.cpp 94ac463110d7b83ee633fe32a6b66b0a8e181ba2 
  src/cli/resolve.cpp 3a12f123e0969382a79d045c15f372e2f5eea02e 
  src/docker/executor.hpp 495fad5b1afbea829e7b87103d68dfa672a458cf 
  src/examples/balloon_framework.cpp 50add6a16297383beefd312de212381b8f14fa8a 
  src/examples/disk_full_framework.cpp 78f13d34f183628372f925f450f76f250a5d81d1 
  src/examples/dynamic_reservation_framework.cpp 
c9a68637ea2dfbb0a9367f14358b5e5de46f3331 
  src/examples/load_generator_framework.cpp 
81c8c237dedd27615fe34ad629978a7359ee1efd 
  src/examples/long_lived_framework.cpp 
bfb253efbe80f52147123abdd804877c175e6f5e 
  src/examples/no_executor_framework.cpp 
466854263cab96e764a3aedca5e59e54bdc6f500 
  src/examples/persistent_volume_framework.cpp 
a2a6944a8eed4fd72cc22e0628e38ac7f1a6260b 
  src/examples/test_framework.cpp a9106404bd5f63ac7b3f76c7368dcf5b02128a9d 
  src/examples/test_http_framework.cpp d6e248cd1e7776b5e63764f1d1740d3e29dcfaa4 
  src/launcher/executor.cpp dec1e07f8f55019b74bbc911a588083160202989 
  src/local/main.cpp 578b65efac1dd8ec201bfcc85de353ca6b867148 
  src/master/main.cpp 9d2fd92dbc2fc5af166edec907adbdb541eb0dd3 
  src/slave/container_loggers/lib_logrotate.hpp 
f91db3edc3d0cc93fedda906d175cbcca0c00c12 
  src/slave/container_loggers/logrotate.hpp 
f906a167f8897385af5f54e1e77cb790121a 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
c87e6715a3f00a6ed2f6da03bd7492f9e77ffc21 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
20fb6ab35ffa85917c6705d79f22f0d54a416353 
  src/slave/containerizer/mesos/launch.cpp 
8a30ff8bd6f9263d68a4344b79f2374a2ae53c04 
  src/slave/containerizer/mesos/mount.cpp 
4b90666d9c75dadd70e6b690ca17fc5699825442 
  src/slave/main.cpp 219914d594ace009201d9caf8209ca3dd4eaa1a5 
  src/tests/active_user_test_helper.cpp 
80bd0acff47689be95a42bad3cb867e0faa68554 
  src/tests/containerizer/capabilities_test_helper.cpp 
2d7be6ddaf4a5e687e928e4651df68ea9b234202 

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


Testing
---

Tested as part of the review chain ending in 
https://reviews.apache.org/r/52388/ on various Linux configurations in internal 
CI.


Thanks,

Benjamin Bannier

Re: Review Request 52390: Fully qualified addresses of Flag members in add calls in stout.

[Michael Park]

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


Fix it, then Ship it!





3rdparty/stout/tests/flags_tests.cpp (line 724)


`{` on the newline. I'll just fix this before I commit. Just leaving an 
note.


- Michael Park


On Oct. 17, 2016, 4:13 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52390/
> ---
> 
> (Updated Oct. 17, 2016, 4:13 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-3335
> https://issues.apache.org/jira/browse/MESOS-3335
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While right now we can technically `add` variables to `Flags` classes
> which are not members, the in order to have correct copy semantics for
> `Flags` only member variables should be used.
> 
> Here we changed all instances to a full pointer-to-member syntax in
> the current code.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/README.md 693e3a3e76300f7fa6c6f6518497edf44cc9f6a2 
>   3rdparty/stout/include/stout/flags.hpp 
> b23b6f68b7da93f1c4fa4d4bddc350a615716041 
>   3rdparty/stout/include/stout/flags/flags.hpp 
> e00aedfe1a1fbb5b8ad4786c6a3ca6db5d5faff0 
>   3rdparty/stout/tests/flags_tests.cpp 
> b2de02dbd920601367f5e42d8a39a96a26551425 
>   3rdparty/stout/tests/subcommand_tests.cpp 
> 9cdb1adc0047f2c7baf1089ddc5afa2052b26f57 
> 
> Diff: https://reviews.apache.org/r/52390/diff/
> 
> 
> Testing
> ---
> 
> Tested as part of the review chain ending in 
> https://reviews.apache.org/r/52388/ on various Linux configurations in 
> internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 52624: Replaced POSIX `int` with `FileDesc` abstraction in `src` folder.

[Mesos ReviewBot]

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



Bad patch!

Reviews applied: [52624, 52625, 52972, 52544, 52364, 52210, 52192]

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

Error:
2016-10-18 08:08:06 URL:https://reviews.apache.org/r/52624/diff/raw/ 
[6801/6801] -> "52624.patch" [1]
error: patch failed: cmake/CompilationConfigure.cmake:97
error: cmake/CompilationConfigure.cmake: patch does not apply

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

- Mesos ReviewBot


On Oct. 18, 2016, 2:46 a.m., Daniel Pravat wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52624/
> ---
> 
> (Updated Oct. 18, 2016, 2:46 a.m.)
> 
> 
> Review request for mesos, Alex Naparu, Alex Clemmer, Joseph Wu, and Michael 
> Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> On POSIX this should have no effect.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 11a8507eb773391073a7b945e2aac503262f86b7 
>   src/files/files.cpp 9c634ac928887b3f1a111f67ebb3fc5229c6fa16 
>   src/health-check/health_checker.cpp 
> 96ae1a733ff3d211b84d0893b4603873af1c89f0 
>   src/slave/containerizer/fetcher.cpp 
> 11104d66e6dd05d8eb1d37a2e3250aca19278110 
>   src/slave/containerizer/mesos/containerizer.cpp 
> eac70d955e08142a2d054039d610a3d516b1b57e 
>   src/slave/containerizer/mesos/launch.hpp 
> f8bac0650965a49562b9910bf6140ded8dbb69ac 
>   src/slave/containerizer/mesos/launch.cpp 
> 8a30ff8bd6f9263d68a4344b79f2374a2ae53c04 
>   src/slave/state.cpp a94bb8d7029295abef70d6595ebc732ac1ab87a8 
>   src/slave/status_update_manager.cpp 
> 056a684b52756d5c6309e7e2167a1532c4e60957 
> 
> Diff: https://reviews.apache.org/r/52624/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Daniel Pravat
> 
>

Re: Review Request 46824: Fully qualified addresses of Flag members in add calls.

[Michael Park]

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




src/local/main.cpp (lines 48 - 63)


Is there a reason why we don't just add this directly to `local::Flags`?



src/master/main.cpp (lines 123 - 168)


Similar to above, why don't we just add these to `master::Flags`?



src/master/main.cpp (lines 179 - 182)


I'm not sure what this is?



src/slave/containerizer/mesos/isolators/network/cni/cni.cpp (lines 1563 - 1590)


In most other places we seem to have opted to not specify the first part. 
e.g.,
```
ActiveUserTestHelper::Flags::Flags()
{
  add(::user,
  "user",
  "The expected user name.");
}
```



src/slave/main.cpp (lines 97 - 151)


One more. Can we just add these to `slave::Flags`?


- Michael Park


On Oct. 17, 2016, 3:24 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46824/
> ---
> 
> (Updated Oct. 17, 2016, 3:24 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Michael Park.
> 
> 
> Bugs: MESOS-3335
> https://issues.apache.org/jira/browse/MESOS-3335
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While right now we can technically `add` variables to `Flags` classes
> which are not members, the in order to have correct copy semantics for
> `Flags` only member variables should be used.
> 
> Here we changed all instances to a full pointer-to-member syntax in
> the current code.
> 
> 
> Diffs
> -
> 
>   src/cli/execute.cpp 94ac463110d7b83ee633fe32a6b66b0a8e181ba2 
>   src/cli/resolve.cpp 3a12f123e0969382a79d045c15f372e2f5eea02e 
>   src/docker/executor.hpp 495fad5b1afbea829e7b87103d68dfa672a458cf 
>   src/examples/balloon_framework.cpp 50add6a16297383beefd312de212381b8f14fa8a 
>   src/examples/disk_full_framework.cpp 
> 78f13d34f183628372f925f450f76f250a5d81d1 
>   src/examples/dynamic_reservation_framework.cpp 
> c9a68637ea2dfbb0a9367f14358b5e5de46f3331 
>   src/examples/load_generator_framework.cpp 
> 81c8c237dedd27615fe34ad629978a7359ee1efd 
>   src/examples/long_lived_framework.cpp 
> bfb253efbe80f52147123abdd804877c175e6f5e 
>   src/examples/no_executor_framework.cpp 
> 466854263cab96e764a3aedca5e59e54bdc6f500 
>   src/examples/persistent_volume_framework.cpp 
> a2a6944a8eed4fd72cc22e0628e38ac7f1a6260b 
>   src/examples/test_framework.cpp a9106404bd5f63ac7b3f76c7368dcf5b02128a9d 
>   src/examples/test_http_framework.cpp 
> d6e248cd1e7776b5e63764f1d1740d3e29dcfaa4 
>   src/launcher/executor.cpp dec1e07f8f55019b74bbc911a588083160202989 
>   src/local/main.cpp 578b65efac1dd8ec201bfcc85de353ca6b867148 
>   src/master/main.cpp 9d2fd92dbc2fc5af166edec907adbdb541eb0dd3 
>   src/slave/container_loggers/lib_logrotate.hpp 
> f91db3edc3d0cc93fedda906d175cbcca0c00c12 
>   src/slave/container_loggers/logrotate.hpp 
> f906a167f8897385af5f54e1e77cb790121a 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 7c35c3056326a8a135e4ad944ac741cda96cab99 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 20fb6ab35ffa85917c6705d79f22f0d54a416353 
>   src/slave/containerizer/mesos/launch.cpp 
> 8a30ff8bd6f9263d68a4344b79f2374a2ae53c04 
>   src/slave/containerizer/mesos/mount.cpp 
> 4b90666d9c75dadd70e6b690ca17fc5699825442 
>   src/slave/main.cpp 219914d594ace009201d9caf8209ca3dd4eaa1a5 
>   src/tests/active_user_test_helper.cpp 
> 80bd0acff47689be95a42bad3cb867e0faa68554 
>   src/tests/containerizer/capabilities_test_helper.cpp 
> 2d7be6ddaf4a5e687e928e4651df68ea9b234202 
> 
> Diff: https://reviews.apache.org/r/46824/diff/
> 
> 
> Testing
> ---
> 
> Tested as part of the review chain ending in 
> https://reviews.apache.org/r/52388/ on various Linux configurations in 
> internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 52783: Added documentation for mesos-containerizer Linux capabilities support.

[Benjamin Bannier]

> On Oct. 18, 2016, 6:55 a.m., Avinash sridharan wrote:
> > docs/linux_capabilities.md, line 3
> > 
> >
> > s/described/describes
> > 
> > to add support for `Linux Capabilities`

I do not understand your second request. How should the sentence read after 
applying you suggestion?


> On Oct. 18, 2016, 6:55 a.m., Avinash sridharan wrote:
> > docs/configuration.md, line 1003
> > 
> >
> > s/the agent/that the agent/

Not a native speaker but to me this seems to just make the sentence more wordy 
(and one should probably use `which` instead of `that`). Maybe a native speaker 
could chime in how this should be phrased.


- Benjamin


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


On Oct. 18, 2016, 9:51 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52783/
> ---
> 
> (Updated Oct. 18, 2016, 9:51 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-6376
> https://issues.apache.org/jira/browse/MESOS-6376
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for mesos-containerizer Linux capabilities support.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md c83a58eb6884c8d8c37880a745e04cf0b789ebdc 
>   docs/linux_capabilities.md PRE-CREATION 
>   docs/mesos-containerizer.md 76544625838845fe2817347fea483a60ab250f99 
> 
> Diff: https://reviews.apache.org/r/52783/diff/
> 
> 
> Testing
> ---
> 
> Checked with local renderer.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 52783: Added documentation for mesos-containerizer Linux capabilities support.

[Benjamin Bannier]

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

(Updated Oct. 18, 2016, 9:51 a.m.)


Review request for mesos and Jie Yu.


Changes
---

Addressed review comments.


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


Repository: mesos


Description
---

Added documentation for mesos-containerizer Linux capabilities support.


Diffs (updated)
-

  docs/configuration.md c83a58eb6884c8d8c37880a745e04cf0b789ebdc 
  docs/linux_capabilities.md PRE-CREATION 
  docs/mesos-containerizer.md 76544625838845fe2817347fea483a60ab250f99 

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


Testing
---

Checked with local renderer.


Thanks,

Benjamin Bannier