Re: Review Request 67938: Made gRPC call options configurable.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67938 was successfully built and tested.

Reviews applied: `['67938']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1940/mesos-review-67938

- Mesos Reviewbot Windows


On July 17, 2018, 1:27 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67938/
> ---
> 
> (Updated July 17, 2018, 1:27 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-9055
> https://issues.apache.org/jira/browse/MESOS-9055
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Made gRPC call options configurable.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/grpc.hpp 
> 0ff8184791277ed02b30481b352be60f10743428 
>   3rdparty/libprocess/src/tests/grpc_tests.cpp 
> eb9621b674ebbeda0b264001e5100a49a217629b 
> 
> 
> Diff: https://reviews.apache.org/r/67938/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 67922: Removed unused `TestStore` mock.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On July 15, 2018, 5:36 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67922/
> ---
> 
> (Updated July 15, 2018, 5:36 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Gilbert Song, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed unused `TestStore` mock.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9c13eaaa8bd62f10539ebbb6ab16c7303dabfcc1 
>   src/tests/containerizer/store.hpp 0589be698d3bf835293fbe7ea817a11a7bb91f67 
> 
> 
> Diff: https://reviews.apache.org/r/67922/diff/1/
> 
> 
> Testing
> ---
> 
> make check (Fedora 28)
> 
> `TestStore` usage was removed in 
> [r/52539](https://reviews.apache.org/r/52539).
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 67938: Made gRPC call options configurable.

[Chun-Hung Hsiao]

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

(Updated July 17, 2018, 1:27 a.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description
---

Made gRPC call options configurable.


Diffs (updated)
-

  3rdparty/libprocess/include/process/grpc.hpp 
0ff8184791277ed02b30481b352be60f10743428 
  3rdparty/libprocess/src/tests/grpc_tests.cpp 
eb9621b674ebbeda0b264001e5100a49a217629b 


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

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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Review Request 67938: Made gRPC call options configurable.

[Chun-Hung Hsiao]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Made gRPC call options configurable.


Diffs
-

  3rdparty/libprocess/include/process/grpc.hpp 
0ff8184791277ed02b30481b352be60f10743428 
  3rdparty/libprocess/src/tests/grpc_tests.cpp 
eb9621b674ebbeda0b264001e5100a49a217629b 


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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 67921: Bug fix for semaphore decomission "deadlock".

[Benjamin Mahler]

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




3rdparty/libprocess/src/semaphore.hpp
Lines 267-274 (original), 267-288 (patched)


Despite the comment below, it looks like we do want to keep some of the if 
condition re-structuring here but ideally in a separate patch? That would help 
make the fix simple and clearer (i.e. decommission doesn't "finish" because a 
new wait() can come in and grab a signal since it's not FIFO, therefore we now 
loop to completion).



3rdparty/libprocess/src/semaphore.hpp
Lines 379-386 (original), 393-400 (patched)


As discussed offline, the fix seems a little odd in its current place, 
since signal was doing its job correctly: it's not a FIFO semaphore, so someone 
can arrive and steal a signal before an existing waiter.

So, maybe we put the burden on decommission to signal until it's definitely 
finished? E.g.

```
  void decomission()
  {
comissioned.store(false);

while (waiters.load() > 0) {
  signal();
}
  }
```


- Benjamin Mahler


On July 15, 2018, 5:09 p.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67921/
> ---
> 
> (Updated July 15, 2018, 5:09 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8239
> https://issues.apache.org/jira/browse/MESOS-8239
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixes MESOS-8239.
> 
> When using the DecomissionableLastInFirstOutFixedSizeSemaphore it's
> possible that waiting threads may never be properly signaled. This bug
> fix makes sure that every waiting thread gets a signal after a
> decomission.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/semaphore.hpp 
> 50501b9797894ad274eb73f74b3eed00cd719114 
> 
> 
> Diff: https://reviews.apache.org/r/67921/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 67923: Improved performance of cgroups::read by verifying after failure.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On July 15, 2018, 7:03 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67923/
> ---
> 
> (Updated July 15, 2018, 7:03 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-8418
> https://issues.apache.org/jira/browse/MESOS-8418
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It turns out that cgroups::verify is expensive and leads to a severe
> performance issue on the agent during container metrics collection
> if there are a lot of containers on the agent. See MESOS-8418.
> 
> Since cgroups::verify serves to provide a helpful error message,
> this patch preserves the error message, but only if the read fails.
> 
> Longer term, there probably needs to be some re-structuring of the
> code to make verification caller-controlled, or perhaps the verify
> code can occur consistently post-operation (as done in this patch),
> or perhaps verify can be optimized substantially.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp b12e63c112a7aa7a6f7150359ff5409f8214067e 
> 
> 
> Diff: https://reviews.apache.org/r/67923/diff/1/
> 
> 
> Testing
> ---
> 
> Ran through internal CI.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 67936: Fixed ephemeral ports deallocation in network/port_mapping isolator.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67936 was successfully built and tested.

Reviews applied: `['67936']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1939/mesos-review-67936

- Mesos Reviewbot Windows


