date:20180509

Re: Review Request 67041: Windows: Ported the rest of `path_tests.cpp`.

2018-05-09 Thread Mesos Reviewbot


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



Patch looks great!

Reviews applied: [67041]

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

- Mesos Reviewbot


On May 10, 2018, 6:07 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67041/
> ---
> 
> (Updated May 10, 2018, 6:07 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-3442
> https://issues.apache.org/jira/browse/MESOS-3442
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This port does not attempt to use `path::join()` to avoid duplicated
> test logic since that function strips the path separator from its
> arguments, thus making it difficult to correctly construct a path like
> `a//b/`. Instead, a semi-programatical approach was taken to duplicate
> the POSIX paths and then replace `/` with `\`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/path_tests.cpp 
> 433a6c60bb6991410e54ab6a6dcd2499c664c37b 
> 
> 
> Diff: https://reviews.apache.org/r/67041/diff/1/
> 
> 
> Testing
> ---
> 
> ```
> ./3rdparty/stout/tests/stout-tests.exe --gtest_filter="PathTest.*"
> Note: Google Test filter = PathTest.*-
> [==] Running 7 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 7 tests from PathTest
> [ RUN  ] PathTest.Basename
> [   OK ] PathTest.Basename (1 ms)
> [ RUN  ] PathTest.Dirname
> [   OK ] PathTest.Dirname (0 ms)
> [ RUN  ] PathTest.Extension
> [   OK ] PathTest.Extension (0 ms)
> [ RUN  ] PathTest.Join
> [   OK ] PathTest.Join (1 ms)
> [ RUN  ] PathTest.Absolute
> [   OK ] PathTest.Absolute (0 ms)
> [ RUN  ] PathTest.Comparison
> [   OK ] PathTest.Comparison (0 ms)
> [ RUN  ] PathTest.FromURI
> [   OK ] PathTest.FromURI (0 ms)
> [--] 7 tests from PathTest (2 ms total)
> 
> [--] Global test environment tear-down
> [==] 7 tests from 1 test case ran. (2 ms total)
> [  PASSED  ] 7 tests.
> 
> 3rdparty/stout/tests/stout-tests --gtest_filter="PathTest.*"
> Note: Google Test filter = PathTest.*-
> [==] Running 7 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 7 tests from PathTest
> [ RUN  ] PathTest.Basename
> [   OK ] PathTest.Basename (0 ms)
> [ RUN  ] PathTest.Dirname
> [   OK ] PathTest.Dirname (0 ms)
> [ RUN  ] PathTest.Extension
> [   OK ] PathTest.Extension (0 ms)
> [ RUN  ] PathTest.Join
> [   OK ] PathTest.Join (0 ms)
> [ RUN  ] PathTest.Absolute
> [   OK ] PathTest.Absolute (0 ms)
> [ RUN  ] PathTest.Comparison
> [   OK ] PathTest.Comparison (0 ms)
> [ RUN  ] PathTest.FromURI
> [   OK ] PathTest.FromURI (0 ms)
> [--] 7 tests from PathTest (0 ms total)
> 
> [--] Global test environment tear-down
> [==] 7 tests from 1 test case ran. (0 ms total)
> [  PASSED  ] 7 tests.
> ```
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 67051: Windows: Enabled more agent tests.

2018-05-09 Thread Mesos Reviewbot Windows


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



PASS: Mesos patch 67051 was successfully built and tested.

Reviews applied: `['67051']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67051

- Mesos Reviewbot Windows


On May 9, 2018, 7:54 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67051/
> ---
> 
> (Updated May 9, 2018, 7:54 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Chun-Hung Hsiao, Eric Mumau, John 
> Kordich, Joseph Wu, Meng Zhu, and Radhika Jandhyala.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These just worked when enabled.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 53c96302258d518f597cd8bd6d0c01896a456edf 
> 
> 
> Diff: https://reviews.apache.org/r/67051/diff/1/
> 
> 
> Testing
> ---
> 
> ```
> [==] Running 4 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 4 tests from SlaveTest
> [ RUN  ] SlaveTest.StateEndpointUnavailableDuringRecovery
> [   OK ] SlaveTest.StateEndpointUnavailableDuringRecovery (302 ms)
> [ RUN  ] SlaveTest.AgentFailoverTerminatesExecutorWithNoTask
> [   OK ] SlaveTest.AgentFailoverTerminatesExecutorWithNoTask (2557 ms)
> [ RUN  ] SlaveTest.AgentFailoverTerminatesHTTPExecutorWithNoTask
> I0509 16:58:54.789753  1312 executor.cpp:192] Version: 1.7.0
> E0509 16:58:54.817754  9864 executor.cpp:702] End-Of-File received from 
> agent. The agent closed the event stream
> I0509 16:58:54.814756 11868 default_executor.cpp:202] Received SUBSCRIBED 
> event
> I0509 16:58:54.819756 11868 default_executor.cpp:206] Subscribed executor on 
> andschwa-pc.mshome.net
> I0509 16:58:54.821755  7880 default_executor.cpp:174] Disconnected from agent
> W0509 16:58:54.826753 14992 executor.cpp:654] Received '404 Not Found' () for 
> SUBSCRIBE
> W0509 16:58:55.824151  3324 executor.cpp:769] Dropping SUBSCRIBE: Executor is 
> in state SUBSCRIBING
> W0509 16:58:56.825736  5512 executor.cpp:769] Dropping SUBSCRIBE: Executor is 
> in state SUBSCRIBED
> W0509 16:58:56.826731  5512 executor.cpp:769] Dropping SUBSCRIBE: Executor is 
> in state SUBSCRIBED
> [   OK ] SlaveTest.AgentFailoverTerminatesHTTPExecutorWithNoTask (3242 ms)
> [ RUN  ] SlaveTest.ResourceProviderPublishAll
> I0509 16:58:58.369606  3412 exec.cpp:162] Version: 1.7.0
> I0509 16:58:58.949059 14492 exec.cpp:236] Executor registered on agent 
> 94020100-3efa-43f0-bb87-c48da21310be-S0
> I0509 16:58:58.953034 14024 executor.cpp:178] Received SUBSCRIBED event
> I0509 16:58:58.956063 14024 executor.cpp:182] Subscribed executor on 
> andschwa-pc.mshome.net
> I0509 16:58:58.967048  3412 executor.cpp:178] Received LAUNCH event
> I0509 16:58:58.969033  3412 executor.cpp:665] Starting task 
> 5c6630a0-e432-47e8-9642-6467cc54e030
> I0509 16:58:59.024057  3412 executor.cpp:485] Running 
> 'C:\Users\andschwa\src\mesos\build\master\src\mesos-containerizer.exe launch 
> '
> I0509 16:58:59.531059  3412 executor.cpp:678] Forked command at 8472
> I0509 16:59:01.976801 14492 exec.cpp:445] Executor asked to shutdown
> I0509 16:59:01.976801  8324 executor.cpp:178] Received SHUTDOWN event
> I0509 16:59:01.976801  8324 executor.cpp:781] Shutting down
> I0509 16:59:01.976801  8324 executor.cpp:894] Sending SIGTERM to process tree 
> at pid 8472
> I0509 16:59[   OK ] SlaveTest.ResourceProviderPublishAll (5111 ms)
> [--] 4 tests from SlaveTest (11339 ms total)
> 
> [--] Global test environment tear-down
> [==] 4 tests from 1 test case ran. (11890 ms total)
> [  PASSED  ] 4 tests.
> ```
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66669: Added clean up of `containers_` map in composing containerizer.

2018-05-09 Thread Qian Zhang


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


Ship it!





src/slave/containerizer/composing.cpp
Line 358 (original), 358-363 (patched)


So besides removing the container from the `containers_` map, we also 
eliminate a call to `ComposingContainerizerProcess::destroy()`, My 
understanding is we assume the underlying containerizer (Mesos/Docker 
containerizer) will always do the destroy, so here in composing containerizer 
we just need to remove the container from the `containers_` map. If so, can you 
please update the commit message for that?


- Qian Zhang


On April 17, 2018, 11:23 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9/
> ---
> 
> (Updated April 17, 2018, 11:23 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, Jie Yu, and Qian 
> Zhang.
> 
> 
> Bugs: MESOS-8714
> https://issues.apache.org/jira/browse/MESOS-8714
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds callbacks on `wait()` and `destroy()` in composing
> containerizer to remove a container from the `containers_` map after
> the container's termination.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.cpp 
> 186102c66d373dcd799cadd9fed7d1c8cb894971 
> 
> 
> Diff: https://reviews.apache.org/r/9/diff/2/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Review Request 67051: Windows: Enabled more agent tests.

2018-05-09 Thread Andrew Schwartzmeyer


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

Review request for mesos, Akash Gupta, Chun-Hung Hsiao, Eric Mumau, John 
Kordich, Joseph Wu, Meng Zhu, and Radhika Jandhyala.


Repository: mesos


Description
---

These just worked when enabled.


Diffs
-

  src/tests/slave_tests.cpp 53c96302258d518f597cd8bd6d0c01896a456edf 


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


Testing
---

```
[==] Running 4 tests from 1 test case.
[--] Global test environment set-up.
[--] 4 tests from SlaveTest
[ RUN  ] SlaveTest.StateEndpointUnavailableDuringRecovery
[   OK ] SlaveTest.StateEndpointUnavailableDuringRecovery (302 ms)
[ RUN  ] SlaveTest.AgentFailoverTerminatesExecutorWithNoTask
[   OK ] SlaveTest.AgentFailoverTerminatesExecutorWithNoTask (2557 ms)
[ RUN  ] SlaveTest.AgentFailoverTerminatesHTTPExecutorWithNoTask
I0509 16:58:54.789753  1312 executor.cpp:192] Version: 1.7.0
E0509 16:58:54.817754  9864 executor.cpp:702] End-Of-File received from agent. 
The agent closed the event stream
I0509 16:58:54.814756 11868 default_executor.cpp:202] Received SUBSCRIBED event
I0509 16:58:54.819756 11868 default_executor.cpp:206] Subscribed executor on 
andschwa-pc.mshome.net
I0509 16:58:54.821755  7880 default_executor.cpp:174] Disconnected from agent
W0509 16:58:54.826753 14992 executor.cpp:654] Received '404 Not Found' () for 
SUBSCRIBE
W0509 16:58:55.824151  3324 executor.cpp:769] Dropping SUBSCRIBE: Executor is 
in state SUBSCRIBING
W0509 16:58:56.825736  5512 executor.cpp:769] Dropping SUBSCRIBE: Executor is 
in state SUBSCRIBED
W0509 16:58:56.826731  5512 executor.cpp:769] Dropping SUBSCRIBE: Executor is 
in state SUBSCRIBED
[   OK ] SlaveTest.AgentFailoverTerminatesHTTPExecutorWithNoTask (3242 ms)
[ RUN  ] SlaveTest.ResourceProviderPublishAll
I0509 16:58:58.369606  3412 exec.cpp:162] Version: 1.7.0
I0509 16:58:58.949059 14492 exec.cpp:236] Executor registered on agent 
94020100-3efa-43f0-bb87-c48da21310be-S0
I0509 16:58:58.953034 14024 executor.cpp:178] Received SUBSCRIBED event
I0509 16:58:58.956063 14024 executor.cpp:182] Subscribed executor on 
andschwa-pc.mshome.net
I0509 16:58:58.967048  3412 executor.cpp:178] Received LAUNCH event
I0509 16:58:58.969033  3412 executor.cpp:665] Starting task 
5c6630a0-e432-47e8-9642-6467cc54e030
I0509 16:58:59.024057  3412 executor.cpp:485] Running 
'C:\Users\andschwa\src\mesos\build\master\src\mesos-containerizer.exe launch 
'
I0509 16:58:59.531059  3412 executor.cpp:678] Forked command at 8472
I0509 16:59:01.976801 14492 exec.cpp:445] Executor asked to shutdown
I0509 16:59:01.976801  8324 executor.cpp:178] Received SHUTDOWN event
I0509 16:59:01.976801  8324 executor.cpp:781] Shutting down
I0509 16:59:01.976801  8324 executor.cpp:894] Sending SIGTERM to process tree 
at pid 8472
I0509 16:59[   OK ] SlaveTest.ResourceProviderPublishAll (5111 ms)
[--] 4 tests from SlaveTest (11339 ms total)

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


Thanks,

Andrew Schwartzmeyer

Re: Review Request 66668: Removed `destroyed` from `Container` struct in composing containerizer.

2018-05-09 Thread Qian Zhang



> On May 9, 2018, 11:15 p.m., Qian Zhang wrote:
> > src/slave/containerizer/composing.cpp
> > Lines 658-661 (original)
> > 
> >
> > If we remove these codes, will the container be missed to delete and 
> > erased from `containers_`?

Ah, Just saw the code to delete it in the next patch. Anyway I think it may be 
better to put that change into this patch :-)


- Qian


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


On April 17, 2018, 11:23 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8/
> ---
> 
> (Updated April 17, 2018, 11:23 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, Jie Yu, and Qian 
> Zhang.
> 
> 
> Bugs: MESOS-8712
> https://issues.apache.org/jira/browse/MESOS-8712
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, we were using `destroyed` promise for each container to
> guarantee that calling `destroy()` on a container returns a non-empty
> value when the destroy-in-progress stops an launch-in-progress using
> the next containerizer. Since we are unifying the semantics of the
> return type for both `wait()` and `destroy()` operations, we can
> remove the `destroyed` promise.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.cpp 
> 186102c66d373dcd799cadd9fed7d1c8cb894971 
> 
> 
> Diff: https://reviews.apache.org/r/8/diff/2/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 66635: Introduced wrapper for access to cgroups system access.

2018-05-09 Thread Mesos Reviewbot


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



Patch looks great!

Reviews applied: [66634, 66987, 66635]

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

- Mesos Reviewbot


On May 9, 2018, 7:55 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66635/
> ---
> 
> (Updated May 9, 2018, 7:55 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-8786
> https://issues.apache.org/jira/browse/MESOS-8786
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Different cgroups subsystems are modelled as actors. In this patch we
> introduce wrapper classes which `dispatch` to the processes. This
> removes e.g., races from mixing naked and `dispatch`'ed method calls.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> 5763c9880728f02e44116fd50e62b592a8ef69b6 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 4431ce13d28035b0c5c037b2848ae03aeaf65562 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 65c1e47a569f320b63b54e5f4fc1da374d02ee0d 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> 9afa02b207e6272836e5a36d69fb48f1f4d02150 
> 
> 
> Diff: https://reviews.apache.org/r/66635/diff/5/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66668: Removed `destroyed` from `Container` struct in composing containerizer.

2018-05-09 Thread Qian Zhang



> On May 9, 2018, 11:15 p.m., Qian Zhang wrote:
> > I checked the callers of `Containerizer::destroy()`, it seems no one 
> > actually cares about its return value (`Option`), so 
> > why do we need to return that? Can we just make it return `Future`?

And then for the last line of `ComposingContainerizerProcess::destroy`:
```
return container->containerizer->wait(containerId);
```
We could change it to something like:
```
return container->containerizer->wait(containerId)
.onAny([](const Future

Re: Review Request 66230: Added test for updating framework info.

2018-05-09 Thread Mesos Reviewbot Windows


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



FAIL: Failed to apply the dependent review: 66229.

Failed command: `python.exe .\support\apply-reviews.py -n -r 66229`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66230

Relevant logs:

- 
[apply-review-66229-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66230/logs/apply-review-66229-stdout.log):

```
error: patch failed: src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp:684
error: src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp: patch does not 
apply
```

- Mesos Reviewbot Windows


On May 9, 2018, 6:15 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66230/
> ---
> 
> (Updated May 9, 2018, 6:15 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Till Toenshoff.
> 
> 
> Bugs: MESOS-7258
> https://issues.apache.org/jira/browse/MESOS-7258
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Includes tests to validate UPDATE_FRAMEWORK call as well as specific
> tests for adding/removing framework roles.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 135ae4389623385a6638158f5f23d6daca14a0ad 
>   src/tests/scheduler_tests.cpp 749420a23bc1a3873bd4d5aee56e78cff79bb1af 
> 
> 
> Diff: https://reviews.apache.org/r/66230/diff/9/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 66229: Implemented UPDATE_FRAMEWORK call.

2018-05-09 Thread Kapil Arya


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

(Updated May 9, 2018, 9:15 p.m.)


Review request for mesos, Benjamin Mahler and Till Toenshoff.


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


Repository: mesos


Description
---

Implemented UPDATE_FRAMEWORK call.


Diffs (updated)
-

  src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp 
ea8d54ff198a5529d61a41bcb6e5806378761091 
  src/master/http.cpp 135ae4389623385a6638158f5f23d6daca14a0ad 
  src/master/master.hpp c30cf082d9ddf2e5dd97e0d887d6d07d8e03cbd4 
  src/master/master.cpp c723a291ed8d219ced4492bc905ac6b52683ae47 
  src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 


Diff: https://reviews.apache.org/r/66229/diff/13/

Changes: https://reviews.apache.org/r/66229/diff/12-13/


Testing
---


Thanks,

Kapil Arya

Re: Review Request 66230: Added test for updating framework info.

2018-05-09 Thread Kapil Arya


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

(Updated May 9, 2018, 9:15 p.m.)


Review request for mesos, Benjamin Mahler and Till Toenshoff.


Summary (updated)
-

Added test for updating framework info.


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


Repository: mesos


Description (updated)
---

Includes tests to validate UPDATE_FRAMEWORK call as well as specific
tests for adding/removing framework roles.


Diffs (updated)
-

  src/master/http.cpp 135ae4389623385a6638158f5f23d6daca14a0ad 
  src/tests/scheduler_tests.cpp 749420a23bc1a3873bd4d5aee56e78cff79bb1af 


Diff: https://reviews.apache.org/r/66230/diff/9/

Changes: https://reviews.apache.org/r/66230/diff/8-9/


Testing
---


Thanks,

Kapil Arya

Re: Review Request 66228: Added Call for updating framework info.

2018-05-09 Thread Kapil Arya


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

(Updated May 9, 2018, 9:14 p.m.)


Review request for mesos, Benjamin Mahler and Till Toenshoff.


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


Repository: mesos


Description
---

Added Call for updating framework info.


Diffs (updated)
-

  include/mesos/scheduler/scheduler.proto 
f6a780a7b75878b9e74402a28a25bb868f7ac36f 
  include/mesos/v1/scheduler/scheduler.proto 
fcfec5e417463103e98dd6917722b4fde41cac7c 


Diff: https://reviews.apache.org/r/66228/diff/9/

Changes: https://reviews.apache.org/r/66228/diff/8-9/


Testing
---


Thanks,

Kapil Arya

Review Request 67049: Modified Master::updateFramework to optionally return an Error.

2018-05-09 Thread Kapil Arya


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

Review request for mesos, Benjamin Mahler and Till Toenshoff.


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


Repository: mesos


Description
---

If the new FrameworkInfo contains non-updatable fields, the `update`
function can now return an Error which can be propagated to the
framework.


Diffs
-

  src/master/master.hpp c30cf082d9ddf2e5dd97e0d887d6d07d8e03cbd4 
  src/master/master.cpp c723a291ed8d219ced4492bc905ac6b52683ae47 


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


Testing
---


Thanks,

Kapil Arya

Review Request 67048: Updated some Master functions to accept repeated fields.

2018-05-09 Thread Kapil Arya


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

Review request for mesos, Benjamin Mahler and Till Toenshoff.


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


Repository: mesos


Description
---

This prevents un-necessary copying of suppressed roles that are
part of subscribe calls.


Diffs
-

  src/master/master.hpp c30cf082d9ddf2e5dd97e0d887d6d07d8e03cbd4 
  src/master/master.cpp c723a291ed8d219ced4492bc905ac6b52683ae47 


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


Testing
---


Thanks,

Kapil Arya

Review Request 67047: Consolidated master framework validation at one place.

2018-05-09 Thread Kapil Arya


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

Review request for mesos, Benjamin Mahler and Till Toenshoff.


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


Repository: mesos


Description
---

Consolidated master framework validation at one place.


Diffs
-

  src/master/master.hpp c30cf082d9ddf2e5dd97e0d887d6d07d8e03cbd4 
  src/master/master.cpp c723a291ed8d219ced4492bc905ac6b52683ae47 
  src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 


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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 66872: Updated FrameworkInfo comparator.

2018-05-09 Thread Kapil Arya


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

(Updated May 9, 2018, 9:11 p.m.)


Review request for mesos, Benjamin Mahler and Till Toenshoff.


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


Repository: mesos


Description (updated)
---

The comparator is more strict and checks for all fields.  Also removed
some existing un-needed comparisons from test cases that were clashing
with the new implementation. This is because the previous
implementation's limited comparison (just `name` and `user` fields)
succeeded whereas it fails with the newer implementaion due to the
stricter/fuller nature.


Diffs (updated)
-

  include/mesos/type_utils.hpp 19ea81716496bcc0117a1b0ff157a0374f38bbfa 
  include/mesos/v1/mesos.hpp fda3eb42061f820869a2d8da939fccadc4e5ddfb 
  src/common/type_utils.cpp 33d63809b61a18e03ff745c88f024c71dd221ca2 
  src/tests/master_allocator_tests.cpp e1aef8a9625a805e7ad2dfad37bfeedee82f160d 
  src/v1/mesos.cpp 9b2df2dd798dff24a91a2604ab53c7fbb5ecfbcf 


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

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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 67044: Renamed a method in the master metrics.

2018-05-09 Thread Mesos Reviewbot Windows


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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['66819', '66820', '66821', '66822', '66823', '66845', 
'66824', '66825', '66846', '66847', '66841', '66842', '66843', '66844', 
'66855', '66861', '66856', '66870', '66874', '67043', '67044']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67044

Relevant logs:

- 
[mesos-tests-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67044/logs/mesos-tests-cmake-stdout.log):

```
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(256):
 warning C4090: 'function': different 'const' qualifiers 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(166):
 warning C4716: 'pthread_cond_broadcast': must return a value 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(205):
 warning C4716: 'pthread_cond_wait': must return a value 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(124):
 warning C4996: 'fopen': This function or variable may be unsafe. Consider 
using fopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See 
online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(128):
 warning C4267: 'initializing': conversion from 'size_t' to 'int', possible 
loss of data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(279):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(301):
 warning C4267: 'initializing': conversion from 'size_t' to 'int', possible 
loss of data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(368):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(372):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(512):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(543):
 warning C4996: 'strcpy': This function or variable may be unsafe. Consider 
using strcpy_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. 
See online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(548):
 warning C4996: 'fopen': This function or variable may be unsafe. Consider 
using fopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See 
online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(569):
 warning C4996: 'strcpy': This function or variable may be unsafe. Consider 
using strcpy_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS.

Re: Review Request 67043: Made the master task state metrics track reregistered agents' tasks.

2018-05-09 Thread Mesos Reviewbot Windows


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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['66819', '66820', '66821', '66822', '66823', '66845', 
'66824', '66825', '66846', '66847', '66841', '66842', '66843', '66844', 
'66855', '66861', '66856', '66870', '66874', '67043']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67043

Relevant logs:

- 
[mesos-tests-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67043/logs/mesos-tests-cmake-stdout.log):

```
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(256):
 warning C4090: 'function': different 'const' qualifiers 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(166):
 warning C4716: 'pthread_cond_broadcast': must return a value 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(205):
 warning C4716: 'pthread_cond_wait': must return a value 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(124):
 warning C4996: 'fopen': This function or variable may be unsafe. Consider 
using fopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See 
online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(128):
 warning C4267: 'initializing': conversion from 'size_t' to 'int', possible 
loss of data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(279):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(301):
 warning C4267: 'initializing': conversion from 'size_t' to 'int', possible 
loss of data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(368):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(372):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(512):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(543):
 warning C4996: 'strcpy': This function or variable may be unsafe. Consider 
using strcpy_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. 
See online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(548):
 warning C4996: 'fopen': This function or variable may be unsafe. Consider 
using fopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See 
online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(569):
 warning C4996: 'strcpy': This function or variable may be unsafe. Consider 
using strcpy_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. 
See online

Re: Review Request 67044: Renamed a method in the master metrics.

2018-05-09 Thread Greg Mann


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

(Updated May 9, 2018, 11:51 p.m.)


Review request for mesos, Benjamin Mahler, Gaston Kleiman, Gilbert Song, and 
Vinod Kone.


Repository: mesos


Description
---

With the recent addition of new methods to the `master::Metrics`
class, this change makes the naming more consistent.


Diffs
-

  src/master/master.cpp 41862db9900acde85a62d2fea85459691c68556e 
  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


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


Testing
---


Thanks,

Greg Mann

Re: Review Request 67043: Made the master task state metrics track reregistered agents' tasks.

2018-05-09 Thread Greg Mann


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

(Updated May 9, 2018, 11:47 p.m.)


Review request for mesos, Benjamin Mahler, Gaston Kleiman, Gilbert Song, and 
Vinod Kone.


Repository: mesos


Description
---

Made the master task state metrics track reregistered agents' tasks.


Diffs
-

  src/master/master.cpp 41862db9900acde85a62d2fea85459691c68556e 
  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


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


Testing
---


Thanks,

Greg Mann

Re: Review Request 67044: Renamed a method in the master metrics.

2018-05-09 Thread Mesos Reviewbot Windows


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



FAIL: Failed to apply the current review.

Failed command: `python.exe .\support\apply-reviews.py -n -r 67044`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67044

Relevant logs:

- 
[apply-review-67044-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67044/logs/apply-review-67044-stdout.log):

```
error: patch failed: src/master/master.cpp:10901
error: src/master/master.cpp: patch does not apply
```

- Mesos Reviewbot Windows


On May 9, 2018, 11:42 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67044/
> ---
> 
> (Updated May 9, 2018, 11:42 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gaston Kleiman, Gilbert Song, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With the recent addition of new methods to the `master::Metrics`
> class, this change makes the naming more consistent.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 41862db9900acde85a62d2fea85459691c68556e 
>   src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
>   src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 
> 
> 
> Diff: https://reviews.apache.org/r/67044/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 67043: Made the master task state metrics track reregistered agents' tasks.

2018-05-09 Thread Mesos Reviewbot Windows


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



FAIL: Failed to apply the current review.

Failed command: `python.exe .\support\apply-reviews.py -n -r 67043`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67043

Relevant logs:

- 
[apply-review-67043-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67043/logs/apply-review-67043-stdout.log):

```
error: patch failed: src/master/metrics.hpp:216
error: src/master/metrics.hpp: patch does not apply
error: patch failed: src/master/metrics.cpp:551
error: src/master/metrics.cpp: patch does not apply
```

- Mesos Reviewbot Windows


On May 9, 2018, 11:40 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67043/
> ---
> 
> (Updated May 9, 2018, 11:40 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gaston Kleiman, Gilbert Song, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made the master task state metrics track reregistered agents' tasks.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 41862db9900acde85a62d2fea85459691c68556e 
>   src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
>   src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 
> 
> 
> Diff: https://reviews.apache.org/r/67043/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Review Request 67044: Renamed a method in the master metrics.

2018-05-09 Thread Greg Mann


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

Review request for mesos, Benjamin Mahler, Gaston Kleiman, Gilbert Song, and 
Vinod Kone.


Repository: mesos


Description
---

With the recent addition of new methods to the `master::Metrics`
class, this change makes the naming more consistent.


Diffs
-

  src/master/master.cpp 41862db9900acde85a62d2fea85459691c68556e 
  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


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


Testing
---


Thanks,

Greg Mann

Re: Review Request 67041: Windows: Ported the rest of `path_tests.cpp`.

2018-05-09 Thread Mesos Reviewbot Windows


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



PASS: Mesos patch 67041 was successfully built and tested.

Reviews applied: `['67041']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67041

- Mesos Reviewbot Windows


On May 9, 2018, 10:07 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67041/
> ---
> 
> (Updated May 9, 2018, 10:07 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Radhika Jandhyala.
> 
> 
> Bugs: MESOS-3442
> https://issues.apache.org/jira/browse/MESOS-3442
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This port does not attempt to use `path::join()` to avoid duplicated
> test logic since that function strips the path separator from its
> arguments, thus making it difficult to correctly construct a path like
> `a//b/`. Instead, a semi-programatical approach was taken to duplicate
> the POSIX paths and then replace `/` with `\`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/path_tests.cpp 
> 433a6c60bb6991410e54ab6a6dcd2499c664c37b 
> 
> 
> Diff: https://reviews.apache.org/r/67041/diff/1/
> 
> 
> Testing
> ---
> 
> ```
> ./3rdparty/stout/tests/stout-tests.exe --gtest_filter="PathTest.*"
> Note: Google Test filter = PathTest.*-
> [==] Running 7 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 7 tests from PathTest
> [ RUN  ] PathTest.Basename
> [   OK ] PathTest.Basename (1 ms)
> [ RUN  ] PathTest.Dirname
> [   OK ] PathTest.Dirname (0 ms)
> [ RUN  ] PathTest.Extension
> [   OK ] PathTest.Extension (0 ms)
> [ RUN  ] PathTest.Join
> [   OK ] PathTest.Join (1 ms)
> [ RUN  ] PathTest.Absolute
> [   OK ] PathTest.Absolute (0 ms)
> [ RUN  ] PathTest.Comparison
> [   OK ] PathTest.Comparison (0 ms)
> [ RUN  ] PathTest.FromURI
> [   OK ] PathTest.FromURI (0 ms)
> [--] 7 tests from PathTest (2 ms total)
> 
> [--] Global test environment tear-down
> [==] 7 tests from 1 test case ran. (2 ms total)
> [  PASSED  ] 7 tests.
> 
> 3rdparty/stout/tests/stout-tests --gtest_filter="PathTest.*"
> Note: Google Test filter = PathTest.*-
> [==] Running 7 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 7 tests from PathTest
> [ RUN  ] PathTest.Basename
> [   OK ] PathTest.Basename (0 ms)
> [ RUN  ] PathTest.Dirname
> [   OK ] PathTest.Dirname (0 ms)
> [ RUN  ] PathTest.Extension
> [   OK ] PathTest.Extension (0 ms)
> [ RUN  ] PathTest.Join
> [   OK ] PathTest.Join (0 ms)
> [ RUN  ] PathTest.Absolute
> [   OK ] PathTest.Absolute (0 ms)
> [ RUN  ] PathTest.Comparison
> [   OK ] PathTest.Comparison (0 ms)
> [ RUN  ] PathTest.FromURI
> [   OK ] PathTest.FromURI (0 ms)
> [--] 7 tests from PathTest (0 ms total)
> 
> [--] Global test environment tear-down
> [==] 7 tests from 1 test case ran. (0 ms total)
> [  PASSED  ] 7 tests.
> ```
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Review Request 67043: Made the master task state metrics track reregistered agents' tasks.

2018-05-09 Thread Greg Mann


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

Review request for mesos, Benjamin Mahler, Gaston Kleiman, Gilbert Song, and 
Vinod Kone.


Repository: mesos


Description
---

Made the master task state metrics track reregistered agents' tasks.


Diffs
-

  src/master/master.cpp 41862db9900acde85a62d2fea85459691c68556e 
  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


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


Testing
---


Thanks,

Greg Mann

Re: Review Request 67008: Added documentation to website for Python components.

2018-05-09 Thread Mesos Reviewbot Windows


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



PASS: Mesos patch 67008 was successfully built and tested.

Reviews applied: `['67008']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67008

- Mesos Reviewbot Windows


On May 8, 2018, 3:39 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67008/
> ---
> 
> (Updated May 8, 2018, 3:39 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Eric Chung, and Kevin Klues.
> 
> 
> Bugs: MESOS-8894
> https://issues.apache.org/jira/browse/MESOS-8894
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation to website for Python components.
> 
> 
> Diffs
> -
> 
>   docs/home.md 5471c70d4023f88adae2234aec267e4d6fea83df 
>   docs/python.md PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67008/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Review Request 67041: Windows: Ported the rest of `path_tests.cpp`.

2018-05-09 Thread Andrew Schwartzmeyer


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

Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, and 
Radhika Jandhyala.


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


Repository: mesos


Description
---

This port does not attempt to use `path::join()` to avoid duplicated
test logic since that function strips the path separator from its
arguments, thus making it difficult to correctly construct a path like
`a//b/`. Instead, a semi-programatical approach was taken to duplicate
the POSIX paths and then replace `/` with `\`.


Diffs
-

  3rdparty/stout/tests/path_tests.cpp 433a6c60bb6991410e54ab6a6dcd2499c664c37b 


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


Testing
---

```
./3rdparty/stout/tests/stout-tests.exe --gtest_filter="PathTest.*"
Note: Google Test filter = PathTest.*-
[==] Running 7 tests from 1 test case.
[--] Global test environment set-up.
[--] 7 tests from PathTest
[ RUN  ] PathTest.Basename
[   OK ] PathTest.Basename (1 ms)
[ RUN  ] PathTest.Dirname
[   OK ] PathTest.Dirname (0 ms)
[ RUN  ] PathTest.Extension
[   OK ] PathTest.Extension (0 ms)
[ RUN  ] PathTest.Join
[   OK ] PathTest.Join (1 ms)
[ RUN  ] PathTest.Absolute
[   OK ] PathTest.Absolute (0 ms)
[ RUN  ] PathTest.Comparison
[   OK ] PathTest.Comparison (0 ms)
[ RUN  ] PathTest.FromURI
[   OK ] PathTest.FromURI (0 ms)
[--] 7 tests from PathTest (2 ms total)

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

3rdparty/stout/tests/stout-tests --gtest_filter="PathTest.*"
Note: Google Test filter = PathTest.*-
[==] Running 7 tests from 1 test case.
[--] Global test environment set-up.
[--] 7 tests from PathTest
[ RUN  ] PathTest.Basename
[   OK ] PathTest.Basename (0 ms)
[ RUN  ] PathTest.Dirname
[   OK ] PathTest.Dirname (0 ms)
[ RUN  ] PathTest.Extension
[   OK ] PathTest.Extension (0 ms)
[ RUN  ] PathTest.Join
[   OK ] PathTest.Join (0 ms)
[ RUN  ] PathTest.Absolute
[   OK ] PathTest.Absolute (0 ms)
[ RUN  ] PathTest.Comparison
[   OK ] PathTest.Comparison (0 ms)
[ RUN  ] PathTest.FromURI
[   OK ] PathTest.FromURI (0 ms)
[--] 7 tests from PathTest (0 ms total)

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


Thanks,

Andrew Schwartzmeyer

Re: Review Request 67022: Refreshed checkpointed SlaveInfo if `--reconfiguration_policy==any`.

2018-05-09 Thread Mesos Reviewbot


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



Bad review!

Reviews applied: [67022]

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

- Mesos Reviewbot


On May 9, 2018, 12:40 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67022/
> ---
> 
> (Updated May 9, 2018, 12:40 a.m.)
> 
> 
> Review request for mesos, Eric Chung and Jason Lai.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refreshed checkpointed SlaveInfo if `--reconfiguration_policy==any`.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.cpp a319b5ea633c41fd8a252c5e1617ac52d1480ba5 
>   src/slave/slave.cpp c6d9152d9de4184f9107bb8242b41d468d76e018 
> 
> 
> Diff: https://reviews.apache.org/r/67022/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 67008: Added documentation to website for Python components.

2018-05-09 Thread Mesos Reviewbot


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



Patch looks great!

Reviews applied: [67008]

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

- Mesos Reviewbot


On May 8, 2018, 3:39 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67008/
> ---
> 
> (Updated May 8, 2018, 3:39 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Eric Chung, and Kevin Klues.
> 
> 
> Bugs: MESOS-8894
> https://issues.apache.org/jira/browse/MESOS-8894
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation to website for Python components.
> 
> 
> Diffs
> -
> 
>   docs/home.md 5471c70d4023f88adae2234aec267e4d6fea83df 
>   docs/python.md PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67008/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 66874: Added per-framework metrics for active task states.

2018-05-09 Thread Greg Mann


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

(Updated May 9, 2018, 7:50 p.m.)


Review request for mesos, Benjamin Mahler, Gaston Kleiman, Gilbert Song, and 
Vinod Kone.


Repository: mesos


Description
---

Added per-framework metrics for active task states.


Diffs (updated)
-

  src/master/master.hpp 5ec764b5c7f96bab786084cccf20fd8a17319718 
  src/master/master.cpp 41862db9900acde85a62d2fea85459691c68556e 
  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


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

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


Testing
---


Thanks,

Greg Mann

Re: Review Request 66870: Added per-framework metrics for suppressed roles.

2018-05-09 Thread Greg Mann


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

(Updated May 9, 2018, 7:37 p.m.)


Review request for mesos, Benjamin Mahler, Gaston Kleiman, Gilbert Song, and 
Vinod Kone.


Changes
---

Made use of common helpers.


Repository: mesos


Description
---

Added per-framework metrics for suppressed roles.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
1000968be6a2935a4cac571414d7f06d7df7acf0 
  src/master/allocator/mesos/metrics.hpp 
6d386225c301d5ab44f3cc0ecdd1478fb5162e5b 
  src/master/allocator/mesos/metrics.cpp 
5194f5b3b3f507b36f02300775230157db3dd477 


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

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


Testing
---


Thanks,

Greg Mann

Re: Review Request 66882: Added `normalizeMetricKey()` and `getFrameworkMetricPrefix()`.

2018-05-09 Thread Gaston Kleiman


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

(Updated May 9, 2018, 12:35 p.m.)


Review request for mesos, Benjamin Mahler, Gilbert Song, Greg Mann, and Vinod 
Kone.


Changes
---

Added extra blank lines.


Repository: mesos


Description
---

Added `normalizeMetricKey()` and `getFrameworkMetricPrefix()`.


Diffs (updated)
-

  src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
  src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 


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

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


Testing
---


Thanks,

Gaston Kleiman

Re: Review Request 66861: Added per-framework DRF position metrics to the allocator.

2018-05-09 Thread Greg Mann


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

(Updated May 9, 2018, 7:32 p.m.)


Review request for mesos, Benjamin Mahler, Gaston Kleiman, Gilbert Song, and 
Vinod Kone.


Changes
---

Made use of the new common helpers.


Repository: mesos


Description
---

During each allocation cycle, the allocator re-sorts roles and
frameworks for each agent in the cluster. This means that for each
agent there exists a total order of (role, framework) tuples.

This patch adds per-framework, per-role metrics which track the
minimum and maximum positions attained by the framework in this
sorting process, from the most recent allocation cycle.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
1000968be6a2935a4cac571414d7f06d7df7acf0 
  src/master/allocator/mesos/metrics.hpp 
6d386225c301d5ab44f3cc0ecdd1478fb5162e5b 
  src/master/allocator/mesos/metrics.cpp 
5194f5b3b3f507b36f02300775230157db3dd477 


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

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


Testing
---


Thanks,

Greg Mann

Re: Review Request 66844: Added framework metrics for filtered resources to the allocator.

2018-05-09 Thread Greg Mann


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

(Updated May 9, 2018, 7:31 p.m.)


Review request for mesos, Benjamin Mahler, Gaston Kleiman, Gilbert Song, and 
Vinod Kone.


Changes
---

Made use of the new common helpers.


Repository: mesos


Description
---

These metrics count the number of times that the allocator has
filtered resources out of this framework's offer stream,
with separate metrics for different types of filtering.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
955ae3e6a9e3c790fb260311ec4b4ef725a826d3 
  src/master/allocator/mesos/hierarchical.cpp 
1000968be6a2935a4cac571414d7f06d7df7acf0 
  src/master/allocator/mesos/metrics.hpp 
6d386225c301d5ab44f3cc0ecdd1478fb5162e5b 
  src/master/allocator/mesos/metrics.cpp 
5194f5b3b3f507b36f02300775230157db3dd477 


Diff: https://reviews.apache.org/r/66844/diff/6/

Changes: https://reviews.apache.org/r/66844/diff/5-6/


Testing
---


Thanks,

Greg Mann

Re: Review Request 66843: Added 'FrameworkMetrics' to the allocator.

2018-05-09 Thread Greg Mann


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

(Updated May 9, 2018, 7:28 p.m.)


Review request for mesos, Benjamin Mahler, Gaston Kleiman, Gilbert Song, and 
Vinod Kone.


Changes
---

Removed unnecessary helpers from 'FrameworkMetrics'.


Repository: mesos


Description
---

This struct will hold per-framework metrics tracked
by the allocator.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
955ae3e6a9e3c790fb260311ec4b4ef725a826d3 
  src/master/allocator/mesos/hierarchical.cpp 
1000968be6a2935a4cac571414d7f06d7df7acf0 
  src/master/allocator/mesos/metrics.hpp 
6d386225c301d5ab44f3cc0ecdd1478fb5162e5b 
  src/master/allocator/mesos/metrics.cpp 
5194f5b3b3f507b36f02300775230157db3dd477 


Diff: https://reviews.apache.org/r/66843/diff/6/

Changes: https://reviews.apache.org/r/66843/diff/5-6/


Testing
---


Thanks,

Greg Mann

Re: Review Request 67024: Renamed `GRPC_UNSECURE` to `GRPC_VARIANT` in CMake.

2018-05-09 Thread Chun-Hung Hsiao


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

(Updated May 9, 2018, 7:04 p.m.)


Review request for mesos, Andrew Schwartzmeyer and Benjamin Bannier.


Changes
---

Addressed Benjamin's comment.


Repository: mesos


Description
---

Renamed `GRPC_UNSECURE` to `GRPC_VARIANT` in CMake.


Diffs (updated)
-

  3rdparty/CMakeLists.txt 0075c133ef60bfef06f4123c14cae8a5672440f5 


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

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


Testing
---

sudo ninja check


Thanks,

Chun-Hung Hsiao

Re: Review Request 66997: Removed the OpenSSL dependency for building gRPC in libprocess.

2018-05-09 Thread Chun-Hung Hsiao


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

(Updated May 9, 2018, 7:03 p.m.)


Review request for mesos, Benjamin Bannier, Gaston Kleiman, and Jie Yu.


Changes
---

Addressed Benjamin's comment.


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


Repository: mesos


Description
---

When the SSL build feature is disabled, libprocess now builds
`libgrpc_unsecure` and `libgrpc++_unsecure` instead of `libgrpc` and
`libgrpc++`, so the SSL headers and libraries are no longer required.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/Makefile.am 
7fb77f8f29e00a276cfb0efc5324ac838c8b5176 
  3rdparty/libprocess/Makefile.am 70edb2ec3ad8b222cd28064f1012c39c335c57a5 
  3rdparty/libprocess/configure.ac fd65d7062ee42eccf2a68d23d02e456d07da6513 


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

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


Testing
---

sudo make check in Mesos build and libprocess standalone build, with openssl 
1.0.2l and 1.1.0f.


Thanks,

Chun-Hung Hsiao

Re: Review Request 66996: Removed the OpenSSL dependency for building gRPC in Mesos.

2018-05-09 Thread Chun-Hung Hsiao


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

(Updated May 9, 2018, 7:02 p.m.)


Review request for mesos, Benjamin Bannier, Gaston Kleiman, and Jie Yu.


Changes
---

Addressed Benjamin's comment.


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


Repository: mesos


Description
---

When the SSL build feature is disabled, Mesos now builds
`libgrpc_unsecure` and `libgrpc++_unsecure` instead of `libgrpc` and
`libgrpc++`, so the SSL headers and libraries are no longer required.


Diffs (updated)
-

  3rdparty/Makefile.am 8d9fa85dd65a94d91670d54dab36deea80d14996 
  configure.ac 429797c35b93a6df69ba0cb0fc28cb66a3171074 
  src/Makefile.am 7e91681e3b8b217f8b23fa5347e059640c62fc65 
  src/python/native_common/ext_modules.py.in 
87387fd580c40b252fc82f98b5238b9b9120903a 


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

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


Testing
---

This patch does not work standalone. Tests are done in the next patch.


Thanks,

Chun-Hung Hsiao

Re: Review Request 67024: Renamed `GRPC_UNSECURE` to `GRPC_VARIANT` in CMake.

2018-05-09 Thread Andrew Schwartzmeyer


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


Ship it!




Ship It!

- Andrew Schwartzmeyer


On May 8, 2018, 7:27 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67024/
> ---
> 
> (Updated May 8, 2018, 7:27 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Renamed `GRPC_UNSECURE` to `GRPC_VARIANT` in CMake.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 0075c133ef60bfef06f4123c14cae8a5672440f5 
> 
> 
> Diff: https://reviews.apache.org/r/67024/diff/1/
> 
> 
> Testing
> ---
> 
> sudo ninja check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 67025: Fixed the link order for gRPC in CMake.

2018-05-09 Thread Andrew Schwartzmeyer


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


Ship it!




Examined this with Chun over DM. Brad King (of CMake) has stated at least in 
mailing list threads that the order of the libraries specified here are (or at 
least should be) preserved when linking.

- Andrew Schwartzmeyer


On May 8, 2018, 7:28 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67025/
> ---
> 
> (Updated May 8, 2018, 7:28 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the link order for gRPC in CMake.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 0075c133ef60bfef06f4123c14cae8a5672440f5 
> 
> 
> Diff: https://reviews.apache.org/r/67025/diff/1/
> 
> 
> Testing
> ---
> 
> sudo ninja check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 67025: Fixed the link order for gRPC in CMake.

2018-05-09 Thread Chun-Hung Hsiao



> On May 9, 2018, 10:35 a.m., Benjamin Bannier wrote:
> > 3rdparty/CMakeLists.txt
> > Line 1009 (original), 1009 (patched)
> > 
> >
> > Why are we moving `libgrpc++` before `libgrpc` here in addition to 
> > moving `libgpr` to the back?
> > 
> > I see both the C++ and C libraries depend on `libgpr` so it should 
> > definitely be on the back to allow correct static links; I see no 
> > dependencies between `libgprc++` and `libgprc` though so we might as well 
> > stick to lexicographic order.

`libgrpc` is not gRPC's C wrapper. It's the core library. And despite that the 
current build doesn't generate linkage between `libgrpc` and `libgrpc++`, the 
C++ wrappers do include the core library headers, so it seems safer and more 
future-proof to me to keep this order. Dropping this.


- Chun-Hung


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


On May 9, 2018, 2:28 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67025/
> ---
> 
> (Updated May 9, 2018, 2:28 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the link order for gRPC in CMake.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 0075c133ef60bfef06f4123c14cae8a5672440f5 
> 
> 
> Diff: https://reviews.apache.org/r/67025/diff/1/
> 
> 
> Testing
> ---
> 
> sudo ninja check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 67018: Removed unnecessary gRPC build flags in libprocess.

2018-05-09 Thread Chun-Hung Hsiao


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

(Updated May 9, 2018, 6:29 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Addressed Benjamin's comment.


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


Repository: mesos


Description
---

The `-Wno-deprecated-declarations` and `-Wno-unused-function` flags are
no longer required to build the bundled gRPC 1.10.0.


Diffs (updated)
-

  3rdparty/libprocess/3rdparty/Makefile.am 
7fb77f8f29e00a276cfb0efc5324ac838c8b5176 


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

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


Testing
---

Tests done later in chain.


Thanks,

Chun-Hung Hsiao

Re: Review Request 66822: Added per Framework Calls to metrics.

2018-05-09 Thread Gaston Kleiman


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




src/master/metrics.hpp
Lines 239 (patched)


Thsi map should be named `call_types` for consistency with all the maps in 
`Master::Metrics`.


- Gaston Kleiman


On April 30, 2018, 11:45 a.m., Gilbert Song wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66822/
> ---
> 
> (Updated April 30, 2018, 11:45 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gaston Kleiman, Greg Mann, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added per Framework Calls to metrics.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 135ae4389623385a6638158f5f23d6daca14a0ad 
>   src/master/master.cpp c723a291ed8d219ced4492bc905ac6b52683ae47 
>   src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
>   src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 
> 
> 
> Diff: https://reviews.apache.org/r/66822/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gilbert Song
> 
>

Re: Review Request 67021: Windows: Enabled `ProtobufTest` suite.

2018-05-09 Thread Radhika Jandhyala via Review Board


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


Ship it!




Ship It!

- Radhika Jandhyala


On May 8, 2018, 11:41 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67021/
> ---
> 
> (Updated May 8, 2018, 11:41 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Radhika Jandhyala.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These tests just worked once added to the build. Also, out-of-date
> code was deleted from `systems_tests.cpp`, and a note added to the
> build file that it (and `signals_tests.cpp`) will be not be ported to
> Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/CMakeLists.txt 
> 28674c9873b7bbccb6b990ec16b7e40a5bf4f9ec 
>   3rdparty/stout/tests/os/systems_tests.cpp 
> a2a05ce28d11cdb97a511e5392a05522a45826c8 
> 
> 
> Diff: https://reviews.apache.org/r/67021/diff/1/
> 
> 
> Testing
> ---
> 
> I just noticed that these weren't enabled when checking something else. 
> Turned them on, they passed. Will push when CI passes.
> 
> ```
> Note: Google Test filter = ProtobufTest.*-
> [==] Running 12 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 12 tests from ProtobufTest
> [ RUN  ] ProtobufTest.JSON
> [   OK ] ProtobufTest.JSON (17 ms)
> [ RUN  ] ProtobufTest.JSONArray
> [   OK ] ProtobufTest.JSONArray (1 ms)
> [ RUN  ] ProtobufTest.JsonLargeIntegers
> [   OK ] ProtobufTest.JsonLargeIntegers (2 ms)
> [ RUN  ] ProtobufTest.SimpleMessageEquals
> [   OK ] ProtobufTest.SimpleMessageEquals (0 ms)
> [ RUN  ] ProtobufTest.ParseJSONArray
> [   OK ] ProtobufTest.ParseJSONArray (1 ms)
> [ RUN  ] ProtobufTest.ParseJSONNull
> [   OK ] ProtobufTest.ParseJSONNull (0 ms)
> [ RUN  ] ProtobufTest.ParseJSONNestedError
> [   OK ] ProtobufTest.ParseJSONNestedError (0 ms)
> [ RUN  ] ProtobufTest.ParseJSONUnrecognizedEnum
> [   OK ] ProtobufTest.ParseJSONUnrecognizedEnum (1 ms)
> [ RUN  ] ProtobufTest.Jsonify
> [   OK ] ProtobufTest.Jsonify (1 ms)
> [ RUN  ] ProtobufTest.JsonifyArray
> [   OK ] ProtobufTest.JsonifyArray (0 ms)
> [ RUN  ] ProtobufTest.JsonifyLargeIntegers
> [   OK ] ProtobufTest.JsonifyLargeIntegers (1 ms)
> [ RUN  ] ProtobufTest.JsonifyMap
> [   OK ] ProtobufTest.JsonifyMap (6 ms)
> [--] 12 tests from ProtobufTest (30 ms total)
> 
> [--] Global test environment tear-down
> [==] 12 tests from 1 test case ran. (31 ms total)
> [  PASSED  ] 12 tests.
> ```
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 67021: Windows: Enabled `ProtobufTest` suite.

2018-05-09 Thread Mesos Reviewbot Windows


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



PASS: Mesos patch 67021 was successfully built and tested.

Reviews applied: `['67021']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67021

- Mesos Reviewbot Windows


On May 8, 2018, 4:41 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67021/
> ---
> 
> (Updated May 8, 2018, 4:41 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Radhika Jandhyala.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These tests just worked once added to the build. Also, out-of-date
> code was deleted from `systems_tests.cpp`, and a note added to the
> build file that it (and `signals_tests.cpp`) will be not be ported to
> Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/CMakeLists.txt 
> 28674c9873b7bbccb6b990ec16b7e40a5bf4f9ec 
>   3rdparty/stout/tests/os/systems_tests.cpp 
> a2a05ce28d11cdb97a511e5392a05522a45826c8 
> 
> 
> Diff: https://reviews.apache.org/r/67021/diff/1/
> 
> 
> Testing
> ---
> 
> I just noticed that these weren't enabled when checking something else. 
> Turned them on, they passed. Will push when CI passes.
> 
> ```
> Note: Google Test filter = ProtobufTest.*-
> [==] Running 12 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 12 tests from ProtobufTest
> [ RUN  ] ProtobufTest.JSON
> [   OK ] ProtobufTest.JSON (17 ms)
> [ RUN  ] ProtobufTest.JSONArray
> [   OK ] ProtobufTest.JSONArray (1 ms)
> [ RUN  ] ProtobufTest.JsonLargeIntegers
> [   OK ] ProtobufTest.JsonLargeIntegers (2 ms)
> [ RUN  ] ProtobufTest.SimpleMessageEquals
> [   OK ] ProtobufTest.SimpleMessageEquals (0 ms)
> [ RUN  ] ProtobufTest.ParseJSONArray
> [   OK ] ProtobufTest.ParseJSONArray (1 ms)
> [ RUN  ] ProtobufTest.ParseJSONNull
> [   OK ] ProtobufTest.ParseJSONNull (0 ms)
> [ RUN  ] ProtobufTest.ParseJSONNestedError
> [   OK ] ProtobufTest.ParseJSONNestedError (0 ms)
> [ RUN  ] ProtobufTest.ParseJSONUnrecognizedEnum
> [   OK ] ProtobufTest.ParseJSONUnrecognizedEnum (1 ms)
> [ RUN  ] ProtobufTest.Jsonify
> [   OK ] ProtobufTest.Jsonify (1 ms)
> [ RUN  ] ProtobufTest.JsonifyArray
> [   OK ] ProtobufTest.JsonifyArray (0 ms)
> [ RUN  ] ProtobufTest.JsonifyLargeIntegers
> [   OK ] ProtobufTest.JsonifyLargeIntegers (1 ms)
> [ RUN  ] ProtobufTest.JsonifyMap
> [   OK ] ProtobufTest.JsonifyMap (6 ms)
> [--] 12 tests from ProtobufTest (30 ms total)
> 
> [--] Global test environment tear-down
> [==] 12 tests from 1 test case ran. (31 ms total)
> [  PASSED  ] 12 tests.
> ```
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 67025: Fixed the link order for gRPC in CMake.

2018-05-09 Thread Mesos Reviewbot


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



Patch looks great!

Reviews applied: [67017, 67018, 66996, 66997, 67024, 67025]

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

- Mesos Reviewbot


On May 8, 2018, 7:28 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67025/
> ---
> 
> (Updated May 8, 2018, 7:28 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the link order for gRPC in CMake.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 0075c133ef60bfef06f4123c14cae8a5672440f5 
> 
> 
> Diff: https://reviews.apache.org/r/67025/diff/1/
> 
> 
> Testing
> ---
> 
> sudo ninja check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66993: Removed unnecessary expectation on termination.

2018-05-09 Thread Mesos Reviewbot Windows


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



PASS: Mesos patch 66993 was successfully built and tested.

Reviews applied: `['66993']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66993

- Mesos Reviewbot Windows


On May 8, 2018, 8:36 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66993/
> ---
> 
> (Updated May 8, 2018, 8:36 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and James Peach.
> 
> 
> Bugs: MESOS-8884
> https://issues.apache.org/jira/browse/MESOS-8884
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test was flaky because termination could already happened when we
> set up the expectation. Given that we already verified task state, I do
> not see checking container termination explicitly is necessary, so
> removing the expectation should fix the flakiness.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> d834e531a550028cd57ac409c9312dd22138e8d5 
> 
> 
> Diff: https://reviews.apache.org/r/66993/diff/2/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="DockerContainerizerTest.ROOT_DOCKER_MaxCompletionTime" 
> ./bin/mesos-tests.sh --gtest_repeat=100 --verbose
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 67021: Windows: Enabled `ProtobufTest` suite.

2018-05-09 Thread Mesos Reviewbot


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



Patch looks great!

Reviews applied: [67021]

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

- Mesos Reviewbot


On May 8, 2018, 11:41 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67021/
> ---
> 
> (Updated May 8, 2018, 11:41 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> and Radhika Jandhyala.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These tests just worked once added to the build. Also, out-of-date
> code was deleted from `systems_tests.cpp`, and a note added to the
> build file that it (and `signals_tests.cpp`) will be not be ported to
> Windows.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/CMakeLists.txt 
> 28674c9873b7bbccb6b990ec16b7e40a5bf4f9ec 
>   3rdparty/stout/tests/os/systems_tests.cpp 
> a2a05ce28d11cdb97a511e5392a05522a45826c8 
> 
> 
> Diff: https://reviews.apache.org/r/67021/diff/1/
> 
> 
> Testing
> ---
> 
> I just noticed that these weren't enabled when checking something else. 
> Turned them on, they passed. Will push when CI passes.
> 
> ```
> Note: Google Test filter = ProtobufTest.*-
> [==] Running 12 tests from 1 test case.
> [--] Global test environment set-up.
> [--] 12 tests from ProtobufTest
> [ RUN  ] ProtobufTest.JSON
> [   OK ] ProtobufTest.JSON (17 ms)
> [ RUN  ] ProtobufTest.JSONArray
> [   OK ] ProtobufTest.JSONArray (1 ms)
> [ RUN  ] ProtobufTest.JsonLargeIntegers
> [   OK ] ProtobufTest.JsonLargeIntegers (2 ms)
> [ RUN  ] ProtobufTest.SimpleMessageEquals
> [   OK ] ProtobufTest.SimpleMessageEquals (0 ms)
> [ RUN  ] ProtobufTest.ParseJSONArray
> [   OK ] ProtobufTest.ParseJSONArray (1 ms)
> [ RUN  ] ProtobufTest.ParseJSONNull
> [   OK ] ProtobufTest.ParseJSONNull (0 ms)
> [ RUN  ] ProtobufTest.ParseJSONNestedError
> [   OK ] ProtobufTest.ParseJSONNestedError (0 ms)
> [ RUN  ] ProtobufTest.ParseJSONUnrecognizedEnum
> [   OK ] ProtobufTest.ParseJSONUnrecognizedEnum (1 ms)
> [ RUN  ] ProtobufTest.Jsonify
> [   OK ] ProtobufTest.Jsonify (1 ms)
> [ RUN  ] ProtobufTest.JsonifyArray
> [   OK ] ProtobufTest.JsonifyArray (0 ms)
> [ RUN  ] ProtobufTest.JsonifyLargeIntegers
> [   OK ] ProtobufTest.JsonifyLargeIntegers (1 ms)
> [ RUN  ] ProtobufTest.JsonifyMap
> [   OK ] ProtobufTest.JsonifyMap (6 ms)
> [--] 12 tests from ProtobufTest (30 ms total)
> 
> [--] Global test environment tear-down
> [==] 12 tests from 1 test case ran. (31 ms total)
> [  PASSED  ] 12 tests.
> ```
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 66635: Introduced wrapper for access to cgroups system access.

2018-05-09 Thread Mesos Reviewbot Windows


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



PASS: Mesos patch 66635 was successfully built and tested.

Reviews applied: `['66634', '66987', '66635']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66635

- Mesos Reviewbot Windows


On May 9, 2018, 7:55 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66635/
> ---
> 
> (Updated May 9, 2018, 7:55 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-8786
> https://issues.apache.org/jira/browse/MESOS-8786
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Different cgroups subsystems are modelled as actors. In this patch we
> introduce wrapper classes which `dispatch` to the processes. This
> removes e.g., races from mixing naked and `dispatch`'ed method calls.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> 5763c9880728f02e44116fd50e62b592a8ef69b6 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 4431ce13d28035b0c5c037b2848ae03aeaf65562 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 65c1e47a569f320b63b54e5f4fc1da374d02ee0d 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> 9afa02b207e6272836e5a36d69fb48f1f4d02150 
> 
> 
> Diff: https://reviews.apache.org/r/66635/diff/5/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66668: Removed `destroyed` from `Container` struct in composing containerizer.

2018-05-09 Thread Qian Zhang


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



I checked the callers of `Containerizer::destroy()`, it seems no one actually 
cares about its return value (`Option`), so why do we 
need to return that? Can we just make it return `Future`?


src/slave/containerizer/composing.cpp
Line 610 (original), 594 (patched)


Do we still need the `switch case`? It seems an `if else` is more readable.



src/slave/containerizer/composing.cpp
Lines 658-661 (original)


If we remove these codes, will the container be missed to delete and erased 
from `containers_`?


- Qian Zhang


On April 17, 2018, 11:23 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/8/
> ---
> 
> (Updated April 17, 2018, 11:23 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, Jie Yu, and Qian 
> Zhang.
> 
> 
> Bugs: MESOS-8712
> https://issues.apache.org/jira/browse/MESOS-8712
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, we were using `destroyed` promise for each container to
> guarantee that calling `destroy()` on a container returns a non-empty
> value when the destroy-in-progress stops an launch-in-progress using
> the next containerizer. Since we are unifying the semantics of the
> return type for both `wait()` and `destroy()` operations, we can
> remove the `destroyed` promise.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.cpp 
> 186102c66d373dcd799cadd9fed7d1c8cb894971 
> 
> 
> Diff: https://reviews.apache.org/r/8/diff/2/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 66882: Added `normalizeMetricKey()` and `getFrameworkMetricPrefix()`.

2018-05-09 Thread Greg Mann


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


Fix it, then Ship it!





src/master/metrics.hpp
Lines 282-283 (original), 282-286 (patched)


Nit: two newlines between these.


- Greg Mann


On May 9, 2018, 12:06 a.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66882/
> ---
> 
> (Updated May 9, 2018, 12:06 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gilbert Song, Greg Mann, and Vinod 
> Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `normalizeMetricKey()` and `getFrameworkMetricPrefix()`.
> 
> 
> Diffs
> -
> 
>   src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
>   src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 
> 
> 
> Diff: https://reviews.apache.org/r/66882/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 67022: Refreshed checkpointed SlaveInfo if `--reconfiguration_policy==any`.

2018-05-09 Thread Mesos Reviewbot Windows


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



FAIL: Failed to apply the dependent review: 64384.

Failed command: `python.exe .\support\apply-reviews.py -n -r 64384`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67022

Relevant logs:

- 
[apply-review-64384-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67022/logs/apply-review-64384-stdout.log):

```
error: patch failed: src/slave/compatibility.cpp:49
error: src/slave/compatibility.cpp: patch does not apply
error: patch failed: src/slave/flags.cpp:467
error: src/slave/flags.cpp: patch does not apply
```

- Mesos Reviewbot Windows


On May 9, 2018, 8:40 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67022/
> ---
> 
> (Updated May 9, 2018, 8:40 a.m.)
> 
> 
> Review request for mesos, Eric Chung and Jason Lai.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refreshed checkpointed SlaveInfo if `--reconfiguration_policy==any`.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.cpp a319b5ea633c41fd8a252c5e1617ac52d1480ba5 
>   src/slave/slave.cpp c6d9152d9de4184f9107bb8242b41d468d76e018 
> 
> 
> Diff: https://reviews.apache.org/r/67022/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 67025: Fixed the link order for gRPC in CMake.

2018-05-09 Thread Mesos Reviewbot Windows


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



PASS: Mesos patch 67025 was successfully built and tested.

Reviews applied: `['67017', '67018', '66996', '66997', '67024', '67025']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67025

- Mesos Reviewbot Windows


On May 9, 2018, 2:28 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67025/
> ---
> 
> (Updated May 9, 2018, 2:28 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the link order for gRPC in CMake.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 0075c133ef60bfef06f4123c14cae8a5672440f5 
> 
> 
> Diff: https://reviews.apache.org/r/67025/diff/1/
> 
> 
> Testing
> ---
> 
> sudo ninja check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 67032: Removed pip install requirements-test.in in new CLI bootstrap script.

2018-05-09 Thread Kevin Klues


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


Ship it!




Ship It!

- Kevin Klues


On May 9, 2018, 1:13 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67032/
> ---
> 
> (Updated May 9, 2018, 1:13 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed pip install requirements-test.in in new CLI bootstrap script.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/bootstrap 87ac3cae790144fe642c9eb86106b05d60a9d14d 
> 
> 
> Diff: https://reviews.apache.org/r/67032/diff/1/
> 
> 
> Testing
> ---
> 
> Building the new CLI e.g., with cmake from a fresh build dir fails without 
> this patch, but works with it.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 67032: Removed pip install requirements-test.in in new CLI bootstrap script.

2018-05-09 Thread Benjamin Bannier


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


Ship it!




Worth mentioning in _Testing done_ that building the new CLI e.g., with cmake 
from a fresh build dir fails without this patch, but works with it ;)

- Benjamin Bannier


On May 9, 2018, 3:13 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67032/
> ---
> 
> (Updated May 9, 2018, 3:13 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed pip install requirements-test.in in new CLI bootstrap script.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/bootstrap 87ac3cae790144fe642c9eb86106b05d60a9d14d 
> 
> 
> Diff: https://reviews.apache.org/r/67032/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 66993: Removed unnecessary expectation on termination.

2018-05-09 Thread Mesos Reviewbot


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



Patch looks great!

Reviews applied: [66993]

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

- Mesos Reviewbot


On May 8, 2018, 8:36 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66993/
> ---
> 
> (Updated May 8, 2018, 8:36 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and James Peach.
> 
> 
> Bugs: MESOS-8884
> https://issues.apache.org/jira/browse/MESOS-8884
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test was flaky because termination could already happened when we
> set up the expectation. Given that we already verified task state, I do
> not see checking container termination explicitly is necessary, so
> removing the expectation should fix the flakiness.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> d834e531a550028cd57ac409c9312dd22138e8d5 
> 
> 
> Diff: https://reviews.apache.org/r/66993/diff/2/
> 
> 
> Testing
> ---
> 
> GTEST_FILTER="DockerContainerizerTest.ROOT_DOCKER_MaxCompletionTime" 
> ./bin/mesos-tests.sh --gtest_repeat=100 --verbose
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Review Request 67032: Removed pip install requirements-test.in in new CLI bootstrap script.

2018-05-09 Thread Armand Grillet


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

Review request for mesos and Kevin Klues.


Repository: mesos


Description
---

Removed pip install requirements-test.in in new CLI bootstrap script.


Diffs
-

  src/python/cli_new/bootstrap 87ac3cae790144fe642c9eb86106b05d60a9d14d 


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


Testing
---


Thanks,

Armand Grillet

Re: Review Request 64970: Use tox for linting and testing code living uder src/python.

2018-05-09 Thread Kevin Klues


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




support/mesos-style.py
Lines 379 (patched)


I am about to commit this, but I wanted to note some changes for posterity.

I am adding a bump to tox 3.0.0 as a precommit to this RR so that we can 
additionally pass the '-qq' to tox to suppress its output. I am also modifying 
this RR to pass this option here.

We want to see the output of the commands it issues, but we don't want to 
see the output of tox itself. The '-qq' option gives us this ability.


- Kevin Klues


On March 14, 2018, 3:05 p.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64970/
> ---
> 
> (Updated March 14, 2018, 3:05 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use tox for linting and testing code living uder src/python.
> 
> At the moment, all linting is done through the same `pylint`
> installation under support/.virtualenv, which requires ALL dependencies
> (i.e. pip-requirements.txt, requirements.in scattered in various
> directories) to be installed in the same virtualenv, making things
> really messy -- e.g. when I've changed some code under `src/python/lib`,
> but don't have the dev virtualenv activated, linting will fail since
> none of the dependencies under `src/python/lib` have been installed.
> 
> Using tox, we can solve this problem by distributing a "test spec"
> (tox.ini) in each of the python source directories which are aware of
> its local dependencies only. To test or lint the code there would be as
> simple as running `tox -e py27-lint `, and the corresponding
> virtualenv and test dependencies would automatically be setup.
> 
> This patch modifies `support/mesos-style.py` to install `tox` in
> `support/.virtualenv` and delegates linting to a `tox` call when it sees
> python directories that have tox setup for it. Linting for all other
> languages will not be effected.
> 
> Testing Done:
> 1. intentionally create a lint error, such as extra spaces before a
> parens in a python file
> 2. run the pre-commit hook and see tox in action
> 
> Reviewed at https://reviews.apache.org/r/64970/
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/tests/__init__.py PRE-CREATION 
>   src/python/cli_new/tox.ini PRE-CREATION 
>   src/python/lib/requirements-test.in 
> b2b73aab65377d9310797203ea84c5150ae60805 
>   src/python/lib/tox.ini fd5e89c77c8608fea21e9caad814c6e111ad57db 
>   support/mesos-style.py 47ec36949010fa511d1b3974739c5ad5c03f6f7f 
> 
> 
> Diff: https://reviews.apache.org/r/64970/diff/6/
> 
> 
> Testing
> ---
> 
> 1. intentionally create a lint error, such as extra spaces before a parens in 
> a python file
> 2. run the pre-commit hook and see tox in action
> 
> 
> Thanks,
> 
> Eric Chung
> 
>

Re: Review Request 66649: Added pb2gen.sh for generating python protobuf bindings.

2018-05-09 Thread Kevin Klues


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




src/python/lib/pb2gen.sh
Lines 64 (patched)


Is there some way to simplify this. Maybe by breaking it out into multiple 
steps? It will be really hard to maintain this going forward otherwise.



src/python/lib/pb2gen.sh
Lines 76 (patched)


Is there some way to simplify this. Maybe by breaking it out into multiple 
steps? It will be really hard to maintain this going forward otherwise.


- Kevin Klues


On April 17, 2018, 8:22 p.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66649/
> ---
> 
> (Updated April 17, 2018, 8:22 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, Kevin Klues, and Zhitao 
> Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The current state of support for python in protoc has two serious
> issues:
> 1. The `__init__.py` files necessary to mark a directory as a python
> package aren't generated.
> 2. The import paths in each of the generated .py files do not reflect
> the `--python_path` option passed to `protoc`.
> 
> This results in incomplete code generated, preventing it from being used
> out of the box. To address this issue, we're adding a `pb2gen.sh` script
> to do end-to-end code generation for python protobuf bindings: the
> script generates the bindings based on what's in the `include` dir, then
> postprocesses the generated code to add proper import paths and the
> `__init__.py` files.
> 
> 
> Diffs
> -
> 
>   src/python/lib/pb2gen.sh PRE-CREATION 
>   src/python/lib/requirements.in 0742f3d846c99c1c4907d9628fb49845564563b2 
> 
> 
> Diff: https://reviews.apache.org/r/66649/diff/3/
> 
> 
> Testing
> ---
> 
> 1. under `src/python/lib`, run `pb2gen.sh`
> 2. initialize and activate virtualenv: `virtualenv env && . env/bin/activate`
> 3. install reqs: `pip install -r requirements.in`
> 4. try to import modules from generated python code: `python -c 'from 
> mesos.pb2.mesos.v1.master import master_pb2'`
> 
> 
> Thanks,
> 
> Eric Chung
> 
>

Re: Review Request 66996: Removed the OpenSSL dependency for building gRPC in Mesos.

2018-05-09 Thread Benjamin Bannier


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


Fix it, then Ship it!





configure.ac
Lines 2071 (patched)


We should be able to use `GRPC_VARIANT` here as well as the name should not 
collide with what we do on the automake side.


- Benjamin Bannier


On May 9, 2018, 2:50 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66996/
> ---
> 
> (Updated May 9, 2018, 2:50 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, and Jie Yu.
> 
> 
> Bugs: MESOS-8798
> https://issues.apache.org/jira/browse/MESOS-8798
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When the SSL build feature is disabled, Mesos now builds
> `libgrpc_unsecure` and `libgrpc++_unsecure` instead of `libgrpc` and
> `libgrpc++`, so the SSL headers and libraries are no longer required.
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am 8d9fa85dd65a94d91670d54dab36deea80d14996 
>   configure.ac 429797c35b93a6df69ba0cb0fc28cb66a3171074 
>   src/Makefile.am 7e91681e3b8b217f8b23fa5347e059640c62fc65 
>   src/python/native_common/ext_modules.py.in 
> 87387fd580c40b252fc82f98b5238b9b9120903a 
> 
> 
> Diff: https://reviews.apache.org/r/66996/diff/2/
> 
> 
> Testing
> ---
> 
> This patch does not work standalone. Tests are done in the next patch.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66997: Removed the OpenSSL dependency for building gRPC in libprocess.

2018-05-09 Thread Benjamin Bannier


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


Fix it, then Ship it!





3rdparty/libprocess/3rdparty/Makefile.am
Lines 218-221 (patched)


Thanks for adding the comment.



3rdparty/libprocess/configure.ac
Lines 1164 (patched)


We should be able to use `GRPC_VARIANT` here as well as the name should not 
collide with what we do on the automake side.


- Benjamin Bannier


On May 9, 2018, 2:52 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66997/
> ---
> 
> (Updated May 9, 2018, 2:52 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, and Jie Yu.
> 
> 
> Bugs: MESOS-8798
> https://issues.apache.org/jira/browse/MESOS-8798
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When the SSL build feature is disabled, libprocess now builds
> `libgrpc_unsecure` and `libgrpc++_unsecure` instead of `libgrpc` and
> `libgrpc++`, so the SSL headers and libraries are no longer required.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/Makefile.am 
> 7fb77f8f29e00a276cfb0efc5324ac838c8b5176 
>   3rdparty/libprocess/Makefile.am 70edb2ec3ad8b222cd28064f1012c39c335c57a5 
>   3rdparty/libprocess/configure.ac fd65d7062ee42eccf2a68d23d02e456d07da6513 
> 
> 
> Diff: https://reviews.apache.org/r/66997/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check in Mesos build and libprocess standalone build, with openssl 
> 1.0.2l and 1.1.0f.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 67025: Fixed the link order for gRPC in CMake.

2018-05-09 Thread Benjamin Bannier

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

Fix it, then Ship it!

3rdparty/CMakeLists.txt
Line 1009 (original), 1009 (patched)

Why are we moving `libgrpc++` before `libgrpc` here in addition to moving 
`libgpr` to the back?

I see both the C++ and C libraries depend on `libgpr` so it should 
definitely be on the back to allow correct static links; I see no dependencies 
between `libgprc++` and `libgprc` though so we might as well stick to 
lexicographic order.

- Benjamin Bannier

On May 9, 2018, 4:28 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67025/
> ---
> 
> (Updated May 9, 2018, 4:28 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the link order for gRPC in CMake.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 0075c133ef60bfef06f4123c14cae8a5672440f5 
> 
> 
> Diff: https://reviews.apache.org/r/67025/diff/1/
> 
> 
> Testing
> ---
> 
> sudo ninja check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 67024: Renamed `GRPC_UNSECURE` to `GRPC_VARIANT` in CMake.

2018-05-09 Thread Benjamin Bannier


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


Fix it, then Ship it!




Ship It!


3rdparty/CMakeLists.txt
Line 1026 (original), 1026 (patched)


Not new, but could you also set this variable on the other branch?

set(GRPC_VARIANT "")


- Benjamin Bannier


On May 9, 2018, 4:27 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67024/
> ---
> 
> (Updated May 9, 2018, 4:27 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Renamed `GRPC_UNSECURE` to `GRPC_VARIANT` in CMake.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 0075c133ef60bfef06f4123c14cae8a5672440f5 
> 
> 
> Diff: https://reviews.apache.org/r/67024/diff/1/
> 
> 
> Testing
> ---
> 
> sudo ninja check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66510: Unified return type of `wait` and `destroy` containerizer methods.

2018-05-09 Thread Qian Zhang



> On May 9, 2018, 4:34 p.m., Qian Zhang wrote:
> > src/slave/containerizer/composing.cpp
> > Line 375 (original), 377 (patched)
> > 
> >
> > Not yours, but `after launch returned false` seems not correct to me 
> > since launch does not return true/false anymore. So it should be something 
> > like `after launch returned NOT_SUPPORTED`?

I see such kind of comments a couple of times in composing.cpp, better to 
update all of them in a separte patch.


- Qian


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


On April 9, 2018, 11:44 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66510/
> ---
> 
> (Updated April 9, 2018, 11:44 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, Jie Yu, and Qian 
> Zhang.
> 
> 
> Bugs: MESOS-8706
> https://issues.apache.org/jira/browse/MESOS-8706
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch changes the return type of `destroy()` method for all
> containerizers. As a result, `destroy()` and `wait()` methods have
> got the same signature and return type. In fact, all these methods
> depend on the same container termination promise, so this unification
> makes both containerizer interface and internal logic more consistent.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.hpp 
> 1221d9b61b7cfbdbaa560dbafb0158225331ed6d 
>   src/slave/containerizer/composing.cpp 
> cd840a5409023d32701329bf37806f2e7ed6ffd2 
>   src/slave/containerizer/containerizer.hpp 
> 836283aa8aa827459558d567e090a230c32351ed 
>   src/slave/containerizer/docker.hpp f1b56c8edced17e9786c86adb9b92c829ed81635 
>   src/slave/containerizer/docker.cpp 31d64a7ea2a331a084c0f1707b82e938dfe6390f 
>   src/slave/containerizer/mesos/containerizer.hpp 
> cba4ed2e20622070b46071f35907d8b32c25b2fc 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 7473a871e626833733f39375b778aff70529dc63 
>   src/slave/http.cpp d500fde22d01d2c66104b72ff39d9de4e3a411cd 
> 
> 
> Diff: https://reviews.apache.org/r/66510/diff/2/
> 
> 
> Testing
> ---
> 
> Tests can't be compiled after this change.
> See the next patch in the chain.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 67018: Removed unnecessary gRPC build flags in libprocess.

2018-05-09 Thread Benjamin Bannier


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


Fix it, then Ship it!




Ship It!


3rdparty/libprocess/3rdparty/Makefile.am
Line 230 (original), 222 (patched)


Let's make this a separate patch.


- Benjamin Bannier


On May 9, 2018, 12:47 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67018/
> ---
> 
> (Updated May 9, 2018, 12:47 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8798
> https://issues.apache.org/jira/browse/MESOS-8798
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `-Wno-deprecated-declarations` and `-Wno-unused-function` flags are
> no longer required to build the bundled gRPC 1.10.0.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/Makefile.am 
> 7fb77f8f29e00a276cfb0efc5324ac838c8b5176 
> 
> 
> Diff: https://reviews.apache.org/r/67018/diff/1/
> 
> 
> Testing
> ---
> 
> Tests done later in chain.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66510: Unified return type of `wait` and `destroy` containerizer methods.

2018-05-09 Thread Qian Zhang


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




src/slave/containerizer/composing.cpp
Line 375 (original), 377 (patched)


Not yours, but `after launch returned false` seems not correct to me since 
launch does not return true/false anymore. So it should be something like 
`after launch returned NOT_SUPPORTED`?



src/slave/containerizer/composing.cpp
Line 516 (original), 519 (patched)


Ditto.


- Qian Zhang


On April 9, 2018, 11:44 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66510/
> ---
> 
> (Updated April 9, 2018, 11:44 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, Jie Yu, and Qian 
> Zhang.
> 
> 
> Bugs: MESOS-8706
> https://issues.apache.org/jira/browse/MESOS-8706
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch changes the return type of `destroy()` method for all
> containerizers. As a result, `destroy()` and `wait()` methods have
> got the same signature and return type. In fact, all these methods
> depend on the same container termination promise, so this unification
> makes both containerizer interface and internal logic more consistent.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.hpp 
> 1221d9b61b7cfbdbaa560dbafb0158225331ed6d 
>   src/slave/containerizer/composing.cpp 
> cd840a5409023d32701329bf37806f2e7ed6ffd2 
>   src/slave/containerizer/containerizer.hpp 
> 836283aa8aa827459558d567e090a230c32351ed 
>   src/slave/containerizer/docker.hpp f1b56c8edced17e9786c86adb9b92c829ed81635 
>   src/slave/containerizer/docker.cpp 31d64a7ea2a331a084c0f1707b82e938dfe6390f 
>   src/slave/containerizer/mesos/containerizer.hpp 
> cba4ed2e20622070b46071f35907d8b32c25b2fc 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 7473a871e626833733f39375b778aff70529dc63 
>   src/slave/http.cpp d500fde22d01d2c66104b72ff39d9de4e3a411cd 
> 
> 
> Diff: https://reviews.apache.org/r/66510/diff/2/
> 
> 
> Testing
> ---
> 
> Tests can't be compiled after this change.
> See the next patch in the chain.
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 66933: Added a realm for resource provider authentication.

2018-05-09 Thread Benjamin Bannier



> On May 8, 2018, 12:35 p.m., Benjamin Bannier wrote:
> > src/slave/constants.hpp
> > Lines 171 (patched)
> > 
> >
> > I don't believe it makes sense to treat resource providers separately 
> > from other internal components using HTTP communication, what about e.g.,
> > 
> > // Name of the agent HTTP authentication realm for internal Mesos 
> > components.
> > constexpr char INTERNAL_HTTP_AUTHENTICATION_REALM[] = 
> > "mesos-internal";
> 
> Jan Schlicht wrote:
> While the resource provider API currently is only used internally, it's 
> expected to be used externally as well. If it would be internal only, we 
> would communicate using actors.

Good point about about external resource providers.


- Benjamin


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


On May 7, 2018, 2:12 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66933/
> ---
> 
> (Updated May 7, 2018, 2:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Chun-Hung Hsiao.
> 
> 
> Bugs: MESOS-8774
> https://issues.apache.org/jira/browse/MESOS-8774
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp d1d15c39c87b9712c96b9f7516a7a0a3e56514cf 
>   src/slave/slave.cpp c6d9152d9de4184f9107bb8242b41d468d76e018 
>   src/tests/cluster.cpp c071da69500e1d8a223f255904acf7e28100e774 
> 
> 
> Diff: https://reviews.apache.org/r/66933/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 66933: Added a realm for resource provider authentication.

2018-05-09 Thread Benjamin Bannier


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


Ship it!




Ship It!

- Benjamin Bannier


On May 7, 2018, 2:12 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66933/
> ---
> 
> (Updated May 7, 2018, 2:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Chun-Hung Hsiao.
> 
> 
> Bugs: MESOS-8774
> https://issues.apache.org/jira/browse/MESOS-8774
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.hpp d1d15c39c87b9712c96b9f7516a7a0a3e56514cf 
>   src/slave/slave.cpp c6d9152d9de4184f9107bb8242b41d468d76e018 
>   src/tests/cluster.cpp c071da69500e1d8a223f255904acf7e28100e774 
> 
> 
> Diff: https://reviews.apache.org/r/66933/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 66635: Introduced wrapper for access to cgroups system access.

2018-05-09 Thread Benjamin Bannier



> On May 9, 2018, 1:10 a.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp
> > Lines 98 (patched)
> > 
> >
> > Could we switch the order with `SubsystemProcess`? This is the semantic 
> > we follow in the agent code for practice. A forward declaration is needed.

Done, also move the impl of `Subsystem::name` to the cpp file.


- Benjamin


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


On May 9, 2018, 9:55 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66635/
> ---
> 
> (Updated May 9, 2018, 9:55 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, haosdent huang, and Jie Yu.
> 
> 
> Bugs: MESOS-8786
> https://issues.apache.org/jira/browse/MESOS-8786
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Different cgroups subsystems are modelled as actors. In this patch we
> introduce wrapper classes which `dispatch` to the processes. This
> removes e.g., races from mixing naked and `dispatch`'ed method calls.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> 5763c9880728f02e44116fd50e62b592a8ef69b6 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 4431ce13d28035b0c5c037b2848ae03aeaf65562 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
> 65c1e47a569f320b63b54e5f4fc1da374d02ee0d 
>   src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
> 9afa02b207e6272836e5a36d69fb48f1f4d02150 
> 
> 
> Diff: https://reviews.apache.org/r/66635/diff/5/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66635: Introduced wrapper for access to cgroups system access.

2018-05-09 Thread Benjamin Bannier


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

(Updated May 9, 2018, 9:55 a.m.)


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


Changes
---

Switch order as requested by Gilbert.


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


Repository: mesos


Description
---

Different cgroups subsystems are modelled as actors. In this patch we
introduce wrapper classes which `dispatch` to the processes. This
removes e.g., races from mixing naked and `dispatch`'ed method calls.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
5763c9880728f02e44116fd50e62b592a8ef69b6 
  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
4431ce13d28035b0c5c037b2848ae03aeaf65562 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.hpp 
65c1e47a569f320b63b54e5f4fc1da374d02ee0d 
  src/slave/containerizer/mesos/isolators/cgroups/subsystem.cpp 
9afa02b207e6272836e5a36d69fb48f1f4d02150 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 67022: Refreshed checkpointed SlaveInfo if `--reconfiguration_policy==any`.

2018-05-09 Thread Eric Chung


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


Ship it!




Ship It!

- Eric Chung


On May 9, 2018, 12:40 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67022/
> ---
> 
> (Updated May 9, 2018, 12:40 a.m.)
> 
> 
> Review request for mesos, Eric Chung and Jason Lai.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refreshed checkpointed SlaveInfo if `--reconfiguration_policy==any`.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.cpp a319b5ea633c41fd8a252c5e1617ac52d1480ba5 
>   src/slave/slave.cpp c6d9152d9de4184f9107bb8242b41d468d76e018 
> 
> 
> Diff: https://reviews.apache.org/r/67022/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 67017: Removed unnecessary gRPC build flags in Mesos.

2018-05-09 Thread Benjamin Bannier


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


Ship it!




Ship It!

- Benjamin Bannier


On Mai 9, 2018, 12:46 vorm., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67017/
> ---
> 
> (Updated Mai 9, 2018, 12:46 vorm.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8798
> https://issues.apache.org/jira/browse/MESOS-8798
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `-Wno-deprecated-declarations` and `-Wno-unused-function` flags are
> no longer required to build the bundled gRPC 1.10.0.
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am 8d9fa85dd65a94d91670d54dab36deea80d14996 
> 
> 
> Diff: https://reviews.apache.org/r/67017/diff/1/
> 
> 
> Testing
> ---
> 
> Tests done later in chain.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66964: Ported (some) Python 2 support scripts to Python 3.

2018-05-09 Thread Eric Chung


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



Mostly lgtm, but it would be even better if we could also put in some unit 
tests, e.g. using tox.

- Eric Chung


On May 4, 2018, 8:44 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66964/
> ---
> 
> (Updated May 4, 2018, 8:44 p.m.)
> 
> 
> Review request for mesos, Armand Grillet, Benjamin Mahler, Eric Chung, 
> Clement Michaud, Gaston Kleiman, Joseph Wu, and Kevin Klues.
> 
> 
> Bugs: MESOS-7658
> https://issues.apache.org/jira/browse/MESOS-7658
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> According to https://docs.python.org/3/howto/pyporting.html, we can
> make our support scripts compatible with both Python 2 and 3 provided
> we only worry about supporting Python 2.7.
> 
> Using the Python-Future quick start guide
> http://python-future.org/quickstart.html, I installed a Python 3.6.5
> environment, ran `pip install future`, and then ran `futurize -w` on
> the modified scripts.
> 
> In my unchanged system environment, attempting to use the scripts
> resulted in a traceback with the error `ImportError: No module named
> future`, since the `future` package is sadly not installed by default.
> On Ubuntu 17.10, I was able to `apt install python-future` to add it
> to my system's Python 2 installation, so would be a new requirement
> despite the Python 2 support.
> 
> The ported scripts were chosen in order to support using Python 3 on
> the Windows build bot, so that author names with non-ASCII characters
> can be used. The rest of the scripts should follow, but are not
> necessary, and require more extensive testing and fixes.
> 
> A manual change was made to `post-reviews` to deal with the stdout of
> child processes. In `execute` we explicitly decode the stdout to a
> string to match the existing expectation (in Python 2, it was already
> a string, but in Python 3 bytes and strings are different types). When
> printing the stdout of a child process, we use `sys.stdout.buffer` to
> print bytes instead of a string in order to preserve ANSI color
> escapes in the output. This also fixes an existing bug on Windows
> where the script would not correctly display colors.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py b6801b926d9fc32172f102586d49e0464da71359 
>   support/mesos-split.py 7ef5c86d55e61b80b5a7c8f137323aa056f272ed 
>   support/post-reviews.py a6646f2eca45f911aad2403b8b0fef3a9323 
>   support/push-commits.py cd751cbf3c7e64005f1da6c96eb33c6fc62c7ae3 
>   support/verify-reviews.py fbc2460051e96761c9669db001c2fa13906f0cca 
> 
> 
> Diff: https://reviews.apache.org/r/66964/diff/1/
> 
> 
> Testing
> ---
> 
> This is a work-in-progress. I used the automatic porting script `futurize` on 
> some of our support scripts (I tried on all of them, but some (like 
> `cpplint.py`) are going to take more work). So I'd prefer to start with the 
> set of scripts necessary to change the Windows ReviewBot over to Python 3. 
> Also, `mesos-style.py` _does not_ like these changes; it throws a bunch of 
> errors and warnings when linting these files:
> 
> ```
> W: 39, 0: Redefining built-in 'str' (redefined-builtin)
> E: 37, 0: Unable to import 'future' (import-error)
> E: 39, 0: Unable to import 'builtins' (import-error)
> C: 39, 0: Import "from builtins import str" should be placed at the top of 
> the module (wrong-import-position)
> C: 40, 0: Import "import atexit" should be placed at the top of the module 
> (wrong-import-position)
> C: 41, 0: Import "import json" should be placed at the top of the module 
> (wrong-import-position)
> C: 42, 0: Import "import os" should be placed at the top of the module 
> (wrong-import-position)
> C: 43, 0: Import "import platform" should be placed at the top of the module 
> (wrong-import-position)
> C: 44, 0: Import "import subprocess" should be placed at the top of the 
> module (wrong-import-position)
> C: 45, 0: Import "import sys" should be placed at the top of the module 
> (wrong-import-position)
> E: 46, 0: No name 'request' in module 'urllib' (no-name-in-module)
> E: 46, 0: Unable to import 'urllib.request' (import-error)
> C: 46, 0: Import "import urllib.request" should be placed at the top of the 
> module (wrong-import-position)
> E: 47, 0: No name 'parse' in module 'urllib' (no-name-in-module)
> E: 47, 0: Unable to import 'urllib.parse' (import-error)
> C: 47, 0: Import "import urllib.parse" should be placed at the top of the 
> module (wrong-import-position)
> E: 48, 0: No name 'error' in module 'urllib' (no-name-in-module)
> E: 48, 0: Unable to import 'urllib.error' (import-error)
> C: 48, 0: Import "import

72 matches

Mail list logo