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

[Benjamin Mahler]

> On July 20, 2018, 10:58 a.m., Benno Evers wrote:
> > 3rdparty/stout/include/stout/jsonify.hpp
> > Line 187 (original), 145 (patched)
> > 
> >
> > I would suggest to add a comment explaining that rapidjson itself says 
> > that the intended way of handling a `false` return value is to terminate 
> > the process, otherwise this looks like a very extreme way of error handling.

`Bool()`, `Double()` and so on always return true, hence the check here. 
Alternatively I could ignore the returned boolean if that lowers the overhead 
for the reader (although hopefully they don't go off wondering why we ignore 
the returned boolean?).

However, both `String()` and `Key()` can return flase only when write 
validation is turned on (per the comment on those).


> On July 20, 2018, 10:58 a.m., Benno Evers wrote:
> > 3rdparty/stout/include/stout/jsonify.hpp
> > Line 417 (original), 324 (patched)
> > 
> >
> > I feel like the previous comment was a little bit more helpful, since 
> > its not immediately obvious that an empty object corresponds to the string 
> > "{}".

Hm.. I wanted to communicate the json structures that get written rather than 
the characters that go into the buffer, so I said things like "an empty object" 
/ "empty array" / "empty string". Perhaps this would be clearer as "empty json 
object" / "empty json array" / "empty json string"?


- Benjamin


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


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

[Benjamin Mahler]

> On July 20, 2018, 9:29 p.m., Michael Park wrote:
> > Looks good!
> > 
> > It might be worth considering using the `OStreamWrapper` stuff for the 
> > `ostream` API.
> > I know writing to `StringBuffer` is faster than writing to 
> > `OStreamWrapper`, but
> > I think just writing to `OStreamWrapper` would be faster than writing to 
> > `StringBuffer`
> > then copying that into a `std::string` then writing that to `ostream` in 
> > the end anyway.

> I think just writing to OStreamWrapper would be faster than writing to 
> StringBuffer

That had seemed doubtful to me based on their documentation:
http://rapidjson.org/md_doc_stream.html#iostreamWrapper

I would be interesting to get the numbers! However, we don't write to ostream 
in the end any longer:
https://reviews.apache.org/r/67992/

I could remove the `<<` operator but I figured I would just leave it untouched 
for now.


> On July 20, 2018, 9:29 p.m., Michael Park wrote:
> > 3rdparty/stout/include/stout/jsonify.hpp
> > Line 137 (original), 95 (patched)
> > 
> >
> > Looks like `GetString` returns a `const char*`. We should provide the 
> > length here: `{buffer.GetString(), buffer.GetSize()}`.

Ah nice catch! This should avoid walking the string right? I'll re-run the 
numbers.


