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

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [67936]

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

- Mesos Reviewbot


On July 17, 2018, 8:12 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67936/
> ---
> 
> (Updated July 17, 2018, 8:12 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/2/
> 
> 
> 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 67965: Optimized ranges subtraction operation.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['67965']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
 
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\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 

Re: Review Request 68030: Fixed a linking issue with gRPC.

[Chun-Hung Hsiao]

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


Fix it, then Ship it!





3rdparty/libprocess/include/process/grpc.hpp
Lines 211 (patched)


Nit: std::chrono::...

Also let's use backticks instead of single quotes.


- Chun-Hung Hsiao


On July 24, 2018, 10:49 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68030/
> ---
> 
> (Updated July 24, 2018, 10:49 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Chun-Hung Hsiao.
> 
> 
> Bugs: MESOS-9094
> https://issues.apache.org/jira/browse/MESOS-9094
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/grpc.hpp 
> 28a854fb96443ea13afbd034dc6d20bfbce1fec1 
> 
> 
> Diff: https://reviews.apache.org/r/68030/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 67990: Fixed libprocess tests against rapidjson.

[Benjamin Mahler]

> On July 24, 2018, 6:43 p.m., Alexander Rukletsov wrote:
> > Can you please call out this change in the changelog? It is unfortunate we 
> > do such a change without notifying people on the user and dev lists with 
> > enough time buffer, maybe you can do it right after the chain lands so that 
> > folks can prepare to the 1.7 release?

Sounds good, thanks for the suggestion!


- Benjamin


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


On July 20, 2018, 3:38 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67990/
> ---
> 
> (Updated July 20, 2018, 3:38 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benno Evers.
> 
> 
> Bugs: MESOS-9092
> https://issues.apache.org/jira/browse/MESOS-9092
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The libprocess tests were checking the serialized format of metrics,
> which previously escaped forward slashes. However, this is not what
> rapidjson does and it's also valid json according to ECMA-404.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/metrics_tests.cpp 
> 8590bdb4da1bead462a5b91c23bbc31905e57a47 
> 
> 
> Diff: https://reviews.apache.org/r/67990/diff/1/
> 
> 
> Testing
> ---
> 
> Tested at the end of this chain, since this is split across 
> stout/libprocess/mesos.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 67965: Optimized ranges subtraction operation.

[Meng Zhu]

> On July 24, 2018, 2:06 p.m., Vinod Kone wrote:
> > src/common/values.cpp
> > Lines 290-311 (patched)
> > 
> >
> > This part is a bit hard to follow. Can you break it down into 4 cases
> > 
> > 1) left is subsumed in right
> > 2) right is subsumed in left
> > 3) left overlaps right but left's start is smaller
> > 4) left overlaps right but right's start is smaller
> > 
> > 
> > Even if some of these cases ends up with same resulting code, I think 
> > it's worth duplicating for readability.

I think it is actually more complicated if we separate into the said four 
cases, there will be more corner cases concerning equal signs. I think while 
the separation makes it easy to reason the logic of the individual case, it 
makes it relatively difficult to be confident that the code has covered all the 
individual corner cases (or a single case is only covered once).


- Meng


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