On July 16, 2018, 11:14 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67936/
> ---
> 
> (Updated July 16, 2018, 11:14 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-9080
> https://issues.apache.org/jira/browse/MESOS-9080
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ephemeral ports are allocated for a container during the preparation
> phase and have to be deallocated during container cleanup regardless of
> whether the container process was isolated or not. Otherwise those ports
> can be leaked if the container is destroyed during preparation.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> a29282ed0488d83966f222084031ed1d2b6ab1f5 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 553f53043a52cfe235fc2b2e88fb54d0f2725d3e 
> 
> 
> Diff: https://reviews.apache.org/r/67936/diff/1/
> 
> 
> Testing
> ---
> 
> Added `PortMappingIsolatorTest.ROOT_CleanupNotIsolated` test to verify that 
> ephemeral ports are properly deallocated when the container was not isolated 
> and manually checked that if fails without the fix. Ran `sudo make check`.
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Re: Review Request 67928: Added balloon framework metric for tasks which were running.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67928 was successfully built and tested.

Reviews applied: `['67928']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1938/mesos-review-67928

- Mesos Reviewbot Windows


On July 16, 2018, 10:01 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67928/
> ---
> 
> (Updated July 16, 2018, 10:01 p.m.)
> 
> 
> Review request for mesos and Meng Zhu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The framework currently exposes metric counters for various expected
> and unexpected task termination reasons. Interpreting these counters
> can be non-trivial since tasks might fail due to benign, but unknown
> external reasons.
> 
> This patch adds a counter for the tasks which actually made it to the
> running stage which can be correlated with the different terminal task
> counts.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_framework.cpp cbc25e45258b05a645363d4229c0a701baa5481c 
> 
> 
> Diff: https://reviews.apache.org/r/67928/diff/2/
> 
> 
> Testing
> ---
> 
> Started the framework with a `local` master and examine the metric.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67923: Improved performance of cgroups::read by verifying after failure.

[Benjamin Mahler]

> On July 16, 2018, 8:54 p.m., Jie Yu wrote:
> > LGTM! Thanks Ben. I think longer term, we will modify caller to call 
> > `verify`, making read/write/create pure helper for writing to cgroup 
> > filesystem.

Yeah, I'll copy the longer options from the commit description into a TODO so 
it's clear to any readers.


- Benjamin


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


On July 16, 2018, 2:03 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67923/
> ---
> 
> (Updated July 16, 2018, 2:03 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-8418
> https://issues.apache.org/jira/browse/MESOS-8418
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It turns out that cgroups::verify is expensive and leads to a severe
> performance issue on the agent during container metrics collection
> if there are a lot of containers on the agent. See MESOS-8418.
> 
> Since cgroups::verify serves to provide a helpful error message,
> this patch preserves the error message, but only if the read fails.
> 
> Longer term, there probably needs to be some re-structuring of the
> code to make verification caller-controlled, or perhaps the verify
> code can occur consistently post-operation (as done in this patch),
> or perhaps verify can be optimized substantially.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp b12e63c112a7aa7a6f7150359ff5409f8214067e 
> 
> 
> Diff: https://reviews.apache.org/r/67923/diff/1/
> 
> 
> Testing
> ---
> 
> Ran through internal CI.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 67936: Fixed ephemeral ports deallocation in network/port_mapping isolator.

[Jie Yu]

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


Fix it, then Ship it!





src/tests/containerizer/port_mapping_tests.cpp
Lines 1826 (patched)


instead of hard code 512 which is fragile, i'd get `ephemeral_ports` 
resources, and use more than half of that.


- Jie Yu


On July 16, 2018, 11:14 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67936/
> ---
> 
> (Updated July 16, 2018, 11:14 p.m.)
> 
> 
> Review request for mesos and Jie Yu.
> 
> 
> Bugs: MESOS-9080
> https://issues.apache.org/jira/browse/MESOS-9080
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Ephemeral ports are allocated for a container during the preparation
> phase and have to be deallocated during container cleanup regardless of
> whether the container process was isolated or not. Otherwise those ports
> can be leaked if the container is destroyed during preparation.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> a29282ed0488d83966f222084031ed1d2b6ab1f5 
>   src/tests/containerizer/port_mapping_tests.cpp 
> 553f53043a52cfe235fc2b2e88fb54d0f2725d3e 
> 
> 
> Diff: https://reviews.apache.org/r/67936/diff/1/
> 
> 
> Testing
> ---
> 
> Added `PortMappingIsolatorTest.ROOT_CleanupNotIsolated` test to verify that 
> ephemeral ports are properly deallocated when the container was not isolated 
> and manually checked that if fails without the fix. Ran `sudo make check`.
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Re: Review Request 67933: Replaced `` with `` in stout.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67933 was successfully built and tested.

Reviews applied: `['67933']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1937/mesos-review-67933

- Mesos Reviewbot Windows


On July 16, 2018, 9:43 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67933/
> ---
> 
> (Updated July 16, 2018, 9:43 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, John Kordich, Joseph Wu, and Radhika 
> Jandhyala.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The intent is that `os.hpp` includes the correct OS-specific header,
> `windows/os.hpp` or `posix/os.hpp`, automatically. They should never
> be included manually.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/kill.hpp 
> bd944357bd741d62aea78c839d87b80853e4a852 
>   3rdparty/stout/include/stout/os/windows/rm.hpp 
> 1c66e3e6fa20116866842378077e10052c147289 
>   3rdparty/stout/include/stout/windows/net.hpp 
> d70f967773d23672f53dd7dd6123434f5e81eeb0 
> 
> 
> Diff: https://reviews.apache.org/r/67933/diff/1/
> 
> 
> Testing
> ---
> 
> Old patch I found on my computer that I figured I should post. `ninja tests ; 
> ctest -V` on Windows.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Review Request 67936: Fixed ephemeral ports deallocation in network/port_mapping isolator.

[Ilya Pronin]

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

Review request for mesos and Jie Yu.


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


Repository: mesos


Description
---

Ephemeral ports are allocated for a container during the preparation
phase and have to be deallocated during container cleanup regardless of
whether the container process was isolated or not. Otherwise those ports
can be leaked if the container is destroyed during preparation.


Diffs
-

  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
a29282ed0488d83966f222084031ed1d2b6ab1f5 
  src/tests/containerizer/port_mapping_tests.cpp 
553f53043a52cfe235fc2b2e88fb54d0f2725d3e 


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


Testing
---

Added `PortMappingIsolatorTest.ROOT_CleanupNotIsolated` test to verify that 
ephemeral ports are properly deallocated when the container was not isolated 
and manually checked that if fails without the fix. Ran `sudo make check`.


Thanks,

Ilya Pronin

Re: Review Request 67931: Support Image Manifest Version 2 Schema 2.

[Andrew Schwartzmeyer]

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



Ah, sorry, I will get the rest of this reviewed tomorrow.


3rdparty/cmake/Versions.cmake
Lines 7-8 (original), 7-8 (patched)


It looks like cURL has since posted 7.61.0, so as long as we're updating 
this, let's update to 7.61.0, and get it added to 
https://github.com/mesos/3rdparty/ so we can get the ReviewBot to pass. (We 
spoke offine so I know you're already doing this, just posting it for 
posterity.)



src/CMakeLists.txt
Lines 170 (patched)


I'm not certain we can just add it to `AGENT_SRC` as that'll also add it 
for POSIX systems. Instead, like how it's added to `LINUX_SRC`, let's also add 
it to `WIN32_SRC`.

I would suggest, take this existing snippet in this file (which shouldn't 
exist given the existence of `WIN32_SRC`:

```
if (WIN32)
  list(APPEND AGENT_SRC
slave/containerizer/mesos/isolators/windows/cpu.cpp
slave/containerizer/mesos/isolators/windows/mem.cpp)
else ()
```

and instead make:

```
set(WIN32)SRC
  slave/containerizer/mesos/isolators/docker/runtime.cpp
  slave/containerizer/mesos/isolators/windows/cpu.cpp
  slave/containerizer/mesos/isolators/windows/mem.cpp
  ... )
```



src/CMakeLists.txt
Lines 424-427 (original), 426-427 (patched)


With this being true, we can just move `uri/fetchers/docker.cpp` to the 
point where we're setting `URI_SRC` to begin with.



src/uri/fetchers/docker.cpp
Lines 102-105 (patched)


I think there's a `strings::replace` to do this.


- Andrew Schwartzmeyer


On July 16, 2018, 11:35 a.m., Liangyu Zhao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67931/
> ---
> 
> (Updated July 16, 2018, 11:35 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support parsing schema 2 and fetching blob with multiple URLs as
> specified in schema 2. Update `curl` version to 7.60.0 because of bug
> encountered in version 7.57.0.
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake 0a897d808cd9e05ac0d1a4e1ca61b8f3538f0c4a 
>   include/mesos/docker/spec.hpp 2879414dc42ffe633ac74b51e1bb116698c41162 
>   include/mesos/docker/v2_2.hpp PRE-CREATION 
>   include/mesos/docker/v2_2.proto PRE-CREATION 
>   include/mesos/uri/fetcher.hpp 760d6b33234d8efdc533c0c6f05e83a5c7d1f56b 
>   src/CMakeLists.txt 10b0946d6f49c7e9c201bad6f9f1b41cc8460fe5 
>   src/Makefile.am 228e168c22f3fd0367f029c506171c6979b31c07 
>   src/docker/spec.cpp 96fbf1f9cf1c2c4b2383607a97990f3a9156e6d9 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 98129d006cda9b65804b518619b6addc8990410a 
>   src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
> a5683e3fe15dd35596122fcc0c580ae9d3adf7f2 
>   src/uri/fetcher.hpp fbf64c6767dea3aa0798f68db8322ce47cd8ad36 
>   src/uri/fetcher.cpp 489c7ce0198ee6803dcc8eb9710b292fa743a0e8 
>   src/uri/fetchers/docker.hpp cdbab9a5684a68a729be12bc06d331acca137da5 
>   src/uri/fetchers/docker.cpp 55ca118660872a933a2dc186723bec6a39ee80f7 
> 
> 
> Diff: https://reviews.apache.org/r/67931/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Liangyu Zhao
> 
>

Re: Review Request 67928: Added balloon framework metric for tasks which were running.

[Benjamin Bannier]

> On July 16, 2018, 8:34 p.m., Meng Zhu wrote:
> > src/examples/balloon_framework.cpp
> > Lines 304 (patched)
> > 
> >
> > stati --> statistics?

This was supposed to be a plural of _status_ which of cause is _status_ (that's 
a long _u_), but I completely messed it up. I went with _status updates_ for 
now to avoid _statuses_.


> On July 16, 2018, 8:34 p.m., Meng Zhu wrote:
> > src/examples/balloon_framework.cpp
> > Lines 309 (patched)
> > 
> >
> > ditto

Ditto.


> On July 16, 2018, 8:34 p.m., Meng Zhu wrote:
> > src/examples/balloon_framework.cpp
> > Line 304 (original), 316 (patched)
> > 
> >
> > Let's add back the default.

I explicitly enumerated all possible cases here to forcibly trigger compiler 
errors should we add another task status. This is usually what we prefer in the 
code if possible.

Dropping.


- Benjamin


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


On July 17, 2018, 12:01 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67928/
> ---
> 
> (Updated July 17, 2018, 12:01 a.m.)
> 
> 
> Review request for mesos and Meng Zhu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The framework currently exposes metric counters for various expected
> and unexpected task termination reasons. Interpreting these counters
> can be non-trivial since tasks might fail due to benign, but unknown
> external reasons.
> 
> This patch adds a counter for the tasks which actually made it to the
> running stage which can be correlated with the different terminal task
> counts.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_framework.cpp cbc25e45258b05a645363d4229c0a701baa5481c 
> 
> 
> Diff: https://reviews.apache.org/r/67928/diff/2/
> 
> 
> Testing
> ---
> 
> Started the framework with a `local` master and examine the metric.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67928: Added balloon framework metric for tasks which were running.

[Benjamin Bannier]

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

(Updated July 17, 2018, 12:01 a.m.)


Review request for mesos and Meng Zhu.


Changes
---

Replaced pseudo Latin in comments with English.


Repository: mesos


Description
---

The framework currently exposes metric counters for various expected
and unexpected task termination reasons. Interpreting these counters
can be non-trivial since tasks might fail due to benign, but unknown
external reasons.

This patch adds a counter for the tasks which actually made it to the
running stage which can be correlated with the different terminal task
counts.


Diffs (updated)
-

  src/examples/balloon_framework.cpp cbc25e45258b05a645363d4229c0a701baa5481c 


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

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


Testing
---

Started the framework with a `local` master and examine the metric.


Thanks,

Benjamin Bannier

Review Request 67933: Replaced `` with `` in stout.

[Andrew Schwartzmeyer]

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

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


Repository: mesos


Description
---

The intent is that `os.hpp` includes the correct OS-specific header,
`windows/os.hpp` or `posix/os.hpp`, automatically. They should never
be included manually.


Diffs
-

  3rdparty/stout/include/stout/os/windows/kill.hpp 
bd944357bd741d62aea78c839d87b80853e4a852 
  3rdparty/stout/include/stout/os/windows/rm.hpp 
1c66e3e6fa20116866842378077e10052c147289 
  3rdparty/stout/include/stout/windows/net.hpp 
d70f967773d23672f53dd7dd6123434f5e81eeb0 


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


Testing
---

Old patch I found on my computer that I figured I should post. `ninja tests ; 
ctest -V` on Windows.


Thanks,

Andrew Schwartzmeyer

Re: Review Request 67869: Add use of `override` to the Mesos C++ style guide.

[Andrew Schwartzmeyer]

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


Ship it!




As mentioned in my reply to Ben, I am _for_ this change, as I believe that if 
we care about it, we need to call it out in addition to its presence in the 
Google Style Guide (that thing is enormous!).

- Andrew Schwartzmeyer


On July 11, 2018, 10:03 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67869/
> ---
> 
> (Updated July 11, 2018, 10:03 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Greg Mann, 
> Mesos Reviewbot, Till Toenshoff, and Zhitao Li.
> 
> 
> Bugs: MESOS-9065
> https://issues.apache.org/jira/browse/MESOS-9065
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add an explicit guideline to the Mesos C++ style guide that the use of
> `override` keyword is required. Update cpplint and clang-tidy tooling
> to encourage this.
> 
> 
> Diffs
> -
> 
>   docs/c++-style-guide.md 80a1af318984d47bdfa641aa12e28963d3183fea 
>   support/clang-tidy fcffb39089573a708dc3f2052f639a1a3621040d 
>   support/mesos-style.py 27ed553cb1d9e0c3c750b414eafe0144c3442c43 
> 
> 
> Diff: https://reviews.apache.org/r/67869/diff/2/
> 
> 
> Testing
> ---
> 
> Manual. The `cpplint` change doesn't appear to enforce the use of `override`.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 67814: Added per-framework metrics for offer operations.

[Gastón Kleiman]

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


Ship it!




Ship It!

- Gastón Kleiman


On July 10, 2018, 2:23 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67814/
> ---
> 
> (Updated July 10, 2018, 2:23 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Chun-Hung Hsiao, Gastón Kleiman, 
> Gilbert Song, and Vinod Kone.
> 
> 
> Bugs: MESOS-8845
> https://issues.apache.org/jira/browse/MESOS-8845
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added per-framework metrics for offer operations.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 0c0d6ca375b806a02625261f7e272a126bc4debe 
>   src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
>   src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 
> 
> 
> Diff: https://reviews.apache.org/r/67814/diff/3/
> 
> 
> Testing
> ---
> 
> Testing details can be found at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 67868: Apply the `override` keyword to Mesos.

[Andrew Schwartzmeyer]

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


Ship it!




Ship It!

- Andrew Schwartzmeyer


On July 11, 2018, 10:03 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67868/
> ---
> 
> (Updated July 11, 2018, 10:03 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Greg Mann, 
> Mesos Reviewbot, Till Toenshoff, and Zhitao Li.
> 
> 
> Bugs: MESOS-9065
> https://issues.apache.org/jira/browse/MESOS-9065
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Apply the `override` keyword to Mesos.
> 
> 
> Diffs
> -
> 
>   include/mesos/executor.hpp 6a9e6fc6b1ab94be2ade9a96de4fb11364b46a13 
>   include/mesos/scheduler.hpp d4cc96e86b27cfe2e3a3de448fd0f5e06b1564df 
>   include/mesos/state/in_memory.hpp 3fcf4620b7252c632ca9416d06a5bd1b9ed5a35a 
>   include/mesos/state/leveldb.hpp 265e08b58ea4a50458587f884a98e24cdc5d1aa2 
>   include/mesos/state/log.hpp 54cbdc8d67ef9dfc47b321d5837e09dd56920c94 
>   include/mesos/state/protobuf.hpp c15696ab79b61f5487ee4a849d62b34b91ca1614 
>   include/mesos/state/zookeeper.hpp fee93989c74db031ff2343e06eb25c5a7b88510f 
>   include/mesos/v1/executor.hpp 9a2eb454b0d5d5dce7af03e3100dfd7ae078b5a1 
>   include/mesos/v1/scheduler.hpp b1dfbb1454b550c8a2cd018b1ca9e3533b9171f7 
>   include/mesos/zookeeper/group.hpp 7983e50ba9d240f19a2b3ae02d7fa8339e77f2fa 
>   include/mesos/zookeeper/watcher.hpp 
> b2961a54eae74c5810df5d085a39652991685930 
>   src/authentication/cram_md5/authenticatee.hpp 
> d77b2108c83951f0495b67f6494fb7410592e848 
>   src/authentication/cram_md5/authenticatee.cpp 
> d43dedf1ec86d3f25811718101ff273a7c0d7333 
>   src/authentication/cram_md5/authenticator.hpp 
> a68b2cf43fb5e45bdc2073a9031b72b6f7a1c1bc 
>   src/authentication/cram_md5/authenticator.cpp 
> 0aad3aa9a6a2c70a6e98fc75a3443ea340d0404d 
>   src/authorizer/local/authorizer.hpp 
> 794443ae9e0ec66845bf611d45d6a037ec96ddf4 
>   src/authorizer/local/authorizer.cpp 
> 056b1723f5ec34271f1afe3ad810bc498db8f28e 
>   src/checks/checker_process.hpp 6364af8f0bfbdf7f6596dc0bf32049806bd2ba83 
>   src/cli/execute.cpp 89e5130784e6f18850a743c4e0219245f7658ff3 
>   src/common/http.hpp 76e8f71b5687031344612e1742d3bb50040db758 
>   src/common/recordio.hpp 88e789c7f65c7ac2a97473d656ab15b3803cc886 
>   src/docker/executor.cpp d9f5a736c1cb28a55db1b4fdc793bc706c7cb1fe 
>   src/examples/balloon_executor.cpp 5281db1718f3db462d40d65dfa9ce0e3d9adcc34 
>   src/examples/balloon_framework.cpp cbc25e45258b05a645363d4229c0a701baa5481c 
>   src/examples/disk_full_framework.cpp 
> dfbdddae7f50d314e67db59300e5140b0476f807 
>   src/examples/docker_no_executor_framework.cpp 
> 0cbda07872fcf7db091c17ef49f5bcea6d80e660 
>   src/examples/dynamic_reservation_framework.cpp 
> e8d8d09dbda95f77e4b7dbbc8e031cd962e3a45f 
>   src/examples/example_module_impl.cpp 
> b84ff73ba96999072a03461f46717c2f104ae9ff 
>   src/examples/load_generator_framework.cpp 
> 3ae25b221185ec6bd4bd84c478be9a511d763162 
>   src/examples/long_lived_executor.cpp 
> 21b511d1d6d0793569ebdb2ecc5c9bc37f3de632 
>   src/examples/long_lived_framework.cpp 
> 0e4e3d00f86ce810ecc273baa4c7d5d124a1a29d 
>   src/examples/no_executor_framework.cpp 
> dc75d7802e43164293c5d68ae78eb7fba6deba64 
>   src/examples/persistent_volume_framework.cpp 
> 43375503f1bdb74fba7994f73beb5b5486026e73 
>   src/examples/test_anonymous_module.cpp 
> c41b7fc6d5feb7760603df1cda8cc0697a815324 
>   src/examples/test_csi_plugin.cpp 9c4da8811cc260bcf3bccfea3036a7964cb75697 
>   src/examples/test_csi_user_framework.cpp 
> 91212e990af711633f7b890b0c9e10587f9efe7c 
>   src/examples/test_executor.cpp d9b7cdd7dbaf61bc54d7490ab9e7f188ea45f571 
>   src/examples/test_framework.cpp aaf952c20cd3d722d00504228c297c053079bf50 
>   src/examples/test_hook_module.cpp 9b9d75c2bb81bf80e8edb919a04e0854e858f49d 
>   src/examples/test_http_executor.cpp 
> fb9209f630c9c0fa6c1e866eeaf178c560372edc 
>   src/examples/test_http_framework.cpp 
> 9ea3c6f35bf6464b4c6cfa7ba2eaf4febc168396 
>   src/exec/exec.cpp ca4f065af18934a59161ad0ee63daba59beeb115 
>   src/executor/executor.cpp ab67caefd82f46eacd33221270b2c408ef70cd17 
>   src/executor/v0_v1executor.hpp ef9bae6ab75279814a5ba9081aa663107195d8e6 
>   src/executor/v0_v1executor.cpp 086cfc74c820657cb3dd7d531f2de44a97832782 
>   src/files/files.cpp 4b8713a2783ec4c4205af071b6cfff3397ec4c1d 
>   src/java/jni/org_apache_mesos_MesosExecutorDriver.cpp 
> 39dba40bf94df27a870296810712999009b50506 
>   src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp 
> 1f9572c00e1285c5a13f3c2e59c058aab6794ebe 
>   src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp 
> 1deeb08337de7cf367c93b8143a035186c06f2bd 
>   sr

Re: Review Request 67927: Removed potentially unneeded lambda capture.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67927 was successfully built and tested.

Reviews applied: `['67927']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1936/mesos-review-67927

- Mesos Reviewbot Windows


On July 16, 2018, 12:22 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67927/
> ---
> 
> (Updated July 16, 2018, 12:22 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Dario Rexin.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In some instances variables used in the body of a lambda do not need
> to be captured explicitly, see e.g.,
> https://stackoverflow.com/a/43468519/176922. Support for this across
> compiler vendors seems to be incomplete at this point; while e.g.,
> clang emits a warning (which we potentially turn into a fatal error)
> for this, msvc still seems to require such variables to be captured.
> 
> To provide a warning free build this patch moves from the usually
> preferred explicit capture list to an implicit capture by reference.
> This allows us to capture all needed variables correctly without
> having to capture too much.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> e8ef21fd9c203ac5e10c17694393322151231435 
> 
> 
> Diff: https://reviews.apache.org/r/67927/diff/3/
> 
> 
> Testing
> ---
> 
> Tested on a number of platforms and compilers in internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67867: Apply the `override` keyword to libprocess.

[Andrew Schwartzmeyer]

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


Ship it!




Ship It!

- Andrew Schwartzmeyer


On July 11, 2018, 10:09 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67867/
> ---
> 
> (Updated July 11, 2018, 10:09 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Greg Mann, 
> Mesos Reviewbot, Till Toenshoff, and Zhitao Li.
> 
> 
> Bugs: MESOS-9065
> https://issues.apache.org/jira/browse/MESOS-9065
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Apply the `override` keyword to libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/README.md 9846ecea45f742da74dd39d1701b5177d2f3f125 
>   3rdparty/libprocess/examples/example.cpp 
> 0db2d03bccb7b4910acfd7e08248f022bc74b6ef 
>   3rdparty/libprocess/include/process/async.hpp 
> 116c90c01655cd1df67616d9c927f6769f5c92da 
>   3rdparty/libprocess/include/process/collect.hpp 
> 5263f0134808a0161b16b3b45871f711b0f5cb45 
>   3rdparty/libprocess/include/process/event.hpp 
> ec64eb786b6cf30bf03ba07a30c6e44d2db9173a 
>   3rdparty/libprocess/include/process/filter.hpp 
> 3f4a8272c451e333ebf9ff79c5f0fee329a8450a 
>   3rdparty/libprocess/include/process/firewall.hpp 
> 0a7b985380893820d0193f7b2eb657ee8c07430e 
>   3rdparty/libprocess/include/process/gmock.hpp 
> b5ead1c2ecacdb468a319c98edd2ffc831a9f74c 
>   3rdparty/libprocess/include/process/grpc.hpp 
> 0ff8184791277ed02b30481b352be60f10743428 
>   3rdparty/libprocess/include/process/gtest.hpp 
> 79ea2b1f6a2a16ff9c5aae281c900e50940877bf 
>   3rdparty/libprocess/include/process/help.hpp 
> 3393b71c484bb8c55c6a4cd2a27dcb798e841a80 
>   3rdparty/libprocess/include/process/limiter.hpp 
> b4b6a7bb48aa19fa40c0ea9f8759b8f553f43ca6 
>   3rdparty/libprocess/include/process/logging.hpp 
> aad7ce8caa193e6aff1a295fef899b49fcfce7b0 
>   3rdparty/libprocess/include/process/metrics/counter.hpp 
> 15aeeb5710636d4e11b862faee50fd6ea4d1cb07 
>   3rdparty/libprocess/include/process/metrics/metrics.hpp 
> f9b72029b2c85826c91b1d7656b0af94dc87010c 
>   3rdparty/libprocess/include/process/metrics/pull_gauge.hpp 
> 5c2a227425f31c439bd48b58171d4038a7f04e5f 
>   3rdparty/libprocess/include/process/metrics/push_gauge.hpp 
> 5c39846172fc46c3022e9aa420460f5b0fb6db2e 
>   3rdparty/libprocess/include/process/metrics/timer.hpp 
> 0a9c0227c457c6c81a59f65f901a5464ee00983d 
>   3rdparty/libprocess/include/process/process.hpp 
> c36df991b6a2c120ab0562e8ff907f9fbf8630d1 
>   3rdparty/libprocess/include/process/profiler.hpp 
> 74890ae7b10c15b77ef1d112a7ebfb3a45b29ed2 
>   3rdparty/libprocess/include/process/protobuf.hpp 
> 5a75a83d294e2d14f81503b113724893c0e140f3 
>   3rdparty/libprocess/include/process/reap.hpp 
> 628bc0a6ec59bfe6bfa7c54906ea266ac31a71a7 
>   3rdparty/libprocess/include/process/run.hpp 
> 0d282c989b8ddeb9ddb31a050cd52afe01623a66 
>   3rdparty/libprocess/include/process/sequence.hpp 
> 24712b16953347eb05a28ad50fbdfad17bf64f19 
>   3rdparty/libprocess/include/process/system.hpp 
> 81ded8af1015fe3ecd578ed98759b66f917ee264 
>   3rdparty/libprocess/src/encoder.hpp 
> 70b5ec479e90c0eb6ac729b465739b581729a956 
>   3rdparty/libprocess/src/http.cpp dc38716960da98b861ee69ae310068e2db1245bb 
>   3rdparty/libprocess/src/http_proxy.hpp 
> 5b6e7e8786ed9eab50cd4c2cfdec455c92d72eca 
>   3rdparty/libprocess/src/memory_profiler.hpp 
> 617f8de27949f2e536e53c122d7269f2a919814b 
>   3rdparty/libprocess/src/poll_socket.hpp 
> eebd7182e433adabf6bb20c41d6ecf1863adf992 
>   3rdparty/libprocess/src/process.cpp 
> b3bd0286e09dcb9d0867679c6a509c34e309a72e 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> 604122a2f33d7abfab92631d9eac33b6753e846b 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 5429034e9f4a1a6a4c4dc53a9f7e01dca119ea24 
>   3rdparty/libprocess/src/tests/metrics_tests.cpp 
> a5b41ed3f4491244656222681f443e7ed55393c2 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 2f8a8234aa1cb6128a85254efc81868c19e6fdac 
>   3rdparty/libprocess/src/tests/profiler_tests.cpp 
> 995bd02f6ecce484cd9b2aca355c2707d73d40b2 
> 
> 
> Diff: https://reviews.apache.org/r/67867/diff/3/
> 
> 
> Testing
> ---
> 
> make check (Fedora 28)
> 
> The compliation database was generated using the method in 
> `support/mesos-tidy/entrypoint.sh`. I also used `-DENABLE_GRPC=YES`, 
> `-DENABLE_JEMALLOC_ALLOCATOR` and `-DENABLE_JAVA`. I wasn't able to get the 
> SSL support to build on F28, and not sure how the Python support works in 
> cmake.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 67923: Improved performance of cgroups::read by verifying after failure.

[Jie Yu]

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


Ship it!




LGTM! Thanks Ben. I think longer term, we will modify caller to call `verify`, 
making read/write/create pure helper for writing to cgroup filesystem.

- Jie Yu


On July 16, 2018, 2:03 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67923/
> ---
> 
> (Updated July 16, 2018, 2:03 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-8418
> https://issues.apache.org/jira/browse/MESOS-8418
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It turns out that cgroups::verify is expensive and leads to a severe
> performance issue on the agent during container metrics collection
> if there are a lot of containers on the agent. See MESOS-8418.
> 
> Since cgroups::verify serves to provide a helpful error message,
> this patch preserves the error message, but only if the read fails.
> 
> Longer term, there probably needs to be some re-structuring of the
> code to make verification caller-controlled, or perhaps the verify
> code can occur consistently post-operation (as done in this patch),
> or perhaps verify can be optimized substantially.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp b12e63c112a7aa7a6f7150359ff5409f8214067e 
> 
> 
> Diff: https://reviews.apache.org/r/67923/diff/1/
> 
> 
> Testing
> ---
> 
> Ran through internal CI.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 67869: Add use of `override` to the Mesos C++ style guide.

[Andrew Schwartzmeyer]

> On July 16, 2018, 1:31 p.m., Benjamin Bannier wrote:
> > docs/c++-style-guide.md
> > Lines 651-662 (patched)
> > 
> >
> > Like I already mentioned on the mailing list, I am still not a fan of 
> > adding redundant information which is already in the Google style guide 
> > here. That way we'd ask ideal contributors to read the Google style guide, 
> > our style guide, and figure out where any the delta is.

IMHO it cannot be reasonably expected that a dev is going to read the entire 
Google Style Guide. I refer to it plenty often, but if we think this part is 
_important_, then I think that it is on us to call it out by adding it here.

Sure, the _ideal_ contributor thinks about every possible syntactical rule or 
formatting rule and tries to find the answer as they write it (I do this, but 
I'm crazy, and still get it wrong a lot), but realistically I think the most we 
can expect is that this guide is read, and hopefully it sinks in as "gotchas" 
that we care most about.


- Andrew


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


On July 11, 2018, 10:03 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67869/
> ---
> 
> (Updated July 11, 2018, 10:03 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Greg Mann, 
> Mesos Reviewbot, Till Toenshoff, and Zhitao Li.
> 
> 
> Bugs: MESOS-9065
> https://issues.apache.org/jira/browse/MESOS-9065
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add an explicit guideline to the Mesos C++ style guide that the use of
> `override` keyword is required. Update cpplint and clang-tidy tooling
> to encourage this.
> 
> 
> Diffs
> -
> 
>   docs/c++-style-guide.md 80a1af318984d47bdfa641aa12e28963d3183fea 
>   support/clang-tidy fcffb39089573a708dc3f2052f639a1a3621040d 
>   support/mesos-style.py 27ed553cb1d9e0c3c750b414eafe0144c3442c43 
> 
> 
> Diff: https://reviews.apache.org/r/67869/diff/2/
> 
> 
> Testing
> ---
> 
> Manual. The `cpplint` change doesn't appear to enforce the use of `override`.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 67812: Added per-framework offer metrics.

[Gastón Kleiman]

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


Ship it!




Ship It!

- Gastón Kleiman


On July 9, 2018, 11:18 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67812/
> ---
> 
> (Updated July 9, 2018, 11:18 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Chun-Hung Hsiao, Gastón Kleiman, 
> Gilbert Song, and Vinod Kone.
> 
> 
> Bugs: MESOS-8848
> https://issues.apache.org/jira/browse/MESOS-8848
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added per-framework offer metrics.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp ddc8df0ea82241be6c733237feef1553c7669eb2 
>   src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
>   src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 
> 
> 
> Diff: https://reviews.apache.org/r/67812/diff/4/
> 
> 
> Testing
> ---
> 
> Testing details can be found at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 67488: Updated CLI to Python 3.

[Andrew Schwartzmeyer]

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


Ship it!




Did you have an automated way of finding where we should apply `override` in 
existing code? I'd like to see if there's any Windows-only code/tests we should 
fix too.

- Andrew Schwartzmeyer


On July 6, 2018, 6:56 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67488/
> ---
> 
> (Updated July 6, 2018, 6:56 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Eric Chung, 
> and Kevin Klues.
> 
> 
> Bugs: MESOS-8955
> https://issues.apache.org/jira/browse/MESOS-8955
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The build tools are also up to date thus the CLI can still be built
> using Autotools and CMake. No features have been added to the CLI.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 61387d77b12a17571a31430db3ca1fe0bbb66a21 
>   configure.ac 66cc28a5a34949bcadc038551249f3781ea9d45b 
>   src/Makefile.am db42e71d90ff2066e104f4b9c269c5e78a9a6ada 
>   src/python/cli_new/CMakeLists.txt ef8da70757e2721f4ac1bee46d0b5d95e81298ca 
>   src/python/cli_new/README.md 3d646e91a8c7c72d4ee1b1180454e5f587295053 
>   src/python/cli_new/bin/main.py 53130383d8ca2ed40c97224b3a6e98aa6b6b107c 
>   src/python/cli_new/bootstrap fb6fbc449a970ccf960914ed910204f3984ea61f 
>   src/python/cli_new/lib/cli/config.py 
> 6f92622725d8a042a2a728fd38c977ac690ef6be 
>   src/python/cli_new/lib/cli/docopt.py 
> 86a4e9c74326fb80cc59487113f07358dd96960d 
>   src/python/cli_new/lib/cli/http.py 03d6031cb3273575f41d4d06d9a409f74488a16b 
>   src/python/cli_new/lib/cli/plugins/agent/main.py 
> 59280ece8ebd00bb96df3675b6356a26cc48a2c0 
>   src/python/cli_new/lib/cli/plugins/task/main.py 
> cc6cff56c71262729a8870017bef2e97636abe5a 
>   src/python/cli_new/lib/cli/tests/base.py 
> 89360e6cac5ca910044a5a82fab7237510edee7f 
>   src/python/cli_new/lib/cli/tests/tests.py 
> 79e1036f6d11c63884091fe43672607b03955c1a 
>   src/python/cli_new/lib/cli/util.py 307b22293a9c7199ad7088dfd0db6dff83a08ac8 
>   src/python/cli_new/tests/main.py acf2e0868555da0eb1c1cee7fb30b1e80783f1e1 
>   src/python/cli_new/tox.ini 58ca3807e3d6096296b4cd09a5cec32b32444d91 
>   src/python/lib/tox.ini 05b633e837fa39a36fb2c5a0778513ce743099db 
>   support/mesos-style.py 27ed553cb1d9e0c3c750b414eafe0144c3442c43 
>   support/python3/mesos-style.py 350ef909e3e7a1c927140cf4475547d704ac2ad5 
> 
> 
> Diff: https://reviews.apache.org/r/67488/diff/12/
> 
> 
> Testing
> ---
> 
> Testing done on Fedora 25 with `python` being Python 2.7, `python3` being 
> Python 3.5 and `python36` being Python 3.6.
> 
> 
> For Autotools:
> 
> ```
> $ ./bootstrap
> $ mkdir build
> $ cd build
> $ PYTHON_3=python36 ../configure --enable-new-cli --disable-java
> $ make check
> ```
> 
> For CMake:
> 
> ```
> $ ./bootstrap
> $ mkdir build
> $ cd build
> $ cmake .. -DENABLE_NEW_CLI=1 -DPYTHON=python36
> $ cmake --build . -- -j16
> $ ./src/mesos
> Mesos CLI
> 
> Usage:
>   mesos (-h | --help)
>   mesos --version
>   mesos  [...]
> 
> Options:
>   -h --help  Show this screen.
>   --version  Show version info.
> 
> Commands:
>   agent   Interacts with the Mesos agents
>   config  Interacts with the Mesos CLI configuration file
>   taskInteracts with the tasks running in a Mesos cluster
> 
> See 'mesos help ' for more information on a specific command.
> $ cmake --build . --target tests -- -j16
> $ ctest -R CLI
> Test project /home/agrillet/apache-mesos/build
> Start 4: CLITests
> 1/1 Test #4: CLITests .   Passed3.63 sec
> 
> 100% tests passed, 0 tests failed out of 1
> ```
> 
> Checked that the the CLI tests were run, that the content of the directory 
> build/src/cli was as expected, and that build/src/mesos was correctly running.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 67869: Add use of `override` to the Mesos C++ style guide.

[Benjamin Bannier]

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


Ship it!





docs/c++-style-guide.md
Lines 651-662 (patched)


Like I already mentioned on the mailing list, I am still not a fan of 
adding redundant information which is already in the Google style guide here. 
That way we'd ask ideal contributors to read the Google style guide, our style 
guide, and figure out where any the delta is.


- Benjamin Bannier


On July 12, 2018, 7:03 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67869/
> ---
> 
> (Updated July 12, 2018, 7:03 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Greg Mann, 
> Mesos Reviewbot, Till Toenshoff, and Zhitao Li.
> 
> 
> Bugs: MESOS-9065
> https://issues.apache.org/jira/browse/MESOS-9065
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add an explicit guideline to the Mesos C++ style guide that the use of
> `override` keyword is required. Update cpplint and clang-tidy tooling
> to encourage this.
> 
> 
> Diffs
> -
> 
>   docs/c++-style-guide.md 80a1af318984d47bdfa641aa12e28963d3183fea 
>   support/clang-tidy fcffb39089573a708dc3f2052f639a1a3621040d 
>   support/mesos-style.py 27ed553cb1d9e0c3c750b414eafe0144c3442c43 
> 
> 
> Diff: https://reviews.apache.org/r/67869/diff/2/
> 
> 
> Testing
> ---
> 
> Manual. The `cpplint` change doesn't appear to enforce the use of `override`.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 67922: Removed unused `TestStore` mock.

[Andrew Schwartzmeyer]

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


Ship it!




Ship It!

- Andrew Schwartzmeyer


On July 15, 2018, 5:36 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67922/
> ---
> 
> (Updated July 15, 2018, 5:36 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Gilbert Song, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed unused `TestStore` mock.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 9c13eaaa8bd62f10539ebbb6ab16c7303dabfcc1 
>   src/tests/containerizer/store.hpp 0589be698d3bf835293fbe7ea817a11a7bb91f67 
> 
> 
> Diff: https://reviews.apache.org/r/67922/diff/1/
> 
> 
> Testing
> ---
> 
> make check (Fedora 28)
> 
> `TestStore` usage was removed in 
> [r/52539](https://reviews.apache.org/r/52539).
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 67868: Apply the `override` keyword to Mesos.

[Benjamin Bannier]

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


Ship it!




Ship It!

- Benjamin Bannier


On July 12, 2018, 7:03 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67868/
> ---
> 
> (Updated July 12, 2018, 7:03 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Greg Mann, 
> Mesos Reviewbot, Till Toenshoff, and Zhitao Li.
> 
> 
> Bugs: MESOS-9065
> https://issues.apache.org/jira/browse/MESOS-9065
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Apply the `override` keyword to Mesos.
> 
> 
> Diffs
> -
> 
>   include/mesos/executor.hpp 6a9e6fc6b1ab94be2ade9a96de4fb11364b46a13 
>   include/mesos/scheduler.hpp d4cc96e86b27cfe2e3a3de448fd0f5e06b1564df 
>   include/mesos/state/in_memory.hpp 3fcf4620b7252c632ca9416d06a5bd1b9ed5a35a 
>   include/mesos/state/leveldb.hpp 265e08b58ea4a50458587f884a98e24cdc5d1aa2 
>   include/mesos/state/log.hpp 54cbdc8d67ef9dfc47b321d5837e09dd56920c94 
>   include/mesos/state/protobuf.hpp c15696ab79b61f5487ee4a849d62b34b91ca1614 
>   include/mesos/state/zookeeper.hpp fee93989c74db031ff2343e06eb25c5a7b88510f 
>   include/mesos/v1/executor.hpp 9a2eb454b0d5d5dce7af03e3100dfd7ae078b5a1 
>   include/mesos/v1/scheduler.hpp b1dfbb1454b550c8a2cd018b1ca9e3533b9171f7 
>   include/mesos/zookeeper/group.hpp 7983e50ba9d240f19a2b3ae02d7fa8339e77f2fa 
>   include/mesos/zookeeper/watcher.hpp 
> b2961a54eae74c5810df5d085a39652991685930 
>   src/authentication/cram_md5/authenticatee.hpp 
> d77b2108c83951f0495b67f6494fb7410592e848 
>   src/authentication/cram_md5/authenticatee.cpp 
> d43dedf1ec86d3f25811718101ff273a7c0d7333 
>   src/authentication/cram_md5/authenticator.hpp 
> a68b2cf43fb5e45bdc2073a9031b72b6f7a1c1bc 
>   src/authentication/cram_md5/authenticator.cpp 
> 0aad3aa9a6a2c70a6e98fc75a3443ea340d0404d 
>   src/authorizer/local/authorizer.hpp 
> 794443ae9e0ec66845bf611d45d6a037ec96ddf4 
>   src/authorizer/local/authorizer.cpp 
> 056b1723f5ec34271f1afe3ad810bc498db8f28e 
>   src/checks/checker_process.hpp 6364af8f0bfbdf7f6596dc0bf32049806bd2ba83 
>   src/cli/execute.cpp 89e5130784e6f18850a743c4e0219245f7658ff3 
>   src/common/http.hpp 76e8f71b5687031344612e1742d3bb50040db758 
>   src/common/recordio.hpp 88e789c7f65c7ac2a97473d656ab15b3803cc886 
>   src/docker/executor.cpp d9f5a736c1cb28a55db1b4fdc793bc706c7cb1fe 
>   src/examples/balloon_executor.cpp 5281db1718f3db462d40d65dfa9ce0e3d9adcc34 
>   src/examples/balloon_framework.cpp cbc25e45258b05a645363d4229c0a701baa5481c 
>   src/examples/disk_full_framework.cpp 
> dfbdddae7f50d314e67db59300e5140b0476f807 
>   src/examples/docker_no_executor_framework.cpp 
> 0cbda07872fcf7db091c17ef49f5bcea6d80e660 
>   src/examples/dynamic_reservation_framework.cpp 
> e8d8d09dbda95f77e4b7dbbc8e031cd962e3a45f 
>   src/examples/example_module_impl.cpp 
> b84ff73ba96999072a03461f46717c2f104ae9ff 
>   src/examples/load_generator_framework.cpp 
> 3ae25b221185ec6bd4bd84c478be9a511d763162 
>   src/examples/long_lived_executor.cpp 
> 21b511d1d6d0793569ebdb2ecc5c9bc37f3de632 
>   src/examples/long_lived_framework.cpp 
> 0e4e3d00f86ce810ecc273baa4c7d5d124a1a29d 
>   src/examples/no_executor_framework.cpp 
> dc75d7802e43164293c5d68ae78eb7fba6deba64 
>   src/examples/persistent_volume_framework.cpp 
> 43375503f1bdb74fba7994f73beb5b5486026e73 
>   src/examples/test_anonymous_module.cpp 
> c41b7fc6d5feb7760603df1cda8cc0697a815324 
>   src/examples/test_csi_plugin.cpp 9c4da8811cc260bcf3bccfea3036a7964cb75697 
>   src/examples/test_csi_user_framework.cpp 
> 91212e990af711633f7b890b0c9e10587f9efe7c 
>   src/examples/test_executor.cpp d9b7cdd7dbaf61bc54d7490ab9e7f188ea45f571 
>   src/examples/test_framework.cpp aaf952c20cd3d722d00504228c297c053079bf50 
>   src/examples/test_hook_module.cpp 9b9d75c2bb81bf80e8edb919a04e0854e858f49d 
>   src/examples/test_http_executor.cpp 
> fb9209f630c9c0fa6c1e866eeaf178c560372edc 
>   src/examples/test_http_framework.cpp 
> 9ea3c6f35bf6464b4c6cfa7ba2eaf4febc168396 
>   src/exec/exec.cpp ca4f065af18934a59161ad0ee63daba59beeb115 
>   src/executor/executor.cpp ab67caefd82f46eacd33221270b2c408ef70cd17 
>   src/executor/v0_v1executor.hpp ef9bae6ab75279814a5ba9081aa663107195d8e6 
>   src/executor/v0_v1executor.cpp 086cfc74c820657cb3dd7d531f2de44a97832782 
>   src/files/files.cpp 4b8713a2783ec4c4205af071b6cfff3397ec4c1d 
>   src/java/jni/org_apache_mesos_MesosExecutorDriver.cpp 
> 39dba40bf94df27a870296810712999009b50506 
>   src/java/jni/org_apache_mesos_MesosSchedulerDriver.cpp 
> 1f9572c00e1285c5a13f3c2e59c058aab6794ebe 
>   src/java/jni/org_apache_mesos_v1_scheduler_V0Mesos.cpp 
> 1deeb08337de7cf367c93b8143a035186c06f2bd 
>   src/laun

Re: Review Request 67867: Apply the `override` keyword to libprocess.

[Benjamin Bannier]

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


Ship it!




Ship It!

- Benjamin Bannier


On July 12, 2018, 7:09 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67867/
> ---
> 
> (Updated July 12, 2018, 7:09 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Greg Mann, 
> Mesos Reviewbot, Till Toenshoff, and Zhitao Li.
> 
> 
> Bugs: MESOS-9065
> https://issues.apache.org/jira/browse/MESOS-9065
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Apply the `override` keyword to libprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/README.md 9846ecea45f742da74dd39d1701b5177d2f3f125 
>   3rdparty/libprocess/examples/example.cpp 
> 0db2d03bccb7b4910acfd7e08248f022bc74b6ef 
>   3rdparty/libprocess/include/process/async.hpp 
> 116c90c01655cd1df67616d9c927f6769f5c92da 
>   3rdparty/libprocess/include/process/collect.hpp 
> 5263f0134808a0161b16b3b45871f711b0f5cb45 
>   3rdparty/libprocess/include/process/event.hpp 
> ec64eb786b6cf30bf03ba07a30c6e44d2db9173a 
>   3rdparty/libprocess/include/process/filter.hpp 
> 3f4a8272c451e333ebf9ff79c5f0fee329a8450a 
>   3rdparty/libprocess/include/process/firewall.hpp 
> 0a7b985380893820d0193f7b2eb657ee8c07430e 
>   3rdparty/libprocess/include/process/gmock.hpp 
> b5ead1c2ecacdb468a319c98edd2ffc831a9f74c 
>   3rdparty/libprocess/include/process/grpc.hpp 
> 0ff8184791277ed02b30481b352be60f10743428 
>   3rdparty/libprocess/include/process/gtest.hpp 
> 79ea2b1f6a2a16ff9c5aae281c900e50940877bf 
>   3rdparty/libprocess/include/process/help.hpp 
> 3393b71c484bb8c55c6a4cd2a27dcb798e841a80 
>   3rdparty/libprocess/include/process/limiter.hpp 
> b4b6a7bb48aa19fa40c0ea9f8759b8f553f43ca6 
>   3rdparty/libprocess/include/process/logging.hpp 
> aad7ce8caa193e6aff1a295fef899b49fcfce7b0 
>   3rdparty/libprocess/include/process/metrics/counter.hpp 
> 15aeeb5710636d4e11b862faee50fd6ea4d1cb07 
>   3rdparty/libprocess/include/process/metrics/metrics.hpp 
> f9b72029b2c85826c91b1d7656b0af94dc87010c 
>   3rdparty/libprocess/include/process/metrics/pull_gauge.hpp 
> 5c2a227425f31c439bd48b58171d4038a7f04e5f 
>   3rdparty/libprocess/include/process/metrics/push_gauge.hpp 
> 5c39846172fc46c3022e9aa420460f5b0fb6db2e 
>   3rdparty/libprocess/include/process/metrics/timer.hpp 
> 0a9c0227c457c6c81a59f65f901a5464ee00983d 
>   3rdparty/libprocess/include/process/process.hpp 
> c36df991b6a2c120ab0562e8ff907f9fbf8630d1 
>   3rdparty/libprocess/include/process/profiler.hpp 
> 74890ae7b10c15b77ef1d112a7ebfb3a45b29ed2 
>   3rdparty/libprocess/include/process/protobuf.hpp 
> 5a75a83d294e2d14f81503b113724893c0e140f3 
>   3rdparty/libprocess/include/process/reap.hpp 
> 628bc0a6ec59bfe6bfa7c54906ea266ac31a71a7 
>   3rdparty/libprocess/include/process/run.hpp 
> 0d282c989b8ddeb9ddb31a050cd52afe01623a66 
>   3rdparty/libprocess/include/process/sequence.hpp 
> 24712b16953347eb05a28ad50fbdfad17bf64f19 
>   3rdparty/libprocess/include/process/system.hpp 
> 81ded8af1015fe3ecd578ed98759b66f917ee264 
>   3rdparty/libprocess/src/encoder.hpp 
> 70b5ec479e90c0eb6ac729b465739b581729a956 
>   3rdparty/libprocess/src/http.cpp dc38716960da98b861ee69ae310068e2db1245bb 
>   3rdparty/libprocess/src/http_proxy.hpp 
> 5b6e7e8786ed9eab50cd4c2cfdec455c92d72eca 
>   3rdparty/libprocess/src/memory_profiler.hpp 
> 617f8de27949f2e536e53c122d7269f2a919814b 
>   3rdparty/libprocess/src/poll_socket.hpp 
> eebd7182e433adabf6bb20c41d6ecf1863adf992 
>   3rdparty/libprocess/src/process.cpp 
> b3bd0286e09dcb9d0867679c6a509c34e309a72e 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> 604122a2f33d7abfab92631d9eac33b6753e846b 
>   3rdparty/libprocess/src/tests/http_tests.cpp 
> 5429034e9f4a1a6a4c4dc53a9f7e01dca119ea24 
>   3rdparty/libprocess/src/tests/metrics_tests.cpp 
> a5b41ed3f4491244656222681f443e7ed55393c2 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 2f8a8234aa1cb6128a85254efc81868c19e6fdac 
>   3rdparty/libprocess/src/tests/profiler_tests.cpp 
> 995bd02f6ecce484cd9b2aca355c2707d73d40b2 
> 
> 
> Diff: https://reviews.apache.org/r/67867/diff/3/
> 
> 
> Testing
> ---
> 
> make check (Fedora 28)
> 
> The compliation database was generated using the method in 
> `support/mesos-tidy/entrypoint.sh`. I also used `-DENABLE_GRPC=YES`, 
> `-DENABLE_JEMALLOC_ALLOCATOR` and `-DENABLE_JAVA`. I wasn't able to get the 
> SSL support to build on F28, and not sure how the Python support works in 
> cmake.
> 
> 
> Thanks,
> 
> James Peach
> 
>

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

[Gastón Kleiman]

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


Fix it, then Ship it!





src/master/metrics.cpp
Lines 551-554 (patched)


Per offline discussion we can inline this method in 
`getFrameworkMetricPrefix()`.


- Gastón Kleiman


On July 10, 2018, 8:39 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67875/
> ---
> 
> (Updated July 10, 2018, 8:39 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gastón Kleiman, and Gilbert Song.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added `normalizeMetricKey()` and `getFrameworkMetricPrefix()`.
> 
> 
> Diffs
> -
> 
>   src/master/metrics.hpp ec76dbcd1d1fa5349d62ce73fb9603e1986a776b 
>   src/master/metrics.cpp e46ead79f3f29e285426f9d061337077f453aa45 
> 
> 
> Diff: https://reviews.apache.org/r/67875/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 67932: Use `wclayer` from `hcsshim` to extract file layers.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['67930', '67931', '67932']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1935/mesos-review-67932

Relevant logs:

- 
[stout-tests-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1935/mesos-review-67932/logs/stout-tests-cmake-stdout.log):

```
 d:\dcos\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6\bzlib.c(1056): 
warning C4267: '=': conversion from 'size_t' to 'Int32', possible loss of data 
[D:\DCOS\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6-build\bzip2.vcxproj] 
[D:\DCOS\mesos\3rdparty\bzip2-1.0.6.vcxproj]
 d:\dcos\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6\bzlib.c(1191): 
warning C4267: '=': conversion from 'size_t' to 'Int32', possible loss of data 
[D:\DCOS\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6-build\bzip2.vcxproj] 
[D:\DCOS\mesos\3rdparty\bzip2-1.0.6.vcxproj]
 d:\dcos\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6\bzlib.c(1417): 
warning C4996: 'strcat': This function or variable may be unsafe. Consider 
using strcat_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. 
See online help for details. 
[D:\DCOS\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6-build\bzip2.vcxproj] 
[D:\DCOS\mesos\3rdparty\bzip2-1.0.6.vcxproj]
 d:\dcos\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6\bzlib.c(1418): 
warning C4996: 'strcat': This function or variable may be unsafe. Consider 
using strcat_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. 
See online help for details. 
[D:\DCOS\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6-build\bzip2.vcxproj] 
[D:\DCOS\mesos\3rdparty\bzip2-1.0.6.vcxproj]
 d:\dcos\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6\bzlib.c(1423): 
warning C4996: 'setmode': The POSIX name for this item is deprecated. Instead, 
use the ISO C and C++ conformant name: _setmode. See online help for details. 
[D:\DCOS\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6-build\bzip2.vcxproj] 
[D:\DCOS\mesos\3rdparty\bzip2-1.0.6.vcxproj]
 d:\dcos\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6\bzlib.c(1425): 
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\bzip2-1.0.6\src\bzip2-1.0.6-build\bzip2.vcxproj] 
[D:\DCOS\mesos\3rdparty\bzip2-1.0.6.vcxproj]
 d:\dcos\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6\bzlib.c(1431): 
warning C4996: 'fdopen': The POSIX name for this item is deprecated. Instead, 
use the ISO C and C++ conformant name: _fdopen. See online help for details. 
[D:\DCOS\mesos\3rdparty\bzip2-1.0.6\src\bzip2-1.0.6-build\bzip2.vcxproj] 
[D:\DCOS\mesos\3rdparty\bzip2-1.0.6.vcxproj]


   "D:\DCOS\mesos\3rdparty\stout\tests\stout-tests.vcxproj" (default 
target) (1) ->
   "D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj" (default target) (8) ->
   (CustomBuild target) -> 
 CUSTOMBUILD : error : downloading 
'https://github.com/mesos/3rdparty/raw/master/curl-7.60.0.tar.gz' failed 
[D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj]
 CUSTOMBUILD : The requested URL returned error : 404 Not Found 
[D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj]
 CUSTOMBUILD : error : downloading 
'https://github.com/mesos/3rdparty/raw/master/curl-7.60.0.tar.gz' failed 
[D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj]
 CUSTOMBUILD : The requested URL returned error : 404 Not Found 
[D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj]
 CUSTOMBUILD : error : downloading 
'https://github.com/mesos/3rdparty/raw/master/curl-7.60.0.tar.gz' failed 
[D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj]
 CUSTOMBUILD : The requested URL returned error : 404 Not Found 
[D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj]
 CUSTOMBUILD : error : downloading 
'https://github.com/mesos/3rdparty/raw/master/curl-7.60.0.tar.gz' failed 
[D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj]
 CUSTOMBUILD : The requested URL returned error : 404 Not Found 
[D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj]
 CUSTOMBUILD : error : downloading 
'https://github.com/mesos/3rdparty/raw/master/curl-7.60.0.tar.gz' failed 
[D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj]
 CUSTOMBUILD : The requested URL returned error : 404 Not Found 
[D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj]
 CUSTOMBUILD : error : downloading 
'https://github.com/mesos/3rdparty/raw/master/curl-7.60.0.tar.gz' failed 
[D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj]
 CUSTOMBUILD : The requested URL returned error : 404 Not Found 
[D:\DCOS\mesos\3rdparty\curl-7.60.0.vcxproj]

Re: Review Request 67871: Optimized the generation of metrics snapshots.

[Benjamin Mahler]

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


Fix it, then Ship it!





3rdparty/libprocess/src/metrics/metrics.cpp
Lines 174 (patched)


How about s/result/waited/?


- Benjamin Mahler


On July 16, 2018, 4:34 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67871/
> ---
> 
> (Updated July 16, 2018, 4:34 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gastón Kleiman, and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Profiling of metrics generation revealed a large amount of time spent
> in map operations. This patch does three things to mitigate this:
> 
>  * Stores the metrics as an ordered map so that we only pay the price
>of sorting when the metric is first added.
>  * Makes use of vectors instead of maps for intermediate objects,
>which eliminates the need for another intermediate object.
>  * Hints when inserting into the returned map, reducing the cost of
>insertion into that ordered container.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/metrics/metrics.hpp 
> f9b72029b2c85826c91b1d7656b0af94dc87010c 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> 4883c9acaa0cc568e27944661a8208f7b2a356a1 
> 
> 
> Diff: https://reviews.apache.org/r/67871/diff/4/
> 
> 
> Testing
> ---
> 
> WITH per-framework metrics, BEFORE optimizations:
> ```
> [ RUN  ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/0
> Test setup: 1 agents with a total of 105 frameworks
> unversioned /metrics/snapshot' response took 157.1449ms
> v1 'master::call::GetMetrics' application/x-protobuf response took 
> 152.599692ms
> v1 'master::call::GetMetrics' application/json response took 198.918334ms
> [   OK ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/0
>  (835 ms)
> [ RUN  ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/1
> Test setup: 1 agents with a total of 1005 frameworks
> unversioned /metrics/snapshot' response took 1.319444199secs
> v1 'master::call::GetMetrics' application/x-protobuf response took 
> 1.257644596secs
> v1 'master::call::GetMetrics' application/json response took 1.527225235secs
> [   OK ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/1
>  (6553 ms)
> [ RUN  ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/2
> Test setup: 1 agents with a total of 10005 frameworks
> unversioned /metrics/snapshot' response took 15.479365874secs
> v1 'master::call::GetMetrics' application/x-protobuf response took 
> 14.542866983secs
> v1 'master::call::GetMetrics' application/json response took 18.05492789secs
> [   OK ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/2
>  (75455 ms)
> [ RUN  ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/3
> Test setup: 1 agents with a total of 20005 frameworks
> unversioned /metrics/snapshot' response took 31.908301664secs
> v1 'master::call::GetMetrics' application/x-protobuf response took 
> 32.128689785secs
> v1 'master::call::GetMetrics' application/json response took 33.669376185secs
> [   OK ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/3
>  (150440 ms)
> ```
> 
> WITH per-framework metrics, AFTER optimizations:
> ```
> [ RUN  ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/0
> Test setup: 1 agents with a total of 105 frameworks
> unversioned /metrics/snapshot' response took 104.577895ms
> v1 'master::call::GetMetrics' application/x-protobuf response took 74.262533ms
> v1 'master::call::GetMetrics' application/json response took 100.218618ms
> [   OK ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/0
>  (562 ms)
> [ RUN  ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/1
> Test setup: 1 agents with a total of 1005 frameworks
> unversioned /metrics/snapshot' response took 921.175877ms
> v1 'master::call::GetMetrics' application/x-protobuf response took 
> 780.277639ms
> v1 'master::call::GetMetrics' application/json response took 1.16865secs
> [   OK ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/1
>  (5424 ms)
> [ RUN  ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/2
> Test setup: 1 agents with a total of 10005 frameworks
> unversioned /metrics/snapshot' response took 10.2

Re: Review Request 67927: Removed potentially unneeded lambda capture.

[Benjamin Hindman]

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


Ship it!




Looks good and thanks for a great comment!

- Benjamin Hindman


On July 16, 2018, 7:22 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67927/
> ---
> 
> (Updated July 16, 2018, 7:22 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Dario Rexin.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In some instances variables used in the body of a lambda do not need
> to be captured explicitly, see e.g.,
> https://stackoverflow.com/a/43468519/176922. Support for this across
> compiler vendors seems to be incomplete at this point; while e.g.,
> clang emits a warning (which we potentially turn into a fatal error)
> for this, msvc still seems to require such variables to be captured.
> 
> To provide a warning free build this patch moves from the usually
> preferred explicit capture list to an implicit capture by reference.
> This allows us to capture all needed variables correctly without
> having to capture too much.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> e8ef21fd9c203ac5e10c17694393322151231435 
> 
> 
> Diff: https://reviews.apache.org/r/67927/diff/3/
> 
> 
> Testing
> ---
> 
> Tested on a number of platforms and compilers in internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67871: Optimized the generation of metrics snapshots.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67871 was successfully built and tested.

Reviews applied: `['67871']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1934/mesos-review-67871

- Mesos Reviewbot Windows


On July 16, 2018, 9:34 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67871/
> ---
> 
> (Updated July 16, 2018, 9:34 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gastón Kleiman, and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Profiling of metrics generation revealed a large amount of time spent
> in map operations. This patch does three things to mitigate this:
> 
>  * Stores the metrics as an ordered map so that we only pay the price
>of sorting when the metric is first added.
>  * Makes use of vectors instead of maps for intermediate objects,
>which eliminates the need for another intermediate object.
>  * Hints when inserting into the returned map, reducing the cost of
>insertion into that ordered container.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/metrics/metrics.hpp 
> f9b72029b2c85826c91b1d7656b0af94dc87010c 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> 4883c9acaa0cc568e27944661a8208f7b2a356a1 
> 
> 
> Diff: https://reviews.apache.org/r/67871/diff/4/
> 
> 
> Testing
> ---
> 
> WITH per-framework metrics, BEFORE optimizations:
> ```
> [ RUN  ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/0
> Test setup: 1 agents with a total of 105 frameworks
> unversioned /metrics/snapshot' response took 157.1449ms
> v1 'master::call::GetMetrics' application/x-protobuf response took 
> 152.599692ms
> v1 'master::call::GetMetrics' application/json response took 198.918334ms
> [   OK ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/0
>  (835 ms)
> [ RUN  ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/1
> Test setup: 1 agents with a total of 1005 frameworks
> unversioned /metrics/snapshot' response took 1.319444199secs
> v1 'master::call::GetMetrics' application/x-protobuf response took 
> 1.257644596secs
> v1 'master::call::GetMetrics' application/json response took 1.527225235secs
> [   OK ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/1
>  (6553 ms)
> [ RUN  ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/2
> Test setup: 1 agents with a total of 10005 frameworks
> unversioned /metrics/snapshot' response took 15.479365874secs
> v1 'master::call::GetMetrics' application/x-protobuf response took 
> 14.542866983secs
> v1 'master::call::GetMetrics' application/json response took 18.05492789secs
> [   OK ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/2
>  (75455 ms)
> [ RUN  ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/3
> Test setup: 1 agents with a total of 20005 frameworks
> unversioned /metrics/snapshot' response took 31.908301664secs
> v1 'master::call::GetMetrics' application/x-protobuf response took 
> 32.128689785secs
> v1 'master::call::GetMetrics' application/json response took 33.669376185secs
> [   OK ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/3
>  (150440 ms)
> ```
> 
> WITH per-framework metrics, AFTER optimizations:
> ```
> [ RUN  ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/0
> Test setup: 1 agents with a total of 105 frameworks
> unversioned /metrics/snapshot' response took 104.577895ms
> v1 'master::call::GetMetrics' application/x-protobuf response took 74.262533ms
> v1 'master::call::GetMetrics' application/json response took 100.218618ms
> [   OK ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/0
>  (562 ms)
> [ RUN  ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/1
> Test setup: 1 agents with a total of 1005 frameworks
> unversioned /metrics/snapshot' response took 921.175877ms
> v1 'master::call::GetMetrics' application/x-protobuf response took 
> 780.277639ms
> v1 'master::call::GetMetrics' application/json response took 1.16865secs
> [   OK ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/1
>  (5424 ms)
> [ RUN  ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/2
> Test setup: 1 agents with a total of 10005 f

Re: Review Request 67927: Removed potentially unneeded lambda capture.

[Benjamin Bannier]

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

(Updated July 16, 2018, 9:22 p.m.)


Review request for mesos, Benjamin Hindman and Dario Rexin.


Changes
---

Added extended comment.


Summary (updated)
-

Removed potentially unneeded lambda capture.


Repository: mesos


Description
---

In some instances variables used in the body of a lambda do not need
to be captured explicitly, see e.g.,
https://stackoverflow.com/a/43468519/176922. Support for this across
compiler vendors seems to be incomplete at this point; while e.g.,
clang emits a warning (which we potentially turn into a fatal error)
for this, msvc still seems to require such variables to be captured.

To provide a warning free build this patch moves from the usually
preferred explicit capture list to an implicit capture by reference.
This allows us to capture all needed variables correctly without
having to capture too much.


Diffs (updated)
-

  3rdparty/libprocess/src/tests/benchmarks.cpp 
e8ef21fd9c203ac5e10c17694393322151231435 


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

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


Testing
---

Tested on a number of platforms and compilers in internal CI.


Thanks,

Benjamin Bannier

Review Request 67932: Use `wclayer` from `hcsshim` to extract file layers.

[Liangyu Zhao via Review Board]

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

Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, and Joseph Wu.


Repository: mesos


Description
---

The `tar` command cannot work successfully on Windows, so use `wclayer`
instead. Note that the folder generated from extraction also cannot be
deleted by `rmdir`, so the GC is also changed to use `wclayer remove`.


Diffs
-

  src/common/command_utils.hpp 525f9c1c37b74f7e4cc71fdc8d52944226998ddc 
  src/common/command_utils.cpp 7dfcc9ff74bcf044d47b803ebc42cf63fba89d17 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
a5683e3fe15dd35596122fcc0c580ae9d3adf7f2 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
85aad25ac8ecfda125be85fb46d882c3982f3930 


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


Testing
---


Thanks,

Liangyu Zhao

Review Request 67931: Support Image Manifest Version 2 Schema 2.

[Liangyu Zhao via Review Board]

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

Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, and Joseph Wu.


Repository: mesos


Description
---

Support parsing schema 2 and fetching blob with multiple URLs as
specified in schema 2. Update `curl` version to 7.60.0 because of bug
encountered in version 7.57.0.


Diffs
-

  3rdparty/cmake/Versions.cmake 0a897d808cd9e05ac0d1a4e1ca61b8f3538f0c4a 
  include/mesos/docker/spec.hpp 2879414dc42ffe633ac74b51e1bb116698c41162 
  include/mesos/docker/v2_2.hpp PRE-CREATION 
  include/mesos/docker/v2_2.proto PRE-CREATION 
  include/mesos/uri/fetcher.hpp 760d6b33234d8efdc533c0c6f05e83a5c7d1f56b 
  src/CMakeLists.txt 10b0946d6f49c7e9c201bad6f9f1b41cc8460fe5 
  src/Makefile.am 228e168c22f3fd0367f029c506171c6979b31c07 
  src/docker/spec.cpp 96fbf1f9cf1c2c4b2383607a97990f3a9156e6d9 
  src/slave/containerizer/mesos/containerizer.cpp 
98129d006cda9b65804b518619b6addc8990410a 
  src/slave/containerizer/mesos/provisioner/docker/registry_puller.cpp 
a5683e3fe15dd35596122fcc0c580ae9d3adf7f2 
  src/uri/fetcher.hpp fbf64c6767dea3aa0798f68db8322ce47cd8ad36 
  src/uri/fetcher.cpp 489c7ce0198ee6803dcc8eb9710b292fa743a0e8 
  src/uri/fetchers/docker.hpp cdbab9a5684a68a729be12bc06d331acca137da5 
  src/uri/fetchers/docker.cpp 55ca118660872a933a2dc186723bec6a39ee80f7 


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


Testing
---


Thanks,

Liangyu Zhao

Review Request 67930: Get tests ready for Windows UCR development.

[Liangyu Zhao via Review Board]

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

Review request for mesos, Akash Gupta, Andrew Schwartzmeyer, and Joseph Wu.


Repository: mesos


Description
---

Enabled Internet test environment on Windows. Disabled Internet
HealthCheckTests on Windows, since they require complete development.
Modified DockerFetcherPluginTest to fetch `microsoft/nanoserver` for
more extensive test for fetcher on Windows.


Diffs
-

  src/tests/environment.cpp 3b84c0a413193badbbdb6d3ee71d74f3ab85c90b 
  src/tests/health_check_tests.cpp 34102e2e3ff90c194bea83ae8a702181121d6522 
  src/tests/uri_fetcher_tests.cpp 816d8ec2503ea79d069c062dfa2f01224b263bf6 


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


Testing
---


Thanks,

Liangyu Zhao

Re: Review Request 67928: Added balloon framework metric for tasks which were running.

[Meng Zhu]

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


Fix it, then Ship it!





src/examples/balloon_framework.cpp
Lines 304 (patched)


stati --> statistics?



src/examples/balloon_framework.cpp
Lines 309 (patched)


ditto



src/examples/balloon_framework.cpp
Line 304 (original), 316 (patched)


Let's add back the default.


- Meng Zhu


On July 16, 2018, 7:48 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67928/
> ---
> 
> (Updated July 16, 2018, 7:48 a.m.)
> 
> 
> Review request for mesos and Meng Zhu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The framework currently exposes metric counters for various expected
> and unexpected task termination reasons. Interpreting these counters
> can be non-trivial since tasks might fail due to benign, but unknown
> external reasons.
> 
> This patch adds a counter for the tasks which actually made it to the
> running stage which can be correlated with the different terminal task
> counts.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_framework.cpp cbc25e45258b05a645363d4229c0a701baa5481c 
> 
> 
> Diff: https://reviews.apache.org/r/67928/diff/1/
> 
> 
> Testing
> ---
> 
> Started the framework with a `local` master and examine the metric.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67927: Removed unneeded lambda capture.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67927 was successfully built and tested.

Reviews applied: `['67927']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1933/mesos-review-67927

- Mesos Reviewbot Windows


On July 16, 2018, 12:37 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67927/
> ---
> 
> (Updated July 16, 2018, 12:37 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Dario Rexin.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In some instances variables used in the body of a lambda do not need
> to be captured explicitly, see e.g.,
> https://stackoverflow.com/a/43468519/176922. Support for this across
> compiler vendors seems to be incomplete at this point; while e.g.,
> clang emits a warning (which we potentially turn into a fatal error)
> for this, msvc still seems to require such variables to be captured.
> 
> To provide a warning free build this patch moves from the usually
> preferred explicit capture list to an implicit capture by reference.
> This allows us to capture all needed variables correctly without
> having to capture too much.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> e8ef21fd9c203ac5e10c17694393322151231435 
> 
> 
> Diff: https://reviews.apache.org/r/67927/diff/2/
> 
> 
> Testing
> ---
> 
> Tested on a number of platforms and compilers in internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67871: Optimized the generation of metrics snapshots.

[Greg Mann]

> On July 14, 2018, 7:54 p.m., Benjamin Mahler wrote:
> > 3rdparty/libprocess/src/metrics/metrics.cpp
> > Lines 164-172 (original), 168-177 (patched)
> > 
> >
> > Whoops, this expression contains both a moving and use of `futures` and 
> > the evaluation order is undefined: 
> > 
> > https://en.cppreference.com/w/cpp/language/eval_order
> > 
> > If the compiler decides to evaluate defer before await, await will see 
> > an empty vector and we'll likely see the timeout CHECK fail if no timeout 
> > was passed.
> > 
> > I've been tripped up by this a bunch of times with moves, and one of 
> > those times was exactly in this spot!
> > 
> > https://issues.apache.org/jira/browse/MESOS-8970

lol, thanks for catching this Ben!! Much appreciated :) I've pushed an update 
to ensure our desired order of evaluation.


- Greg


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


On July 16, 2018, 4:34 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67871/
> ---
> 
> (Updated July 16, 2018, 4:34 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gastón Kleiman, and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Profiling of metrics generation revealed a large amount of time spent
> in map operations. This patch does three things to mitigate this:
> 
>  * Stores the metrics as an ordered map so that we only pay the price
>of sorting when the metric is first added.
>  * Makes use of vectors instead of maps for intermediate objects,
>which eliminates the need for another intermediate object.
>  * Hints when inserting into the returned map, reducing the cost of
>insertion into that ordered container.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/metrics/metrics.hpp 
> f9b72029b2c85826c91b1d7656b0af94dc87010c 
>   3rdparty/libprocess/src/metrics/metrics.cpp 
> 4883c9acaa0cc568e27944661a8208f7b2a356a1 
> 
> 
> Diff: https://reviews.apache.org/r/67871/diff/4/
> 
> 
> Testing
> ---
> 
> WITH per-framework metrics, BEFORE optimizations:
> ```
> [ RUN  ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/0
> Test setup: 1 agents with a total of 105 frameworks
> unversioned /metrics/snapshot' response took 157.1449ms
> v1 'master::call::GetMetrics' application/x-protobuf response took 
> 152.599692ms
> v1 'master::call::GetMetrics' application/json response took 198.918334ms
> [   OK ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/0
>  (835 ms)
> [ RUN  ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/1
> Test setup: 1 agents with a total of 1005 frameworks
> unversioned /metrics/snapshot' response took 1.319444199secs
> v1 'master::call::GetMetrics' application/x-protobuf response took 
> 1.257644596secs
> v1 'master::call::GetMetrics' application/json response took 1.527225235secs
> [   OK ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/1
>  (6553 ms)
> [ RUN  ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/2
> Test setup: 1 agents with a total of 10005 frameworks
> unversioned /metrics/snapshot' response took 15.479365874secs
> v1 'master::call::GetMetrics' application/x-protobuf response took 
> 14.542866983secs
> v1 'master::call::GetMetrics' application/json response took 18.05492789secs
> [   OK ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/2
>  (75455 ms)
> [ RUN  ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/3
> Test setup: 1 agents with a total of 20005 frameworks
> unversioned /metrics/snapshot' response took 31.908301664secs
> v1 'master::call::GetMetrics' application/x-protobuf response took 
> 32.128689785secs
> v1 'master::call::GetMetrics' application/json response took 33.669376185secs
> [   OK ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/3
>  (150440 ms)
> ```
> 
> WITH per-framework metrics, AFTER optimizations:
> ```
> [ RUN  ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/0
> Test setup: 1 agents with a total of 105 frameworks
> unversioned /metrics/snapshot' response took 104.577895ms
> v1 'master::call::GetMetrics' application/x-protobuf response took 74.262533ms
> v1 'master::call::GetMetrics' application/json response took 100.218618ms
> [   OK ] 
> AgentFrameworkTaskCountContentType/MasterMetricsQu

Re: Review Request 67871: Optimized the generation of metrics snapshots.

[Greg Mann]

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

(Updated July 16, 2018, 4:34 p.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, and James Peach.


Repository: mesos


Description
---

Profiling of metrics generation revealed a large amount of time spent
in map operations. This patch does three things to mitigate this:

 * Stores the metrics as an ordered map so that we only pay the price
   of sorting when the metric is first added.
 * Makes use of vectors instead of maps for intermediate objects,
   which eliminates the need for another intermediate object.
 * Hints when inserting into the returned map, reducing the cost of
   insertion into that ordered container.


Diffs (updated)
-

  3rdparty/libprocess/include/process/metrics/metrics.hpp 
f9b72029b2c85826c91b1d7656b0af94dc87010c 
  3rdparty/libprocess/src/metrics/metrics.cpp 
4883c9acaa0cc568e27944661a8208f7b2a356a1 


Diff: https://reviews.apache.org/r/67871/diff/4/

Changes: https://reviews.apache.org/r/67871/diff/3-4/


Testing
---

WITH per-framework metrics, BEFORE optimizations:
```
[ RUN  ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/0
Test setup: 1 agents with a total of 105 frameworks
unversioned /metrics/snapshot' response took 157.1449ms
v1 'master::call::GetMetrics' application/x-protobuf response took 152.599692ms
v1 'master::call::GetMetrics' application/json response took 198.918334ms
[   OK ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/0
 (835 ms)
[ RUN  ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/1
Test setup: 1 agents with a total of 1005 frameworks
unversioned /metrics/snapshot' response took 1.319444199secs
v1 'master::call::GetMetrics' application/x-protobuf response took 
1.257644596secs
v1 'master::call::GetMetrics' application/json response took 1.527225235secs
[   OK ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/1
 (6553 ms)
[ RUN  ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/2
Test setup: 1 agents with a total of 10005 frameworks
unversioned /metrics/snapshot' response took 15.479365874secs
v1 'master::call::GetMetrics' application/x-protobuf response took 
14.542866983secs
v1 'master::call::GetMetrics' application/json response took 18.05492789secs
[   OK ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/2
 (75455 ms)
[ RUN  ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/3
Test setup: 1 agents with a total of 20005 frameworks
unversioned /metrics/snapshot' response took 31.908301664secs
v1 'master::call::GetMetrics' application/x-protobuf response took 
32.128689785secs
v1 'master::call::GetMetrics' application/json response took 33.669376185secs
[   OK ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/3
 (150440 ms)
```

WITH per-framework metrics, AFTER optimizations:
```
[ RUN  ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/0
Test setup: 1 agents with a total of 105 frameworks
unversioned /metrics/snapshot' response took 104.577895ms
v1 'master::call::GetMetrics' application/x-protobuf response took 74.262533ms
v1 'master::call::GetMetrics' application/json response took 100.218618ms
[   OK ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/0
 (562 ms)
[ RUN  ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/1
Test setup: 1 agents with a total of 1005 frameworks
unversioned /metrics/snapshot' response took 921.175877ms
v1 'master::call::GetMetrics' application/x-protobuf response took 780.277639ms
v1 'master::call::GetMetrics' application/json response took 1.16865secs
[   OK ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/1
 (5424 ms)
[ RUN  ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/2
Test setup: 1 agents with a total of 10005 frameworks
unversioned /metrics/snapshot' response took 10.2413387secs
v1 'master::call::GetMetrics' application/x-protobuf response took 
9.407992945secs
v1 'master::call::GetMetrics' application/json response took 10.582584848secs
[   OK ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/2
 (57206 ms)
[ RUN  ] 
AgentFrameworkTaskCountContentType/MasterMetricsQuery_BENCHMARK_Test.GetMetrics/3
Test setup: 1 agents with a total of 20005 frameworks
unversioned /metrics/snapshot' response took 19.930542079secs
v1 'master::call::GetMetrics' application/x-protobuf response took 
20.318739763secs
v1 'master::call::GetMetrics' application/json response took 

Re: Review Request 67921: Bug fix for semaphore decomission "deadlock".

[Dario Rexin]

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


Ship it!




Ship It!

- Dario Rexin


On July 15, 2018, 5:09 p.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67921/
> ---
> 
> (Updated July 15, 2018, 5:09 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8239
> https://issues.apache.org/jira/browse/MESOS-8239
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixes MESOS-8239.
> 
> When using the DecomissionableLastInFirstOutFixedSizeSemaphore it's
> possible that waiting threads may never be properly signaled. This bug
> fix makes sure that every waiting thread gets a signal after a
> decomission.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/semaphore.hpp 
> 50501b9797894ad274eb73f74b3eed00cd719114 
> 
> 
> Diff: https://reviews.apache.org/r/67921/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 67928: Added balloon framework metric for tasks which were running.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67928 was successfully built and tested.

Reviews applied: `['67928']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1932/mesos-review-67928

- Mesos Reviewbot Windows


On July 16, 2018, 2:48 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67928/
> ---
> 
> (Updated July 16, 2018, 2:48 p.m.)
> 
> 
> Review request for mesos and Meng Zhu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The framework currently exposes metric counters for various expected
> and unexpected task termination reasons. Interpreting these counters
> can be non-trivial since tasks might fail due to benign, but unknown
> external reasons.
> 
> This patch adds a counter for the tasks which actually made it to the
> running stage which can be correlated with the different terminal task
> counts.
> 
> 
> Diffs
> -
> 
>   src/examples/balloon_framework.cpp cbc25e45258b05a645363d4229c0a701baa5481c 
> 
> 
> Diff: https://reviews.apache.org/r/67928/diff/1/
> 
> 
> Testing
> ---
> 
> Started the framework with a `local` master and examine the metric.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67866: Apply the `override` keyword to stout.

[Benjamin Bannier]

> On July 16, 2018, 5:37 p.m., Benjamin Bannier wrote:
> > Ship It!

> When doing automated clang-tidy refactors it is also always very useful to 
> call out how the compilation database was generated (e.g., what flags were 
> used). Could you add that to either the commit message or the Testing done 
> section?


- Benjamin


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


On July 12, 2018, 7:02 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67866/
> ---
> 
> (Updated July 12, 2018, 7:02 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Greg Mann, 
> Mesos Reviewbot, Till Toenshoff, and Zhitao Li.
> 
> 
> Bugs: MESOS-9065
> https://issues.apache.org/jira/browse/MESOS-9065
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Apply the `override` keyword to stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/lambda.hpp 
> 252de895ac679715ef20015d8887b1a78f264fe8 
>   3rdparty/stout/include/stout/tests/utils.hpp 
> e7336a586cc3ad4147cfe963604059026fc1e3f1 
>   3rdparty/stout/tests/main.cpp 144399d86c9177b2ec9a2ae8e95c55d5aa0defd2 
>   3rdparty/stout/tests/os/rmdir_tests.cpp 
> 518afc0ddedc4c53d01ac08a36234964f088420d 
>   3rdparty/stout/tests/os/sendfile_tests.cpp 
> cc1fa113eae5c2c68e2f4f21c35b700650db787c 
>   3rdparty/stout/tests/subcommand_tests.cpp 
> 211d28df1bba069c848000a19d3f6038e71d8159 
> 
> 
> Diff: https://reviews.apache.org/r/67866/diff/2/
> 
> 
> Testing
> ---
> 
> make check (Fedora 28)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 67866: Apply the `override` keyword to stout.

[Benjamin Bannier]

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


Ship it!




Ship It!

- Benjamin Bannier


On July 12, 2018, 7:02 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67866/
> ---
> 
> (Updated July 12, 2018, 7:02 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Greg Mann, 
> Mesos Reviewbot, Till Toenshoff, and Zhitao Li.
> 
> 
> Bugs: MESOS-9065
> https://issues.apache.org/jira/browse/MESOS-9065
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Apply the `override` keyword to stout.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/lambda.hpp 
> 252de895ac679715ef20015d8887b1a78f264fe8 
>   3rdparty/stout/include/stout/tests/utils.hpp 
> e7336a586cc3ad4147cfe963604059026fc1e3f1 
>   3rdparty/stout/tests/main.cpp 144399d86c9177b2ec9a2ae8e95c55d5aa0defd2 
>   3rdparty/stout/tests/os/rmdir_tests.cpp 
> 518afc0ddedc4c53d01ac08a36234964f088420d 
>   3rdparty/stout/tests/os/sendfile_tests.cpp 
> cc1fa113eae5c2c68e2f4f21c35b700650db787c 
>   3rdparty/stout/tests/subcommand_tests.cpp 
> 211d28df1bba069c848000a19d3f6038e71d8159 
> 
> 
> Diff: https://reviews.apache.org/r/67866/diff/2/
> 
> 
> Testing
> ---
> 
> make check (Fedora 28)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 67488: Updated CLI to Python 3.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On July 6, 2018, 1:56 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67488/
> ---
> 
> (Updated July 6, 2018, 1:56 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Eric Chung, 
> and Kevin Klues.
> 
> 
> Bugs: MESOS-8955
> https://issues.apache.org/jira/browse/MESOS-8955
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The build tools are also up to date thus the CLI can still be built
> using Autotools and CMake. No features have been added to the CLI.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 61387d77b12a17571a31430db3ca1fe0bbb66a21 
>   configure.ac 66cc28a5a34949bcadc038551249f3781ea9d45b 
>   src/Makefile.am db42e71d90ff2066e104f4b9c269c5e78a9a6ada 
>   src/python/cli_new/CMakeLists.txt ef8da70757e2721f4ac1bee46d0b5d95e81298ca 
>   src/python/cli_new/README.md 3d646e91a8c7c72d4ee1b1180454e5f587295053 
>   src/python/cli_new/bin/main.py 53130383d8ca2ed40c97224b3a6e98aa6b6b107c 
>   src/python/cli_new/bootstrap fb6fbc449a970ccf960914ed910204f3984ea61f 
>   src/python/cli_new/lib/cli/config.py 
> 6f92622725d8a042a2a728fd38c977ac690ef6be 
>   src/python/cli_new/lib/cli/docopt.py 
> 86a4e9c74326fb80cc59487113f07358dd96960d 
>   src/python/cli_new/lib/cli/http.py 03d6031cb3273575f41d4d06d9a409f74488a16b 
>   src/python/cli_new/lib/cli/plugins/agent/main.py 
> 59280ece8ebd00bb96df3675b6356a26cc48a2c0 
>   src/python/cli_new/lib/cli/plugins/task/main.py 
> cc6cff56c71262729a8870017bef2e97636abe5a 
>   src/python/cli_new/lib/cli/tests/base.py 
> 89360e6cac5ca910044a5a82fab7237510edee7f 
>   src/python/cli_new/lib/cli/tests/tests.py 
> 79e1036f6d11c63884091fe43672607b03955c1a 
>   src/python/cli_new/lib/cli/util.py 307b22293a9c7199ad7088dfd0db6dff83a08ac8 
>   src/python/cli_new/tests/main.py acf2e0868555da0eb1c1cee7fb30b1e80783f1e1 
>   src/python/cli_new/tox.ini 58ca3807e3d6096296b4cd09a5cec32b32444d91 
>   src/python/lib/tox.ini 05b633e837fa39a36fb2c5a0778513ce743099db 
>   support/mesos-style.py 27ed553cb1d9e0c3c750b414eafe0144c3442c43 
>   support/python3/mesos-style.py 350ef909e3e7a1c927140cf4475547d704ac2ad5 
> 
> 
> Diff: https://reviews.apache.org/r/67488/diff/12/
> 
> 
> Testing
> ---
> 
> Testing done on Fedora 25 with `python` being Python 2.7, `python3` being 
> Python 3.5 and `python36` being Python 3.6.
> 
> 
> For Autotools:
> 
> ```
> $ ./bootstrap
> $ mkdir build
> $ cd build
> $ PYTHON_3=python36 ../configure --enable-new-cli --disable-java
> $ make check
> ```
> 
> For CMake:
> 
> ```
> $ ./bootstrap
> $ mkdir build
> $ cd build
> $ cmake .. -DENABLE_NEW_CLI=1 -DPYTHON=python36
> $ cmake --build . -- -j16
> $ ./src/mesos
> Mesos CLI
> 
> Usage:
>   mesos (-h | --help)
>   mesos --version
>   mesos  [...]
> 
> Options:
>   -h --help  Show this screen.
>   --version  Show version info.
> 
> Commands:
>   agent   Interacts with the Mesos agents
>   config  Interacts with the Mesos CLI configuration file
>   taskInteracts with the tasks running in a Mesos cluster
> 
> See 'mesos help ' for more information on a specific command.
> $ cmake --build . --target tests -- -j16
> $ ctest -R CLI
> Test project /home/agrillet/apache-mesos/build
> Start 4: CLITests
> 1/1 Test #4: CLITests .   Passed3.63 sec
> 
> 100% tests passed, 0 tests failed out of 1
> ```
> 
> Checked that the the CLI tests were run, that the content of the directory 
> build/src/cli was as expected, and that build/src/mesos was correctly running.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 67488: Updated CLI to Python 3.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On July 6, 2018, 1:56 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67488/
> ---
> 
> (Updated July 6, 2018, 1:56 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Eric Chung, 
> and Kevin Klues.
> 
> 
> Bugs: MESOS-8955
> https://issues.apache.org/jira/browse/MESOS-8955
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The build tools are also up to date thus the CLI can still be built
> using Autotools and CMake. No features have been added to the CLI.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 61387d77b12a17571a31430db3ca1fe0bbb66a21 
>   configure.ac 66cc28a5a34949bcadc038551249f3781ea9d45b 
>   src/Makefile.am db42e71d90ff2066e104f4b9c269c5e78a9a6ada 
>   src/python/cli_new/CMakeLists.txt ef8da70757e2721f4ac1bee46d0b5d95e81298ca 
>   src/python/cli_new/README.md 3d646e91a8c7c72d4ee1b1180454e5f587295053 
>   src/python/cli_new/bin/main.py 53130383d8ca2ed40c97224b3a6e98aa6b6b107c 
>   src/python/cli_new/bootstrap fb6fbc449a970ccf960914ed910204f3984ea61f 
>   src/python/cli_new/lib/cli/config.py 
> 6f92622725d8a042a2a728fd38c977ac690ef6be 
>   src/python/cli_new/lib/cli/docopt.py 
> 86a4e9c74326fb80cc59487113f07358dd96960d 
>   src/python/cli_new/lib/cli/http.py 03d6031cb3273575f41d4d06d9a409f74488a16b 
>   src/python/cli_new/lib/cli/plugins/agent/main.py 
> 59280ece8ebd00bb96df3675b6356a26cc48a2c0 
>   src/python/cli_new/lib/cli/plugins/task/main.py 
> cc6cff56c71262729a8870017bef2e97636abe5a 
>   src/python/cli_new/lib/cli/tests/base.py 
> 89360e6cac5ca910044a5a82fab7237510edee7f 
>   src/python/cli_new/lib/cli/tests/tests.py 
> 79e1036f6d11c63884091fe43672607b03955c1a 
>   src/python/cli_new/lib/cli/util.py 307b22293a9c7199ad7088dfd0db6dff83a08ac8 
>   src/python/cli_new/tests/main.py acf2e0868555da0eb1c1cee7fb30b1e80783f1e1 
>   src/python/cli_new/tox.ini 58ca3807e3d6096296b4cd09a5cec32b32444d91 
>   src/python/lib/tox.ini 05b633e837fa39a36fb2c5a0778513ce743099db 
>   support/mesos-style.py 27ed553cb1d9e0c3c750b414eafe0144c3442c43 
>   support/python3/mesos-style.py 350ef909e3e7a1c927140cf4475547d704ac2ad5 
> 
> 
> Diff: https://reviews.apache.org/r/67488/diff/12/
> 
> 
> Testing
> ---
> 
> Testing done on Fedora 25 with `python` being Python 2.7, `python3` being 
> Python 3.5 and `python36` being Python 3.6.
> 
> 
> For Autotools:
> 
> ```
> $ ./bootstrap
> $ mkdir build
> $ cd build
> $ PYTHON_3=python36 ../configure --enable-new-cli --disable-java
> $ make check
> ```
> 
> For CMake:
> 
> ```
> $ ./bootstrap
> $ mkdir build
> $ cd build
> $ cmake .. -DENABLE_NEW_CLI=1 -DPYTHON=python36
> $ cmake --build . -- -j16
> $ ./src/mesos
> Mesos CLI
> 
> Usage:
>   mesos (-h | --help)
>   mesos --version
>   mesos  [...]
> 
> Options:
>   -h --help  Show this screen.
>   --version  Show version info.
> 
> Commands:
>   agent   Interacts with the Mesos agents
>   config  Interacts with the Mesos CLI configuration file
>   taskInteracts with the tasks running in a Mesos cluster
> 
> See 'mesos help ' for more information on a specific command.
> $ cmake --build . --target tests -- -j16
> $ ctest -R CLI
> Test project /home/agrillet/apache-mesos/build
> Start 4: CLITests
> 1/1 Test #4: CLITests .   Passed3.63 sec
> 
> 100% tests passed, 0 tests failed out of 1
> ```
> 
> Checked that the the CLI tests were run, that the content of the directory 
> build/src/cli was as expected, and that build/src/mesos was correctly running.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 67488: Updated CLI to Python 3.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On July 6, 2018, 1:56 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67488/
> ---
> 
> (Updated July 6, 2018, 1:56 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, Eric Chung, 
> and Kevin Klues.
> 
> 
> Bugs: MESOS-8955
> https://issues.apache.org/jira/browse/MESOS-8955
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The build tools are also up to date thus the CLI can still be built
> using Autotools and CMake. No features have been added to the CLI.
> 
> 
> Diffs
> -
> 
>   cmake/CompilationConfigure.cmake 61387d77b12a17571a31430db3ca1fe0bbb66a21 
>   configure.ac 66cc28a5a34949bcadc038551249f3781ea9d45b 
>   src/Makefile.am db42e71d90ff2066e104f4b9c269c5e78a9a6ada 
>   src/python/cli_new/CMakeLists.txt ef8da70757e2721f4ac1bee46d0b5d95e81298ca 
>   src/python/cli_new/README.md 3d646e91a8c7c72d4ee1b1180454e5f587295053 
>   src/python/cli_new/bin/main.py 53130383d8ca2ed40c97224b3a6e98aa6b6b107c 
>   src/python/cli_new/bootstrap fb6fbc449a970ccf960914ed910204f3984ea61f 
>   src/python/cli_new/lib/cli/config.py 
> 6f92622725d8a042a2a728fd38c977ac690ef6be 
>   src/python/cli_new/lib/cli/docopt.py 
> 86a4e9c74326fb80cc59487113f07358dd96960d 
>   src/python/cli_new/lib/cli/http.py 03d6031cb3273575f41d4d06d9a409f74488a16b 
>   src/python/cli_new/lib/cli/plugins/agent/main.py 
> 59280ece8ebd00bb96df3675b6356a26cc48a2c0 
>   src/python/cli_new/lib/cli/plugins/task/main.py 
> cc6cff56c71262729a8870017bef2e97636abe5a 
>   src/python/cli_new/lib/cli/tests/base.py 
> 89360e6cac5ca910044a5a82fab7237510edee7f 
>   src/python/cli_new/lib/cli/tests/tests.py 
> 79e1036f6d11c63884091fe43672607b03955c1a 
>   src/python/cli_new/lib/cli/util.py 307b22293a9c7199ad7088dfd0db6dff83a08ac8 
>   src/python/cli_new/tests/main.py acf2e0868555da0eb1c1cee7fb30b1e80783f1e1 
>   src/python/cli_new/tox.ini 58ca3807e3d6096296b4cd09a5cec32b32444d91 
>   src/python/lib/tox.ini 05b633e837fa39a36fb2c5a0778513ce743099db 
>   support/mesos-style.py 27ed553cb1d9e0c3c750b414eafe0144c3442c43 
>   support/python3/mesos-style.py 350ef909e3e7a1c927140cf4475547d704ac2ad5 
> 
> 
> Diff: https://reviews.apache.org/r/67488/diff/12/
> 
> 
> Testing
> ---
> 
> Testing done on Fedora 25 with `python` being Python 2.7, `python3` being 
> Python 3.5 and `python36` being Python 3.6.
> 
> 
> For Autotools:
> 
> ```
> $ ./bootstrap
> $ mkdir build
> $ cd build
> $ PYTHON_3=python36 ../configure --enable-new-cli --disable-java
> $ make check
> ```
> 
> For CMake:
> 
> ```
> $ ./bootstrap
> $ mkdir build
> $ cd build
> $ cmake .. -DENABLE_NEW_CLI=1 -DPYTHON=python36
> $ cmake --build . -- -j16
> $ ./src/mesos
> Mesos CLI
> 
> Usage:
>   mesos (-h | --help)
>   mesos --version
>   mesos  [...]
> 
> Options:
>   -h --help  Show this screen.
>   --version  Show version info.
> 
> Commands:
>   agent   Interacts with the Mesos agents
>   config  Interacts with the Mesos CLI configuration file
>   taskInteracts with the tasks running in a Mesos cluster
> 
> See 'mesos help ' for more information on a specific command.
> $ cmake --build . --target tests -- -j16
> $ ctest -R CLI
> Test project /home/agrillet/apache-mesos/build
> Start 4: CLITests
> 1/1 Test #4: CLITests .   Passed3.63 sec
> 
> 100% tests passed, 0 tests failed out of 1
> ```
> 
> Checked that the the CLI tests were run, that the content of the directory 
> build/src/cli was as expected, and that build/src/mesos was correctly running.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Review Request 67928: Added balloon framework metric for tasks which were running.

[Benjamin Bannier]

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

Review request for mesos and Meng Zhu.


Repository: mesos


Description
---

The framework currently exposes metric counters for various expected
and unexpected task termination reasons. Interpreting these counters
can be non-trivial since tasks might fail due to benign, but unknown
external reasons.

This patch adds a counter for the tasks which actually made it to the
running stage which can be correlated with the different terminal task
counts.


Diffs
-

  src/examples/balloon_framework.cpp cbc25e45258b05a645363d4229c0a701baa5481c 


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


Testing
---

Started the framework with a `local` master and examine the metric.


Thanks,

Benjamin Bannier

Re: Review Request 67925: Fixed image link in docker volume isolator docs.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67925 was successfully built and tested.

Reviews applied: `['67903', '67904', '67905', '67925']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1931/mesos-review-67925

- Mesos Reviewbot Windows


On July 16, 2018, 12:13 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67925/
> ---
> 
> (Updated July 16, 2018, 12:13 p.m.)
> 
> 
> Review request for mesos, Dave Lester and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed image link in docker volume isolator docs.
> 
> 
> Diffs
> -
> 
>   docs/isolators/docker-volume.md f34b8fa149c32636e870cd9a248376c2d943ec9e 
> 
> 
> Diff: https://reviews.apache.org/r/67925/diff/1/
> 
> 
> Testing
> ---
> 
> Verified that the image is now displayed.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67927: Removed unneeded lambda capture.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['67927']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1930/mesos-review-67927

Relevant logs:

- 
[libprocess-tests-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1930/mesos-review-67927/logs/libprocess-tests-cmake-stdout.log):

```
 cl : Command line warning D9002: ignoring unknown option '-fPIC' 
[D:\DCOS\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2-build\http_parser.vcxproj]
 [D:\DCOS\mesos\3rdparty\http_parser-2.6.2.vcxproj]
 
d:\dcos\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2\http_parser.c(1415):
 warning C4244: '+=': conversion from '__int64' to 'uint32_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2-build\http_parser.vcxproj]
 [D:\DCOS\mesos\3rdparty\http_parser-2.6.2.vcxproj]
 
d:\dcos\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2\http_parser.c(1520):
 warning C4244: '+=': conversion from '__int64' to 'uint32_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2-build\http_parser.vcxproj]
 [D:\DCOS\mesos\3rdparty\http_parser-2.6.2.vcxproj]
 
d:\dcos\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2\http_parser.c(1689):
 warning C4244: '+=': conversion from '__int64' to 'uint32_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2-build\http_parser.vcxproj]
 [D:\DCOS\mesos\3rdparty\http_parser-2.6.2.vcxproj]
 
d:\dcos\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2\http_parser.c(2273):
 warning C4244: '=': conversion from '__int64' to 'uint16_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2-build\http_parser.vcxproj]
 [D:\DCOS\mesos\3rdparty\http_parser-2.6.2.vcxproj]
 
d:\dcos\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2\http_parser.c(2280):
 warning C4244: '=': conversion from '__int64' to 'uint16_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2-build\http_parser.vcxproj]
 [D:\DCOS\mesos\3rdparty\http_parser-2.6.2.vcxproj]
 
d:\dcos\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2\http_parser.c(2292):
 warning C4244: '=': conversion from '__int64' to 'uint16_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2-build\http_parser.vcxproj]
 [D:\DCOS\mesos\3rdparty\http_parser-2.6.2.vcxproj]
 
d:\dcos\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2\http_parser.c(2301):
 warning C4244: '=': conversion from '__int64' to 'uint16_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2-build\http_parser.vcxproj]
 [D:\DCOS\mesos\3rdparty\http_parser-2.6.2.vcxproj]
 
d:\dcos\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2\http_parser.c(2401):
 warning C4244: '=': conversion from '__int64' to 'uint16_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2-build\http_parser.vcxproj]
 [D:\DCOS\mesos\3rdparty\http_parser-2.6.2.vcxproj]
 cl : Command line warning D9002: ignoring unknown option '-fPIC' 
[D:\DCOS\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2-build\http_parser.vcxproj]
 [D:\DCOS\mesos\3rdparty\http_parser-2.6.2.vcxproj]
 
d:\dcos\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2\http_parser.c(1415):
 warning C4244: '+=': conversion from '__int64' to 'uint32_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2-build\http_parser.vcxproj]
 [D:\DCOS\mesos\3rdparty\http_parser-2.6.2.vcxproj]
 
d:\dcos\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2\http_parser.c(1520):
 warning C4244: '+=': conversion from '__int64' to 'uint32_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2-build\http_parser.vcxproj]
 [D:\DCOS\mesos\3rdparty\http_parser-2.6.2.vcxproj]
 
d:\dcos\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2\http_parser.c(1689):
 warning C4244: '+=': conversion from '__int64' to 'uint32_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2-build\http_parser.vcxproj]
 [D:\DCOS\mesos\3rdparty\http_parser-2.6.2.vcxproj]
 
d:\dcos\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2\http_parser.c(2273):
 warning C4244: '=': conversion from '__int64' to 'uint16_t', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\http_parser-2.6.2\src\http_parser-2.6.2-build\http_parser.vcxproj]
 [D:\DCOS\mesos\3rd

Re: Review Request 67924: Mentioned linters `mesos-style.py` and `mesos-tidy.sh` in style guide.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67924 was successfully built and tested.

Reviews applied: `['67924']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1929/mesos-review-67924

- Mesos Reviewbot Windows


On July 16, 2018, 11:23 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67924/
> ---
> 
> (Updated July 16, 2018, 11:23 a.m.)
> 
> 
> Review request for mesos and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Mentioned linters `mesos-style.py` and `mesos-tidy.sh` in style guide.
> 
> 
> Diffs
> -
> 
>   docs/c++-style-guide.md 80a1af318984d47bdfa641aa12e28963d3183fea 
> 
> 
> Diff: https://reviews.apache.org/r/67924/diff/1/
> 
> 
> Testing
> ---
> 
> Verified good site rendering.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 67927: Removed unneeded lambda capture.

[Benjamin Bannier]

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

Review request for mesos, Benjamin Hindman and Dario Rexin.


Repository: mesos


Description
---

In some instances variables used in the body of a lambda do not need
to be captured explicitly. This patch removes the unneeded capture of
a constant integer initialized with a compile-time constant in a
"reachable scope". See e.g.,
https://stackoverflow.com/a/43468519/176922 for a more detailed
explanation.


Diffs
-

  3rdparty/libprocess/src/tests/benchmarks.cpp 
e8ef21fd9c203ac5e10c17694393322151231435 


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


Testing
---

Tested on a number of platforms and compilers in internal CI.


Thanks,

Benjamin Bannier

Review Request 67925: Fixed image link in docker volume isolator docs.

[Benjamin Bannier]

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

Review request for mesos, Dave Lester and Till Toenshoff.


Repository: mesos


Description
---

Fixed image link in docker volume isolator docs.


Diffs
-

  docs/isolators/docker-volume.md f34b8fa149c32636e870cd9a248376c2d943ec9e 


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


Testing
---

Verified that the image is now displayed.


Thanks,

Benjamin Bannier

Review Request 67924: Mentioned linters `mesos-style.py` and `mesos-tidy.sh` in style guide.

[Benjamin Bannier]

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

Review request for mesos and James Peach.


Repository: mesos


Description
---

Mentioned linters `mesos-style.py` and `mesos-tidy.sh` in style guide.


Diffs
-

  docs/c++-style-guide.md 80a1af318984d47bdfa641aa12e28963d3183fea 


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


Testing
---

Verified good site rendering.


Thanks,

Benjamin Bannier

Re: Review Request 67923: Improved performance of cgroups::read by verifying after failure.

[Stéphane Cottin via Review Board]

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


Ship it!




Ship It!

- Stéphane Cottin


On July 16, 2018, 2:03 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67923/
> ---
> 
> (Updated July 16, 2018, 2:03 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-8418
> https://issues.apache.org/jira/browse/MESOS-8418
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It turns out that cgroups::verify is expensive and leads to a severe
> performance issue on the agent during container metrics collection
> if there are a lot of containers on the agent. See MESOS-8418.
> 
> Since cgroups::verify serves to provide a helpful error message,
> this patch preserves the error message, but only if the read fails.
> 
> Longer term, there probably needs to be some re-structuring of the
> code to make verification caller-controlled, or perhaps the verify
> code can occur consistently post-operation (as done in this patch),
> or perhaps verify can be optimized substantially.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp b12e63c112a7aa7a6f7150359ff5409f8214067e 
> 
> 
> Diff: https://reviews.apache.org/r/67923/diff/1/
> 
> 
> Testing
> ---
> 
> Ran through internal CI.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>