Re: Review Request 59129: Introduced `inet6::Address` to handle IPv6 addresses in `libprocess`.

2017-05-25 Thread Avinash sridharan

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

(Updated May 26, 2017, 6:38 a.m.)


Review request for mesos, Benjamin Hindman and Jie Yu.


Changes
---

Addressed some of Ben's comments.


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


Repository: mesos


Description
---

Introduced `inet6::Address` to handle IPv6 addresses in `libprocess`.


Diffs (updated)
-

  3rdparty/libprocess/include/process/address.hpp 
6b143c3d00c1d7ebd1697c26b6d312a64f30839a 
  3rdparty/libprocess/src/socket.cpp 85920a1abb3a2b2da167647dcf1351b825c72c80 


Diff: https://reviews.apache.org/r/59129/diff/2/

Changes: https://reviews.apache.org/r/59129/diff/1-2/


Testing
---

make check.


Thanks,

Avinash sridharan



Re: Review Request 59578: Made MasterTest.MaxCompletedTasksPerFrameworkFlag less fragile.

2017-05-25 Thread Mesos Reviewbot

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



Patch looks great!

Reviews applied: [59578]

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 May 25, 2017, 6:44 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59578/
> ---
> 
> (Updated May 25, 2017, 6:44 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rather than depending on batch allocations to eventually occur, pause
> the clock and explicitly advance it when we want to trigger a batch
> allocation. This improves both test robustness and speed.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp 1dfe5fd7c33c3c479175aa522fef1956f11f265c 
> 
> 
> Diff: https://reviews.apache.org/r/59578/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 59413: Fix bytes.hpp constexpr compilation failure with GCC 7.1.

2017-05-25 Thread Mesos Reviewbot

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



Patch looks great!

Reviews applied: [59454, 59413]

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 May 25, 2017, 11:13 a.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59413/
> ---
> 
> (Updated May 25, 2017, 11:13 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and 
> Neil Conway.
> 
> 
> Bugs: MESOS-7520
> https://issues.apache.org/jira/browse/MESOS-7520
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Many of the `constexpr` variables fail to compile with errors such as `error: 
> 'Megabytes(32).Megabytes::' is not a constant expression because 
> it refers to an incompletely initialized variable`. 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80829
> 
> This patch changes around the `Bytes` class a bit by getting rid of the small 
> classes that extend `Bytes`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/bytes.hpp 98223db50 
> 
> 
> Diff: https://reviews.apache.org/r/59413/diff/5/
> 
> 
> Testing
> ---
> 
> `./bootstrap && mkdir build && cd build && ../configure --disable-python 
> --disable-java --disable-optimize --disable-hardening &&  make -j$(nproc) && 
> make check -j$(nproc)`
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>



Re: Review Request 59584: Introduced executor reconnect retries on the agent.

2017-05-25 Thread Greg Mann

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




src/slave/flags.cpp
Lines 355 (patched)


s/sent to the executor/sent to the executor during recovery/



src/slave/flags.cpp
Lines 356 (patched)


s/MESOS-5322/MESOS-5332/



src/slave/flags.cpp
Lines 365-366 (patched)


Maybe something like:

these "old" executors will reply on their half-open connection and receive 
a RST; without any retries, they will fail to reconnect and be killed by the 
agent once the executor re-registration timeout elapses.



src/slave/slave.cpp
Lines 5964-5965 (patched)


Ditto, as above.



src/slave/slave.cpp
Lines 5967 (patched)


s/an optional/optional/



src/slave/slave.cpp
Lines 5972-5973 (patched)


Is this TODO necessary, since this entire block only executes when 
`executor->pid.isSome() && executor->pid.get()`?



src/slave/slave.cpp
Lines 5975-5979 (patched)


Why const ref for the IDs but not for the retry interval?


- Greg Mann


On May 26, 2017, 12:56 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59584/
> ---
> 
> (Updated May 26, 2017, 12:56 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-5332, MESOS-7057 and MESOS-7569
> https://issues.apache.org/jira/browse/MESOS-5332
> https://issues.apache.org/jira/browse/MESOS-7057
> https://issues.apache.org/jira/browse/MESOS-7569
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> PID-based v0 executors using Mesos libraries >= 1.1.2 always re-link
> with the agent upon receiving the reconnect message. This avoids the
> executor replying on a half-open TCP connection to the old agent
> (possible if netfilter is dropping packets, see: MESOS-7057).
> However, PID-based executors using Mesos libraries < 1.1.2 do not
> re-link and are therefore prone to replying on a half-open connection
> after the agent restarts. If we only send a single reconnect message,
> these "old" executors will reply on their half-open connection,
> receive a RST, and think the agent just died. To ensure these "old"
> executors can reconnect in the presence of netfilter dropping packets,
> we introduced optional retries of the reconnect message. This results
> in "old" executors correctly establishing a link when processing the
> second reconnect message.
> 
> Generally, users should not enable this flag unless they are affected
> by this issue.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.hpp b66995630f89dfb95a6d0cf66efc5d7590e90cbc 
>   src/slave/flags.cpp 0c8276e425a6a7d22ee68edc6cc25b331635ec44 
>   src/slave/slave.cpp 15e4d68714556ca30a766acd3b9729367df680c3 
> 
> 
> Diff: https://reviews.apache.org/r/59584/diff/1/
> 
> 
> Testing
> ---
> 
> Added tests in follow up reviews.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>



Re: Review Request 59594: Minor logging cleanup to put open/close quotes on the same line.

2017-05-25 Thread Vinod Kone

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


Ship it!




Ship It!

- Vinod Kone


On May 26, 2017, 1:05 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59594/
> ---
> 
> (Updated May 26, 2017, 1:05 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 15e4d68714556ca30a766acd3b9729367df680c3 
> 
> 
> Diff: https://reviews.apache.org/r/59594/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>



Re: Review Request 59592: Added logging of executor re-registration messages.

2017-05-25 Thread Vinod Kone

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


Ship it!




Ship It!

- Vinod Kone


On May 26, 2017, 1:04 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59592/
> ---
> 
> (Updated May 26, 2017, 1:04 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 15e4d68714556ca30a766acd3b9729367df680c3 
> 
> 
> Diff: https://reviews.apache.org/r/59592/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>



Re: Review Request 59593: Avoided use of [] operator for read only map access.

2017-05-25 Thread Vinod Kone

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


Ship it!




Ship It!

- Vinod Kone


On May 26, 2017, 1:05 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59593/
> ---
> 
> (Updated May 26, 2017, 1:05 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 15e4d68714556ca30a766acd3b9729367df680c3 
> 
> 
> Diff: https://reviews.apache.org/r/59593/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>



Re: Review Request 59591: Removed a use of the 'default' switch case.

2017-05-25 Thread Vinod Kone

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


Ship it!




Ship It!

- Vinod Kone


On May 26, 2017, 1:03 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59591/
> ---
> 
> (Updated May 26, 2017, 1:03 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For enums, we instead rely on the compiler to warn us when we
> miss a switch case. With the presence of the 'default' case,
> this doesn't occur. See MESOS-3754.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 15e4d68714556ca30a766acd3b9729367df680c3 
> 
> 
> Diff: https://reviews.apache.org/r/59591/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>



Re: Review Request 59589: Don't crash when re-registering executor from an unknown framework.

2017-05-25 Thread Vinod Kone

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


Ship it!




Ship It!

- Vinod Kone


On May 26, 2017, 1:01 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59589/
> ---
> 
> (Updated May 26, 2017, 1:01 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rather than crashing the agent, the agent will tell the executor
> to shut down.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 15e4d68714556ca30a766acd3b9729367df680c3 
> 
> 
> Diff: https://reviews.apache.org/r/59589/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>



Re: Review Request 59590: Don't crash the agent when an unknown executor re-registers.

2017-05-25 Thread Vinod Kone

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


Ship it!




Ship It!

- Vinod Kone


On May 26, 2017, 1:02 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59590/
> ---
> 
> (Updated May 26, 2017, 1:02 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rather than crashing the agent, the agent will tell the executor
> to shut down.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 15e4d68714556ca30a766acd3b9729367df680c3 
> 
> 
> Diff: https://reviews.apache.org/r/59590/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>



Re: Review Request 59587: Added a test for ignoring executor re-registrations.

2017-05-25 Thread Vinod Kone

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


Ship it!





src/tests/slave_tests.cpp
Lines 7174 (patched)


ditto from previous review. helper?


- Vinod Kone


On May 26, 2017, 12:21 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59587/
> ---
> 
> (Updated May 26, 2017, 12:21 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Greg Mann, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When the executor reconnect retry is enabled, the agent will ignore
> any subsequent executor re-registrations since the agent cannot
> correctly handle these in the steady state case.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp d124d594fb4f20db3f5dacbc7482d97161cca2df 
> 
> 
> Diff: https://reviews.apache.org/r/59587/diff/1/
> 
> 
> Testing
> ---
> 
> Ran in repetition.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>



Re: Review Request 59586: Added a test for shutting down executors that re-register.

2017-05-25 Thread Vinod Kone

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


Ship it!





src/tests/slave_tests.cpp
Lines 7069-7074 (patched)


couldn't use any helper here?


- Vinod Kone


On May 26, 2017, 12:21 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59586/
> ---
> 
> (Updated May 26, 2017, 12:21 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Greg Mann, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When already (re-)registered executors attempt to re-register,
> and the agent does not have the optional reconnect retry enabled,
> the agent will shut down the executor.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp d124d594fb4f20db3f5dacbc7482d97161cca2df 
> 
> 
> Diff: https://reviews.apache.org/r/59586/diff/1/
> 
> 
> Testing
> ---
> 
> Ran in repetition.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>



Re: Review Request 59585: Added a test for the optional executor reconnect retry in the agent.

2017-05-25 Thread Vinod Kone

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


Fix it, then Ship it!





src/tests/slave_recovery_tests.cpp
Lines 806 (patched)


s/that/that when/



src/tests/slave_recovery_tests.cpp
Lines 847 (patched)


maybe instead of ignoring subsequent updates test that no further updates 
are sent as a result of this change.



src/tests/slave_recovery_tests.cpp
Lines 859 (patched)


if you want to follow the above advice, you need to ensure ack is 
checkpointed.



src/tests/slave_recovery_tests.cpp
Lines 882 (patched)


// The first attempt by the executor to re-register is dropped so that the 
agent will retry the reconnect.


- Vinod Kone


On May 26, 2017, 12:21 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59585/
> ---
> 
> (Updated May 26, 2017, 12:21 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Greg Mann, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This tests that the retries occur if the agent does not receive
> a re-registration from the executor within the timeout interval.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_recovery_tests.cpp 0aa87f534fbc655e3f1aa2ab7f56a1b6be7a8755 
> 
> 
> Diff: https://reviews.apache.org/r/59585/diff/1/
> 
> 
> Testing
> ---
> 
> Ran in repetition.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>



Re: Review Request 59584: Introduced executor reconnect retries on the agent.

2017-05-25 Thread Vinod Kone

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




src/slave/slave.cpp
Lines 5956 (patched)


Maybe just link to the flag help here instead of duplicating?



src/slave/slave.cpp
Lines 6006 (patched)


Can you log this as well like we do for the first message?

```
LOG(INFO) << "Re-sending reconnect request to executor " << *executor;
```


- Vinod Kone


On May 26, 2017, 12:56 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59584/
> ---
> 
> (Updated May 26, 2017, 12:56 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Greg Mann, and Vinod Kone.
> 
> 
> Bugs: MESOS-5332, MESOS-7057 and MESOS-7569
> https://issues.apache.org/jira/browse/MESOS-5332
> https://issues.apache.org/jira/browse/MESOS-7057
> https://issues.apache.org/jira/browse/MESOS-7569
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> PID-based v0 executors using Mesos libraries >= 1.1.2 always re-link
> with the agent upon receiving the reconnect message. This avoids the
> executor replying on a half-open TCP connection to the old agent
> (possible if netfilter is dropping packets, see: MESOS-7057).
> However, PID-based executors using Mesos libraries < 1.1.2 do not
> re-link and are therefore prone to replying on a half-open connection
> after the agent restarts. If we only send a single reconnect message,
> these "old" executors will reply on their half-open connection,
> receive a RST, and think the agent just died. To ensure these "old"
> executors can reconnect in the presence of netfilter dropping packets,
> we introduced optional retries of the reconnect message. This results
> in "old" executors correctly establishing a link when processing the
> second reconnect message.
> 
> Generally, users should not enable this flag unless they are affected
> by this issue.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.hpp b66995630f89dfb95a6d0cf66efc5d7590e90cbc 
>   src/slave/flags.cpp 0c8276e425a6a7d22ee68edc6cc25b331635ec44 
>   src/slave/slave.cpp 15e4d68714556ca30a766acd3b9729367df680c3 
> 
> 
> Diff: https://reviews.apache.org/r/59584/diff/1/
> 
> 
> Testing
> ---
> 
> Added tests in follow up reviews.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>



Re: Review Request 59600: Added `--resource_providers` flag to the agent.

2017-05-25 Thread Chun-Hung Hsiao

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


Fix it, then Ship it!




Any manual checks in addition to `make check`?


docs/configuration.md
Lines 1851 (patched)


Missing a period.



docs/configuration.md
Lines 1854 (patched)


Is `to the master` an error here?



src/slave/flags.cpp
Lines 102 (patched)


`to the master`?


- Chun-Hung Hsiao


On May 26, 2017, 1:12 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59600/
> ---
> 
> (Updated May 26, 2017, 1:12 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Joseph Wu, and 
> Jan Schlicht.
> 
> 
> Bugs: MESOS-7571
> https://issues.apache.org/jira/browse/MESOS-7571
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `--resource_providers` flag to the agent.
> 
> 
> Diffs
> -
> 
>   docs/configuration.md 59e1bbe7502c7f55fb3fb3167ccdccc1294e6db7 
>   include/mesos/type_utils.hpp c7872c0cd7f19d923e8be1f33a775d5eb17a0cf7 
>   src/common/parse.hpp 64eabf89f81a6708cfac89081e0d9f9586721693 
>   src/common/type_utils.cpp 5f8e72b97d6766f9729079f6c6c013bc117cedb9 
>   src/slave/flags.hpp b66995630f89dfb95a6d0cf66efc5d7590e90cbc 
>   src/slave/flags.cpp 0c8276e425a6a7d22ee68edc6cc25b331635ec44 
> 
> 
> Diff: https://reviews.apache.org/r/59600/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 59601: Removed an unused declaration in type_utils.hpp.

2017-05-25 Thread Chun-Hung Hsiao

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


Ship it!




Ship It!

- Chun-Hung Hsiao


On May 26, 2017, 1:13 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59601/
> ---
> 
> (Updated May 26, 2017, 1:13 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Joseph Wu, Jan 
> Schlicht, and Zhongbo Tian.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed an unused declaration in type_utils.hpp.
> 
> 
> Diffs
> -
> 
>   include/mesos/type_utils.hpp c7872c0cd7f19d923e8be1f33a775d5eb17a0cf7 
> 
> 
> Diff: https://reviews.apache.org/r/59601/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Re: Review Request 59353: Enabled DOCKER and ROOT filter flags on Windows.

2017-05-25 Thread Joseph Wu

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


Fix it, then Ship it!




Mostly just white-space issues, so I'll fix those up before committing.


src/tests/default_executor_tests.cpp
Line 1430 (original), 1430-1431 (patched)


Whitespace:
```
TEST_P_TEMP_DISABLED_ON_WINDOWS(
PersistentVolumeDefaultExecutor, ROOT_PersistentResources)
```



src/tests/default_executor_tests.cpp
Line 1572 (original), 1573-1574 (patched)


Ditto about whitespace... and throughout this patch.



src/tests/environment.cpp
Line 268 (original)


```
#if defined(__linux__) || defined(__WINDOWS__)
```

We still don't support Docker tests on Posix, so this check should be 
modified rather than removed.


- Joseph Wu


On May 17, 2017, 2:35 p.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59353/
> ---
> 
> (Updated May 17, 2017, 2:35 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Jeff Coffler, Joseph Wu, and 
> Li Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled DOCKER and ROOT filter flags on Windows.
> 
> 
> Diffs
> -
> 
>   src/tests/CMakeLists.txt 9f2af9cdd1cf50485f4cd84ce67bcceba64b9328 
>   src/tests/containerizer/docker_tests.cpp 
> 3d3c9afbe81ebdcdf7c5542c56f9ca0e17e2484d 
>   src/tests/default_executor_tests.cpp 
> 22af7e973f8e6ca583c3126a80bc092bf88fea33 
>   src/tests/environment.cpp 047798c781707c42641baa9473177ae25a451989 
>   src/tests/health_check_tests.cpp 6c1b9a0ead6edb34c20cf4973fffcff913c5d7ad 
>   src/tests/hook_tests.cpp 02d8f800c3eb9b1e617a14c78c2ef1e45d1c72bb 
>   src/tests/slave_tests.cpp 8f81f77aa29d16616d5a38197729baab0fb0cab5 
> 
> 
> Diff: https://reviews.apache.org/r/59353/diff/1/
> 
> 
> Testing
> ---
> 
> Tested on both Windows and Linux.  Everything passes on Linux, but there are 
> a number of tests that are failing on Windows. The tests that are failing on 
> Windows are related to the tests that are currently failing on master on 
> Windows. These tests are failing due to some long path changes in the Windows 
> creators update, and I believe Andy is working on fixes to them.
> 
> 
> Thanks,
> 
> John Kordich
> 
>



Re: Review Request 59578: Made MasterTest.MaxCompletedTasksPerFrameworkFlag less fragile.

2017-05-25 Thread Neil Conway


> On May 26, 2017, 12:03 a.m., Kevin Klues wrote:
> > src/tests/master_tests.cpp
> > Lines 5635-5636 (patched)
> > 
> >
> > Is this something standard we've been doing recently to fix flaky 
> > tests? I'm not familiar with this pattern. of pausing the clocl and then 
> > advancing it manually.

We do this fairly often in the tests (`ag "Clock::pause" src/tests | wc -l` => 
366). In my opinion, the advantages are:

* Tests that rely on the clock making progress are more likely to be flaky, 
because there is no synchronization between how fast the test executes and when 
clock-based timers go off.
* A test that is written to assume that clock advances at a certain point has 
an _implicit_ timing dependency. Pausing the clock and advancing the clock 
makes this dependency explicit.
* The test usually runs faster; rather than waiting for the system clock (which 
might be quite a while), we can immediately advance the clock the appropriate 
amount.

Personally, I think we should pause the clock in all tests by default 
(MESOS-4101).


- Neil


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


On May 25, 2017, 6:44 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59578/
> ---
> 
> (Updated May 25, 2017, 6:44 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rather than depending on batch allocations to eventually occur, pause
> the clock and explicitly advance it when we want to trigger a batch
> allocation. This improves both test robustness and speed.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp 1dfe5fd7c33c3c479175aa522fef1956f11f265c 
> 
> 
> Diff: https://reviews.apache.org/r/59578/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Review Request 59601: Removed an unused declaration in type_utils.hpp.

2017-05-25 Thread Jie Yu

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

Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Joseph Wu, Jan 
Schlicht, and Zhongbo Tian.


Repository: mesos


Description
---

Removed an unused declaration in type_utils.hpp.


Diffs
-

  include/mesos/type_utils.hpp c7872c0cd7f19d923e8be1f33a775d5eb17a0cf7 


Diff: https://reviews.apache.org/r/59601/diff/1/


Testing
---

make check


Thanks,

Jie Yu



Review Request 59600: Added `--resource_providers` flag to the agent.

2017-05-25 Thread Jie Yu

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

Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Joseph Wu, and Jan 
Schlicht.


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


Repository: mesos


Description
---

Added `--resource_providers` flag to the agent.


Diffs
-

  docs/configuration.md 59e1bbe7502c7f55fb3fb3167ccdccc1294e6db7 
  include/mesos/type_utils.hpp c7872c0cd7f19d923e8be1f33a775d5eb17a0cf7 
  src/common/parse.hpp 64eabf89f81a6708cfac89081e0d9f9586721693 
  src/common/type_utils.cpp 5f8e72b97d6766f9729079f6c6c013bc117cedb9 
  src/slave/flags.hpp b66995630f89dfb95a6d0cf66efc5d7590e90cbc 
  src/slave/flags.cpp 0c8276e425a6a7d22ee68edc6cc25b331635ec44 


Diff: https://reviews.apache.org/r/59600/diff/1/


Testing
---

make check


Thanks,

Jie Yu



Re: Review Request 59596: Renamed the ifdef guard for v1 resource provider header.

2017-05-25 Thread Chun-Hung Hsiao

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


Fix it, then Ship it!





include/mesos/v1/resource_provider/resource_provider.hpp
Line 17 (original), 17 (patched)


Should we also fix the ifdef guard in 
`include/mesos/resource_provider/resource_provider.hpp`? Also there are a 
couple more headers that also only include corresponding protobuf headers but 
not using this `_PROTO_` naming, such as `include/mesos/docker/v1.hpp`. They 
can be fixed later, just leave a note here.


- Chun-Hung Hsiao


On May 26, 2017, 1:01 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59596/
> ---
> 
> (Updated May 26, 2017, 1:01 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Joseph Wu, and 
> Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Renamed the ifdef guard for v1 resource provider header.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/resource_provider/resource_provider.hpp 
> 2b8c8afab852621fb49b132813d512d0c96bc68c 
> 
> 
> Diff: https://reviews.apache.org/r/59596/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



Review Request 59599: Added 'type' and 'name' fields to ResourceProviderInfo.

2017-05-25 Thread Jie Yu

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

Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Joseph Wu, and Jan 
Schlicht.


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


Repository: mesos


Description
---

Added 'type' and 'name' fields to ResourceProviderInfo.


Diffs
-

  include/mesos/mesos.proto b063ddaca60398db0ca8ef8a6a41e472b9f4a404 
  include/mesos/v1/mesos.proto bc7dbe0d99de6e5a23b842751fce43896a6fcce9 


Diff: https://reviews.apache.org/r/59599/diff/1/


Testing
---

make check


Thanks,

Jie Yu



Review Request 59598: Added registration logic to storage local resource provider.

2017-05-25 Thread Jie Yu

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

Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Joseph Wu, and Jan 
Schlicht.


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


Repository: mesos


Description
---

Added registration logic to storage local resource provider.


Diffs
-

  src/resource_provider/storage/provider.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/59598/diff/1/


Testing
---

make check


Thanks,

Jie Yu



Review Request 59594: Minor logging cleanup to put open/close quotes on the same line.

2017-05-25 Thread Benjamin Mahler

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

Review request for mesos, Greg Mann and Vinod Kone.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/slave/slave.cpp 15e4d68714556ca30a766acd3b9729367df680c3 


Diff: https://reviews.apache.org/r/59594/diff/1/


Testing
---


Thanks,

Benjamin Mahler



Review Request 59593: Avoided use of [] operator for read only map access.

2017-05-25 Thread Benjamin Mahler

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

Review request for mesos, Greg Mann and Vinod Kone.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/slave/slave.cpp 15e4d68714556ca30a766acd3b9729367df680c3 


Diff: https://reviews.apache.org/r/59593/diff/1/


Testing
---


Thanks,

Benjamin Mahler



Review Request 59592: Added logging of executor re-registration messages.

2017-05-25 Thread Benjamin Mahler

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

Review request for mesos, Greg Mann and Vinod Kone.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/slave/slave.cpp 15e4d68714556ca30a766acd3b9729367df680c3 


Diff: https://reviews.apache.org/r/59592/diff/1/


Testing
---


Thanks,

Benjamin Mahler



Review Request 59591: Removed a use of the 'default' switch case.

2017-05-25 Thread Benjamin Mahler

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

Review request for mesos, Greg Mann and Vinod Kone.


Repository: mesos


Description
---

For enums, we instead rely on the compiler to warn us when we
miss a switch case. With the presence of the 'default' case,
this doesn't occur. See MESOS-3754.


Diffs
-

  src/slave/slave.cpp 15e4d68714556ca30a766acd3b9729367df680c3 


Diff: https://reviews.apache.org/r/59591/diff/1/


Testing
---


Thanks,

Benjamin Mahler



Review Request 59590: Don't crash the agent when an unknown executor re-registers.

2017-05-25 Thread Benjamin Mahler

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

Review request for mesos, Greg Mann and Vinod Kone.


Repository: mesos


Description
---

Rather than crashing the agent, the agent will tell the executor
to shut down.


Diffs
-

  src/slave/slave.cpp 15e4d68714556ca30a766acd3b9729367df680c3 


Diff: https://reviews.apache.org/r/59590/diff/1/


Testing
---


Thanks,

Benjamin Mahler



Review Request 59596: Renamed the ifdef guard for v1 resource provider header.

2017-05-25 Thread Jie Yu

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

Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Joseph Wu, and Jan 
Schlicht.


Repository: mesos


Description
---

Renamed the ifdef guard for v1 resource provider header.


Diffs
-

  include/mesos/v1/resource_provider/resource_provider.hpp 
2b8c8afab852621fb49b132813d512d0c96bc68c 


Diff: https://reviews.apache.org/r/59596/diff/1/


Testing
---


Thanks,

Jie Yu



Review Request 59589: Don't crash when re-registering executor from an unknown framework.

2017-05-25 Thread Benjamin Mahler

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

Review request for mesos, Greg Mann and Vinod Kone.


Repository: mesos


Description
---

Rather than crashing the agent, the agent will tell the executor
to shut down.


Diffs
-

  src/slave/slave.cpp 15e4d68714556ca30a766acd3b9729367df680c3 


Diff: https://reviews.apache.org/r/59589/diff/1/


Testing
---


Thanks,

Benjamin Mahler



Re: Review Request 59584: Introduced executor reconnect retries on the agent.

2017-05-25 Thread Benjamin Mahler

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

(Updated May 26, 2017, 12:56 a.m.)


Review request for mesos, Anand Mazumdar, Greg Mann, and Vinod Kone.


Changes
---

Added MESOS-7569.


Bugs: MESOS-5332, MESOS-7057 and MESOS-7569
https://issues.apache.org/jira/browse/MESOS-5332
https://issues.apache.org/jira/browse/MESOS-7057
https://issues.apache.org/jira/browse/MESOS-7569


Repository: mesos


Description
---

PID-based v0 executors using Mesos libraries >= 1.1.2 always re-link
with the agent upon receiving the reconnect message. This avoids the
executor replying on a half-open TCP connection to the old agent
(possible if netfilter is dropping packets, see: MESOS-7057).
However, PID-based executors using Mesos libraries < 1.1.2 do not
re-link and are therefore prone to replying on a half-open connection
after the agent restarts. If we only send a single reconnect message,
these "old" executors will reply on their half-open connection,
receive a RST, and think the agent just died. To ensure these "old"
executors can reconnect in the presence of netfilter dropping packets,
we introduced optional retries of the reconnect message. This results
in "old" executors correctly establishing a link when processing the
second reconnect message.

Generally, users should not enable this flag unless they are affected
by this issue.


Diffs
-

  src/slave/flags.hpp b66995630f89dfb95a6d0cf66efc5d7590e90cbc 
  src/slave/flags.cpp 0c8276e425a6a7d22ee68edc6cc25b331635ec44 
  src/slave/slave.cpp 15e4d68714556ca30a766acd3b9729367df680c3 


Diff: https://reviews.apache.org/r/59584/diff/1/


Testing
---

Added tests in follow up reviews.


Thanks,

Benjamin Mahler



Re: Review Request 59413: Fix bytes.hpp constexpr compilation failure with GCC 7.1.

2017-05-25 Thread Mesos Reviewbot Windows

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



Patch looks great!

Reviews applied: [59454, 59413]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On May 25, 2017, 6:13 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59413/
> ---
> 
> (Updated May 25, 2017, 6:13 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and 
> Neil Conway.
> 
> 
> Bugs: MESOS-7520
> https://issues.apache.org/jira/browse/MESOS-7520
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Many of the `constexpr` variables fail to compile with errors such as `error: 
> 'Megabytes(32).Megabytes::' is not a constant expression because 
> it refers to an incompletely initialized variable`. 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80829
> 
> This patch changes around the `Bytes` class a bit by getting rid of the small 
> classes that extend `Bytes`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/bytes.hpp 98223db50 
> 
> 
> Diff: https://reviews.apache.org/r/59413/diff/5/
> 
> 
> Testing
> ---
> 
> `./bootstrap && mkdir build && cd build && ../configure --disable-python 
> --disable-java --disable-optimize --disable-hardening &&  make -j$(nproc) && 
> make check -j$(nproc)`
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>



Review Request 59586: Added a test for shutting down executors that re-register.

2017-05-25 Thread Benjamin Mahler

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

Review request for mesos, Anand Mazumdar, Greg Mann, and Vinod Kone.


Repository: mesos


Description
---

When already (re-)registered executors attempt to re-register,
and the agent does not have the optional reconnect retry enabled,
the agent will shut down the executor.


Diffs
-

  src/tests/slave_tests.cpp d124d594fb4f20db3f5dacbc7482d97161cca2df 


Diff: https://reviews.apache.org/r/59586/diff/1/


Testing
---

Ran in repetition.


Thanks,

Benjamin Mahler



Review Request 59584: Introduced executor reconnect retries on the agent.

2017-05-25 Thread Benjamin Mahler

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

Review request for mesos, Anand Mazumdar, Greg Mann, and Vinod Kone.


Bugs: MESOS-5332 and MESOS-7057
https://issues.apache.org/jira/browse/MESOS-5332
https://issues.apache.org/jira/browse/MESOS-7057


Repository: mesos


Description
---

PID-based v0 executors using Mesos libraries >= 1.1.2 always re-link
with the agent upon receiving the reconnect message. This avoids the
executor replying on a half-open TCP connection to the old agent
(possible if netfilter is dropping packets, see: MESOS-7057).
However, PID-based executors using Mesos libraries < 1.1.2 do not
re-link and are therefore prone to replying on a half-open connection
after the agent restarts. If we only send a single reconnect message,
these "old" executors will reply on their half-open connection,
receive a RST, and think the agent just died. To ensure these "old"
executors can reconnect in the presence of netfilter dropping packets,
we introduced optional retries of the reconnect message. This results
in "old" executors correctly establishing a link when processing the
second reconnect message.

Generally, users should not enable this flag unless they are affected
by this issue.


Diffs
-

  src/slave/flags.hpp b66995630f89dfb95a6d0cf66efc5d7590e90cbc 
  src/slave/flags.cpp 0c8276e425a6a7d22ee68edc6cc25b331635ec44 
  src/slave/slave.cpp 15e4d68714556ca30a766acd3b9729367df680c3 


Diff: https://reviews.apache.org/r/59584/diff/1/


Testing
---

Added tests in follow up reviews.


Thanks,

Benjamin Mahler



Review Request 59587: Added a test for ignoring executor re-registrations.

2017-05-25 Thread Benjamin Mahler

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

Review request for mesos, Anand Mazumdar, Greg Mann, and Vinod Kone.


Repository: mesos


Description
---

When the executor reconnect retry is enabled, the agent will ignore
any subsequent executor re-registrations since the agent cannot
correctly handle these in the steady state case.


Diffs
-

  src/tests/slave_tests.cpp d124d594fb4f20db3f5dacbc7482d97161cca2df 


Diff: https://reviews.apache.org/r/59587/diff/1/


Testing
---

Ran in repetition.


Thanks,

Benjamin Mahler



Review Request 59585: Added a test for the optional executor reconnect retry in the agent.

2017-05-25 Thread Benjamin Mahler

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

Review request for mesos, Anand Mazumdar, Greg Mann, and Vinod Kone.


Repository: mesos


Description
---

This tests that the retries occur if the agent does not receive
a re-registration from the executor within the timeout interval.


Diffs
-

  src/tests/slave_recovery_tests.cpp 0aa87f534fbc655e3f1aa2ab7f56a1b6be7a8755 


Diff: https://reviews.apache.org/r/59585/diff/1/


Testing
---

Ran in repetition.


Thanks,

Benjamin Mahler



Re: Review Request 59233: Updated v6 address for containers running on host network.

2017-05-25 Thread Benjamin Hindman

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


Fix it, then Ship it!





src/slave/slave.cpp
Lines 4424 (patched)


Not that I'm opposed to you setting this explicitly can we also make this 
the default so that we don't have to have funky if checking code other places 
where we want to determine if it's v4 or v6?


- Benjamin Hindman


On May 12, 2017, 10:22 p.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59233/
> ---
> 
> (Updated May 12, 2017, 10:22 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7488
> https://issues.apache.org/jira/browse/MESOS-7488
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently the agent is populating only the v4 address for containers
> running on the host network. However, clusters running on a dual stack
> network need to report v4 and v6 address for containers running on the
> host network. This change allows v6 address specified by operators to be
> advertised for containers running on the host network.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 209aff180db1a68ae360d7c278937fe645efdb2b 
> 
> 
> Diff: https://reviews.apache.org/r/59233/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 49571: Added a benchmark test for allocations.

2017-05-25 Thread James Peach

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


Ship it!




Ship It!

- James Peach


On May 25, 2017, 11:30 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49571/
> ---
> 
> (Updated May 25, 2017, 11:30 p.m.)
> 
> 
> Review request for mesos, James Peach and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5771
> https://issues.apache.org/jira/browse/MESOS-5771
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allocations test has the following resource configurations:
> (1) REGULAR: Offers from every slave have regular resources.
> (2) SHARED: Offers from every slave include a shared resource.
> (3) REGULAR: Offers from every alternate slave contain only regular
> resources; and offers from every other alternate slave contains
> a shared resource.
> 
> This test is parameterized based on number of agents, number of
> frameworks and resource configuration.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> eb2b647d3247a85e8b9b82e5589232c74ad8570f 
>   src/tests/resources_utils.hpp 1f41f02babce5c8174ea2223f4dc7470452fbaf1 
>   src/tests/resources_utils.cpp 2cef55f7312d671307e097c2c4960c8dcf45c1ff 
> 
> 
> Diff: https://reviews.apache.org/r/49571/diff/36/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> Allocations benchmark test results
> ==
> There is no visible impact in performance when shared resources are added in 
> the allocations. The numbers for HEAD are prior to shared resources support 
> (mid 2016) and the numbers indicate improvements in allocations during this 
> timeframe.
> 
> Following is a snapshot with 1000 agents and 200 frameworks.
> 
> With the patch (and no shared resources)
> 
> [ RUN  ] AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/9
> Using 1000 agents and 200 frameworks with resource type 0
> Added 200 frameworks in 6588us
> Added 1000 agents in 1.567347secs
> round 0 allocate() took 1.15531secs to make 1000 offers
> round 10 allocate() took 1.152876secs to make 1000 offers
> round 20 allocate() took 1.15661secs to make 1000 offers
> round 30 allocate() took 1.117733secs to make 1000 offers
> round 40 allocate() took 1.118754secs to make 1000 offers
> round 50 allocate() took 1.11169secs to make 1000 offers
> 
> With the patch (and shared resources on all agents)
> ---
> [ RUN  ] 
> AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/10
> Using 1000 agents and 200 frameworks with resource type 1
> Added 200 frameworks in 6064us
> Added 1000 agents in 1.627008secs
> round 0 allocate() took 1.168253secs to make 1000 offers
> round 10 allocate() took 1.146421secs to make 1000 offers
> round 20 allocate() took 1.16416secs to make 1000 offers
> round 30 allocate() took 1.210476secs to make 1000 offers
> round 40 allocate() took 1.194251secs to make 1000 offers
> round 50 allocate() took 1.17789secs to make 1000 offers
> 
> With the patch (and shared resources on alternate agents)
> -
> [ RUN  ] 
> AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/11
> Using 1000 agents and 200 frameworks with resource type 2
> Added 200 frameworks in 6466us
> Added 1000 agents in 1.568717secs
> round 0 allocate() took 1.153005secs to make 1000 offers
> round 10 allocate() took 1.168169secs to make 1000 offers
> round 20 allocate() took 1.156774secs to make 1000 offers
> round 30 allocate() took 1.183112secs to make 1000 offers
> round 40 allocate() took 1.202452secs to make 1000 offers
> round 50 allocate() took 1.198918secs to make 1000 offers
> 
> Based on HEAD, with all regular resources (no shared resources in HEAD 
> supported)
> -
> [ RUN  ] AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/9
> Using 1000 agents and 200 frameworks with resource type 0
> Added 200 frameworks in 6801us
> Added 1000 agents in 1.721447secs
> round 0 allocate() took 1.502953secs to make 1000 offers
> round 50 allocate() took 1.520157secs to make 1000 offers
> round 100 allocate() took 1.517221secs to make 1000 offers
> round 150 allocate() took 1.526446secs to make 1000 offers
> round 199 allocate() took 1.538005secs to make 1000 offers
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>



Re: Review Request 59413: Fix bytes.hpp constexpr compilation failure with GCC 7.1.

2017-05-25 Thread James Peach

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


Ship it!




Ship It!

- James Peach


On May 25, 2017, 6:13 p.m., Aaron Wood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59413/
> ---
> 
> (Updated May 25, 2017, 6:13 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and 
> Neil Conway.
> 
> 
> Bugs: MESOS-7520
> https://issues.apache.org/jira/browse/MESOS-7520
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Many of the `constexpr` variables fail to compile with errors such as `error: 
> 'Megabytes(32).Megabytes::' is not a constant expression because 
> it refers to an incompletely initialized variable`. 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80829
> 
> This patch changes around the `Bytes` class a bit by getting rid of the small 
> classes that extend `Bytes`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/bytes.hpp 98223db50 
> 
> 
> Diff: https://reviews.apache.org/r/59413/diff/5/
> 
> 
> Testing
> ---
> 
> `./bootstrap && mkdir build && cd build && ../configure --disable-python 
> --disable-java --disable-optimize --disable-hardening &&  make -j$(nproc) && 
> make check -j$(nproc)`
> 
> 
> Thanks,
> 
> Aaron Wood
> 
>



Re: Review Request 59128: Added initialization logic in Mesos agent for IPv6 flags.

2017-05-25 Thread Benjamin Hindman

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


Ship it!




Ship It!

- Benjamin Hindman


On May 10, 2017, 7:04 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59128/
> ---
> 
> (Updated May 10, 2017, 7:04 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7488
> https://issues.apache.org/jira/browse/MESOS-7488
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Operators can now set the IPv6 address for the agent using the `--ip6`
> and `--ip6_discovery_command` flags. Currently, the only place the agent
> will use the IPv6 address would be to advertise v6 addresses for
> containers running on the host network.
> 
> 
> Diffs
> -
> 
>   src/slave/main.cpp 507d59996a90f51c7cd1e0f836a34c2e9c484791 
> 
> 
> Diff: https://reviews.apache.org/r/59128/diff/1/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 59127: Added IPv6 flags for Mesos agent.

2017-05-25 Thread Benjamin Hindman

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


Ship it!




Ship It!

- Benjamin Hindman


On May 10, 2017, 7:03 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59127/
> ---
> 
> (Updated May 10, 2017, 7:03 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7488
> https://issues.apache.org/jira/browse/MESOS-7488
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added IPv6 flags for Mesos agent.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.hpp e5784ef81ad0720c7ec061ee0b28b8fadae77afd 
>   src/slave/flags.cpp ed99fadbf1aa91f7f0a57c9fb351c0247a40c6f4 
> 
> 
> Diff: https://reviews.apache.org/r/59127/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 49571: Added a benchmark test for allocations.

2017-05-25 Thread James Peach


> On May 24, 2017, 11:35 p.m., James Peach wrote:
> > src/tests/hierarchical_allocator_tests.cpp
> > Lines 4995 (patched)
> > 
> >
> > I found that this output wasn't very helpful. How about running through 
> > the offer cycle N times and collecting the results in a 
> > `process::TimeSeries`, then showing the `process::Statistics` once at the 
> > end of the test?
> 
> Anindya Sinha wrote:
> I think we should defer this to a future commit to do this for all 
> benchmarks.

That sounds fine to me. I filed 
[MESOS-756](https://issues.apache.org/jira/browse/MESOS-7567).


- James


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


On May 25, 2017, 11:30 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49571/
> ---
> 
> (Updated May 25, 2017, 11:30 p.m.)
> 
> 
> Review request for mesos, James Peach and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5771
> https://issues.apache.org/jira/browse/MESOS-5771
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allocations test has the following resource configurations:
> (1) REGULAR: Offers from every slave have regular resources.
> (2) SHARED: Offers from every slave include a shared resource.
> (3) REGULAR: Offers from every alternate slave contain only regular
> resources; and offers from every other alternate slave contains
> a shared resource.
> 
> This test is parameterized based on number of agents, number of
> frameworks and resource configuration.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> eb2b647d3247a85e8b9b82e5589232c74ad8570f 
>   src/tests/resources_utils.hpp 1f41f02babce5c8174ea2223f4dc7470452fbaf1 
>   src/tests/resources_utils.cpp 2cef55f7312d671307e097c2c4960c8dcf45c1ff 
> 
> 
> Diff: https://reviews.apache.org/r/49571/diff/36/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> Allocations benchmark test results
> ==
> There is no visible impact in performance when shared resources are added in 
> the allocations. The numbers for HEAD are prior to shared resources support 
> (mid 2016) and the numbers indicate improvements in allocations during this 
> timeframe.
> 
> Following is a snapshot with 1000 agents and 200 frameworks.
> 
> With the patch (and no shared resources)
> 
> [ RUN  ] AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/9
> Using 1000 agents and 200 frameworks with resource type 0
> Added 200 frameworks in 6588us
> Added 1000 agents in 1.567347secs
> round 0 allocate() took 1.15531secs to make 1000 offers
> round 10 allocate() took 1.152876secs to make 1000 offers
> round 20 allocate() took 1.15661secs to make 1000 offers
> round 30 allocate() took 1.117733secs to make 1000 offers
> round 40 allocate() took 1.118754secs to make 1000 offers
> round 50 allocate() took 1.11169secs to make 1000 offers
> 
> With the patch (and shared resources on all agents)
> ---
> [ RUN  ] 
> AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/10
> Using 1000 agents and 200 frameworks with resource type 1
> Added 200 frameworks in 6064us
> Added 1000 agents in 1.627008secs
> round 0 allocate() took 1.168253secs to make 1000 offers
> round 10 allocate() took 1.146421secs to make 1000 offers
> round 20 allocate() took 1.16416secs to make 1000 offers
> round 30 allocate() took 1.210476secs to make 1000 offers
> round 40 allocate() took 1.194251secs to make 1000 offers
> round 50 allocate() took 1.17789secs to make 1000 offers
> 
> With the patch (and shared resources on alternate agents)
> -
> [ RUN  ] 
> AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/11
> Using 1000 agents and 200 frameworks with resource type 2
> Added 200 frameworks in 6466us
> Added 1000 agents in 1.568717secs
> round 0 allocate() took 1.153005secs to make 1000 offers
> round 10 allocate() took 1.168169secs to make 1000 offers
> round 20 allocate() took 1.156774secs to make 1000 offers
> round 30 allocate() took 1.183112secs to make 1000 offers
> round 40 allocate() took 1.202452secs to make 1000 offers
> round 50 allocate() took 1.198918secs to make 1000 offers
> 
> Based on HEAD, with all regular resources (no shared resources in HEAD 
> supported)
> -
> [ RUN  ] AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/9
> Using 1000 agents and 200 frameworks with resou

Re: Review Request 59130: Added storage for IPv6 in a `libprocess` process.

2017-05-25 Thread Benjamin Hindman

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




3rdparty/libprocess/src/process.cpp
Lines 119 (patched)


Let's be explicit and add namespace `inet` too so that the code is very 
clear when we're working with v4 versus v6.



3rdparty/libprocess/src/process.cpp
Line 584 (original), 592 (patched)


Do we want a inline struct of `addresses`?

__addresses__.inet6 ? .v4?
.unix

?


- Benjamin Hindman


On May 10, 2017, 7:08 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59130/
> ---
> 
> (Updated May 10, 2017, 7:08 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7488
> https://issues.apache.org/jira/browse/MESOS-7488
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The operator can specify a v6 address to be used by a `libprocess`
> process by specifying the `LIBPROCESS_IP6` environment variable.
> Currently the `lipbrocess` stores the IPv6 address but doesn't open a
> socket on the v6 address in order to listen or send traffic on the v6
> address.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 96ce7dbc486a2f1d55d2238a8a102bf024b12b1c 
> 
> 
> Diff: https://reviews.apache.org/r/59130/diff/1/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 59578: Made MasterTest.MaxCompletedTasksPerFrameworkFlag less fragile.

2017-05-25 Thread Kevin Klues

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




src/tests/master_tests.cpp
Lines 5635-5636 (patched)


Is this something standard we've been doing recently to fix flaky tests? 
I'm not familiar with this pattern. of pausing the clocl and then advancing it 
manually.


- Kevin Klues


On May 25, 2017, 6:44 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59578/
> ---
> 
> (Updated May 25, 2017, 6:44 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Rather than depending on batch allocations to eventually occur, pause
> the clock and explicitly advance it when we want to trigger a batch
> allocation. This improves both test robustness and speed.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp 1dfe5fd7c33c3c479175aa522fef1956f11f265c 
> 
> 
> Diff: https://reviews.apache.org/r/59578/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 59129: Introduced `inet6::Address` to handle IPv6 addresses in `libprocess`.

2017-05-25 Thread Benjamin Hindman

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




3rdparty/libprocess/include/process/address.hpp
Line 152 (original), 164 (patched)


s/unspec/internal/



3rdparty/libprocess/include/process/address.hpp
Lines 152-154 (original), 164-166 (patched)


Let's leave a comment here explaining that we need an internal Address to 
capture the common functions reused between both inet::Address and 
inet6::Address which are both IP based and that we can't do this for the 
top-level Address because it can potentially hold a unix address which we can't 
have these overloaded functions apply to.



3rdparty/libprocess/include/process/address.hpp
Lines 207 (patched)


s/unspec/internal/



3rdparty/libprocess/include/process/address.hpp
Line 163 (original), 221 (patched)


Move static's up to the top of class (THIS WAS MY BUG NOT YOURS SORRY!).



3rdparty/libprocess/include/process/address.hpp
Lines 223 (patched)


Keep using IP/port constructor after net::IP refactor.



3rdparty/libprocess/include/process/address.hpp
Line 199 (original), 281 (patched)


Move '{' to next line.



3rdparty/libprocess/include/process/address.hpp
Line 200 (original), 282 (patched)


Call IP/port constructor after net::IP refactor.



3rdparty/libprocess/include/process/address.hpp
Lines 285 (patched)


inet6::Address::ANY_ANY() is explicit enough, so let's kill the extra '6'.



3rdparty/libprocess/include/process/address.hpp
Line 253 (original), 358 (patched)


Weird spacing, probably not your bug but can you fix it!



3rdparty/libprocess/include/process/address.hpp
Line 258 (original), 363 (patched)


Extra newline.



3rdparty/libprocess/include/process/address.hpp
Lines 377 (patched)


Extra newline.



3rdparty/libprocess/include/process/address.hpp
Lines 387 (patched)


Pull '{' back to previous line, below too please.



3rdparty/libprocess/include/process/address.hpp
Lines 402 (patched)


s/unspec/internal/



3rdparty/libprocess/include/process/address.hpp
Line 271 (original), 413 (patched)


Compiler won't catch this so just mentioning it for you.


- Benjamin Hindman


On May 10, 2017, 7:07 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59129/
> ---
> 
> (Updated May 10, 2017, 7:07 a.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-7488
> https://issues.apache.org/jira/browse/MESOS-7488
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Introduced `inet6::Address` to handle IPv6 addresses in `libprocess`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/address.hpp 
> 6b143c3d00c1d7ebd1697c26b6d312a64f30839a 
>   3rdparty/libprocess/src/socket.cpp 85920a1abb3a2b2da167647dcf1351b825c72c80 
> 
> 
> Diff: https://reviews.apache.org/r/59129/diff/1/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>



Re: Review Request 59537: Fixed a bug in 'ComposingContainerizerProcess::wait()'.

2017-05-25 Thread Mesos Reviewbot Windows

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



Bad patch!

Reviews applied: [59537]

Failed command: python support/apply-reviews.py -n -r 59537

Error:
Traceback (most recent call last):
  File "support/apply-reviews.py", line 381, in 
reviewboard()
  File "support/apply-reviews.py", line 360, in reviewboard
apply_review()
  File "support/apply-reviews.py", line 126, in apply_review
commit_patch()
  File "support/apply-reviews.py", line 225, in commit_patch
shell(cmd, options['dry_run'])
  File "support/apply-reviews.py", line 111, in shell
error_code = subprocess.call(command, stderr=subprocess.STDOUT, shell=True)
  File "C:\Python27\lib\subprocess.py", line 168, in call
return Popen(*popenargs, **kwargs).wait()
  File "C:\Python27\lib\subprocess.py", line 390, in __init__
errread, errwrite)
  File "C:\Python27\lib\subprocess.py", line 610, in _execute_child
args = '{} /c "{}"'.format (comspec, args)
UnicodeEncodeError: 'ascii' codec can't encode character u'\xf3' in position 
25: ordinal not in range(128)

Full log: http://mesos-winbot.westus.cloudapp.azure.com/../logs/console

- Mesos Reviewbot Windows


On May 25, 2017, 4:33 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59537/
> ---
> 
> (Updated May 25, 2017, 4:33 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Jie Yu, Kevin Klues, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-7546
> https://issues.apache.org/jira/browse/MESOS-7546
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a bug in the Composing Containerizer that would make it always
> immediately return 'None' when trying to wait on a nested container
> that had already been terminated and whose exit status was checkpointed.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.cpp 
> 0b6c76b3d081d86df81a6062ae7a191ba8dadfde 
> 
> 
> Diff: https://reviews.apache.org/r/59537/diff/2/
> 
> 
> Testing
> ---
> 
> `make check` on GNU/Linux, plus manual testing using an agent that uses both 
> the Docker and the Mesos containerizers.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>



Re: Review Request 59555: A prototype to move sample collection into async sampling.

2017-05-25 Thread Mesos Reviewbot Windows

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



Bad review!

Reviews applied: []

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

- Mesos Reviewbot Windows


On May 24, 2017, 11:35 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59555/
> ---
> 
> (Updated May 24, 2017, 11:35 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A prototype to move sample collection into async sampling.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/metrics/metrics.hpp 
> 9c32a88d851c884a5025edb6ea1e27939b484546 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> 7184aa4d0294c20466646c9aa61d90973eca22e1 
> 
> 
> Diff: https://reviews.apache.org/r/59555/diff/1/
> 
> 
> Testing
> ---
> 
> Ran this on a master and able to get result on `/metrics/cached` endpoint.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>



Re: Review Request 58720: CLI: Extended the unit test infrastructure.

2017-05-25 Thread Mesos Reviewbot Windows

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



Patch looks great!

Reviews applied: [58719, 58720]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On May 25, 2017, 9:08 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58720/
> ---
> 
> (Updated May 25, 2017, 9:08 a.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7283
> https://issues.apache.org/jira/browse/MESOS-7283
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This infrastructure includes the ability to bring up a test cluster to
> run the CLI against. Future unit tests will use this infrastructure to
> test their commands against a running mesos cluster. The tests require
> some binaries created when building Mesos.
> 
> 
> Diffs
> -
> 
>   src/cli_new/lib/cli/__init__.py f4fc3f18af5641a4a87143adaba81e62334ccffb 
>   src/cli_new/lib/cli/tests/__init__.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/base.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/constants.py PRE-CREATION 
>   src/cli_new/lib/cli/tests/tests.py PRE-CREATION 
>   src/cli_new/pip-requirements.txt 28613e56a5c6d5c7606a7e58d6125b0c34748e83 
>   src/cli_new/tests/main.py dff5d48b0ddae87960a78f9d05e4ae597912f1f6 
> 
> 
> Diff: https://reviews.apache.org/r/58720/diff/12/
> 
> 
> Testing
> ---
> 
> PEP8 and Pylint used to make sure that the code style is correct. Manuel test:
> 
> $ cd src/cli_new
> $ ./bootstrap
> $ source activate
> 
> (mesos-cli) $ mesos-cli-tests
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>



Re: Review Request 49571: Added a benchmark test for allocations.

2017-05-25 Thread Anindya Sinha

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

(Updated May 25, 2017, 11:30 p.m.)


Review request for mesos, James Peach and Jiang Yan Xu.


Changes
---

Addressed review comments.


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


Repository: mesos


Description
---

Allocations test has the following resource configurations:
(1) REGULAR: Offers from every slave have regular resources.
(2) SHARED: Offers from every slave include a shared resource.
(3) REGULAR: Offers from every alternate slave contain only regular
resources; and offers from every other alternate slave contains
a shared resource.

This test is parameterized based on number of agents, number of
frameworks and resource configuration.


Diffs (updated)
-

  src/tests/hierarchical_allocator_tests.cpp 
eb2b647d3247a85e8b9b82e5589232c74ad8570f 
  src/tests/resources_utils.hpp 1f41f02babce5c8174ea2223f4dc7470452fbaf1 
  src/tests/resources_utils.cpp 2cef55f7312d671307e097c2c4960c8dcf45c1ff 


Diff: https://reviews.apache.org/r/49571/diff/36/

Changes: https://reviews.apache.org/r/49571/diff/35-36/


Testing
---

All tests passed.

Allocations benchmark test results
==
There is no visible impact in performance when shared resources are added in 
the allocations. The numbers for HEAD are prior to shared resources support 
(mid 2016) and the numbers indicate improvements in allocations during this 
timeframe.

Following is a snapshot with 1000 agents and 200 frameworks.

With the patch (and no shared resources)

[ RUN  ] AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/9
Using 1000 agents and 200 frameworks with resource type 0
Added 200 frameworks in 6588us
Added 1000 agents in 1.567347secs
round 0 allocate() took 1.15531secs to make 1000 offers
round 10 allocate() took 1.152876secs to make 1000 offers
round 20 allocate() took 1.15661secs to make 1000 offers
round 30 allocate() took 1.117733secs to make 1000 offers
round 40 allocate() took 1.118754secs to make 1000 offers
round 50 allocate() took 1.11169secs to make 1000 offers

With the patch (and shared resources on all agents)
---
[ RUN  ] AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/10
Using 1000 agents and 200 frameworks with resource type 1
Added 200 frameworks in 6064us
Added 1000 agents in 1.627008secs
round 0 allocate() took 1.168253secs to make 1000 offers
round 10 allocate() took 1.146421secs to make 1000 offers
round 20 allocate() took 1.16416secs to make 1000 offers
round 30 allocate() took 1.210476secs to make 1000 offers
round 40 allocate() took 1.194251secs to make 1000 offers
round 50 allocate() took 1.17789secs to make 1000 offers

With the patch (and shared resources on alternate agents)
-
[ RUN  ] AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/11
Using 1000 agents and 200 frameworks with resource type 2
Added 200 frameworks in 6466us
Added 1000 agents in 1.568717secs
round 0 allocate() took 1.153005secs to make 1000 offers
round 10 allocate() took 1.168169secs to make 1000 offers
round 20 allocate() took 1.156774secs to make 1000 offers
round 30 allocate() took 1.183112secs to make 1000 offers
round 40 allocate() took 1.202452secs to make 1000 offers
round 50 allocate() took 1.198918secs to make 1000 offers

Based on HEAD, with all regular resources (no shared resources in HEAD 
supported)
-
[ RUN  ] AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/9
Using 1000 agents and 200 frameworks with resource type 0
Added 200 frameworks in 6801us
Added 1000 agents in 1.721447secs
round 0 allocate() took 1.502953secs to make 1000 offers
round 50 allocate() took 1.520157secs to make 1000 offers
round 100 allocate() took 1.517221secs to make 1000 offers
round 150 allocate() took 1.526446secs to make 1000 offers
round 199 allocate() took 1.538005secs to make 1000 offers


Thanks,

Anindya Sinha



Re: Review Request 49571: Added a benchmark test for allocations.

2017-05-25 Thread Anindya Sinha


> On May 24, 2017, 11:35 p.m., James Peach wrote:
> > src/tests/hierarchical_allocator_tests.cpp
> > Lines 4972 (patched)
> > 
> >
> > Why do we need to loop that many times? I don't think you'd expect the 
> > performance to vary much over this range because tou are always recovering 
> > the resources so the allocation state doesn't change substantially.

I now run the loop 6 times to ensure we cover each framework which should be 
`max(number_of_frameworks)/min(number_of_agents)`, i.e. `6000/1000 = 6`.


> On May 24, 2017, 11:35 p.m., James Peach wrote:
> > src/tests/hierarchical_allocator_tests.cpp
> > Lines 4995 (patched)
> > 
> >
> > I found that this output wasn't very helpful. How about running through 
> > the offer cycle N times and collecting the results in a 
> > `process::TimeSeries`, then showing the `process::Statistics` once at the 
> > end of the test?

I think we should defer this to a future commit to do this for all benchmarks.


- Anindya


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


On May 25, 2017, 11:30 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49571/
> ---
> 
> (Updated May 25, 2017, 11:30 p.m.)
> 
> 
> Review request for mesos, James Peach and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-5771
> https://issues.apache.org/jira/browse/MESOS-5771
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allocations test has the following resource configurations:
> (1) REGULAR: Offers from every slave have regular resources.
> (2) SHARED: Offers from every slave include a shared resource.
> (3) REGULAR: Offers from every alternate slave contain only regular
> resources; and offers from every other alternate slave contains
> a shared resource.
> 
> This test is parameterized based on number of agents, number of
> frameworks and resource configuration.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> eb2b647d3247a85e8b9b82e5589232c74ad8570f 
>   src/tests/resources_utils.hpp 1f41f02babce5c8174ea2223f4dc7470452fbaf1 
>   src/tests/resources_utils.cpp 2cef55f7312d671307e097c2c4960c8dcf45c1ff 
> 
> 
> Diff: https://reviews.apache.org/r/49571/diff/36/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> Allocations benchmark test results
> ==
> There is no visible impact in performance when shared resources are added in 
> the allocations. The numbers for HEAD are prior to shared resources support 
> (mid 2016) and the numbers indicate improvements in allocations during this 
> timeframe.
> 
> Following is a snapshot with 1000 agents and 200 frameworks.
> 
> With the patch (and no shared resources)
> 
> [ RUN  ] AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/9
> Using 1000 agents and 200 frameworks with resource type 0
> Added 200 frameworks in 6588us
> Added 1000 agents in 1.567347secs
> round 0 allocate() took 1.15531secs to make 1000 offers
> round 10 allocate() took 1.152876secs to make 1000 offers
> round 20 allocate() took 1.15661secs to make 1000 offers
> round 30 allocate() took 1.117733secs to make 1000 offers
> round 40 allocate() took 1.118754secs to make 1000 offers
> round 50 allocate() took 1.11169secs to make 1000 offers
> 
> With the patch (and shared resources on all agents)
> ---
> [ RUN  ] 
> AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/10
> Using 1000 agents and 200 frameworks with resource type 1
> Added 200 frameworks in 6064us
> Added 1000 agents in 1.627008secs
> round 0 allocate() took 1.168253secs to make 1000 offers
> round 10 allocate() took 1.146421secs to make 1000 offers
> round 20 allocate() took 1.16416secs to make 1000 offers
> round 30 allocate() took 1.210476secs to make 1000 offers
> round 40 allocate() took 1.194251secs to make 1000 offers
> round 50 allocate() took 1.17789secs to make 1000 offers
> 
> With the patch (and shared resources on alternate agents)
> -
> [ RUN  ] 
> AllResources/HierarchicalAllocations_BENCHMARK_Test.Allocations/11
> Using 1000 agents and 200 frameworks with resource type 2
> Added 200 frameworks in 6466us
> Added 1000 agents in 1.568717secs
> round 0 allocate() took 1.153005secs to make 1000 offers
> round 10 allocate() took 1.168169secs to make 1000 offers
> round 20 allocate() took 1.156774secs to make 1000 offers
> round 30 allocat

Re: Review Request 59186: Additional linux/capabilities isolator documentation.

2017-05-25 Thread Mesos Reviewbot Windows

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



Patch looks great!

Reviews applied: [59547, 59548, 59549, 59550, 59551, 59552, 59185, 59553, 
59554, 59186]

Passed command: support\windows-build.bat

- Mesos Reviewbot Windows


On May 24, 2017, 4:46 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59186/
> ---
> 
> (Updated May 24, 2017, 4:46 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7477
> https://issues.apache.org/jira/browse/MESOS-7477
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Linux capabilities are somewhat involved, so add some additional
> exposition of how the linux/capabilites isolator handles them.
> 
> 
> Diffs
> -
> 
>   docs/linux_capabilities.md b588aff6842a14bbf7ff5c35931cac61f9019805 
> 
> 
> Diff: https://reviews.apache.org/r/59186/diff/3/
> 
> 
> Testing
> ---
> 
> None.
> 
> 
> Thanks,
> 
> James Peach
> 
>



Review Request 59583: Added a test to verify executor driver message dropping behavior.

2017-05-25 Thread Greg Mann

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

Review request for mesos, Anand Mazumdar, Benjamin Mahler, and Vinod Kone.


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


Repository: mesos


Description
---

This patch adds a test which verifies that the executor driver will drop
RunTaskMessages when it is not connected to the agent.


Diffs
-

  src/tests/slave_tests.cpp d124d594fb4f20db3f5dacbc7482d97161cca2df 


Diff: https://reviews.apache.org/r/59583/diff/1/


Testing
---

`GTEST_FILTER="*DisconnectedExecutorDropsMessages*" bin/mesos-tests.sh 
--gtest_repeat=-1 --gtest_break_on_failure`


Thanks,

Greg Mann



Review Request 59582: Made the executor driver drop some messages when not connected.

2017-05-25 Thread Greg Mann

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

Review request for mesos, Anand Mazumdar, Benjamin Mahler, and Vinod Kone.


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


Repository: mesos


Description
---

This patch updates the executor driver to drop RunTaskMessages,
FrameworkMessages, and StatusUpdateAcknowledgements when it is
not connected to the agent. This facilitates executor reconnect
retry logic which is being added to the agent.


Diffs
-

  src/exec/exec.cpp f36e3707d8804b63ec1ca4e7f16728210f0029de 


Diff: https://reviews.apache.org/r/59582/diff/1/


Testing
---

Testing details can be found at the end of this chain.


Thanks,

Greg Mann



Re: Review Request 59556: Windows: Updated build documentation.

2017-05-25 Thread Andrew Schwartzmeyer


> On May 25, 2017, 9:20 p.m., Joseph Wu wrote:
> > docs/windows.md
> > Lines 10-13 (original), 10-11 (patched)
> > 
> >
> > These double-newlines are part of the markdown style guidelines we use 
> > in Mesos docs.

Oooh.


- Andrew


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


On May 24, 2017, 11:38 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59556/
> ---
> 
> (Updated May 24, 2017, 11:38 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Updated build documentation.
> 
> 
> Diffs
> -
> 
>   docs/windows.md b070c64af508479996d6ef6c3b198c433b5dc600 
> 
> 
> Diff: https://reviews.apache.org/r/59556/diff/1/
> 
> 
> Testing
> ---
> 
> It's build docs, please review. It's somewhat better at least.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Review Request 59581: Resolved a TODO that depended on a newer version of gtest.

2017-05-25 Thread Gastón Kleiman

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
---

Resolved a TODO that depended on a newer version of gtest.


Diffs
-

  src/tests/default_executor_tests.cpp 22af7e973f8e6ca583c3126a80bc092bf88fea33 


Diff: https://reviews.apache.org/r/59581/diff/1/


Testing
---

`make tests` on GNU/Linux


Thanks,

Gastón Kleiman



Re: Review Request 59500: Added Windows ReviewBot launch script.

2017-05-25 Thread Andrew Schwartzmeyer

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

(Updated May 25, 2017, 10:20 p.m.)


Review request for mesos and Joseph Wu.


Changes
---

Address review.


Repository: mesos


Description
---

This adds a support script used to run the Windows ReviewBot in a loop,
while properly logging the output and providing a URL for the log.


Diffs (updated)
-

  support/mesos-reviewbot.ps1 PRE-CREATION 
  support/verify-reviews.py 391bef5c15a7399f037e54600d1b13c9bd261811 


Diff: https://reviews.apache.org/r/59500/diff/2/

Changes: https://reviews.apache.org/r/59500/diff/1-2/


Testing
---


Thanks,

Andrew Schwartzmeyer



Re: Review Request 59500: Added Windows ReviewBot launch script.

2017-05-25 Thread Andrew Schwartzmeyer


> On May 25, 2017, 9:35 p.m., Joseph Wu wrote:
> > support/mesos-reviewbot.ps1
> > Lines 1 (patched)
> > 
> >
> > Need one of these at the top:
> > ```
> > # Licensed to the Apache Software Foundation (ASF) under one
> > # or more contributor license agreements.  See the NOTICE file
> > # distributed with this work for additional information
> > # regarding copyright ownership.  The ASF licenses this file
> > # to you under the Apache License, Version 2.0 (the
> > # "License"); you may not use this file except in compliance
> > # with the License.  You may obtain a copy of the License at
> > #
> > # http://www.apache.org/licenses/LICENSE-2.0
> > #
> > # Unless required by applicable law or agreed to in writing, software
> > # distributed under the License is distributed on an "AS IS" BASIS,
> > # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or 
> > implied.
> > # See the License for the specific language governing permissions and
> > # limitations under the License.
> > ```

Thanks!


> On May 25, 2017, 9:35 p.m., Joseph Wu wrote:
> > support/mesos-reviewbot.ps1
> > Lines 8 (patched)
> > 
> >
> > Did you leave this URL here intentionally?  Seems like something the 
> > user should specify.

I did, since it was what I'm using and it was easier, but I can prompt for it.


> On May 25, 2017, 9:35 p.m., Joseph Wu wrote:
> > support/mesos-reviewbot.ps1
> > Lines 10-16 (patched)
> > 
> >
> > The python check is fine, but adding the default install location 
> > doesn't seem necessary.

But it was handy! I'll throw an error instead.


> On May 25, 2017, 9:35 p.m., Joseph Wu wrote:
> > support/mesos-reviewbot.ps1
> > Lines 21-22 (patched)
> > 
> >
> > The `windows-build.bat` script defaults to this generator now.

Oh yeah, thanks.


> On May 25, 2017, 9:35 p.m., Joseph Wu wrote:
> > support/mesos-reviewbot.ps1
> > Lines 25 (patched)
> > 
> >
> > Does `Sort-Object` sort alphabetically?  Or is it smart enough to sort 
> > `10` after `9`?

I thought it did; I was wrong. Fixing.


> On May 25, 2017, 9:35 p.m., Joseph Wu wrote:
> > support/mesos-reviewbot.ps1
> > Lines 28 (patched)
> > 
> >
> > `verify-reviews.py` concatentates `BUILD_URL` + `console`.  So wouldn't 
> > the url end up being `$folder/$i/console/console`?

Yeah... I must have changed this before posting but without testing. My bad.


- Andrew


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


On May 23, 2017, 8:19 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59500/
> ---
> 
> (Updated May 23, 2017, 8:19 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds a support script used to run the Windows ReviewBot in a loop,
> while properly logging the output and providing a URL for the log.
> 
> 
> Diffs
> -
> 
>   support/mesos-reviewbot.ps1 PRE-CREATION 
>   support/verify-reviews.py 391bef5c15a7399f037e54600d1b13c9bd261811 
> 
> 
> Diff: https://reviews.apache.org/r/59500/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 59500: Added Windows ReviewBot launch script.

2017-05-25 Thread Joseph Wu

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




support/mesos-reviewbot.ps1
Lines 1 (patched)


Need one of these at the top:
```
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements.  See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership.  The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License.  You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
```



support/mesos-reviewbot.ps1
Lines 8 (patched)


Did you leave this URL here intentionally?  Seems like something the user 
should specify.



support/mesos-reviewbot.ps1
Lines 10-16 (patched)


The python check is fine, but adding the default install location doesn't 
seem necessary.



support/mesos-reviewbot.ps1
Lines 21-22 (patched)


The `windows-build.bat` script defaults to this generator now.



support/mesos-reviewbot.ps1
Lines 25 (patched)


Does `Sort-Object` sort alphabetically?  Or is it smart enough to sort `10` 
after `9`?



support/mesos-reviewbot.ps1
Lines 28 (patched)


`verify-reviews.py` concatentates `BUILD_URL` + `console`.  So wouldn't the 
url end up being `$folder/$i/console/console`?


- Joseph Wu


On May 23, 2017, 1:19 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59500/
> ---
> 
> (Updated May 23, 2017, 1:19 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds a support script used to run the Windows ReviewBot in a loop,
> while properly logging the output and providing a URL for the log.
> 
> 
> Diffs
> -
> 
>   support/mesos-reviewbot.ps1 PRE-CREATION 
>   support/verify-reviews.py 391bef5c15a7399f037e54600d1b13c9bd261811 
> 
> 
> Diff: https://reviews.apache.org/r/59500/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 59536: CMake: Added SHA256 hashes for 3rdparty downloads.

2017-05-25 Thread Joseph Wu

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


Ship it!




Ship It!

- Joseph Wu


On May 24, 2017, 12:13 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59536/
> ---
> 
> (Updated May 24, 2017, 12:13 p.m.)
> 
> 
> Review request for mesos, Aaron Wood and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `ExternalProject_Add` uses this hash to verify the tarballs
> automatically. This removes the following warning:
> 
> File will not be verified since no URL_HASH specified
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 03f314fdbb8216ca21abed3108e6986d037b2019 
>   3rdparty/cmake/Versions.cmake d3f572c0b54fea4494fee033c549fa47c301 
> 
> 
> Diff: https://reviews.apache.org/r/59536/diff/1/
> 
> 
> Testing
> ---
> 
> make check on Linux, cmake --build . --target mesos-tests and stout-tests on 
> Windows.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 59556: Windows: Updated build documentation.

2017-05-25 Thread Joseph Wu

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


Ship it!




I can make some of these tweaks before committing.


docs/windows.md
Lines 10-13 (original), 10-11 (patched)


These double-newlines are part of the markdown style guidelines we use in 
Mesos docs.



docs/windows.md
Lines 39 (patched)


s/not/note/



docs/windows.md
Lines 68-69 (original), 77-78 (patched)


This can be updated.  The master can be run in a non-HA setup now.



docs/windows.md
Lines 79-82 (original), 82-85 (patched)


Docker containerizer works now (sorta).  So we can remove this point.


- Joseph Wu


On May 24, 2017, 4:38 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59556/
> ---
> 
> (Updated May 24, 2017, 4:38 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Updated build documentation.
> 
> 
> Diffs
> -
> 
>   docs/windows.md b070c64af508479996d6ef6c3b198c433b5dc600 
> 
> 
> Diff: https://reviews.apache.org/r/59556/diff/1/
> 
> 
> Testing
> ---
> 
> It's build docs, please review. It's somewhat better at least.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 59353: Enabled DOCKER and ROOT filter flags on Windows.

2017-05-25 Thread Li Li

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


Ship it!




Ship It!

- Li Li


On May 17, 2017, 9:35 p.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59353/
> ---
> 
> (Updated May 17, 2017, 9:35 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Jeff Coffler, Joseph Wu, and 
> Li Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled DOCKER and ROOT filter flags on Windows.
> 
> 
> Diffs
> -
> 
>   src/tests/CMakeLists.txt 9f2af9cdd1cf50485f4cd84ce67bcceba64b9328 
>   src/tests/containerizer/docker_tests.cpp 
> 3d3c9afbe81ebdcdf7c5542c56f9ca0e17e2484d 
>   src/tests/default_executor_tests.cpp 
> 22af7e973f8e6ca583c3126a80bc092bf88fea33 
>   src/tests/environment.cpp 047798c781707c42641baa9473177ae25a451989 
>   src/tests/health_check_tests.cpp 6c1b9a0ead6edb34c20cf4973fffcff913c5d7ad 
>   src/tests/hook_tests.cpp 02d8f800c3eb9b1e617a14c78c2ef1e45d1c72bb 
>   src/tests/slave_tests.cpp 8f81f77aa29d16616d5a38197729baab0fb0cab5 
> 
> 
> Diff: https://reviews.apache.org/r/59353/diff/1/
> 
> 
> Testing
> ---
> 
> Tested on both Windows and Linux.  Everything passes on Linux, but there are 
> a number of tests that are failing on Windows. The tests that are failing on 
> Windows are related to the tests that are currently failing on master on 
> Windows. These tests are failing due to some long path changes in the Windows 
> creators update, and I believe Andy is working on fixes to them.
> 
> 
> Thanks,
> 
> John Kordich
> 
>



Re: Review Request 59353: Enabled DOCKER and ROOT filter flags on Windows.

2017-05-25 Thread Li Li

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


Ship it!




Ship It!

- Li Li


On May 17, 2017, 9:35 p.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59353/
> ---
> 
> (Updated May 17, 2017, 9:35 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Jeff Coffler, Joseph Wu, and 
> Li Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled DOCKER and ROOT filter flags on Windows.
> 
> 
> Diffs
> -
> 
>   src/tests/CMakeLists.txt 9f2af9cdd1cf50485f4cd84ce67bcceba64b9328 
>   src/tests/containerizer/docker_tests.cpp 
> 3d3c9afbe81ebdcdf7c5542c56f9ca0e17e2484d 
>   src/tests/default_executor_tests.cpp 
> 22af7e973f8e6ca583c3126a80bc092bf88fea33 
>   src/tests/environment.cpp 047798c781707c42641baa9473177ae25a451989 
>   src/tests/health_check_tests.cpp 6c1b9a0ead6edb34c20cf4973fffcff913c5d7ad 
>   src/tests/hook_tests.cpp 02d8f800c3eb9b1e617a14c78c2ef1e45d1c72bb 
>   src/tests/slave_tests.cpp 8f81f77aa29d16616d5a38197729baab0fb0cab5 
> 
> 
> Diff: https://reviews.apache.org/r/59353/diff/1/
> 
> 
> Testing
> ---
> 
> Tested on both Windows and Linux.  Everything passes on Linux, but there are 
> a number of tests that are failing on Windows. The tests that are failing on 
> Windows are related to the tests that are currently failing on master on 
> Windows. These tests are failing due to some long path changes in the Windows 
> creators update, and I believe Andy is working on fixes to them.
> 
> 
> Thanks,
> 
> John Kordich
> 
>



Re: Review Request 59557: Windows: Bumped required CMake version to 3.8.1.

2017-05-25 Thread Joseph Wu

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


Ship it!




There's no particular reason to require `3.8.1` instead of `3.8.0` (see patch 
notes: https://blog.kitware.com/cmake-3-8-1-available-for-download/ ).  We use 
features from `3.8`, but not from the patch release.

(As discussed offline, I'll lower the requirement before committing.)

- Joseph Wu


On May 24, 2017, 4:37 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59557/
> ---
> 
> (Updated May 24, 2017, 4:37 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Bugs: MESOS-7351
> https://issues.apache.org/jira/browse/MESOS-7351
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The 3.7.x series has a bug where it cannot find Visual Studio 2017
> tools. See https://gitlab.kitware.com/cmake/cmake/issues/16732
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 4ba6dd04ad594b0087784d12b785894d9ae257ac 
> 
> 
> Diff: https://reviews.apache.org/r/59557/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 59545: Added a test to verify the agent flag 'executor_reregistration_timeout'.

2017-05-25 Thread Greg Mann

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

(Updated May 25, 2017, 7:27 p.m.)


Review request for mesos, Anand Mazumdar, Benjamin Mahler, and Vinod Kone.


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


Repository: mesos


Description
---

This patch adds a test to verify the correct behavior of
the agent flag 'executor_reregistration_timeout.


Diffs (updated)
-

  src/tests/slave_tests.cpp d124d594fb4f20db3f5dacbc7482d97161cca2df 


Diff: https://reviews.apache.org/r/59545/diff/3/

Changes: https://reviews.apache.org/r/59545/diff/2-3/


Testing
---

`bin/mesos-tests.sh --gtest_filter="*ExecutorReregistrationTimeoutFlag*" 
--gtest_repeat=-1 --gtest_break_on_failure`


Thanks,

Greg Mann



Re: Review Request 59536: CMake: Added SHA256 hashes for 3rdparty downloads.

2017-05-25 Thread Mesos Reviewbot

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



Patch looks great!

Reviews applied: [59536]

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 May 24, 2017, 7:13 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59536/
> ---
> 
> (Updated May 24, 2017, 7:13 p.m.)
> 
> 
> Review request for mesos, Aaron Wood and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `ExternalProject_Add` uses this hash to verify the tarballs
> automatically. This removes the following warning:
> 
> File will not be verified since no URL_HASH specified
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 03f314fdbb8216ca21abed3108e6986d037b2019 
>   3rdparty/cmake/Versions.cmake d3f572c0b54fea4494fee033c549fa47c301 
> 
> 
> Diff: https://reviews.apache.org/r/59536/diff/1/
> 
> 
> Testing
> ---
> 
> make check on Linux, cmake --build . --target mesos-tests and stout-tests on 
> Windows.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Review Request 59579: Fixed flakiness in MasterTest.EndpointsForHalfRemovedSlave.

2017-05-25 Thread Neil Conway

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

Review request for mesos and Anand Mazumdar.


Repository: mesos


Description
---

The test had a race between setting up an expectation and dispatching an
asynchronous operation that would satisfy the expectation (the former
needs to happen first).


Diffs
-

  src/tests/master_tests.cpp 1dfe5fd7c33c3c479175aa522fef1956f11f265c 


Diff: https://reviews.apache.org/r/59579/diff/1/


Testing
---

`make check`


Thanks,

Neil Conway



Review Request 59578: Made MasterTest.MaxCompletedTasksPerFrameworkFlag less fragile.

2017-05-25 Thread Neil Conway

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

Review request for mesos and Kevin Klues.


Repository: mesos


Description
---

Rather than depending on batch allocations to eventually occur, pause
the clock and explicitly advance it when we want to trigger a batch
allocation. This improves both test robustness and speed.


Diffs
-

  src/tests/master_tests.cpp 1dfe5fd7c33c3c479175aa522fef1956f11f265c 


Diff: https://reviews.apache.org/r/59578/diff/1/


Testing
---

`make check`


Thanks,

Neil Conway



Re: Review Request 59541: Fixed flakiness in MasterAllocatorTest.FrameworkExited.

2017-05-25 Thread Anand Mazumdar

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


Ship it!




LGTM!

- Anand Mazumdar


On May 25, 2017, 6:36 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59541/
> ---
> 
> (Updated May 25, 2017, 6:36 p.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Bugs: MESOS-7552
> https://issues.apache.org/jira/browse/MESOS-7552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test did not pause the clock; hence, batch allocations could occur
> at unpredictable times and cause the test to fail.
> 
> Fix this by pausing the clock and explicitly advancing it as needed to
> trigger batch allocations or other events.
> 
> 
> Diffs
> -
> 
>   src/tests/master_allocator_tests.cpp 
> 750e030d388d5469399bc79fa723095e62b8f2c5 
> 
> 
> Diff: https://reviews.apache.org/r/59541/diff/2/
> 
> 
> Testing
> ---
> 
> `./src/mesos-tests --gtest_filter="MasterAllocatorTest/0.FrameworkExited" 
> --gtest_break_on_failure --gtest_repeat=5000` # Linux
> 
> Without this RR, the test fails within a few hundred iterations.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 59541: Fixed flakiness in MasterAllocatorTest.FrameworkExited.

2017-05-25 Thread Neil Conway

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

(Updated May 25, 2017, 6:36 p.m.)


Review request for mesos and Anand Mazumdar.


Changes
---

Address review comments.


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


Repository: mesos


Description
---

This test did not pause the clock; hence, batch allocations could occur
at unpredictable times and cause the test to fail.

Fix this by pausing the clock and explicitly advancing it as needed to
trigger batch allocations or other events.


Diffs (updated)
-

  src/tests/master_allocator_tests.cpp 750e030d388d5469399bc79fa723095e62b8f2c5 


Diff: https://reviews.apache.org/r/59541/diff/2/

Changes: https://reviews.apache.org/r/59541/diff/1-2/


Testing
---

`./src/mesos-tests --gtest_filter="MasterAllocatorTest/0.FrameworkExited" 
--gtest_break_on_failure --gtest_repeat=5000` # Linux

Without this RR, the test fails within a few hundred iterations.


Thanks,

Neil Conway



Re: Review Request 59541: Fixed flakiness in MasterAllocatorTest.FrameworkExited.

2017-05-25 Thread Neil Conway


> On May 25, 2017, 3:58 a.m., Anand Mazumdar wrote:
> > src/tests/master_allocator_tests.cpp
> > Lines 719-720 (patched)
> > 
> >
> > hmm, not immediately clear to me what guarrantees that this method is 
> > invoked once before the end of the function?
> > 
> > - `driver.stop()` would send the teardown message to the master and 
> > immediately trigger the latch making `driver.join()` return.
> > - If the function finishes before the message is processed by the 
> > master, `removeFramework()` might not be invoked.

Good catch -- I'm not sure offhand if that sequence is possible, but the intent 
here is just to reset the expectation to the default value, so `DoRepeatedly()` 
is probably better anyway.


- Neil


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


On May 24, 2017, 9:32 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59541/
> ---
> 
> (Updated May 24, 2017, 9:32 p.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> Bugs: MESOS-7552
> https://issues.apache.org/jira/browse/MESOS-7552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test did not pause the clock; hence, batch allocations could occur
> at unpredictable times and cause the test to fail.
> 
> Fix this by pausing the clock and explicitly advancing it as needed to
> trigger batch allocations or other events.
> 
> 
> Diffs
> -
> 
>   src/tests/master_allocator_tests.cpp 
> 750e030d388d5469399bc79fa723095e62b8f2c5 
> 
> 
> Diff: https://reviews.apache.org/r/59541/diff/1/
> 
> 
> Testing
> ---
> 
> `./src/mesos-tests --gtest_filter="MasterAllocatorTest/0.FrameworkExited" 
> --gtest_break_on_failure --gtest_repeat=5000` # Linux
> 
> Without this RR, the test fails within a few hundred iterations.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 59545: Added a test to verify the agent flag 'executor_reregistration_timeout'.

2017-05-25 Thread Vinod Kone

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


Fix it, then Ship it!





src/tests/slave_tests.cpp
Line 7069 (original), 7069 (patched)


no need for this.



src/tests/slave_tests.cpp
Lines 7075 (patched)


s/ackCheckpoint/_statusUpdateAcknowledgement/



src/tests/slave_tests.cpp
Line 7077 (original), 7080 (patched)


no need for this.



src/tests/slave_tests.cpp
Lines 7084-7086 (original), 7088-7090 (patched)


no need for this.



src/tests/slave_tests.cpp
Line 7118 (original), 7125 (patched)


no need for this.


- Vinod Kone


On May 25, 2017, 6:01 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59545/
> ---
> 
> (Updated May 25, 2017, 6:01 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-7540
> https://issues.apache.org/jira/browse/MESOS-7540
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a test to verify the correct behavior of
> the agent flag 'executor_reregistration_timeout.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp d124d594fb4f20db3f5dacbc7482d97161cca2df 
> 
> 
> Diff: https://reviews.apache.org/r/59545/diff/2/
> 
> 
> Testing
> ---
> 
> `bin/mesos-tests.sh --gtest_filter="*ExecutorReregistrationTimeoutFlag*" 
> --gtest_repeat=-1 --gtest_break_on_failure`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 58515: Update 3rdparty build to support protobuf 3.3.0.

2017-05-25 Thread Zhitao Li

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

(Updated May 25, 2017, 6:15 p.m.)


Review request for mesos, Anand Mazumdar and Joseph Wu.


Changes
---

Fix typo for CMake


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


Repository: mesos


Description
---

Update 3rdparty build to support protobuf 3.3.0.


Diffs (updated)
-

  3rdparty/cmake/Versions.cmake 728f88fe57aef4dd7be1d40c7c0227ad2db5015e 
  3rdparty/versions.am b8144702adca9dd41199f69cb6fb0c0c4b490dbd 


Diff: https://reviews.apache.org/r/58515/diff/5/

Changes: https://reviews.apache.org/r/58515/diff/4-5/


Testing
---


Thanks,

Zhitao Li



Re: Review Request 59413: Fix bytes.hpp constexpr compilation failure with GCC 7.1.

2017-05-25 Thread Aaron Wood via Review Board

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

(Updated May 25, 2017, 6:13 p.m.)


Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and Neil 
Conway.


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


Repository: mesos


Description
---

Many of the `constexpr` variables fail to compile with errors such as `error: 
'Megabytes(32).Megabytes::' is not a constant expression because it 
refers to an incompletely initialized variable`. 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80829

This patch changes around the `Bytes` class a bit by getting rid of the small 
classes that extend `Bytes`.


Diffs
-

  3rdparty/stout/include/stout/bytes.hpp 98223db50 


Diff: https://reviews.apache.org/r/59413/diff/5/


Testing (updated)
---

`./bootstrap && mkdir build && cd build && ../configure --disable-python 
--disable-java --disable-optimize --disable-hardening &&  make -j$(nproc) && 
make check -j$(nproc)`


Thanks,

Aaron Wood



Re: Review Request 59556: Windows: Updated build documentation.

2017-05-25 Thread Li Li

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


Ship it!




Ship It!

- Li Li


On May 24, 2017, 11:38 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59556/
> ---
> 
> (Updated May 24, 2017, 11:38 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Updated build documentation.
> 
> 
> Diffs
> -
> 
>   docs/windows.md b070c64af508479996d6ef6c3b198c433b5dc600 
> 
> 
> Diff: https://reviews.apache.org/r/59556/diff/1/
> 
> 
> Testing
> ---
> 
> It's build docs, please review. It's somewhat better at least.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 59545: Added a test to verify the agent flag 'executor_reregistration_timeout'.

2017-05-25 Thread Greg Mann

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

(Updated May 25, 2017, 6:01 p.m.)


Review request for mesos, Anand Mazumdar, Benjamin Mahler, and Vinod Kone.


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


Repository: mesos


Description
---

This patch adds a test to verify the correct behavior of
the agent flag 'executor_reregistration_timeout.


Diffs (updated)
-

  src/tests/slave_tests.cpp d124d594fb4f20db3f5dacbc7482d97161cca2df 


Diff: https://reviews.apache.org/r/59545/diff/2/

Changes: https://reviews.apache.org/r/59545/diff/1-2/


Testing
---

`bin/mesos-tests.sh --gtest_filter="*ExecutorReregistrationTimeoutFlag*" 
--gtest_repeat=-1 --gtest_break_on_failure`


Thanks,

Greg Mann



Re: Review Request 59413: Fix bytes.hpp constexpr compilation failure with GCC 7.1.

2017-05-25 Thread Aaron Wood via Review Board

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

(Updated May 25, 2017, 4:42 p.m.)


Review request for mesos, Benjamin Bannier, James Peach, Michael Park, and Neil 
Conway.


Changes
---

Fixed formatting.


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


Repository: mesos


Description
---

Many of the `constexpr` variables fail to compile with errors such as `error: 
'Megabytes(32).Megabytes::' is not a constant expression because it 
refers to an incompletely initialized variable`. 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80829

This patch changes around the `Bytes` class a bit by getting rid of the small 
classes that extend `Bytes`.


Diffs (updated)
-

  3rdparty/stout/include/stout/bytes.hpp 98223db50 


Diff: https://reviews.apache.org/r/59413/diff/5/

Changes: https://reviews.apache.org/r/59413/diff/4-5/


Testing
---

`./bootstrap && mkdir build && cd build && ../configure --disable-python 
--disable-java --disable-optimize --disable-hardening &&  make -j$(nproc)`


Thanks,

Aaron Wood



Re: Review Request 59537: Fixed a bug in 'ComposingContainerizerProcess::wait()'.

2017-05-25 Thread Gastón Kleiman

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

(Updated May 25, 2017, 4:33 p.m.)


Review request for mesos, Alexander Rukletsov, Jie Yu, Kevin Klues, and Vinod 
Kone.


Changes
---

Improved the comment.


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


Repository: mesos


Description
---

Fixed a bug in the Composing Containerizer that would make it always
immediately return 'None' when trying to wait on a nested container
that had already been terminated and whose exit status was checkpointed.


Diffs (updated)
-

  src/slave/containerizer/composing.cpp 
0b6c76b3d081d86df81a6062ae7a191ba8dadfde 


Diff: https://reviews.apache.org/r/59537/diff/2/

Changes: https://reviews.apache.org/r/59537/diff/1-2/


Testing
---

`make check` on GNU/Linux, plus manual testing using an agent that uses both 
the Docker and the Mesos containerizers.


Thanks,

Gastón Kleiman



Re: Review Request 59556: Windows: Updated build documentation.

2017-05-25 Thread Jeff Coffler

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


Ship it!




Ship It!

- Jeff Coffler


On May 24, 2017, 11:38 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59556/
> ---
> 
> (Updated May 24, 2017, 11:38 p.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Windows: Updated build documentation.
> 
> 
> Diffs
> -
> 
>   docs/windows.md b070c64af508479996d6ef6c3b198c433b5dc600 
> 
> 
> Diff: https://reviews.apache.org/r/59556/diff/1/
> 
> 
> Testing
> ---
> 
> It's build docs, please review. It's somewhat better at least.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 59560: Replaced "!(x == y)" with "x != y" when comparing SlaveIDs.

2017-05-25 Thread Vinod Kone

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


Ship it!




Ship It!

- Vinod Kone


On May 25, 2017, 12:37 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59560/
> ---
> 
> (Updated May 25, 2017, 12:37 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replaced "!(x == y)" with "x != y" when comparing SlaveIDs.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp f02d388a46985a3e1a8b5b0597c40865b849f86f 
>   src/slave/slave.cpp 15e4d68714556ca30a766acd3b9729367df680c3 
> 
> 
> Diff: https://reviews.apache.org/r/59560/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 59559: Replaced `.get().` with `->` in the agent.

2017-05-25 Thread Vinod Kone

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


Ship it!




Thanks for the sweep!

- Vinod Kone


On May 25, 2017, 12:37 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59559/
> ---
> 
> (Updated May 25, 2017, 12:37 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replaced `.get().` with `->` in the agent.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.cpp 0c8276e425a6a7d22ee68edc6cc25b331635ec44 
>   src/slave/http.cpp 5beaf91de1ba948658fa7fa299a364432afcf50b 
>   src/slave/main.cpp 947f1f27a3900145cb0bf036088e0ce26f40eeb3 
>   src/slave/slave.cpp 15e4d68714556ca30a766acd3b9729367df680c3 
>   src/slave/state.cpp 33dcc7a148f9a6b1a3216cce45710da8fd819ba6 
>   src/slave/status_update_manager.cpp 
> 0cd88ac3a959bc59622188c534ba61019fdbfdf1 
> 
> 
> Diff: https://reviews.apache.org/r/59559/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 59558: Replaced `.get().` with `->` for access to `Master::leader`.

2017-05-25 Thread Vinod Kone

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


Ship it!




Ship It!

- Vinod Kone


On May 25, 2017, 12:37 a.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59558/
> ---
> 
> (Updated May 25, 2017, 12:37 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Replaced `.get().` with `->` for access to `Master::leader`.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 7060b8fa53e0682681c50e051908ffbbf50fb7da 
>   src/master/master.cpp f02d388a46985a3e1a8b5b0597c40865b849f86f 
> 
> 
> Diff: https://reviews.apache.org/r/59558/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>



Re: Review Request 59545: Added a test to verify the agent flag 'executor_reregistration_timeout'.

2017-05-25 Thread Vinod Kone

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




src/tests/slave_tests.cpp
Lines 7077 (patched)


You need to wait until the acknowledgement is checkpointed by the agent to 
ensure agent doesnt retry after a restart.



src/tests/slave_tests.cpp
Lines 7136 (patched)


what's the guarantee that master finished processing the reregisterslave 
message? need a clock settle?


- Vinod Kone


On May 24, 2017, 11:10 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59545/
> ---
> 
> (Updated May 24, 2017, 11:10 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-7540
> https://issues.apache.org/jira/browse/MESOS-7540
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a test to verify the correct behavior of
> the agent flag 'executor_reregistration_timeout.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp d124d594fb4f20db3f5dacbc7482d97161cca2df 
> 
> 
> Diff: https://reviews.apache.org/r/59545/diff/1/
> 
> 
> Testing
> ---
> 
> `bin/mesos-tests.sh --gtest_filter="*ExecutorReregistrationTimeoutFlag*" 
> --gtest_repeat=-1 --gtest_break_on_failure`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 59537: Fixed a bug in 'ComposingContainerizerProcess::wait()'.

2017-05-25 Thread Jie Yu

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


Fix it, then Ship it!





src/slave/containerizer/composing.cpp
Lines 626 (patched)


I wouldn't say this is a workaround. This is a proper fix for the issue :)



src/slave/containerizer/composing.cpp
Lines 627 (patched)


kill this line


- Jie Yu


On May 24, 2017, 8:30 p.m., Gastón Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59537/
> ---
> 
> (Updated May 24, 2017, 8:30 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Jie Yu, Kevin Klues, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-7546
> https://issues.apache.org/jira/browse/MESOS-7546
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed a bug in the Composing Containerizer that would make it always
> immediately return 'None' when trying to wait on a nested container
> that had already been terminated and whose exit status was checkpointed.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.cpp 
> 0b6c76b3d081d86df81a6062ae7a191ba8dadfde 
> 
> 
> Diff: https://reviews.apache.org/r/59537/diff/1/
> 
> 
> Testing
> ---
> 
> `make check` on GNU/Linux, plus manual testing using an agent that uses both 
> the Docker and the Mesos containerizers.
> 
> 
> Thanks,
> 
> Gastón Kleiman
> 
>



Re: Review Request 59555: A prototype to move sample collection into async sampling.

2017-05-25 Thread Mesos Reviewbot

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



Bad review!

Reviews applied: []

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

- Mesos Reviewbot


On May 24, 2017, 11:35 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59555/
> ---
> 
> (Updated May 24, 2017, 11:35 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A prototype to move sample collection into async sampling.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/metrics/metrics.hpp 
> 9c32a88d851c884a5025edb6ea1e27939b484546 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> 7184aa4d0294c20466646c9aa61d90973eca22e1 
> 
> 
> Diff: https://reviews.apache.org/r/59555/diff/1/
> 
> 
> Testing
> ---
> 
> Ran this on a master and able to get result on `/metrics/cached` endpoint.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>



Re: Review Request 59545: Added a test to verify the agent flag 'executor_reregistration_timeout'.

2017-05-25 Thread Mesos Reviewbot

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



Patch looks great!

Reviews applied: [59545]

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 May 24, 2017, 11:10 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59545/
> ---
> 
> (Updated May 24, 2017, 11:10 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Benjamin Mahler, and Vinod Kone.
> 
> 
> Bugs: MESOS-7540
> https://issues.apache.org/jira/browse/MESOS-7540
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a test to verify the correct behavior of
> the agent flag 'executor_reregistration_timeout.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp d124d594fb4f20db3f5dacbc7482d97161cca2df 
> 
> 
> Diff: https://reviews.apache.org/r/59545/diff/1/
> 
> 
> Testing
> ---
> 
> `bin/mesos-tests.sh --gtest_filter="*ExecutorReregistrationTimeoutFlag*" 
> --gtest_repeat=-1 --gtest_break_on_failure`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>



Re: Review Request 58720: CLI: Extended the unit test infrastructure.

2017-05-25 Thread Armand Grillet

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

(Updated May 25, 2017, 9:08 a.m.)


Review request for mesos and Kevin Klues.


Changes
---

Lambdas removed.


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


Repository: mesos


Description
---

This infrastructure includes the ability to bring up a test cluster to
run the CLI against. Future unit tests will use this infrastructure to
test their commands against a running mesos cluster. The tests require
some binaries created when building Mesos.


Diffs (updated)
-

  src/cli_new/lib/cli/__init__.py f4fc3f18af5641a4a87143adaba81e62334ccffb 
  src/cli_new/lib/cli/tests/__init__.py PRE-CREATION 
  src/cli_new/lib/cli/tests/base.py PRE-CREATION 
  src/cli_new/lib/cli/tests/constants.py PRE-CREATION 
  src/cli_new/lib/cli/tests/tests.py PRE-CREATION 
  src/cli_new/pip-requirements.txt 28613e56a5c6d5c7606a7e58d6125b0c34748e83 
  src/cli_new/tests/main.py dff5d48b0ddae87960a78f9d05e4ae597912f1f6 


Diff: https://reviews.apache.org/r/58720/diff/11/

Changes: https://reviews.apache.org/r/58720/diff/10-11/


Testing
---

PEP8 and Pylint used to make sure that the code style is correct. Manuel test:

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

(mesos-cli) $ mesos-cli-tests


Thanks,

Armand Grillet