On July 24, 2018, 5:23 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67965/
> ---
> 
> (Updated July 24, 2018, 5:23 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gastón Kleiman, and Vinod Kone.
> 
> 
> Bugs: MESOS-9086
> https://issues.apache.org/jira/browse/MESOS-9086
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The current ranges subtraction uses boost IntervalSet.
> Based on the profiling result of MESOS-8989, the ranges
> subtraction operation is about 2~3 times more expensive
> than that of addition. The conversion cost from Ranges
> to IntervalSet may be the culprit.
> 
> This patch optimizes the ranges subtraction operation by
> converting Ranges to a vector of internal sub-ranges, sorting
> the vector based on sub-range start and then applying a
> one-pass algorithm similar to that of addition.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp afe4137f82115dd4ec9967b5eba16a1dd15401c8 
>   src/tests/values_tests.cpp 2f5abedfd461c114d03f5e941d81ebe414188033 
>   src/v1/values.cpp d2c31f6c91998382dec1d8834b40613013716cdd 
> 
> 
> Diff: https://reviews.apache.org/r/67965/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> Benchmarking:
> 
> tl:dr: more than 80% performance improvment across board. Performance of 
> subtraction is now on par or better than the addition, especially when there 
> are a large number of sub-ranges.
> 
> Build with -O2 optimization, ran on a multicore machine with peak frequency 
> at 2.2GHz:
> 
> Took 1.617706ms to perform 1000 'a += b' operations on ports:[1-6, 11-16, 
> 21-26...91-96] and ports:[3-8, 13-18, 23-28..., 93-98] with 10 sub-ranges
> Took 1.607999ms to perform 1000 'a -= b' operations on ports:[1-6, 11-16, 
> 21-26...91-96] and ports:[3-8, 13-18, 23-28..., 93-98] with 10 sub-ranges
> Took 3.094113ms to perform 1000 'a + b' operations on ports:[1-6, 11-16, 
> 21-26...91-96] and ports:[3-8, 13-18, 23-28..., 93-98] with 10 sub-ranges
> Took 3.152291ms to perform 1000 'a - b' operations on ports:[1-6, 11-16, 
> 21-26...91-96] and ports:[3-8, 13-18, 23-28..., 93-98] with 10 sub-ranges
> 
> Took 14.908924ms to perform 1000 'a += b' operations on ports:[1-6, 11-16, 
> 21-26...991-996] and ports:[3-8, 13-18, 23-28..., 993-998] with 100 sub-ranges
> Took 13.564767ms to perform 1000 'a -= b' operations on ports:[1-6, 11-16, 
> 21-26...991-996] and ports:[3-8, 13-18, 23-28..., 993-998] with 100 sub-ranges
> Took 25.523443ms to perform 1000 'a + b' operations on ports:[1-6, 11-16, 
> 21-26...991-996] and ports:[3-8, 13-18, 23-28..., 993-998] with 100 sub-ranges
> Took 24.871216ms to perform 1000 'a - b' operations on ports:[1-6, 11-16, 
> 21-26...991-996] and ports:[3-8, 13-18, 23-28..., 993-998] with 100 sub-ranges
> 
> Took 234.22483ms to perform 1000 'a += b' operations on ports:[1-6, 11-16, 
> 21-26...9991-9996] and ports:[3-8, 13-18, 23-28..., 9993-9998] with 1000 
> sub-ranges
> Took 144.417381ms to perform 1000 'a -= b' operations on ports:[1-6, 11-16, 
> 21-26...9991-9996] and ports:[3-8, 13-18, 23-28..., 9993-9998] with 1000 
> sub-ranges
> Took 322.548021ms to perform 1000 'a + b' operations on ports:[1-6, 11-16, 
> 21-26...9991-9996] and ports:[3-8, 13-18, 23-28..., 9993-9998] with 1000 
> sub-ranges
> Took 227.835441ms to perform 1000 'a - b' operations on ports:[1-6, 11-16, 
> 21-26...9991-9996] and ports:[3-8, 13-18, 23-28..., 9993-9998] with 1000 
> sub-ranges
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 67965: Optimized ranges subtraction operation.

[Meng Zhu]

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

(Updated July 24, 2018, 5:23 p.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, and Vinod Kone.


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


Repository: mesos


Description
---

The current ranges subtraction uses boost IntervalSet.
Based on the profiling result of MESOS-8989, the ranges
subtraction operation is about 2~3 times more expensive
than that of addition. The conversion cost from Ranges
to IntervalSet may be the culprit.

This patch optimizes the ranges subtraction operation by
converting Ranges to a vector of internal sub-ranges, sorting
the vector based on sub-range start and then applying a
one-pass algorithm similar to that of addition.


Diffs (updated)
-

  src/common/values.cpp afe4137f82115dd4ec9967b5eba16a1dd15401c8 
  src/tests/values_tests.cpp 2f5abedfd461c114d03f5e941d81ebe414188033 
  src/v1/values.cpp d2c31f6c91998382dec1d8834b40613013716cdd 


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

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


Testing
---

make check

Benchmarking:

tl:dr: more than 80% performance improvment across board. Performance of 
subtraction is now on par or better than the addition, especially when there 
are a large number of sub-ranges.

Build with -O2 optimization, ran on a multicore machine with peak frequency at 
2.2GHz:

Took 1.617706ms to perform 1000 'a += b' operations on ports:[1-6, 11-16, 
21-26...91-96] and ports:[3-8, 13-18, 23-28..., 93-98] with 10 sub-ranges
Took 1.607999ms to perform 1000 'a -= b' operations on ports:[1-6, 11-16, 
21-26...91-96] and ports:[3-8, 13-18, 23-28..., 93-98] with 10 sub-ranges
Took 3.094113ms to perform 1000 'a + b' operations on ports:[1-6, 11-16, 
21-26...91-96] and ports:[3-8, 13-18, 23-28..., 93-98] with 10 sub-ranges
Took 3.152291ms to perform 1000 'a - b' operations on ports:[1-6, 11-16, 
21-26...91-96] and ports:[3-8, 13-18, 23-28..., 93-98] with 10 sub-ranges

Took 14.908924ms to perform 1000 'a += b' operations on ports:[1-6, 11-16, 
21-26...991-996] and ports:[3-8, 13-18, 23-28..., 993-998] with 100 sub-ranges
Took 13.564767ms to perform 1000 'a -= b' operations on ports:[1-6, 11-16, 
21-26...991-996] and ports:[3-8, 13-18, 23-28..., 993-998] with 100 sub-ranges
Took 25.523443ms to perform 1000 'a + b' operations on ports:[1-6, 11-16, 
21-26...991-996] and ports:[3-8, 13-18, 23-28..., 993-998] with 100 sub-ranges
Took 24.871216ms to perform 1000 'a - b' operations on ports:[1-6, 11-16, 
21-26...991-996] and ports:[3-8, 13-18, 23-28..., 993-998] with 100 sub-ranges

Took 234.22483ms to perform 1000 'a += b' operations on ports:[1-6, 11-16, 
21-26...9991-9996] and ports:[3-8, 13-18, 23-28..., 9993-9998] with 1000 
sub-ranges
Took 144.417381ms to perform 1000 'a -= b' operations on ports:[1-6, 11-16, 
21-26...9991-9996] and ports:[3-8, 13-18, 23-28..., 9993-9998] with 1000 
sub-ranges
Took 322.548021ms to perform 1000 'a + b' operations on ports:[1-6, 11-16, 
21-26...9991-9996] and ports:[3-8, 13-18, 23-28..., 9993-9998] with 1000 
sub-ranges
Took 227.835441ms to perform 1000 'a - b' operations on ports:[1-6, 11-16, 
21-26...9991-9996] and ports:[3-8, 13-18, 23-28..., 9993-9998] with 1000 
sub-ranges


Thanks,

Meng Zhu

Re: Review Request 67987: Added rapidjson to the mesos build.

[Benno Evers]

> On July 24, 2018, 5:41 p.m., Alexander Rukletsov wrote:
> > 3rdparty/Makefile.am
> > Lines 327 (patched)
> > 
> >
> > s/radidjson/rapidjson/
> > 
> > Also, I don't see you using `rapidjsondir`, can you please explain me 
> > why you declare it?

It's how automake works, the key quote from the manual is:

> A given prefix (e.g. `zar`) is valid if a variable of the same name with
> `dir` appended is defined (e.g. `zardir`).


- Benno


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


On July 20, 2018, 3:38 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67987/
> ---
> 
> (Updated July 20, 2018, 3:38 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benno Evers, and James Peach.
> 
> 
> Bugs: MESOS-9092
> https://issues.apache.org/jira/browse/MESOS-9092
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This includes a stripped bundle of the latest release. Stripping
> is required for licensing (see rapidjson.md), but also helps reduce
> the bloat in the mesos git repo.
> 
> Also included is a readme for how to update the dependency.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt b58996d2ed7521e42d6298d174cc8c494b84eb8f 
>   3rdparty/Makefile.am 6123b8a0164d50f0bd771d2e5f485379811e7fcb 
>   3rdparty/README.md fa9a6483ca2dc4980767a2cb723d654e342e6bb8 
>   3rdparty/cmake/Versions.cmake 0a897d808cd9e05ac0d1a4e1ca61b8f3538f0c4a 
>   3rdparty/rapidjson-1.1.0.tar.gz PRE-CREATION 
>   3rdparty/rapidjson.md PRE-CREATION 
>   3rdparty/versions.am 57792800c4b4b67dd55a316ce00480db6c253d34 
>   configure.ac e86c496140ee9732ad3204ef22ae8ce60cd5c079 
>   src/Makefile.am ecb95ef152fd37e5e83c4f6415489f4cf3959578 
> 
> 
> Diff: https://reviews.apache.org/r/67987/diff/3/
> 
> 
> Testing
> ---
> 
> Tested at the end of this chain, since this is split across 
> stout/libprocess/mesos.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 68030: Fixed a linking issue with gRPC.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [68030]

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

- Mesos Reviewbot


On July 24, 2018, 10:49 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68030/
> ---
> 
> (Updated July 24, 2018, 10:49 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Chun-Hung Hsiao.
> 
> 
> Bugs: MESOS-9094
> https://issues.apache.org/jira/browse/MESOS-9094
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/grpc.hpp 
> 28a854fb96443ea13afbd034dc6d20bfbce1fec1 
> 
> 
> Diff: https://reviews.apache.org/r/68030/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 67827: Added a helper `stripIncapableResources` in the allocator.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67827 was successfully built and tested.

Reviews applied: `['67826', '67827']`

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

- Mesos Reviewbot Windows


On July 24, 2018, 12:58 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67827/
> ---
> 
> (Updated July 24, 2018, 12:58 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9104
> https://issues.apache.org/jira/browse/MESOS-9104
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This helper removes any resources that the framework is not
> capable of receiving based on the given framework capability.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> c1a6789f1808a57dd94ede7bbd2636031f136ea3 
>   src/master/allocator/mesos/hierarchical.cpp 
> 7b4e9db3435b51c9ce025b7b522e10db4b907ebb 
> 
> 
> Diff: https://reviews.apache.org/r/67827/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 67965: Optimized ranges subtraction operation.

[Vinod Kone]

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




src/common/values.cpp
Line 231 (original), 231 (patched)


extra blank line.



src/common/values.cpp
Lines 239 (patched)


`ranges`



src/common/values.cpp
Lines 263 (patched)


either use `iteratorLeft` and `iteratorRight` or `itLeft` and `itRight`. 
i've looked around and I don't think we use `itr*` anywhere.



src/common/values.cpp
Lines 265 (patched)


don't think you need the comment here. it's pretty clear?



src/common/values.cpp
Lines 290-311 (patched)


This part is a bit hard to follow. Can you break it down into 4 cases

1) left is subsumed in right
2) right is subsumed in left
3) left overlaps right but left's start is smaller
4) left overlaps right but right's start is smaller

Even if some of these cases ends up with same resulting code, I think it's 
worth duplicating for readability.



src/common/values.cpp
Lines 313 (patched)


I like that you moved this outside the loop!



src/common/values.cpp
Line 371 (original), 464 (patched)


Can you add a TODO here to make this consistent with how we do subtraction?



src/v1/values.cpp
Lines 233 (patched)


See below.


- Vinod Kone


On July 24, 2018, 5:29 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67965/
> ---
> 
> (Updated July 24, 2018, 5:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gastón Kleiman, and Vinod Kone.
> 
> 
> Bugs: MESOS-9086
> https://issues.apache.org/jira/browse/MESOS-9086
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The current ranges subtraction uses boost IntervalSet.
> Based on the profiling result of MESOS-8989, the ranges
> subtraction operation is about 2~3 times more expensive
> than that of addition. The conversion cost from Ranges
> to IntervalSet may be the culprit.
> 
> This patch optimizes the ranges subtraction operation by
> converting Ranges to a vector of internal sub-ranges, sorting
> the vector based on sub-range start and then applying a
> one-pass algorithm similar to that of addition.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp afe4137f82115dd4ec9967b5eba16a1dd15401c8 
>   src/tests/values_tests.cpp 2f5abedfd461c114d03f5e941d81ebe414188033 
>   src/v1/values.cpp d2c31f6c91998382dec1d8834b40613013716cdd 
> 
> 
> Diff: https://reviews.apache.org/r/67965/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> Benchmarking:
> 
> tl:dr: more than 80% performance improvment across board. Performance of 
> subtraction is now on par or better than the addition, especially when there 
> are a large number of sub-ranges.
> 
> Build with -O2 optimization, ran on a multicore machine with peak frequency 
> at 2.2GHz:
> 
> Took 1.617706ms to perform 1000 'a += b' operations on ports:[1-6, 11-16, 
> 21-26...91-96] and ports:[3-8, 13-18, 23-28..., 93-98] with 10 sub-ranges
> Took 1.607999ms to perform 1000 'a -= b' operations on ports:[1-6, 11-16, 
> 21-26...91-96] and ports:[3-8, 13-18, 23-28..., 93-98] with 10 sub-ranges
> Took 3.094113ms to perform 1000 'a + b' operations on ports:[1-6, 11-16, 
> 21-26...91-96] and ports:[3-8, 13-18, 23-28..., 93-98] with 10 sub-ranges
> Took 3.152291ms to perform 1000 'a - b' operations on ports:[1-6, 11-16, 
> 21-26...91-96] and ports:[3-8, 13-18, 23-28..., 93-98] with 10 sub-ranges
> 
> Took 14.908924ms to perform 1000 'a += b' operations on ports:[1-6, 11-16, 
> 21-26...991-996] and ports:[3-8, 13-18, 23-28..., 993-998] with 100 sub-ranges
> Took 13.564767ms to perform 1000 'a -= b' operations on ports:[1-6, 11-16, 
> 21-26...991-996] and ports:[3-8, 13-18, 23-28..., 993-998] with 100 sub-ranges
> Took 25.523443ms to perform 1000 'a + b' operations on ports:[1-6, 11-16, 
> 21-26...991-996] and ports:[3-8, 13-18, 23-28..., 993-998] with 100 sub-ranges
> Took 24.871216ms to perform 1000 'a - b' operations on ports:[1-6, 11-16, 
> 21-26...991-996] and ports:[3-8, 13-18, 23-28..., 993-998] with 100 sub-ranges
> 
> Took 234.22483ms to perform 1000 'a += b' operations on ports:[1-6, 11-16, 
> 21-26...9991-9996] and ports:[3-8, 13-18, 23-28..., 9993-9998] with 1000 
> sub-ranges
> Took 144.417381ms to perform 1000 'a -= b' operations on ports:[1-6, 11-16, 
> 

Re: Review Request 68029: Updated XFS disk isolator docs.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 68029 was successfully built and tested.

Reviews applied: `['67914', '68029']`

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

- Mesos Reviewbot Windows


On July 24, 2018, 5:36 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68029/
> ---
> 
> (Updated July 24, 2018, 5:36 p.m.)
> 
> 
> Review request for mesos and James Peach.
> 
> 
> Bugs: MESOS-9007
> https://issues.apache.org/jira/browse/MESOS-9007
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added project IDs reclaiming mechanism description.
> 
> 
> Diffs
> -
> 
>   docs/isolators/disk-xfs.md 96bb39ec51bfdc4b297099baf1a4cef3efe2c92f 
>   docs/monitoring.md 2f06e9b7b05d9cd3f447cbd3eab43bc493e25cd1 
> 
> 
> Diff: https://reviews.apache.org/r/68029/diff/3/
> 
> 
> Testing
> ---
> 
> N/A.
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Re: Review Request 67878: Added/updated tests to check per-framework metrics.

[Greg Mann]

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

(Updated July 24, 2018, 8:03 p.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, and Gilbert Song.


Changes
---

Tested functionality of 'suppressed' metrics.


Repository: mesos


Description
---

Added/updated tests to check per-framework metrics.


Diffs (updated)
-

  src/tests/master_tests.cpp 44b0ac39f87c6415e130c5e7f505428642739311 
  src/tests/scheduler_tests.cpp bcdd0ec010889b4155a0feebe0248b7593d8bf0c 


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

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


Testing
---


Thanks,

Greg Mann

Re: Review Request 67827: Added a helper `stripIncapableResources` in the allocator.

[Meng Zhu]

> On July 23, 2018, 4:19 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.hpp
> > Lines 685 (patched)
> > 
> >
> > const Resources& per comment below

Done.


> On July 23, 2018, 4:19 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 1812-1813 (original), 1812-1813 (patched)
> > 
> >
> > What is "normal offer behavior"? In this patch we can just adjust the 
> > comment to reflect that we're not doing an explicit copy anymore, and if we 
> > want to document *why* the current approach is used, we can do that in a 
> > separate patch after hearing from Yan?

Commented why we are subtracting already offered resources in this cycle.


> On July 23, 2018, 4:19 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 2044-2047 (original), 2024-2027 (patched)
> > 
> >
> > Ditto here.

Commented why we are subtracting already offered resources in this cycle.


> On July 23, 2018, 4:19 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 2689 (patched)
> > 
> >
> > This can be `const Resources&` to avoid a copy?

Done.


- Meng


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


On July 24, 2018, 12:58 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67827/
> ---
> 
> (Updated July 24, 2018, 12:58 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9104
> https://issues.apache.org/jira/browse/MESOS-9104
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This helper removes any resources that the framework is not
> capable of receiving based on the given framework capability.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> c1a6789f1808a57dd94ede7bbd2636031f136ea3 
>   src/master/allocator/mesos/hierarchical.cpp 
> 7b4e9db3435b51c9ce025b7b522e10db4b907ebb 
> 
> 
> Diff: https://reviews.apache.org/r/67827/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 67827: Added a helper `stripIncapableResources` in the allocator.

[Meng Zhu]

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

(Updated July 24, 2018, 12:58 p.m.)


Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

This helper removes any resources that the framework is not
capable of receiving based on the given framework capability.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
c1a6789f1808a57dd94ede7bbd2636031f136ea3 
  src/master/allocator/mesos/hierarchical.cpp 
7b4e9db3435b51c9ce025b7b522e10db4b907ebb 


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

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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 68001: Fix padding in MpscLinkedQueue.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['68001']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
 
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\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 

Re: Review Request 67826: Made `Slave::getAvailable()` return all shared resources.

[Meng Zhu]

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

(Updated July 24, 2018, 12:50 p.m.)


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


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


Repository: mesos


Description
---

Currently, depending on already allocated resources,
`HierarchicalAllocatorProcess::Slave::getAvailable()`
may not contain all the shared resources.
Thus it does not accurately reflect what is available.

Since shared resources are always allocatable, we should
include all shared resources in the agent available resources.
This would help remove the one off logic for removing and
adding back shared resources in the allocation loop.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
c1a6789f1808a57dd94ede7bbd2636031f136ea3 


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

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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 67826: Made `Slave::getAvailable()` return all shared resources.

[Meng Zhu]

> On July 23, 2018, 4:09 p.m., Benjamin Mahler wrote:
> > Can you clarify why this commit doesn't touch the shared resources logic in 
> > the allocation loop?
> > https://github.com/apache/mesos/blob/1.6.0/src/master/allocator/mesos/hierarchical.cpp#L2070-L2079
> > 
> > Does this patch pass the tests without the next patch applied? Even so, it 
> > seems like this patch should tidy the shared resources logic up in the 
> > allocation loop now that the old assumption (doesn't show up in 
> > `getAvailable()`) isn't true anymore?
> > 
> > Perhaps the following would be a bit simpler to follow, since the 
> > optimization seems to complicate things a bit:
> > 
> > Patch 1: Move shared resources logic from the allocation loop into the 
> > agent struct.
> > Patch 2: Optimize `Slave::updateAvailable` at the expense of `Slave` 
> > construction and `Slave::updateTotal`?

The existing implementation:
```
Resources available = slave.getAvailable().nonShared();
if (framework.capabilities.sharedResources) {
  available += slave.getTotal().shared();
  ...
}
```
does not depend on how much shared resources `getAvailable()` will return -- it 
calls `nonShared()` anyway. I think there is no assumption being broken by this 
patch and thus no need to tidy the logic in the allocation loop.

Added a comment about why all shared resources are always included.


> On July 23, 2018, 4:09 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.hpp
> > Lines 474-476 (patched)
> > 
> >
> > I was going to say this warrants a comment about there always being a 
> > copy of the shared resources available but I noticed its down below on the 
> > member variable.
> > 
> > Per the comment at the top level of the review, shouldn't the 
> > comment/logic in the allocation loop be migrating here in this patch?

See my comment above.


> On July 23, 2018, 4:09 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.hpp
> > Lines 509-511 (patched)
> > 
> >
> > s/traversals/copying/ ?
> > 
> > Isn't the optimization also being done in the Slave constructor? Maybe 
> > we don't need to say exactly which functions the optimization is done?

Done.


- Meng


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


On July 24, 2018, 12:50 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67826/
> ---
> 
> (Updated July 24, 2018, 12:50 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-9104
> https://issues.apache.org/jira/browse/MESOS-9104
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, depending on already allocated resources,
> `HierarchicalAllocatorProcess::Slave::getAvailable()`
> may not contain all the shared resources.
> Thus it does not accurately reflect what is available.
> 
> Since shared resources are always allocatable, we should
> include all shared resources in the agent available resources.
> This would help remove the one off logic for removing and
> adding back shared resources in the allocation loop.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> c1a6789f1808a57dd94ede7bbd2636031f136ea3 
> 
> 
> Diff: https://reviews.apache.org/r/67826/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 67992: Avoid performance cost of ostringstream in http::OK json constructors.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On July 20, 2018, 3:39 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67992/
> ---
> 
> (Updated July 20, 2018, 3:39 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benno Evers.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In the common case where no jsonp is passed this avoids an extra copy
> out from the ostringstream. In the jsonp case, the performance should
> be near identical or better as we now pre-reserve the space needed
> for the body.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/http.cpp cbb910282a4aff8a914dbd13ff20fc70d929c269 
> 
> 
> Diff: https://reviews.apache.org/r/67992/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 67914: Updated XFS disk isolator to reclaim project IDs after disk GC.

[James Peach]

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


Ship it!




Ship It!

- James Peach


On July 24, 2018, 6:19 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67914/
> ---
> 
> (Updated July 24, 2018, 6:19 p.m.)
> 
> 
> Review request for mesos and James Peach.
> 
> 
> Bugs: MESOS-9007
> https://issues.apache.org/jira/browse/MESOS-9007
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently upon container destruction its project ID is unallocated by
> the isolator and removed from the container work directory. However due
> to API limitations we can't unset project IDs on symlinks that may exist
> inside the directory. Because of that the project may still exist until
> the container directory is garbage collected. If the project ID is
> reused for a new container, any lingering symlinks that still have that
> project ID will contribute to disk usage of the new container. Typically
> symlinks don't take much space, but still this leads to inaccuracy in
> disk space usage accounting.
> 
> This patch postpones project ID reclaiming until sandbox GC time. The
> isolator periodically checks if sandboxes of terminated containers still
> exist and deallocates project IDs of the ones that were removed. Checks
> are performed every "disk_watch_interval". This mechanism can be
> improved in the future if we introduce a way for the isolators to learn
> about disk GCs.
> 
> Current number of available project IDs can be tracked with the new
> "containerizer/mesos/disk/project_ids_free" and
> "containerizer/mesos/disk/project_ids_total" metrics.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/xfs/disk.hpp 
> 0891f7709aa4f98758a727856d58e6177d46adca 
>   src/slave/containerizer/mesos/isolators/xfs/disk.cpp 
> 25f52a43b34b141bdaf7c448817423cf4264e22a 
>   src/tests/containerizer/xfs_quota_tests.cpp 
> dc18a8a59d1eb7fae3592ef6ba8c046e4f46ee4a 
> 
> 
> Diff: https://reviews.apache.org/r/67914/diff/5/
> 
> 
> Testing
> ---
> 
> Added `ROOT_XFS_QuotaTest.ProjectIDReclaiming` test that verifies that 
> project ID is reclaimed and reused after sandbox GC. Ran `sudo make check`.
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Re: Review Request 68033: Fixed flaky test `DefaultExecutorTest.TaskWithFileURI`.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 68033 was successfully built and tested.

Reviews applied: `['68033']`

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

- Mesos Reviewbot Windows


On July 24, 2018, 5:28 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68033/
> ---
> 
> (Updated July 24, 2018, 5:28 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-9108
> https://issues.apache.org/jira/browse/MESOS-9108
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed the flay test by removing the last scheduler task
> status acknowledgment which could race with the test
> tear-down (including the scheduler destruction).
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> f23cb07fbdc9dac0acf75714c835e0eedd7dce6f 
> 
> 
> Diff: https://reviews.apache.org/r/68033/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 67993: Avoid resource copying while serving state json.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On July 20, 2018, 5:23 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67993/
> ---
> 
> (Updated July 20, 2018, 5:23 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benno Evers.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The state serving code was constructing `Resources` wrappers in
> order to jsonify resources. This incurs a copy of the underlying
> Resource objects which is unnecessary and can be costly. This
> removes the copy by proving a direct way to jsonify
> `RepeatedPtrField`.
> 
> This shows a small 10-15% reduction in the master StateQuery
> benchmark timings:
> 
>minq1q3   max
> baseline  3.48  3.54  4.12  4.40
> after this patch  2.82  3.11  3.31  3.71
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 9d2b01f5cb0f787d1ea84d298b25b7e0d072cc66 
>   src/common/http.cpp 9dfbfd12e22b215a24d0598b5c87d96807dc7a6c 
>   src/master/http.cpp c855de1b7bb71cb455d111fcd71c4486f421f2c2 
>   src/slave/http.cpp cf93c487b0936baf70238ffe6eba62f752ea9a81 
> 
> 
> Diff: https://reviews.apache.org/r/67993/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 67991: Adjusted Mesos to compile against jsonify rapidjson changes.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On July 20, 2018, 3:38 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67991/
> ---
> 
> (Updated July 20, 2018, 3:38 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benno Evers.
> 
> 
> Bugs: MESOS-9092
> https://issues.apache.org/jira/browse/MESOS-9092
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `StringWriter` no longer allows appending multiple times
> and instead has a `set` function that must be used.
> 
> 
> Diffs
> -
> 
>   src/common/http.cpp 9dfbfd12e22b215a24d0598b5c87d96807dc7a6c 
>   src/master/http.cpp c855de1b7bb71cb455d111fcd71c4486f421f2c2 
>   src/slave/http.cpp cf93c487b0936baf70238ffe6eba62f752ea9a81 
> 
> 
> Diff: https://reviews.apache.org/r/67991/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 67990: Fixed libprocess tests against rapidjson.

[Alexander Rukletsov]

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


Ship it!




Can you please call out this change in the changelog? It is unfortunate we do 
such a change without notifying people on the user and dev lists with enough 
time buffer, maybe you can do it right after the chain lands so that folks can 
prepare to the 1.7 release?

- Alexander Rukletsov


On July 20, 2018, 3:38 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67990/
> ---
> 
> (Updated July 20, 2018, 3:38 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benno Evers.
> 
> 
> Bugs: MESOS-9092
> https://issues.apache.org/jira/browse/MESOS-9092
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The libprocess tests were checking the serialized format of metrics,
> which previously escaped forward slashes. However, this is not what
> rapidjson does and it's also valid json according to ECMA-404.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/metrics_tests.cpp 
> 8590bdb4da1bead462a5b91c23bbc31905e57a47 
> 
> 
> Diff: https://reviews.apache.org/r/67990/diff/1/
> 
> 
> Testing
> ---
> 
> Tested at the end of this chain, since this is split across 
> stout/libprocess/mesos.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 67988: Improved performance of jsonify by integrating with rapidjson.

[Alexander Rukletsov]

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


Fix it, then Ship it!





3rdparty/stout/include/stout/jsonify.hpp
Lines 123 (patched)


Can you please remove the trailing underscore then? It is weird to see it 
in a public symbol.



3rdparty/stout/include/stout/jsonify.hpp
Lines 366-368 (original), 273-275 (patched)


Do we still want / need a `CHECK` then?



3rdparty/stout/include/stout/jsonify.hpp
Line 375 (original), 282-284 (patched)


ditto


- Alexander Rukletsov


On July 20, 2018, 3:38 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67988/
> ---
> 
> (Updated July 20, 2018, 3:38 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benno Evers, and Michael Park.
> 
> 
> Bugs: MESOS-9092
> https://issues.apache.org/jira/browse/MESOS-9092
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This reduces the time needed for the client to finish receiving a
> master's /state response by 50% in the `StateQuery` benchmark:
> 
> minq1q3   max
> baseline   6.52  6.76  7.33  8.26
> rapidjson w/ SIMD  3.48  3.54  4.12  4.4
> rapidjson  3.29  3.32  3.65  3.85
> 
> SIMD is left disabled for now since it showed slightly slower
> results.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/jsonify.hpp 
> 2314980e185ee61cc2ea54f1b4d2a8b35e58121c 
> 
> 
> Diff: https://reviews.apache.org/r/67988/diff/2/
> 
> 
> Testing
> ---
> 
> Tested at the end of this chain, since this is split across 
> stout/libprocess/mesos.
> 
> 
> 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/#review206406
---


Ship it!




Ship It!

- Jie Yu


On July 17, 2018, 8:12 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67936/
> ---
> 
> (Updated July 17, 2018, 8:12 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/2/
> 
> 
> 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 67914: Updated XFS disk isolator to reclaim project IDs after disk GC.

[Ilya Pronin]

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

(Updated July 24, 2018, 11:19 a.m.)


Review request for mesos and James Peach.


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


Repository: mesos


Description
---

Currently upon container destruction its project ID is unallocated by
the isolator and removed from the container work directory. However due
to API limitations we can't unset project IDs on symlinks that may exist
inside the directory. Because of that the project may still exist until
the container directory is garbage collected. If the project ID is
reused for a new container, any lingering symlinks that still have that
project ID will contribute to disk usage of the new container. Typically
symlinks don't take much space, but still this leads to inaccuracy in
disk space usage accounting.

This patch postpones project ID reclaiming until sandbox GC time. The
isolator periodically checks if sandboxes of terminated containers still
exist and deallocates project IDs of the ones that were removed. Checks
are performed every "disk_watch_interval". This mechanism can be
improved in the future if we introduce a way for the isolators to learn
about disk GCs.

Current number of available project IDs can be tracked with the new
"containerizer/mesos/disk/project_ids_free" and
"containerizer/mesos/disk/project_ids_total" metrics.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/xfs/disk.hpp 
0891f7709aa4f98758a727856d58e6177d46adca 
  src/slave/containerizer/mesos/isolators/xfs/disk.cpp 
25f52a43b34b141bdaf7c448817423cf4264e22a 
  src/tests/containerizer/xfs_quota_tests.cpp 
dc18a8a59d1eb7fae3592ef6ba8c046e4f46ee4a 


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

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


Testing
---

Added `ROOT_XFS_QuotaTest.ProjectIDReclaiming` test that verifies that project 
ID is reclaimed and reused after sandbox GC. Ran `sudo make check`.


Thanks,

Ilya Pronin

Re: Review Request 68029: Updated XFS disk isolator docs.

[James Peach]

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


Ship it!




Ship It!

- James Peach


On July 24, 2018, 5:36 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68029/
> ---
> 
> (Updated July 24, 2018, 5:36 p.m.)
> 
> 
> Review request for mesos and James Peach.
> 
> 
> Bugs: MESOS-9007
> https://issues.apache.org/jira/browse/MESOS-9007
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added project IDs reclaiming mechanism description.
> 
> 
> Diffs
> -
> 
>   docs/isolators/disk-xfs.md 96bb39ec51bfdc4b297099baf1a4cef3efe2c92f 
>   docs/monitoring.md 2f06e9b7b05d9cd3f447cbd3eab43bc493e25cd1 
> 
> 
> Diff: https://reviews.apache.org/r/68029/diff/3/
> 
> 
> Testing
> ---
> 
> N/A.
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Re: Review Request 68001: Fix padding in MpscLinkedQueue.

[Dario Rexin]

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

(Updated July 24, 2018, 6:06 p.m.)


Review request for Benjamin Bannier and James Peach.


Repository: mesos


Description
---

This patch aligns the head of MpscLinkedQueue to a new cache line
and adds padding between head and tail to avoid false sharing
between to two and after tail to avoid false sharing with other
objects that could otherwise end up on the same cache line.


Diffs (updated)
-

  3rdparty/libprocess/src/mpsc_linked_queue.hpp 48c95093d 


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

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


Testing
---

make check & benchmarks


Thanks,

Dario Rexin

Re: Review Request 68001: Fix padding in MpscLinkedQueue.

[Dario Rexin]

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

(Updated July 24, 2018, 6:05 p.m.)


Review request for Benjamin Bannier and James Peach.


Repository: mesos


Description
---

This patch aligns the head of MpscLinkedQueue to a new cache line
and adds padding between head and tail to avoid false sharing
between to two and after tail to avoid false sharing with other
objects that could otherwise end up on the same cache line.


Diffs (updated)
-

  3rdparty/libprocess/src/mpsc_linked_queue.hpp 48c95093d 


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

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


Testing
---

make check & benchmarks


Thanks,

Dario Rexin

Re: Review Request 68001: Fix padding in MpscLinkedQueue.

[James Peach]

> On July 24, 2018, 5:05 p.m., James Peach wrote:
> > 3rdparty/libprocess/src/mpsc_linked_queue.hpp
> > Lines 27 (patched)
> > 
> >
> > Chatted with @bbannier, and he pointed out that we can replace 
> > `__COUNTER__` with something like this:
> > 
> > ```
> > #define MPSC_CACHELINE_PAD(varname) \
> >   uint8_t CAT(_pad, __, varname)[64 - (sizeof(var) % 64)]
> > ```
> 
> Benjamin Bannier wrote:
> I am not sure we still need the macro since the `alignas` will enforce 
> the memory layout we want for `head` and `tail`, and we only really need to 
> add it once to add additional padding after `tail`.
> 
> I'd either suggest getting rid of the macro and just manually adding a 
> padding variable after `tail` (its only required use site), or alternatively 
> move its definition down into the class and `#undef`'ing it after last use. 
> To me the former looks simpler, cleaner and easier to understand.

If we don't add the padding after the head, I expect we will still get the 
clang-tidy warning:

```
consider reordering the fields or adding explicit padding members 
[clang-analyzer-optin.performance.Padding]

```

For consistency, I suggested to @dario that we apply both the alignas and the 
padding for both variables.


- James


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


On July 24, 2018, 2:59 p.m., Dario Rexin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68001/
> ---
> 
> (Updated July 24, 2018, 2:59 p.m.)
> 
> 
> Review request for Benjamin Bannier and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch aligns the head of MpscLinkedQueue to a new cache line
> and adds padding between head and tail to avoid false sharing
> between to two and after tail to avoid false sharing with other
> objects that could otherwise end up on the same cache line.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/mpsc_linked_queue.hpp 48c95093d 
> 
> 
> Diff: https://reviews.apache.org/r/68001/diff/2/
> 
> 
> Testing
> ---
> 
> make check & benchmarks
> 
> 
> Thanks,
> 
> Dario Rexin
> 
>

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

[Greg Mann]

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

(Updated July 24, 2018, 6:01 p.m.)


Review request for mesos, Benjamin Mahler, Chun-Hung Hsiao, Gastón Kleiman, 
Gilbert Song, and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added per-framework offer metrics.


Diffs (updated)
-

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


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

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


Testing
---

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


Thanks,

Greg Mann

Re: Review Request 67809: Added per-framework metrics for scheduler events.

[Greg Mann]

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

(Updated July 24, 2018, 5:57 p.m.)


Review request for mesos, Benjamin Mahler, Chun-Hung Hsiao, Gastón Kleiman, 
Gilbert Song, and Vinod Kone.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Added per-framework metrics for scheduler events.


Diffs (updated)
-

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


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

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


Testing
---


Thanks,

Greg Mann

Re: Review Request 67987: Added rapidjson to the mesos build.

[Alexander Rukletsov]

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


Fix it, then Ship it!




Please also add an entry to "docs/configuration/autotools.md"


3rdparty/CMakeLists.txt
Lines 37-43 (original), 37-44 (patched)


Can you please sort the list?



3rdparty/Makefile.am
Lines 129-130 (original), 131-132 (patched)


I hate to say it again, but my eyes bleed when I see this.



3rdparty/Makefile.am
Lines 327 (patched)


s/radidjson/rapidjson/

Also, I don't see you using `rapidjsondir`, can you please explain me why 
you declare it?


- Alexander Rukletsov


On July 20, 2018, 3:38 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67987/
> ---
> 
> (Updated July 20, 2018, 3:38 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benno Evers, and James Peach.
> 
> 
> Bugs: MESOS-9092
> https://issues.apache.org/jira/browse/MESOS-9092
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This includes a stripped bundle of the latest release. Stripping
> is required for licensing (see rapidjson.md), but also helps reduce
> the bloat in the mesos git repo.
> 
> Also included is a readme for how to update the dependency.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt b58996d2ed7521e42d6298d174cc8c494b84eb8f 
>   3rdparty/Makefile.am 6123b8a0164d50f0bd771d2e5f485379811e7fcb 
>   3rdparty/README.md fa9a6483ca2dc4980767a2cb723d654e342e6bb8 
>   3rdparty/cmake/Versions.cmake 0a897d808cd9e05ac0d1a4e1ca61b8f3538f0c4a 
>   3rdparty/rapidjson-1.1.0.tar.gz PRE-CREATION 
>   3rdparty/rapidjson.md PRE-CREATION 
>   3rdparty/versions.am 57792800c4b4b67dd55a316ce00480db6c253d34 
>   configure.ac e86c496140ee9732ad3204ef22ae8ce60cd5c079 
>   src/Makefile.am ecb95ef152fd37e5e83c4f6415489f4cf3959578 
> 
> 
> Diff: https://reviews.apache.org/r/67987/diff/3/
> 
> 
> Testing
> ---
> 
> Tested at the end of this chain, since this is split across 
> stout/libprocess/mesos.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 68001: Fix padding in MpscLinkedQueue.

[Benjamin Bannier]

> On July 24, 2018, 7:05 p.m., James Peach wrote:
> > 3rdparty/libprocess/src/mpsc_linked_queue.hpp
> > Lines 27 (patched)
> > 
> >
> > Chatted with @bbannier, and he pointed out that we can replace 
> > `__COUNTER__` with something like this:
> > 
> > ```
> > #define MPSC_CACHELINE_PAD(varname) \
> >   uint8_t CAT(_pad, __, varname)[64 - (sizeof(var) % 64)]
> > ```

I am not sure we still need the macro since the `alignas` will enforce the 
memory layout we want for `head` and `tail`, and we only really need to add it 
once to add additional padding after `tail`.

I'd either suggest getting rid of the macro and just manually adding a padding 
variable after `tail` (its only required use site), or alternatively move its 
definition down into the class and `#undef`'ing it after last use. To me the 
former looks simpler, cleaner and easier to understand.


- Benjamin


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


On July 24, 2018, 4:59 p.m., Dario Rexin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68001/
> ---
> 
> (Updated July 24, 2018, 4:59 p.m.)
> 
> 
> Review request for Benjamin Bannier and James Peach.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch aligns the head of MpscLinkedQueue to a new cache line
> and adds padding between head and tail to avoid false sharing
> between to two and after tail to avoid false sharing with other
> objects that could otherwise end up on the same cache line.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/mpsc_linked_queue.hpp 48c95093d 
> 
> 
> Diff: https://reviews.apache.org/r/68001/diff/2/
> 
> 
> Testing
> ---
> 
> make check & benchmarks
> 
> 
> Thanks,
> 
> Dario Rexin
> 
>

Re: Review Request 68029: Updated XFS disk isolator docs.

[Ilya Pronin]

> On July 24, 2018, 10:11 a.m., James Peach wrote:
> > docs/isolators/disk-xfs.md
> > Lines 46 (patched)
> > 
> >
> > Make this a link:
> > 
> > ```
> > [`--disk_watch_interval`](configuration/agent.md#disk_watch_interval)
> > ```

Done.


> On July 24, 2018, 10:11 a.m., James Peach wrote:
> > docs/isolators/disk-xfs.md
> > Lines 50 (patched)
> > 
> >
> > Can you document these in docs/monitoring.md as well?

Done.


- Ilya


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


On July 24, 2018, 10:36 a.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68029/
> ---
> 
> (Updated July 24, 2018, 10:36 a.m.)
> 
> 
> Review request for mesos and James Peach.
> 
> 
> Bugs: MESOS-9007
> https://issues.apache.org/jira/browse/MESOS-9007
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added project IDs reclaiming mechanism description.
> 
> 
> Diffs
> -
> 
>   docs/isolators/disk-xfs.md 96bb39ec51bfdc4b297099baf1a4cef3efe2c92f 
>   docs/monitoring.md 2f06e9b7b05d9cd3f447cbd3eab43bc493e25cd1 
> 
> 
> Diff: https://reviews.apache.org/r/68029/diff/3/
> 
> 
> Testing
> ---
> 
> N/A.
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Re: Review Request 68029: Updated XFS disk isolator docs.

[Ilya Pronin]

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

(Updated July 24, 2018, 10:36 a.m.)


Review request for mesos and James Peach.


Changes
---

Addressed review comments.


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


Repository: mesos


Description
---

Added project IDs reclaiming mechanism description.


Diffs (updated)
-

  docs/isolators/disk-xfs.md 96bb39ec51bfdc4b297099baf1a4cef3efe2c92f 
  docs/monitoring.md 2f06e9b7b05d9cd3f447cbd3eab43bc493e25cd1 


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

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


Testing
---

N/A.


Thanks,

Ilya Pronin

Re: Review Request 66856: Tracked completed framework metrics in the allocator.

[Greg Mann]

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

(Updated July 24, 2018, 5:30 p.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, Gilbert Song, and 
Vinod Kone.


Repository: mesos


Description
---

This ensures that per-framework metrics which are tracked in the
allocator will be retained as long as the per-framework metrics
which are tracked in the master.


Diffs (updated)
-

  include/mesos/allocator/allocator.hpp 
c19ab64ff1422a68cfb6eecea7e2cc4132da8dcc 
  src/master/allocator/mesos/allocator.hpp 
4c337470c5722a5bd1f53c67b5d81a497a7b8023 
  src/master/allocator/mesos/hierarchical.hpp 
c1a6789f1808a57dd94ede7bbd2636031f136ea3 
  src/master/allocator/mesos/hierarchical.cpp 
7b4e9db3435b51c9ce025b7b522e10db4b907ebb 
  src/master/master.cpp 2af976f7ea7f81d4b06a45ce13286dbd61b9b144 
  src/tests/allocator.hpp 6139492bf4d18f31e558bd0216bcccd2705af4d7 
  src/tests/api_tests.cpp 182622a62d350ebefc891a385de3f2d35a7c0243 
  src/tests/master_allocator_tests.cpp 9d802c8ba04374ac6df7aac668bcf05d5eb6c407 
  src/tests/master_quota_tests.cpp d4a8db8b34bcd6793af024e998054b795e900832 
  src/tests/reservation_tests.cpp 6ae0055ad3ca2f848c613a293177d02b3140355e 
  src/tests/resource_offers_tests.cpp 6e24cddce65e7f5a0ce8ea4fde8397a19236d8b8 
  src/tests/slave_recovery_tests.cpp 69b49220e00a37b0824203ae208813a60b67862e 


Diff: https://reviews.apache.org/r/66856/diff/8/

Changes: https://reviews.apache.org/r/66856/diff/7-8/


Testing
---

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


Thanks,

Greg Mann

Review Request 68033: Fixed flaky test `DefaultExecutorTest.TaskWithFileURI`.

[Meng Zhu]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Fixed the flay test by removing the last scheduler task
status acknowledgment which could race with the test
tear-down (including the scheduler destruction).


Diffs
-

  src/tests/default_executor_tests.cpp f23cb07fbdc9dac0acf75714c835e0eedd7dce6f 


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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 67986: Added rapidjson to the libprocess build.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On July 20, 2018, 3:38 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67986/
> ---
> 
> (Updated July 20, 2018, 3:38 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benno Evers.
> 
> 
> Bugs: MESOS-9092
> https://issues.apache.org/jira/browse/MESOS-9092
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This follows the approach used for picojson.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/Makefile.am 
> d13e93f0ce8ae054161dcc4017b1f16d0d5596e1 
>   3rdparty/libprocess/Makefile.am e0d35d9fdddcecf6498f9e617a9c5c604a7f9fc6 
>   3rdparty/libprocess/configure.ac 7fa596aabf6f2082aa6cde2a36d8bae7becdef47 
> 
> 
> Diff: https://reviews.apache.org/r/67986/diff/2/
> 
> 
> Testing
> ---
> 
> Tested at the end of this chain, since this is split across 
> stout/libprocess/mesos.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 68029: Updated XFS disk isolator docs.

[James Peach]

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




docs/isolators/disk-xfs.md
Lines 46 (patched)


Make this a link:

```
[`--disk_watch_interval`](configuration/agent.md#disk_watch_interval)
```



docs/isolators/disk-xfs.md
Lines 50 (patched)


Can you document these in docs/monitoring.md as well?


- James Peach


On July 24, 2018, 12:45 a.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68029/
> ---
> 
> (Updated July 24, 2018, 12:45 a.m.)
> 
> 
> Review request for mesos and James Peach.
> 
> 
> Bugs: MESOS-9007
> https://issues.apache.org/jira/browse/MESOS-9007
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added project IDs reclaiming mechanism description.
> 
> 
> Diffs
> -
> 
>   docs/isolators/disk-xfs.md 96bb39ec51bfdc4b297099baf1a4cef3efe2c92f 
> 
> 
> Diff: https://reviews.apache.org/r/68029/diff/2/
> 
> 
> Testing
> ---
> 
> N/A.
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Re: Review Request 67985: Added rapidjson to the stout build.

[Alexander Rukletsov]

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


Fix it, then Ship it!





3rdparty/stout/3rdparty/Makefile.am
Lines 40-45 (original), 41-47 (patched)


Can we fix the order please?



3rdparty/stout/3rdparty/Makefile.am
Lines 60-62 (original), 62-65 (patched)


Can we fix the order here as well?



3rdparty/stout/CMakeLists.txt
Line 32 (original), 33 (patched)


While you're here, can you please fix the order as well?



3rdparty/stout/Makefile.am
Lines 57-58 (original), 57-59 (patched)


Not your fault, but maybe you can fix it before adding more


- Alexander Rukletsov


On July 20, 2018, 3:38 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67985/
> ---
> 
> (Updated July 20, 2018, 3:38 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benno Evers.
> 
> 
> Bugs: MESOS-9092
> https://issues.apache.org/jira/browse/MESOS-9092
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This follows the approach used for picojson.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/3rdparty/Makefile.am 
> cc831a93a57d970bc28d754acd04569780154bee 
>   3rdparty/stout/CMakeLists.txt 9cbb6f2a13fe9201fdebb9e9994d7725e53af083 
>   3rdparty/stout/Makefile.am 44710c21d3c5a2bc2309b0fe10419712523d3187 
>   3rdparty/stout/configure.ac 7e85c7efccdcbca8f3ef1173075384b0060a15c7 
> 
> 
> Diff: https://reviews.apache.org/r/67985/diff/2/
> 
> 
> Testing
> ---
> 
> Tested at the end of this chain, since this is split across 
> stout/libprocess/mesos.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 68001: Fix padding in MpscLinkedQueue.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['68001']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
 
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\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]


   "D:\DCOS\mesos\3rdparty\libprocess\src\tests\libprocess-tests.vcxproj" 
(default target) (1) ->
   "D:\DCOS\mesos\3rdparty\libprocess\src\tests\benchmarks.vcxproj" 
(default 

Re: Review Request 67878: Added/updated tests to check per-framework metrics.

[Greg Mann]

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

(Updated July 24, 2018, 3:44 p.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, and Gilbert Song.


Repository: mesos


Description
---

Added/updated tests to check per-framework metrics.


Diffs (updated)
-

  src/tests/master_tests.cpp 44b0ac39f87c6415e130c5e7f505428642739311 


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

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


Testing
---


Thanks,

Greg Mann

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

[Greg Mann]

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

(Updated July 24, 2018, 3:40 p.m.)


Review request for mesos, Benjamin Mahler, Gastón Kleiman, Gilbert Song, and 
Vinod Kone.


Repository: mesos


Description
---

Added per-framework metrics for suppressed roles.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
707dd6bd0f255a64d759ce87cbf75be57d86b392 
  src/master/allocator/mesos/metrics.hpp 
6d386225c301d5ab44f3cc0ecdd1478fb5162e5b 
  src/master/allocator/mesos/metrics.cpp 
82990b2dc0b827a43a392d898667eaf58c77ea36 


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

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


Testing
---

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


Thanks,

Greg Mann

Re: Review Request 68029: Updated XFS disk isolator docs.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [67915, 67914, 68029]

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

- Mesos Reviewbot


On July 24, 2018, 12:45 a.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68029/
> ---
> 
> (Updated July 24, 2018, 12:45 a.m.)
> 
> 
> Review request for mesos and James Peach.
> 
> 
> Bugs: MESOS-9007
> https://issues.apache.org/jira/browse/MESOS-9007
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added project IDs reclaiming mechanism description.
> 
> 
> Diffs
> -
> 
>   docs/isolators/disk-xfs.md 96bb39ec51bfdc4b297099baf1a4cef3efe2c92f 
> 
> 
> Diff: https://reviews.apache.org/r/68029/diff/2/
> 
> 
> Testing
> ---
> 
> N/A.
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Re: Review Request 68001: Fix padding in MpscLinkedQueue.

[Dario Rexin]

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

(Updated July 24, 2018, 2:59 p.m.)


Review request for Benjamin Bannier and James Peach.


Repository: mesos


Description
---

This patch aligns the head of MpscLinkedQueue to a new cache line
and adds padding between head and tail to avoid false sharing
between to two and after tail to avoid false sharing with other
objects that could otherwise end up on the same cache line.


Diffs (updated)
-

  3rdparty/libprocess/src/mpsc_linked_queue.hpp 48c95093d 


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

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


Testing
---

make check & benchmarks


Thanks,

Dario Rexin

Re: Review Request 68030: Fixed a linking issue with gRPC.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 68030 was successfully built and tested.

Reviews applied: `['68030']`

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

- Mesos Reviewbot Windows


On July 24, 2018, 10:49 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68030/
> ---
> 
> (Updated July 24, 2018, 10:49 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Chun-Hung Hsiao.
> 
> 
> Bugs: MESOS-9094
> https://issues.apache.org/jira/browse/MESOS-9094
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/grpc.hpp 
> 28a854fb96443ea13afbd034dc6d20bfbce1fec1 
> 
> 
> Diff: https://reviews.apache.org/r/68030/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 67983: Enabled `TimeTest.Now` on Windows.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [67951, 67952, 67976, 67977, 67978, 67979, 67980, 67983]

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

- Mesos Reviewbot


On July 20, 2018, 5:19 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67983/
> ---
> 
> (Updated July 20, 2018, 5:19 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, 
> Liangyu Zhao, and Radhika Jandhyala.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> While the timer resolution on Windows does not support 10
> microseconds, it does support 1000 microseconds, which is still fast
> enough for a unit test.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/time_tests.cpp 
> 08ddb56f1789f400b8cd072c53e885c759f13ddc 
> 
> 
> Diff: https://reviews.apache.org/r/67983/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Review Request 68030: Fixed a linking issue with gRPC.

[Jan Schlicht]

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

Review request for mesos, Benjamin Bannier and Chun-Hung Hsiao.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  3rdparty/libprocess/include/process/grpc.hpp 
28a854fb96443ea13afbd034dc6d20bfbce1fec1 


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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 67502: Refactored ReviewBoard API functionality into separate module.

[Armand Grillet]

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


Ship it!




Ship It!

- Armand Grillet


On July 19, 2018, 3:48 p.m., Dragos Schebesch wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67502/
> ---
> 
> (Updated July 19, 2018, 3:48 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Armand Grillet.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored API functionality into separate module -- this was done to clean 
> up the codebase and put all API calls in single place.
> 
> 
> Diffs
> -
> 
>   support/python3/common.py PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/67502/diff/4/
> 
> 
> Testing
> ---
> 
> For example, to call the api on a specific review_url, with some data, we 
> would use the following code:
> 
> ```
> ReviewBoardHandler().api(review_url, data)
> ```
> 
> 
> Thanks,
> 
> Dragos Schebesch
> 
>

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

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


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
> 
>

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

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


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 67905: Added missing anchor.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On July 13, 2018, 7:22 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67905/
> ---
> 
> (Updated July 13, 2018, 7:22 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Dave Lester, and Till 
> Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added missing anchor.
> 
> 
> Diffs
> -
> 
>   docs/upgrades.md ef89a7250fba002b0b41030f19c78268190d2671 
> 
> 
> Diff: https://reviews.apache.org/r/67905/diff/1/
> 
> 
> Testing
> ---
> 
> Verified that the link in overview table now works.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67903: Fixed breadcrumb links in site templates.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On July 13, 2018, 7:22 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67903/
> ---
> 
> (Updated July 13, 2018, 7:22 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Dave Lester, and Till 
> Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch makes sure that we do not leak newlines into breadcrumb
> links created from page titles, and fixes the handling of spaces which
> are represented with dashes.
> 
> 
> Diffs
> -
> 
>   site/source/layouts/basic.erb fc59f00684b61c551f2705b77b3d66a017e1e76e 
> 
> 
> Diff: https://reviews.apache.org/r/67903/diff/1/
> 
> 
> Testing
> ---
> 
> Verified that the breadcrumb _Getting Started_ (explicit `breadcrumb` in 
> yaml) and the _Documentation breadcrumb_ (reachable from any documentation 
> page, e.g., http://mesos.apache.org/documentation/latest/committing/) have a 
> valid link in the source without newline (some browsers seem to automagically 
> clean up the broken link).
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67904: Fixed documentation links.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On July 13, 2018, 7:22 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67904/
> ---
> 
> (Updated July 13, 2018, 7:22 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Dave Lester, and Till 
> Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed documentation links.
> 
> 
> Diffs
> -
> 
>   docs/cni.md 73b0e64a5db85dbe820e96dd916cc77f5fd1401b 
>   docs/csi.md 7c38fc10633aa28d012606150099ab5cc4b60cb6 
>   docs/home.md 770217f0cc3909ce61e4ae72347eda9120762820 
>   docs/modules.md d3e3017fc98929baa4cb5bc29531a368dda0db8c 
>   docs/performance-profiling.md b3608a109e0684ee81bf8b4052dd1013df95331a 
>   site/source/blog/2014-05-29-mesos-0-18-1-and-0-18-2-released.md 
> 4debcba81f21cddd9ab2058aea8585e8e178603f 
>   site/source/blog/2017-03-08-mesos-1-2-0-released.md 
> 4edb977b90fe8121baf3bd5b079642f8be2290cf 
>   site/source/blog/2017-12-12-storage-developments-in-apache-mesos.md 
> 5dfe6c2d0fad69fbef8b03061b7daf7c963ab22b 
>   site/source/community.html.md 3125565f6152dfdd7ba66863045d2e02702de1a7 
> 
> 
> Diff: https://reviews.apache.org/r/67904/diff/2/
> 
> 
> Testing
> ---
> 
> Inspected a couple of updated links by browsing the generated site. 
> Verification run with `html-proofer` shows less broken links.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67998: Improved cmake missing files check script.

[Alexander Rukletsov]

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


Ship it!




Ship It!

- Alexander Rukletsov


On July 20, 2018, 10:08 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67998/
> ---
> 
> (Updated July 20, 2018, 10:08 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Andrew Schwartzmeyer.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch performs some cleanups:
> 
> * do not emit `realpath` errors when validating filename-like strings
>   in the cmake build,
> * prevent stray output of log headings when no issues were found,
> * emit all diagnostics to stderr,
> * return with an error code if issues were found.
> 
> 
> Diffs
> -
> 
>   support/check-cmake-missing-files.sh 
> b17e8715eb0ceb0c82d87372e2b756741ad6a7e9 
> 
> 
> Diff: https://reviews.apache.org/r/67998/diff/1/
> 
> 
> Testing
> ---
> 
> Checked output before and after https://reviews.apache.org/r/67751/.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 67965: Optimized ranges subtraction operation.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67965 was successfully built and tested.

Reviews applied: `['67965']`

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

- Mesos Reviewbot Windows


On July 24, 2018, 5:29 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67965/
> ---
> 
> (Updated July 24, 2018, 5:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Gastón Kleiman, and Vinod Kone.
> 
> 
> Bugs: MESOS-9086
> https://issues.apache.org/jira/browse/MESOS-9086
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The current ranges subtraction uses boost IntervalSet.
> Based on the profiling result of MESOS-8989, the ranges
> subtraction operation is about 2~3 times more expensive
> than that of addition. The conversion cost from Ranges
> to IntervalSet may be the culprit.
> 
> This patch optimizes the ranges subtraction operation by
> converting Ranges to a vector of internal sub-ranges, sorting
> the vector based on sub-range start and then applying a
> one-pass algorithm similar to that of addition.
> 
> 
> Diffs
> -
> 
>   src/common/values.cpp afe4137f82115dd4ec9967b5eba16a1dd15401c8 
>   src/tests/values_tests.cpp 2f5abedfd461c114d03f5e941d81ebe414188033 
>   src/v1/values.cpp d2c31f6c91998382dec1d8834b40613013716cdd 
> 
> 
> Diff: https://reviews.apache.org/r/67965/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> Benchmarking:
> 
> tl:dr: more than 80% performance improvment across board. Performance of 
> subtraction is now on par or better than the addition, especially when there 
> are a large number of sub-ranges.
> 
> Build with -O2 optimization, ran on a multicore machine with peak frequency 
> at 2.2GHz:
> 
> Took 1.617706ms to perform 1000 'a += b' operations on ports:[1-6, 11-16, 
> 21-26...91-96] and ports:[3-8, 13-18, 23-28..., 93-98] with 10 sub-ranges
> Took 1.607999ms to perform 1000 'a -= b' operations on ports:[1-6, 11-16, 
> 21-26...91-96] and ports:[3-8, 13-18, 23-28..., 93-98] with 10 sub-ranges
> Took 3.094113ms to perform 1000 'a + b' operations on ports:[1-6, 11-16, 
> 21-26...91-96] and ports:[3-8, 13-18, 23-28..., 93-98] with 10 sub-ranges
> Took 3.152291ms to perform 1000 'a - b' operations on ports:[1-6, 11-16, 
> 21-26...91-96] and ports:[3-8, 13-18, 23-28..., 93-98] with 10 sub-ranges
> 
> Took 14.908924ms to perform 1000 'a += b' operations on ports:[1-6, 11-16, 
> 21-26...991-996] and ports:[3-8, 13-18, 23-28..., 993-998] with 100 sub-ranges
> Took 13.564767ms to perform 1000 'a -= b' operations on ports:[1-6, 11-16, 
> 21-26...991-996] and ports:[3-8, 13-18, 23-28..., 993-998] with 100 sub-ranges
> Took 25.523443ms to perform 1000 'a + b' operations on ports:[1-6, 11-16, 
> 21-26...991-996] and ports:[3-8, 13-18, 23-28..., 993-998] with 100 sub-ranges
> Took 24.871216ms to perform 1000 'a - b' operations on ports:[1-6, 11-16, 
> 21-26...991-996] and ports:[3-8, 13-18, 23-28..., 993-998] with 100 sub-ranges
> 
> Took 234.22483ms to perform 1000 'a += b' operations on ports:[1-6, 11-16, 
> 21-26...9991-9996] and ports:[3-8, 13-18, 23-28..., 9993-9998] with 1000 
> sub-ranges
> Took 144.417381ms to perform 1000 'a -= b' operations on ports:[1-6, 11-16, 
> 21-26...9991-9996] and ports:[3-8, 13-18, 23-28..., 9993-9998] with 1000 
> sub-ranges
> Took 322.548021ms to perform 1000 'a + b' operations on ports:[1-6, 11-16, 
> 21-26...9991-9996] and ports:[3-8, 13-18, 23-28..., 9993-9998] with 1000 
> sub-ranges
> Took 227.835441ms to perform 1000 'a - b' operations on ports:[1-6, 11-16, 
> 21-26...9991-9996] and ports:[3-8, 13-18, 23-28..., 9993-9998] with 1000 
> sub-ranges
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>