- Benjamin


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


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/1/
> 
> 
> Testing
> ---
> 
> Tested at the end of this chain, since this is split across 
> stout/libprocess/mesos.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 67994: Extended log message for assertion.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [67994]

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, 7:01 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67994/
> ---
> 
> (Updated July 20, 2018, 7:01 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Extended log message for assertion.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 98129d006cda9b65804b518619b6addc8990410a 
> 
> 
> Diff: https://reviews.apache.org/r/67994/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

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/#review206309
---



PASS: Mesos patch 67827 was successfully built and tested.

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

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

- Mesos Reviewbot Windows


On July 20, 2018, 11:30 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67827/
> ---
> 
> (Updated July 20, 2018, 11:30 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 
> 702e7c0aa84b4b672d82c759c25a28a77c78ad50 
>   src/master/allocator/mesos/hierarchical.cpp 
> 707dd6bd0f255a64d759ce87cbf75be57d86b392 
> 
> 
> Diff: https://reviews.apache.org/r/67827/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

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

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [67915, 67914]

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 19, 2018, 11:12 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67914/
> ---
> 
> (Updated July 19, 2018, 11:12 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. 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" metric.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/xfs/disk.hpp 
> 0891f7709aa4f98758a727856d58e6177d46adca 
>   src/slave/containerizer/mesos/isolators/xfs/disk.cpp 
> 25f52a43b34b141bdaf7c448817423cf4264e22a 
>   src/slave/flags.hpp eeb9708f9ec76d83b6719541f4a012544c7c0cbe 
>   src/slave/flags.cpp 58cdc0f1100fe244e5bf1036e1ccf39478d5d478 
>   src/tests/containerizer/xfs_quota_tests.cpp 
> dc18a8a59d1eb7fae3592ef6ba8c046e4f46ee4a 
> 
> 
> Diff: https://reviews.apache.org/r/67914/diff/3/
> 
> 
> 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 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 20, 2018, 4:30 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Rebased.


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 
702e7c0aa84b4b672d82c759c25a28a77c78ad50 
  src/master/allocator/mesos/hierarchical.cpp 
707dd6bd0f255a64d759ce87cbf75be57d86b392 


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

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


Testing
---

make check


Thanks,

Meng Zhu

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 20, 2018, 4:30 p.m.)


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


Changes
---

Rebased.


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


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

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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 67945: Made getters in the Slave class in the allocator return references.

[Meng Zhu]

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

(Updated July 20, 2018, 4:29 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Rebased.


Repository: mesos


Description
---

This eliminates some unnecessary copies.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
702e7c0aa84b4b672d82c759c25a28a77c78ad50 


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

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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 67777: Added a helper to match agent-framework capabilities in the allocator.

[Meng Zhu]

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

(Updated July 20, 2018, 4:28 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Rebased.


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


Repository: mesos


Description
---

`isCapableOfReceivingAgent*(` checks if a framework
is capable of receiving resources on the agent based on
the framework capability.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
702e7c0aa84b4b672d82c759c25a28a77c78ad50 
  src/master/allocator/mesos/hierarchical.cpp 
707dd6bd0f255a64d759ce87cbf75be57d86b392 


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

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


Testing
---

make check


Thanks,

Meng Zhu

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

[Benjamin Mahler]

> On July 20, 2018, 10:46 a.m., Benno Evers wrote:
> > It seems unfortunate that we have to break the API for this. Maybe it would 
> > be possible to have an empty `std::string` member in `StringWriter` that 
> > can serve as a buffer and would be written in the destructor, so we can 
> > emulate `append()`?

There wasn't a use case for append, but thinking over this again, it does seem 
valuable from a performance perspective to not have to force callers to build 
up a string before calling `set()`. I'll see if rapidjson supports the append 
use case and if so add it back.


- Benjamin


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


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.

[Benjamin Mahler]

> On July 20, 2018, 11:19 a.m., Benno Evers wrote:
> > Just to be clear about the consequences, this could imply that users having 
> > written custom tooling to parse the response of `/metrics` might see their 
> > scripts break, right?

Yes, in general, if anyone is using a json de-serializer that doesn't conform 
to the spec they will potentially break since the spec allows different choices 
and rapidjson made different ones than we did.


- Benjamin


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


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 67987: Added rapidjson to the mesos build.

[Benjamin Mahler]

> On July 20, 2018, 10:55 a.m., Benno Evers wrote:
> > 3rdparty/Makefile.am
> > Lines 327 (patched)
> > 
> >
> > Shouldn't the header be under 
> > `$(RAPIDJSON)/include/rapidjson/rapidjson.h`? Also, should we maybe add a 
> > comment that `rapidjson.h` doesn't include any of the other public 
> > rapidjson headers?

This was another bad copy from picojson (which is single header). I've copied 
the approach from elfio now, which puts all headers here.


> On July 20, 2018, 10:55 a.m., Benno Evers wrote:
> > 3rdparty/Makefile.am
> > Lines 517 (patched)
> > 
> >
> > What's the reason that we cannot use the same `DESTDIR=$(INSTALLDIR)` 
> > pattern as for the other entries here? If I remember correctly, 
> > `includedir` should default to `$(DESTDIR)/$(PREFIX)/include`, so I guess 
> > it's to get rid of some default prefix? In this case maybe `PREFIX=` would 
> > be a little bit more explicit and consistent. (especially since the 
> > rapidjson build is cmake-based and doesn't support setting `includedir` 
> > directly)

This was a bad copy; rapidjson doesn't have a makefile. We could run a cmake 
install but I noticed it's not done yet for any others? For now I copied the 
approach taken for elfio since it uses `cp`. Let me know if that makes sense, 
because I'm quite puzzled about the different approaches taken in this file.


- Benjamin


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


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/1/
> 
> 
> Testing
> ---
> 
> Tested at the end of this chain, since this is split across 
> stout/libprocess/mesos.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

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

[Benjamin Mahler]

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




3rdparty/CMakeLists.txt
Lines 444-447 (patched)


Stale comment from copying picojson.



3rdparty/Makefile.am
Lines 514-518 (patched)


Bad copy from picojson, there is no 'make' based install for rapidjson. It 
uses cmake, but I don't see other cmake install examples here so will have to 
see what to do (e.g. follow `cp` approach of other libraries here?).



configure.ac
Lines 1854 (patched)


Needs to be rapidjson/rapidjson.h per benno's comments on previous reviews.


- Benjamin Mahler


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/1/
> 
> 
> Testing
> ---
> 
> Tested at the end of this chain, since this is split across 
> stout/libprocess/mesos.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

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

[Benjamin Mahler]

> On July 20, 2018, 7:28 a.m., Benjamin Bannier wrote:
> > The patch seems to not contain `3rdparty/rapidjson-1.1.0.tar.gz`. Are you 
> > running into https://issues.apache.org/jira/browse/MESOS-3906? If yes, 
> > could you try to upload the patch again after applying haosdent's fix?
> 
> Benjamin Mahler wrote:
> Ah thanks, any reason not to commit that fix?
> 
> Benjamin Bannier wrote:
> This issue is in `rbtools`, not in our tools. It looks like the issue 
> didn't become clear to them in the patch haosdent posted, and the discussion 
> got derailed and ultimately died.

Ah ok, I'll give that a shot and I commented on that review


- Benjamin


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


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/1/
> 
> 
> 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 20, 2018, 11:27 p.m., Benjamin Bannier wrote:
> > 3rdparty/libprocess/src/mpsc_linked_queue.hpp
> > Lines 184-188 (patched)
> > 
> >
> > Wouldn't it be possible to avoid all the manual padding by aligning 
> > both `head` and `tail` on their own cache lines?
> > 
> > // We align `head` to 64 bytes (x86 cache line size) to guarantee 
> > it to
> > // be put on a new cache line. This is to prevent false sharing with
> > // other objects that could otherwise end up on the same cache line 
> > as
> > // this queue. We also align `tail` to avoid false sharing of `head`
> > // with `tail` and to avoid false sharing with other objects.
> > 
> > // We assume a x86_64 cache line size of 64 bytes.
> > //
> > // TODO(drexin): Programatically get the cache line size.
> > #define CACHE_LINE 64
> > 
> > alignas(CACHE_LINE) std::atomic*> head;
> > alignas(CACHE_LINE) Node* tail;
> > 
> > #undef CACHE_LINE
> > 
> > Wouldn't this satisfy the guarantees you list in your comment?
> > 
> > It would also allows us to avoid the macro which has a number of issues 
> > (`__COUNTER__` is a GNU extension, missing check of `sizeof(var) < 64`).

I should have used a real variable for `CACHE_LINE` above to make this less 
nasty, e.g.,

static constexpr std::size_t CACHE_LINE = 64; // Requires e.g., `#include 
`.


- Benjamin


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


On July 20, 2018, 6:51 p.m., Dario Rexin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68001/
> ---
> 
> (Updated July 20, 2018, 6:51 p.m.)
> 
> 
> Review request for Benjamin Bannier.
> 
> 
> 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/1/
> 
> 
> Testing
> ---
> 
> make check & benchmarks
> 
> 
> Thanks,
> 
> Dario Rexin
> 
>

Re: Review Request 67896: Added container-specific cgroups mount for freezer & systemd subsystems.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On July 19, 2018, 2:10 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67896/
> ---
> 
> (Updated July 19, 2018, 2:10 a.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Bugs: MESOS-9070
> https://issues.apache.org/jira/browse/MESOS-9070
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added container-specific cgroups mount for freezer & systemd subsystems.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/constants.hpp 
> 5be39500828aecad01cbc0e27cb20493048f990e 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 8fba6f9d335cc43a7cd0fcb51ed047ba0e7027ed 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> 3bddcece7028745cec6623ac33dbfcaced629629 
> 
> 
> Diff: https://reviews.apache.org/r/67896/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

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

[Michael Park]

> On July 20, 2018, 3:58 a.m., Benno Evers wrote:
> > 3rdparty/stout/include/stout/jsonify.hpp
> > Line 370 (original), 278 (patched)
> > 
> >
> > If we care enough about performance to make a templatized overload for 
> > `const char(&)[N]`, maybe we should also add one for `std::string&&`?

The `const char (&)[N]` exists to avoid constructing a temporary
`std::string` from a string literal argument at the moment.
What cases would adding `std::string&&` improve? We can't exactly
move the `string` anyway right?


- Michael


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


On July 19, 2018, 8:38 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67988/
> ---
> 
> (Updated July 19, 2018, 8:38 p.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/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.

[Michael Park]

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


Fix it, then Ship it!




Looks good!

It might be worth considering using the `OStreamWrapper` stuff for the 
`ostream` API.
I know writing to `StringBuffer` is faster than writing to `OStreamWrapper`, but
I think just writing to `OStreamWrapper` would be faster than writing to 
`StringBuffer`
then copying that into a `std::string` then writing that to `ostream` in the 
end anyway.


3rdparty/stout/include/stout/jsonify.hpp
Line 137 (original), 95 (patched)


Looks like `GetString` returns a `const char*`. We should provide the 
length here: `{buffer.GetString(), buffer.GetSize()}`.


- Michael Park


On July 19, 2018, 8:38 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67988/
> ---
> 
> (Updated July 19, 2018, 8:38 p.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/1/
> 
> 
> Testing
> ---
> 
> Tested at the end of this chain, since this is split across 
> stout/libprocess/mesos.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

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 20, 2018, 8:58 p.m.)


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


Changes
---

Rebase to accommodate our new usage of the `override` keyword.


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 
702e7c0aa84b4b672d82c759c25a28a77c78ad50 
  src/master/allocator/mesos/hierarchical.cpp 
707dd6bd0f255a64d759ce87cbf75be57d86b392 
  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/7/

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


Testing
---

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


Thanks,

Greg Mann

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

[Greg Mann]

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

(Updated July 20, 2018, 8:57 p.m.)


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


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


Repository: mesos


Description
---

Added per-framework metrics for offer operations.


Diffs (updated)
-

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


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

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


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 20, 2018, 8:56 p.m.)


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


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

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


Testing
---


Thanks,

Greg Mann

Re: Review Request 67776: Added per-framework metrics for scheduler calls.

[Greg Mann]

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

(Updated July 20, 2018, 8:55 p.m.)


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


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


Repository: mesos


Description
---

Added per-framework metrics for scheduler calls.


Diffs (updated)
-

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


Diff: https://reviews.apache.org/r/67776/diff/7/

Changes: https://reviews.apache.org/r/67776/diff/6-7/


Testing
---

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


Thanks,

Greg Mann

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

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67983 was successfully built and tested.

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

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

- Mesos Reviewbot Windows


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

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

[Benjamin Bannier]

> On July 20, 2018, 9:28 a.m., Benjamin Bannier wrote:
> > The patch seems to not contain `3rdparty/rapidjson-1.1.0.tar.gz`. Are you 
> > running into https://issues.apache.org/jira/browse/MESOS-3906? If yes, 
> > could you try to upload the patch again after applying haosdent's fix?
> 
> Benjamin Mahler wrote:
> Ah thanks, any reason not to commit that fix?

This issue is in `rbtools`, not in our tools. It looks like the issue didn't 
become clear to them in the patch haosdent posted, and the discussion got 
derailed and ultimately died.


- Benjamin


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


On July 20, 2018, 5: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, 5: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/1/
> 
> 
> Testing
> ---
> 
> Tested at the end of this chain, since this is split across 
> stout/libprocess/mesos.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

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

[Benjamin Mahler]

> On July 20, 2018, 7:28 a.m., Benjamin Bannier wrote:
> > The patch seems to not contain `3rdparty/rapidjson-1.1.0.tar.gz`. Are you 
> > running into https://issues.apache.org/jira/browse/MESOS-3906? If yes, 
> > could you try to upload the patch again after applying haosdent's fix?

Ah thanks, any reason not to commit that fix?


- Benjamin


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


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/1/
> 
> 
> 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/#review206284
---



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/1965/mesos-review-68001

Relevant logs:

- 
[libprocess-tests-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1965/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 t

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

[Andrew Schwartzmeyer]

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

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

Re: Review Request 67891: Added multi-framework scalability guidelines to the documentation.

[Vinod Kone]

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




docs/app-framework-development-guide.md
Lines 55 (patched)


Do we want to give any guidance on what values we considere large? 
Otherwise, it might add more confusion than necessary?


- Vinod Kone


On July 13, 2018, 9:33 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67891/
> ---
> 
> (Updated July 13, 2018, 9:33 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Greg Mann, Meng Zhu, and Vinod Kone.
> 
> 
> Bugs: MESOS-8238
> https://issues.apache.org/jira/browse/MESOS-8238
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Running multiple frameworks requires co-operation from framework
> developers; this outlines some initial guidelines that should be
> followed to ensure running multiple frameworks together performs
> well.
> 
> 
> Diffs
> -
> 
>   docs/app-framework-development-guide.md 
> 035ac1f80e063c75786845475d573c1ee03570c0 
> 
> 
> Diff: https://reviews.apache.org/r/67891/diff/2/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 67996: Granted container user permissions for SANDBOX volume of PARENT type.

[Qian Zhang]

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

(Updated July 20, 2018, 11:38 p.m.)


Review request for mesos, Gilbert Song, Greg Mann, Ilya Pronin, and Jie Yu.


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


Repository: mesos


Description
---

Granted container user permissions for SANDBOX volume of PARENT type.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp 
4896c6811c2c59dcf00871b7a8b6b9b50da0f062 
  src/slave/containerizer/mesos/utils.hpp 
bfd07e28c78fc140e395ffccd11d65545bf007fc 
  src/slave/containerizer/mesos/utils.cpp 
30e76d1d91651975033078f5450e45f5f2fd8ba0 


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

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


Testing
---


Thanks,

Qian Zhang

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

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67998 was successfully built and tested.

Reviews applied: `['67998']`

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

- Mesos Reviewbot Windows


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 67990: Fixed libprocess tests against rapidjson.

[Benno Evers]

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


Ship it!




Just to be clear about the consequences, this could imply that users having 
written custom tooling to parse the response of `/metrics` might see their 
scripts break, right?

- Benno Evers


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 67989: Fixed issues with the JSON serialization tests.

[Benno Evers]

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


Ship it!




Ship It!

- Benno Evers


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/67989/
> ---
> 
> (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
> ---
> 
> There were some issues in the existing JSON serialization tests
> when it comes to rapidjson. For example:
> 
>   - 0x7F does not need to be escaped per ECMA-404. This is what
> rapidjson adheres to.
>   - Forward slash does not need to be escaped.
> 
> This updates the test to have a valid UTF-8 case with a multi-byte
> sequence (which does not need to be escaped per ECMA-404), as well
> as an invalid case where a multibyte sequence is partial (which we
> allow for now since we do not have validation errors exposed).
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/json_tests.cpp 
> 508d8433797a81ec7f720ddc678a007297fec5b3 
>   3rdparty/stout/tests/jsonify_tests.cpp 
> 7de95a4a15912fae3c9269733758e765abc7330d 
> 
> 
> Diff: https://reviews.apache.org/r/67989/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.

[Benno Evers]

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


Fix it, then Ship it!





3rdparty/stout/include/stout/jsonify.hpp
Line 169 (original), 129 (patched)


Let's not use C-style casts.



3rdparty/stout/include/stout/jsonify.hpp
Line 187 (original), 145 (patched)


I would suggest to add a comment explaining that rapidjson itself says that 
the intended way of handling a `false` return value is to terminate the 
process, otherwise this looks like a very extreme way of error handling.



3rdparty/stout/include/stout/jsonify.hpp
Line 327 (original), 251 (patched)


The comment seems outdated, `set()` can only be used to write strings, not 
characters.

Also, just a personal preference, but I would suggest referring to 
functions using `()` after the function name, i.e. `set()` instead of `set`.



3rdparty/stout/include/stout/jsonify.hpp
Line 329 (original), 254 (patched)


I don't really have a good suggestion on how to change it, but this whole 
class seems a bit weird now - we instantiate an object whose only purpose it is 
to have one member function be called, exactly once, and it can't even check 
that because there is no way of signalling the error.

Instinctively, this should probably just free functions like `void 
writeString(rapidjson::Writer*, const std::string&);`, 
but that doesn't really fit within the rest of stouts json framework.



3rdparty/stout/include/stout/jsonify.hpp
Line 366 (original), 273 (patched)


This seems to be missing a word? (same below)



3rdparty/stout/include/stout/jsonify.hpp
Line 370 (original), 278 (patched)


If we care enough about performance to make a templatized overload for 
`const char(&)[N]`, maybe we should also add one for `std::string&&`?



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


I'd suggest `empty_` for consistency.



3rdparty/stout/include/stout/jsonify.hpp
Line 417 (original), 324 (patched)


I feel like the previous comment was a little bit more helpful, since its 
not immediately obvious that an empty object corresponds to the string "{}".


- Benno Evers


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/1/
> 
> 
> Testing
> ---
> 
> Tested at the end of this chain, since this is split across 
> stout/libprocess/mesos.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

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

[Benno Evers]

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




3rdparty/Makefile.am
Lines 325 (patched)


Does it? It looks like `jsonify.hpp` actually adds these includes in the 
next review:

```
#include 
#include 
```



3rdparty/Makefile.am
Lines 327 (patched)


Shouldn't the header be under `$(RAPIDJSON)/include/rapidjson/rapidjson.h`? 
Also, should we maybe add a comment that `rapidjson.h` doesn't include any of 
the other public rapidjson headers?



3rdparty/Makefile.am
Lines 517 (patched)


What's the reason that we cannot use the same `DESTDIR=$(INSTALLDIR)` 
pattern as for the other entries here? If I remember correctly, `includedir` 
should default to `$(DESTDIR)/$(PREFIX)/include`, so I guess it's to get rid of 
some default prefix? In this case maybe `PREFIX=` would be a little bit more 
explicit and consistent. (especially since the rapidjson build is cmake-based 
and doesn't support setting `includedir` directly)



3rdparty/rapidjson.md
Lines 11 (patched)


This is not really related to this review, but also came up when discussing 
the jemalloc patches - if we modify upstreame tarballs, should we maybe 
gpg-sign the result?


- Benno Evers


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/1/
> 
> 
> Testing
> ---
> 
> Tested at the end of this chain, since this is split across 
> stout/libprocess/mesos.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

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

[Benno Evers]

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


Fix it, then Ship it!





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


Same issue as in the prior review.


- Benno Evers


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/1/
> 
> 
> Testing
> ---
> 
> Tested at the end of this chain, since this is split across 
> stout/libprocess/mesos.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

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

[Benno Evers]

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




3rdparty/stout/configure.ac
Lines 496 (patched)


On my machine (Ubuntu 18.04, but probably the same for all debian 
derivatives), the rapidjson headers get installed under 
`/usr/include/rapidjson/rapidjson.h`. So I suspect `AC_CHECK_HEADERS()` 
wouldn't be able to find it when trying to build against an unbundled version 
of rapidjson.


- Benno Evers


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/1/
> 
> 
> Testing
> ---
> 
> Tested at the end of this chain, since this is split across 
> stout/libprocess/mesos.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

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

[Benno Evers]

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



It seems unfortunate that we have to break the API for this. Maybe it would be 
possible to have an empty `std::string` member in `StringWriter` that can serve 
as a buffer and would be written in the destructor, so we can emulate 
`append()`?

- Benno Evers


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 67993: Avoid resource copying while serving state json.

[Benno Evers]

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


Ship it!




Ship It!

- Benno Evers


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 67992: Avoid performance cost of ostringstream in http::OK json constructors.

[Benno Evers]

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


Fix it, then Ship it!





3rdparty/libprocess/src/http.cpp
Line 724 (original), 720 (patched)


Let's not use C-style casts.



3rdparty/libprocess/src/http.cpp
Lines 729 (patched)


Ditto.


- Benno Evers


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/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Review Request 67998: Improved cmake missing files check script.

[Benjamin Bannier]

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

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 67751: WIP: Added missing files to CMake build.

[Benjamin Bannier]

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



This looks overall good to me.

This patch adds some cmake options which can be enabled on the command line, 
but do suprising things (e.g., add files depending on non-existant 3rdparty 
deps which also rely on unset preprocessor symbols). I'd suggest to explicitly 
check that the options are not enabled. This could be a simple

if (ENABLE_PORT_MAPPING_ISOLATOR)
  message(FATAL "The cmake build currently does not support the port 
mapping isolator, see MESOS-XYZ")
endif ()


src/CMakeLists.txt
Lines 317 (patched)


In the autotools build this triggers a `#define ENABLE_XFS_DISK_ISOLATOR`.



src/CMakeLists.txt
Lines 339 (patched)


In the autotools build this triggers a `#define 
ENABLE_NETWORK_PORTS_ISOLATOR`.



src/CMakeLists.txt
Lines 344 (patched)


In the autotools build this triggers a `#define 
ENABLE_PORT_MAPPING_ISOLATOR`.



src/CMakeLists.txt
Lines 604-605 (patched)


Let's remove the comments. They add nothing on this level and would 
hopefully get out of sync anyway.



src/cli/CMakeLists.txt
Lines 33-40 (patched)


Simple and reasonable!


- Benjamin Bannier


On June 27, 2018, 1:12 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67751/
> ---
> 
> (Updated June 27, 2018, 1:12 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James Peach, and Joseph Wu.
> 
> 
> Bugs: MESOS-8994
> https://issues.apache.org/jira/browse/MESOS-8994
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP: Added missing files to CMake build.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 10b0946d6f49c7e9c201bad6f9f1b41cc8460fe5 
>   src/Makefile.am bd94a6488c1c1cc2481b9e9edb25307ced8c0d21 
>   src/cli/CMakeLists.txt 7b2abf2fe14888ec1da11414189f71da972ac427 
>   src/python/executor/CMakeLists.txt PRE-CREATION 
>   src/python/scheduler/CMakeLists.txt PRE-CREATION 
>   src/slave/containerizer/mesos/CMakeLists.txt 
> ba1f92fe7dd59c34c6dee0bc7ecf6f1b5160eee8 
>   src/tests/CMakeLists.txt b9c906d7e91e8e2ce3ec76f972169f9b592a6132 
> 
> 
> Diff: https://reviews.apache.org/r/67751/diff/1/
> 
> 
> Testing
> ---
> 
> This does not add the `option(FOO)` yet to the configuration, not is there 
> logic (yet) to find the necessary libraries to enable those options. How do 
> we want to proceed with this? I was thinking add each `option(ENABLE_XFS)` 
> etc. followed by a `if (ENABLE_XFS) message(FATAL_ERROR "Please add the 
> necessary logic to CMake to build this and see MESOS-1234."` ... but it may 
> honestly take just as much time to add the `find_library` logic myself...
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 67997: Added a test `ROOT_UNPRIVILEGED_USER_ParentTypeDifferentUser`.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the dependent review: 66811.

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

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

Relevant logs:

- 
[apply-review-66811-stdout.log](http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1963/mesos-review-67997/logs/apply-review-66811-stdout.log):

```
error: patch failed: 3rdparty/stout/CMakeLists.txt:30
error: 3rdparty/stout/CMakeLists.txt: patch does not apply
```

- Mesos Reviewbot Windows


On July 20, 2018, 10:21 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67997/
> ---
> 
> (Updated July 20, 2018, 10:21 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, Ilya Pronin, and Jie Yu.
> 
> 
> Bugs: MESOS-8810
> https://issues.apache.org/jira/browse/MESOS-8810
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test `ROOT_UNPRIVILEGED_USER_ParentTypeDifferentUser`.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/volume_sandbox_path_isolator_tests.cpp 
> 97b35a4dfb4c5942858bad5fbc743fd205dd4c3c 
> 
> 
> Diff: https://reviews.apache.org/r/67997/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 67995: Normalized paths passed to mesos-style.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 67995 was successfully built and tested.

Reviews applied: `['67995']`

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

- Mesos Reviewbot Windows


On July 20, 2018, 7:07 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67995/
> ---
> 
> (Updated July 20, 2018, 7:07 a.m.)
> 
> 
> Review request for mesos, Armand Grillet and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This make it possible to pass in slightly messier paths to
> mesos-style, e.g., paths containing double directory separators.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 4f3488cadfe01b0969977171fb40bbc7ad840bc2 
>   support/python3/mesos-style.py cba47c3a60ca47c9d2059574f3f6761b6361a734 
> 
> 
> Diff: https://reviews.apache.org/r/67995/diff/1/
> 
> 
> Testing
> ---
> 
> Before this patch the following invocations lint nothing
> 
> * `./support/mesos-style.py ./support/mesos-style.py`
> * `./support/mesos-style.py support//mesos-style.py`
> 
> After this patch they both successfully (claim to) lint the file.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 67997: Added a test `ROOT_UNPRIVILEGED_USER_ParentTypeDifferentUser`.

[Qian Zhang]

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

Review request for mesos, Gilbert Song, Greg Mann, Ilya Pronin, and Jie Yu.


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


Repository: mesos


Description
---

Added a test `ROOT_UNPRIVILEGED_USER_ParentTypeDifferentUser`.


Diffs
-

  src/tests/containerizer/volume_sandbox_path_isolator_tests.cpp 
97b35a4dfb4c5942858bad5fbc743fd205dd4c3c 


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


Testing
---

sudo make check


Thanks,

Qian Zhang

Review Request 67996: Granted container user permissions for SANDBOX volume of PARENT type.

[Qian Zhang]

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

Review request for mesos, Gilbert Song, Greg Mann, Ilya Pronin, and Jie Yu.


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


Repository: mesos


Description
---

Granted container user permissions for SANDBOX volume of PARENT type.


Diffs
-

  src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp 
4896c6811c2c59dcf00871b7a8b6b9b50da0f062 
  src/slave/containerizer/mesos/utils.hpp 
bfd07e28c78fc140e395ffccd11d65545bf007fc 
  src/slave/containerizer/mesos/utils.cpp 
30e76d1d91651975033078f5450e45f5f2fd8ba0 


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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 67994: Extended log message for assertion.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['67994']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
[   OK ] Endpoint/SlaveEndpointTest.NoAuthorizer/2 (105 ms)
[--] 9 tests from Endpoint/SlaveEndpointTest (963 ms total)

[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (32 
ms)
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (38 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (73 ms 
total)

[--] 1 test from IsolationFlag/CpuIsolatorTest
[ RUN  ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0
[   OK ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0 (650 ms)
[--] 1 test from IsolationFlag/CpuIsolatorTest (674 ms total)

[--] 1 test from IsolationFlag/MemoryIsolatorTest
[ RUN  ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0
[   OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (779 ms)
[--] 1 test from IsolationFlag/MemoryIsolatorTest (802 ms total)

[--] Global test environment tear-down
[==] 1012 tests from 98 test cases ran. (560531 ms total)
[  PASSED  ] 1011 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] CommandExecutorCheckTest.CommandCheckTimeout

 1 FAILED TEST
  YOU HAVE 222 DISABLED TESTS

```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1962/mesos-review-67994/logs/mesos-tests-stderr.log):

```
I0720 08:15:23.196698 14984 slave.cpp:3939] Shutting down framework 
d74fa897-04e0-40f0-86ae-4f9d7427637d-
I0720 08:15:23.196698 17492 master.cpp:10917] Updating the state of task 
81f816cc-2aeb-4d62-b7fc-062109e50fad of framework 
d74fa897-04e0-40f0-86ae-4f9d7427637d- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0720 08:15:23.197832 14984 slave.cpp:6658] Shutting down executor 
'81f816cc-2aeb-4d62-b7fc-062109e50fad' of framework 
d74fa897-04e0-40f0-86ae-4f9d7427637d- at executor(1)@192.10.1.5:56078
I0720 08:15:23.199687 14984 slave.cpp:931] Agent terminating
I0720 08:15:23.199687 17492 master.cpp:11016] Removing task 
81f816cc-2aeb-4d62-b7fc-062109e50fad with resources cpus(allocated: *):4; 
mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
*):[31000-32000] of framework d74fa897-04e0-40f0-86ae-4f9d7427637d- on 
agent d74fa897-04e0-40f0-86ae-4f9d7427637d-S0 at slave(462)@192.10.1.5:54366 
(windows-01.enofukwu14ruplxn0gs3yzmsgf.xx.internal.cloudapp.I0720 
08:15:23.018679 18724 exec.cpp:162] Version: 1.7.0
I0720 08:15:23.044675 18632 exec.cpp:236] Executor registered on agent 
d74fa897-04e0-40f0-86ae-4f9d7427637d-S0
I0720 08:15:23.049669  6484 executor.cpp:182] Received SUBSCRIBED event
I0720 08:15:23.054677  6484 executor.cpp:186] Subscribed executor on 
windows-01.enofukwu14ruplxn0gs3yzmsgf.xx.internal.cloudapp.net
I0720 08:15:23.054677  6484 executor.cpp:182] Received LAUNCH event
I0720 08:15:23.059684  6484 executor.cpp:679] Starting task 
81f816cc-2aeb-4d62-b7fc-062109e50fad
I0720 08:15:23.142678  6484 executor.cpp:499] Running 
'D:\DCOS\mesos\src\mesos-containerizer.exe launch '
I0720 08:15:23.155689  6484 executor.cpp:693] Forked command at 15900
I0720 08:15:23.200686 19196 exec.cpp:445] Executor asked to shutdown
I0720 08:15:23.201696  6484 executor.cpp:182] Received SHUTDOWN event
I0720 08:15:23.201696  6484 executor.cpp:796] Shutting down
I0720 08:15:23.201696  6484 executor.cpp:909] Sending SIGTERM to process tree 
at pid 15net)
W0720 08:15:23.200686 14984 slave.cpp:3935] Ignoring shutdown framework 
d74fa897-04e0-40f0-86ae-4f9d7427637d- because it is terminating
I0720 08:15:23.204685 17492 master.cpp:1330] Agent 
d74fa897-04e0-40f0-86ae-4f9d7427637d-S0 at slave(462)@192.10.1.5:54366 
(windows-01.enofukwu14ruplxn0gs3yzmsgf.xx.internal.cloudapp.net) disconnected
I0720 08:15:23.204685 17492 master.cpp:3340] Disconnecting agent 
d74fa897-04e0-40f0-86ae-4f9d7427637d-S0 at slave(462)@192.10.1.5:54366 
(windows-01.enofukwu14ruplxn0gs3yzmsgf.xx.internal.cloudapp.net)
I0720 08:15:23.204685 17492 master.cpp:3359] Deactivating agent 
d74fa897-04e0-40f0-86ae-4f9d7427637d-S0 at slave(462)@192.10.1.5:54366 
(windows-01.enofukwu1

Re: Review Request 66814: Added a test `FsAclTest.ManipulateAcl`.

[Qian Zhang]

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

(Updated July 20, 2018, 4:13 p.m.)


Review request for mesos, Gilbert Song, Greg Mann, Ilya Pronin, and Jie Yu.


Changes
---

Rebased.


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


Repository: mesos


Description
---

Added a test `FsAclTest.ManipulateAcl`.


Diffs (updated)
-

  3rdparty/stout/tests/os/filesystem_tests.cpp 
09d0a40a74a293dcf6eecde2a443281e4b8d9fe8 


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

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


Testing
---

sudo make check


Thanks,

Qian Zhang

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

[Benjamin Bannier]

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



The patch seems to not contain `3rdparty/rapidjson-1.1.0.tar.gz`. Are you 
running into https://issues.apache.org/jira/browse/MESOS-3906? If yes, could 
you try to upload the patch again after applying haosdent's fix?

- Benjamin Bannier


On July 20, 2018, 5: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, 5: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/1/
> 
> 
> Testing
> ---
> 
> Tested at the end of this chain, since this is split across 
> stout/libprocess/mesos.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 67995: Normalized paths passed to mesos-style.

[Armand Grillet]

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


Ship it!




Ship It!

- Armand Grillet


On juil. 20, 2018, 7:07 matin, Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67995/
> ---
> 
> (Updated juil. 20, 2018, 7:07 matin)
> 
> 
> Review request for mesos, Armand Grillet and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This make it possible to pass in slightly messier paths to
> mesos-style, e.g., paths containing double directory separators.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py 4f3488cadfe01b0969977171fb40bbc7ad840bc2 
>   support/python3/mesos-style.py cba47c3a60ca47c9d2059574f3f6761b6361a734 
> 
> 
> Diff: https://reviews.apache.org/r/67995/diff/1/
> 
> 
> Testing
> ---
> 
> Before this patch the following invocations lint nothing
> 
> * `./support/mesos-style.py ./support/mesos-style.py`
> * `./support/mesos-style.py support//mesos-style.py`
> 
> After this patch they both successfully (claim to) lint the file.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 67995: Normalized paths passed to mesos-style.

[Benjamin Bannier]

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

Review request for mesos, Armand Grillet and Kevin Klues.


Repository: mesos


Description
---

This make it possible to pass in slightly messier paths to
mesos-style, e.g., paths containing double directory separators.


Diffs
-

  support/mesos-style.py 4f3488cadfe01b0969977171fb40bbc7ad840bc2 
  support/python3/mesos-style.py cba47c3a60ca47c9d2059574f3f6761b6361a734 


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


Testing
---

Before this patch the following invocations lint nothing

* `./support/mesos-style.py ./support/mesos-style.py`
* `./support/mesos-style.py support//mesos-style.py`

After this patch they both successfully (claim to) lint the file.


Thanks,

Benjamin Bannier

Review Request 67994: Extended log message for assertion.

[Benjamin Bannier]

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

Review request for mesos, Gilbert Song and Jie Yu.


Repository: mesos


Description
---

Extended log message for assertion.


Diffs
-

  src/slave/containerizer/mesos/containerizer.cpp 
98129d006cda9b65804b518619b6addc8990410a 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier