Re: Review Request 68706: Added master failover reregistration progress metrics.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [68706]

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 Oct. 16, 2018, 4:49 p.m., Xudong Ni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68706/
> ---
> 
> (Updated Oct. 16, 2018, 4:49 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, James Peach, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-9178
> https://issues.apache.org/jira/browse/MESOS-9178
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> During the master failover, the time that the master elected is
> considered as the start of failover. In the progress of
> reregistration, the percentile represents the time when such
> percentile of agents finished registration again; The percentile of
> these data as in this metrics can represent overall reregistration
> progress; In case of degradation towards to the end of reregistration,
> the high percentile can reflect it; In the case there are unreachable
> agents in the failover, if certain percentile recovery couldn't be
> reached, the intiail value of that percentile will not be updated.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 00c6ea94bcb73746aef740236632ede123f5b534 
>   src/master/master.hpp ea7e9242b62fe6c2cc0e717f9a9f2f0c1cc0a390 
>   src/master/master.cpp 868787bb2f9d879531402f83507b322462322efc 
>   src/master/metrics.hpp e1da18e6ba2737f729e1e30653020538150ae898 
>   src/master/metrics.cpp 56a7eef2d279ad3248092d37d19013d3ac110757 
>   src/tests/master_tests.cpp 1db8ed7d81acbcd8bad4b7ca77c501d1d99cc135 
> 
> 
> Diff: https://reviews.apache.org/r/68706/diff/5/
> 
> 
> Testing
> ---
> 
> Automation:
> [ RUN  ] MasterTest.MetricsInMetricsEndpoint
> [   OK ] MasterTest.MetricsInMetricsEndpoint (42 ms)
> 
> Real world cases:
> While reregistrations is in progress: 3277 out of 3582 completed:
> "master/slave_reregistrations": 3277.0,
> "master/slaves_100_percent_reregistered_secs": 0.0,
> "master/slaves_25_percent_reregistered_secs": 5.0,
> "master/slaves_50_percent_reregistered_secs": 11.0,
> "master/slaves_75_percent_reregistered_secs": 20.0,
> "master/slaves_90_percent_reregistered_secs": 30.0,
> "master/slaves_99_percent_reregistered_secs": 0.0,
> 
> 
> While 3582 reregistrations were all completed:
> "master/slave_reregistrations": 3582.0,
> "master/slaves_100_percent_reregistered_secs": 54.0,
> "master/slaves_25_percent_reregistered_secs": 5.0,
> "master/slaves_50_percent_reregistered_secs": 11.0,
> "master/slaves_75_percent_reregistered_secs": 20.0,
> "master/slaves_90_percent_reregistered_secs": 30.0,
> "master/slaves_99_percent_reregistered_secs": 39.0,
> 
> 
> Thanks,
> 
> Xudong Ni
> 
>

Re: Review Request 69032: Optimized resources filter operation.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 69032 was successfully built and tested.

Reviews applied: `['69032']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2478/mesos-review-69032

- Mesos Reviewbot Windows


On Oct. 16, 2018, 11:16 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69032/
> ---
> 
> (Updated Oct. 16, 2018, 11:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Gastón Kleiman.
> 
> 
> Bugs: MESOS-9325
> https://issues.apache.org/jira/browse/MESOS-9325
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, `Resources::filter()` operation uses `add()` which
> scans the resources vector, a O(n) operation. This is not
> necessary. `filter()` operation should only remove `Resource`
> entires. This patch optimizes the performance by directly
> `push_back` the resource to the vector without scanning.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp f0f3df579550d874e477e2d6e7d0e87df079358a 
>   src/v1/resources.cpp 4d2b64f6e30ef96b47fc0b72b00158864e245650 
> 
> 
> Diff: https://reviews.apache.org/r/69032/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 69032: Optimized resources filter operation.

[Benjamin Mahler]

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


Ship it!




Can you also post the filter benchmark results?

https://github.com/apache/mesos/blob/1.7.0/src/tests/resources_tests.cpp#L3865-L3920


src/common/resources.cpp
Line 1550 (original), 1550 (patched)


Consider reserving the result vector?



src/common/resources.cpp
Line 1555 (original), 1555 (patched)


Probably warrants a comment about why we just add it to the vector instead 
of adding?


- Benjamin Mahler


On Oct. 16, 2018, 11:16 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69032/
> ---
> 
> (Updated Oct. 16, 2018, 11:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Gastón Kleiman.
> 
> 
> Bugs: MESOS-9325
> https://issues.apache.org/jira/browse/MESOS-9325
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently, `Resources::filter()` operation uses `add()` which
> scans the resources vector, a O(n) operation. This is not
> necessary. `filter()` operation should only remove `Resource`
> entires. This patch optimizes the performance by directly
> `push_back` the resource to the vector without scanning.
> 
> 
> Diffs
> -
> 
>   src/common/resources.cpp f0f3df579550d874e477e2d6e7d0e87df079358a 
>   src/v1/resources.cpp 4d2b64f6e30ef96b47fc0b72b00158864e245650 
> 
> 
> Diff: https://reviews.apache.org/r/69032/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 63655: Switched to `net::socketpair` in `ns::clone`.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63655 was successfully built and tested.

Reviews applied: `['63654', '63655']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2477/mesos-review-63655

- Mesos Reviewbot Windows


On Oct. 16, 2018, 10:26 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63655/
> ---
> 
> (Updated Oct. 16, 2018, 10:26 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8156
> https://issues.apache.org/jira/browse/MESOS-8156
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Started using to `net::socketpair` in `ns::clone` so that we don't
> have to deal with `SOCK_CLOEXEC` portability.
> 
> 
> Diffs
> -
> 
>   src/linux/ns.cpp ffa9b65b7caf891a5bee1e20be128c37487335e4 
> 
> 
> Diff: https://reviews.apache.org/r/63655/diff/4/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26, macOS)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Review Request 69032: Optimized resources filter operation.

[Meng Zhu]

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

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


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


Repository: mesos


Description
---

Currently, `Resources::filter()` operation uses `add()` which
scans the resources vector, a O(n) operation. This is not
necessary. `filter()` operation should only remove `Resource`
entires. This patch optimizes the performance by directly
`push_back` the resource to the vector without scanning.


Diffs
-

  src/common/resources.cpp f0f3df579550d874e477e2d6e7d0e87df079358a 
  src/v1/resources.cpp 4d2b64f6e30ef96b47fc0b72b00158864e245650 


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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 63655: Switched to `net::socketpair` in `ns::clone`.

[James Peach]

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

(Updated Oct. 16, 2018, 10:26 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description
---

Started using to `net::socketpair` in `ns::clone` so that we don't
have to deal with `SOCK_CLOEXEC` portability.


Diffs (updated)
-

  src/linux/ns.cpp ffa9b65b7caf891a5bee1e20be128c37487335e4 


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

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


Testing
---

make check (Fedora 26, macOS)


Thanks,

James Peach

Re: Review Request 63654: Added a `net::socketpair` helper to stout.

[James Peach]

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

(Updated Oct. 16, 2018, 10:26 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description
---

Added a `net::socketpair` helper to stout that deals with
automatically setting O_CLOEXEC and disabling SIGPIPE where
necessary.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/posix/socket.hpp 
5557394e9ce8e4addc07d328d55274a9d573dfcd 
  3rdparty/stout/tests/os/sendfile_tests.cpp 
e334f489756e6f3352e7b1e84aabb9d40b734138 


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

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


Testing
---

make check (Fedora 26, macOS)


Thanks,

James Peach

Re: Review Request 68089: Added LevelDB compaction after replicated log truncation.

[Benjamin Mahler]

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




src/log/leveldb.cpp
Lines 412-413 (original), 412 (patched)


What happened here? Did you intend to make the deletion synchronous for 
both autoCompact cases?


- Benjamin Mahler


On July 27, 2018, 8:12 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68089/
> ---
> 
> (Updated July 27, 2018, 8:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jie Yu.
> 
> 
> Bugs: MESOS-184
> https://issues.apache.org/jira/browse/MESOS-184
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> LevelDB compaction algorithm doesn't seem to work well with out key
> usage pattern and background compaction doesn't get triggered:
> https://github.com/google/leveldb/issues/603. Because of that the
> replicated log storage grows over time until it consumes all disk space
> on the partition. As a workaround this change adds manual
> leveldb::DB::CompactRange() invocation. Compaction is triggered after
> every truncation action persistence that involved removal of stored
> keys. By default this workaround is disabled and has to be turned on
> with autoCompact parameter of the Log constructor.
> 
> 
> Diffs
> -
> 
>   include/mesos/log/log.hpp ba355331d682a7f2bdc534b4eb4402e2347528d8 
>   src/log/leveldb.hpp 1d5842a13cfe99fa3ea2ac73eabb2a0b55e15239 
>   src/log/leveldb.cpp 72eeffb17e2de39594de284bc01063445f82ab2c 
>   src/log/log.hpp 5612a1a2c4f0d3bba3d946b365f34616180eb426 
>   src/log/log.cpp 6a7080ab5115c777bd5039acb0f830563866781f 
>   src/log/replica.hpp 082c81cd92e1d46f8014cd23953878f9e23a19d7 
>   src/log/replica.cpp 25c128029c9d54f92145e2294f8f59d11b27da2a 
>   src/tests/log_tests.cpp a8980e3676f51d14087f56338ff45de2927ea992 
> 
> 
> Diff: https://reviews.apache.org/r/68089/diff/1/
> 
> 
> Testing
> ---
> 
> Updated `LogStorageTest` cases to run with and without compaction enabled. 
> Ran `make check`.
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Re: Review Request 68090: Added log_auto_compact flag to the master.

[Benjamin Mahler]

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




src/master/flags.cpp
Lines 124-128 (patched)


Seems like we should clarify that this flag is a workaround of a leveldb 
bug and that it's temporary? Or are you thinking this should be permanent?


- Benjamin Mahler


On July 27, 2018, 8:12 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68090/
> ---
> 
> (Updated July 27, 2018, 8:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jie Yu.
> 
> 
> Bugs: MESOS-184
> https://issues.apache.org/jira/browse/MESOS-184
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new flag enables experimental automatic compaction invocation for
> the LevelDB based replicated log storage. This is a workaround the
> problem described in for MESOS-184.
> 
> 
> Diffs
> -
> 
>   src/master/flags.hpp 3929c297b45e1203e5b00454e88f86988a8b1058 
>   src/master/flags.cpp 8fede0d2cf2254f34134329885f47c3c78dc5846 
>   src/master/main.cpp 2c7b1bb492a0655dec9280e98ff942a15e2ae8f0 
> 
> 
> Diff: https://reviews.apache.org/r/68090/diff/1/
> 
> 
> Testing
> ---
> 
> Ran `make check`. Manually verified that the replicated log storage doesn't 
> grow over time with `--log_auto_compact=true`.
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Re: Review Request 68089: Added LevelDB compaction after replicated log truncation.

[Benjamin Mahler]

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




include/mesos/log/log.hpp
Lines 202-209 (patched)


Can we avoid the additional constructors in favor of an additional default 
parameter?

Alternatively, if you're willing to clean this up, we could introduce a 
Log::Options struct?



include/mesos/log/log.hpp
Lines 206 (patched)


The comment doesn't mention "auto" compaction, can you mention it? Also, is 
MESOS-184 the clear pointer to what this is and when we can remove it (i.e. 
once the leveldb issue gets fixed and we upgrade the bundled leveldb?)



src/log/leveldb.hpp
Lines 45 (patched)


Can you mention that this is only needed to support the auto-compaction 
feature (along with a pointer to that feature).



src/log/leveldb.cpp
Line 464 (original), 471 (patched)


extra newline



src/log/leveldb.cpp
Lines 479 (patched)


Are there error cases to handle?

From what I could tell, it looks like this is an asynchronous operation 
that schedules the background compaction? Is it useful to time it given this?

The log message here seems misleading if it's indeed done in the background?



src/log/log.hpp
Lines 53-55 (original), 53-74 (patched)


Ditto here for avoiding the extra constructors



src/log/replica.hpp
Line 60 (original), 60 (patched)


Can you document this flag including a pointer to why it was added and when 
it can be removed?



src/tests/log_tests.cpp
Lines 132-133 (original), 132-134 (patched)


Can you explain that we want to test all cases with both auto-compaction on 
and off to ensure it has no correctness impact? Is this the only spot where we 
should be doing this?



src/tests/log_tests.cpp
Line 258 (original), 256 (patched)


double semi-colon?


- Benjamin Mahler


On July 27, 2018, 8:12 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68089/
> ---
> 
> (Updated July 27, 2018, 8:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Jie Yu.
> 
> 
> Bugs: MESOS-184
> https://issues.apache.org/jira/browse/MESOS-184
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> LevelDB compaction algorithm doesn't seem to work well with out key
> usage pattern and background compaction doesn't get triggered:
> https://github.com/google/leveldb/issues/603. Because of that the
> replicated log storage grows over time until it consumes all disk space
> on the partition. As a workaround this change adds manual
> leveldb::DB::CompactRange() invocation. Compaction is triggered after
> every truncation action persistence that involved removal of stored
> keys. By default this workaround is disabled and has to be turned on
> with autoCompact parameter of the Log constructor.
> 
> 
> Diffs
> -
> 
>   include/mesos/log/log.hpp ba355331d682a7f2bdc534b4eb4402e2347528d8 
>   src/log/leveldb.hpp 1d5842a13cfe99fa3ea2ac73eabb2a0b55e15239 
>   src/log/leveldb.cpp 72eeffb17e2de39594de284bc01063445f82ab2c 
>   src/log/log.hpp 5612a1a2c4f0d3bba3d946b365f34616180eb426 
>   src/log/log.cpp 6a7080ab5115c777bd5039acb0f830563866781f 
>   src/log/replica.hpp 082c81cd92e1d46f8014cd23953878f9e23a19d7 
>   src/log/replica.cpp 25c128029c9d54f92145e2294f8f59d11b27da2a 
>   src/tests/log_tests.cpp a8980e3676f51d14087f56338ff45de2927ea992 
> 
> 
> Diff: https://reviews.apache.org/r/68089/diff/1/
> 
> 
> Testing
> ---
> 
> Updated `LogStorageTest` cases to run with and without compaction enabled. 
> Ran `make check`.
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Re: Review Request 69053: Documented that UUID is a 128 bits (or 16 bytes).

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 69053 was successfully built and tested.

Reviews applied: `['69052', '69053']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2476/mesos-review-69053

- Mesos Reviewbot Windows


On Oct. 16, 2018, 6:57 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69053/
> ---
> 
> (Updated Oct. 16, 2018, 6:57 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, and Jan 
> Schlicht.
> 
> 
> Bugs: MESOS-8309
> https://issues.apache.org/jira/browse/MESOS-8309
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented that UUID is a 128 bits (or 16 bytes).
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5a985fca39cdfb7e9b4775650a7e5dbe68c3b8ae 
>   include/mesos/v1/mesos.proto a5ebb786a98bd3fd34745ddc553aa7a751c0e337 
> 
> 
> Diff: https://reviews.apache.org/r/69053/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 69053: Documented that UUID is a 128 bits (or 16 bytes).

[Gastón Kleiman]

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


Ship it!




Ship It!

- Gastón Kleiman


On Oct. 16, 2018, 11:57 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69053/
> ---
> 
> (Updated Oct. 16, 2018, 11:57 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, and Jan 
> Schlicht.
> 
> 
> Bugs: MESOS-8309
> https://issues.apache.org/jira/browse/MESOS-8309
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented that UUID is a 128 bits (or 16 bytes).
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5a985fca39cdfb7e9b4775650a7e5dbe68c3b8ae 
>   include/mesos/v1/mesos.proto a5ebb786a98bd3fd34745ddc553aa7a751c0e337 
> 
> 
> Diff: https://reviews.apache.org/r/69053/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 69053: Documented that UUID is a 128 bits (or 16 bytes).

[James DeFelice]

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


Ship it!




Ship It!

- James DeFelice


On Oct. 16, 2018, 6:57 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69053/
> ---
> 
> (Updated Oct. 16, 2018, 6:57 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, and Jan 
> Schlicht.
> 
> 
> Bugs: MESOS-8309
> https://issues.apache.org/jira/browse/MESOS-8309
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Documented that UUID is a 128 bits (or 16 bytes).
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5a985fca39cdfb7e9b4775650a7e5dbe68c3b8ae 
>   include/mesos/v1/mesos.proto a5ebb786a98bd3fd34745ddc553aa7a751c0e337 
> 
> 
> Diff: https://reviews.apache.org/r/69053/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Review Request 69052: Added a test to ensure UUID size, variant, version.

[Benjamin Mahler]

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

Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.


Repository: mesos


Description
---

See summary.


Diffs
-

  3rdparty/stout/tests/uuid_tests.cpp 31cc0162490ed2d9eb84b190b6db302ca5807c23 


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


Testing
---


Thanks,

Benjamin Mahler

Review Request 69053: Documented that UUID is a 128 bits (or 16 bytes).

[Benjamin Mahler]

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

Review request for mesos, Benjamin Bannier, James DeFelice, Jie Yu, and Jan 
Schlicht.


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


Repository: mesos


Description
---

Documented that UUID is a 128 bits (or 16 bytes).


Diffs
-

  include/mesos/mesos.proto 5a985fca39cdfb7e9b4775650a7e5dbe68c3b8ae 
  include/mesos/v1/mesos.proto a5ebb786a98bd3fd34745ddc553aa7a751c0e337 


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


Testing
---


Thanks,

Benjamin Mahler

Re: Review Request 68706: Added master failover reregistration progress metrics.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 68706 was successfully built and tested.

Reviews applied: `['68706']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2475/mesos-review-68706

- Mesos Reviewbot Windows


On Oct. 16, 2018, 4:49 p.m., Xudong Ni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68706/
> ---
> 
> (Updated Oct. 16, 2018, 4:49 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, James Peach, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-9178
> https://issues.apache.org/jira/browse/MESOS-9178
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> During the master failover, the time that the master elected is
> considered as the start of failover. In the progress of
> reregistration, the percentile represents the time when such
> percentile of agents finished registration again; The percentile of
> these data as in this metrics can represent overall reregistration
> progress; In case of degradation towards to the end of reregistration,
> the high percentile can reflect it; In the case there are unreachable
> agents in the failover, if certain percentile recovery couldn't be
> reached, the intiail value of that percentile will not be updated.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 00c6ea94bcb73746aef740236632ede123f5b534 
>   src/master/master.hpp ea7e9242b62fe6c2cc0e717f9a9f2f0c1cc0a390 
>   src/master/master.cpp 868787bb2f9d879531402f83507b322462322efc 
>   src/master/metrics.hpp e1da18e6ba2737f729e1e30653020538150ae898 
>   src/master/metrics.cpp 56a7eef2d279ad3248092d37d19013d3ac110757 
>   src/tests/master_tests.cpp 1db8ed7d81acbcd8bad4b7ca77c501d1d99cc135 
> 
> 
> Diff: https://reviews.apache.org/r/68706/diff/5/
> 
> 
> Testing
> ---
> 
> Automation:
> [ RUN  ] MasterTest.MetricsInMetricsEndpoint
> [   OK ] MasterTest.MetricsInMetricsEndpoint (42 ms)
> 
> Real world cases:
> While reregistrations is in progress: 3277 out of 3582 completed:
> "master/slave_reregistrations": 3277.0,
> "master/slaves_100_percent_reregistered_secs": 0.0,
> "master/slaves_25_percent_reregistered_secs": 5.0,
> "master/slaves_50_percent_reregistered_secs": 11.0,
> "master/slaves_75_percent_reregistered_secs": 20.0,
> "master/slaves_90_percent_reregistered_secs": 30.0,
> "master/slaves_99_percent_reregistered_secs": 0.0,
> 
> 
> While 3582 reregistrations were all completed:
> "master/slave_reregistrations": 3582.0,
> "master/slaves_100_percent_reregistered_secs": 54.0,
> "master/slaves_25_percent_reregistered_secs": 5.0,
> "master/slaves_50_percent_reregistered_secs": 11.0,
> "master/slaves_75_percent_reregistered_secs": 20.0,
> "master/slaves_90_percent_reregistered_secs": 30.0,
> "master/slaves_99_percent_reregistered_secs": 39.0,
> 
> 
> Thanks,
> 
> Xudong Ni
> 
>

Re: Review Request 68706: Added master failover reregistration progress metrics.

[Xudong Ni via Review Board]

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

(Updated Oct. 16, 2018, 4:49 p.m.)


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


Changes
---

Sync master


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


Repository: mesos


Description
---

During the master failover, the time that the master elected is
considered as the start of failover. In the progress of
reregistration, the percentile represents the time when such
percentile of agents finished registration again; The percentile of
these data as in this metrics can represent overall reregistration
progress; In case of degradation towards to the end of reregistration,
the high percentile can reflect it; In the case there are unreachable
agents in the failover, if certain percentile recovery couldn't be
reached, the intiail value of that percentile will not be updated.


Diffs (updated)
-

  docs/monitoring.md 00c6ea94bcb73746aef740236632ede123f5b534 
  src/master/master.hpp ea7e9242b62fe6c2cc0e717f9a9f2f0c1cc0a390 
  src/master/master.cpp 868787bb2f9d879531402f83507b322462322efc 
  src/master/metrics.hpp e1da18e6ba2737f729e1e30653020538150ae898 
  src/master/metrics.cpp 56a7eef2d279ad3248092d37d19013d3ac110757 
  src/tests/master_tests.cpp 1db8ed7d81acbcd8bad4b7ca77c501d1d99cc135 


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

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


Testing
---

Automation:
[ RUN  ] MasterTest.MetricsInMetricsEndpoint
[   OK ] MasterTest.MetricsInMetricsEndpoint (42 ms)

Real world cases:
While reregistrations is in progress: 3277 out of 3582 completed:
"master/slave_reregistrations": 3277.0,
"master/slaves_100_percent_reregistered_secs": 0.0,
"master/slaves_25_percent_reregistered_secs": 5.0,
"master/slaves_50_percent_reregistered_secs": 11.0,
"master/slaves_75_percent_reregistered_secs": 20.0,
"master/slaves_90_percent_reregistered_secs": 30.0,
"master/slaves_99_percent_reregistered_secs": 0.0,


While 3582 reregistrations were all completed:
"master/slave_reregistrations": 3582.0,
"master/slaves_100_percent_reregistered_secs": 54.0,
"master/slaves_25_percent_reregistered_secs": 5.0,
"master/slaves_50_percent_reregistered_secs": 11.0,
"master/slaves_75_percent_reregistered_secs": 20.0,
"master/slaves_90_percent_reregistered_secs": 30.0,
"master/slaves_99_percent_reregistered_secs": 39.0,


Thanks,

Xudong Ni

Re: Review Request 69016: Added a test `SubprocessTest.WhiteListFds`.

[James Peach]

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


Ship it!





3rdparty/libprocess/src/tests/subprocess_tests.cpp
Lines 1067 (patched)


Just a suggestion ... you could avoid the need for temporary files by using 
`dup` or `socket` to create file descriptors.


- James Peach


On Oct. 14, 2018, 2:11 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69016/
> ---
> 
> (Updated Oct. 14, 2018, 2:11 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and James Peach.
> 
> 
> Bugs: MESOS-9152 and MESOS-9164
> https://issues.apache.org/jira/browse/MESOS-9152
> https://issues.apache.org/jira/browse/MESOS-9164
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test `SubprocessTest.WhiteListFds`.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/subprocess_tests.cpp 
> e6742ecfc8f5e0a5cf7f4a53f3df4a3dd48e5ea9 
> 
> 
> Diff: https://reviews.apache.org/r/69016/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 68644: Closed all file descriptors except `whitelist_fds` in posix/subprocess.

[James Peach]

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


Ship it!




- James Peach


On Oct. 14, 2018, 2:05 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68644/
> ---
> 
> (Updated Oct. 14, 2018, 2:05 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and James Peach.
> 
> 
> Bugs: MESOS-9152 and MESOS-9164
> https://issues.apache.org/jira/browse/MESOS-9152
> https://issues.apache.org/jira/browse/MESOS-9164
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Closed all file descriptors except `whitelist_fds` in posix/subprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/posix/subprocess.hpp 
> 007058b61fdcd4716aa793516c842c3cef8c0a29 
>   3rdparty/libprocess/src/subprocess.cpp 
> c0640de2dc4278b884282dfaad98c49c3b067a5b 
> 
> 
> Diff: https://reviews.apache.org/r/68644/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 68644: Closed all file descriptors except `whitelist_fds` in posix/subprocess.

[James Peach]

> On Oct. 12, 2018, 5:40 p.m., James Peach wrote:
> > 3rdparty/libprocess/src/posix/subprocess.hpp
> > Lines 213 (patched)
> > 
> >
> > Since you are planning a different code path for macOS, maybe hoist 
> > this out into a static support function in preparation?
> 
> Qian Zhang wrote:
> Did you mean we should put those codes into a static function like 
> `static int convertStringToInt(const char *name)`?

Yes, exactly.


> On Oct. 12, 2018, 5:40 p.m., James Peach wrote:
> > 3rdparty/libprocess/src/posix/subprocess.hpp
> > Lines 255 (patched)
> > 
> >
> > You can just use `std::find()` here.
> 
> Qian Zhang wrote:
> That's what I was thinking. But it seems `std::find()` may allocate 
> memory (search `allocate memory` in 
> https://en.cppreference.com/w/cpp/algorithm/find )?

Oh, I guess that it's not guaranteed to not allocate. Let's drop this then.


> On Oct. 12, 2018, 5:40 p.m., James Peach wrote:
> > 3rdparty/libprocess/src/posix/subprocess.hpp
> > Lines 275 (patched)
> > 
> >
> > Unfortunately, the `Try` here is not async-signal-safe. However, that 
> > is already used by `UNSET_CLOEXEC`, so I think we can just leave a TODO 
> > here.
> > 
> > Can you file a JIRA to add something like `signal_save::uncloexec()`?
> 
> Qian Zhang wrote:
> I see we also use `Try` in another place in `childMain`, e.g., 
> `Try callback = hook();`, so that one needs to be changed too?
> 
> And I'd like to call `fcntl` here directly to unset the `close-on-exec` 
> flag.

> I see we also use Try in another place in childMain, e.g., Try 
> callback = hook();, so that one needs to be changed too?

Yes, in principle. I don't think we need to address that here though.

> And I'd like to call fcntl here directly to unset the close-on-exec flag.

That seems fine to me. Previously, we put helpers in the `signal_safe` 
namespace, but having a local helper for this case seems OK too.


- James


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


On Oct. 14, 2018, 2:05 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68644/
> ---
> 
> (Updated Oct. 14, 2018, 2:05 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and James Peach.
> 
> 
> Bugs: MESOS-9152 and MESOS-9164
> https://issues.apache.org/jira/browse/MESOS-9152
> https://issues.apache.org/jira/browse/MESOS-9164
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Closed all file descriptors except `whitelist_fds` in posix/subprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/posix/subprocess.hpp 
> 007058b61fdcd4716aa793516c842c3cef8c0a29 
>   3rdparty/libprocess/src/subprocess.cpp 
> c0640de2dc4278b884282dfaad98c49c3b067a5b 
> 
> 
> Diff: https://reviews.apache.org/r/68644/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 68644: Closed all file descriptors except `whitelist_fds` in posix/subprocess.

[James Peach]

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




3rdparty/libprocess/src/posix/subprocess.hpp
Lines 19 (patched)


Should this be inside the `__linux__` guard?


- James Peach


On Oct. 14, 2018, 2:05 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68644/
> ---
> 
> (Updated Oct. 14, 2018, 2:05 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and James Peach.
> 
> 
> Bugs: MESOS-9152 and MESOS-9164
> https://issues.apache.org/jira/browse/MESOS-9152
> https://issues.apache.org/jira/browse/MESOS-9164
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Closed all file descriptors except `whitelist_fds` in posix/subprocess.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/posix/subprocess.hpp 
> 007058b61fdcd4716aa793516c842c3cef8c0a29 
>   3rdparty/libprocess/src/subprocess.cpp 
> c0640de2dc4278b884282dfaad98c49c3b067a5b 
> 
> 
> Diff: https://reviews.apache.org/r/68644/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 68991: Added a test `FsTest.Lsof`.

[James Peach]

> On Oct. 12, 2018, 5:40 p.m., James Peach wrote:
> > 3rdparty/stout/tests/os/filesystem_tests.cpp
> > Lines 830 (patched)
> > 
> >
> > Maybe also add:
> > ```
> > EXPECT_FALSE(std::find(fds->begin(), fds->end(), -1) != fds->end())
> > ```
> 
> Qian Zhang wrote:
> Can you please elaborate a bit why we need to add this?

I was just thinking about a negative test to ensure that we don't have anything 
in the result set that shouldn't be there. Maybe an alternative approach would 
be to loop over the returned descriptors and make a system call, e.g. 
fcntl(F_GETFL), to verify that every entry is an open descriptor.

If you don't think adding more checks helps the test, I'm OK with dropping this 
issue :)


- James


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


On Oct. 14, 2018, 1:35 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68991/
> ---
> 
> (Updated Oct. 14, 2018, 1:35 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and James Peach.
> 
> 
> Bugs: MESOS-9152
> https://issues.apache.org/jira/browse/MESOS-9152
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test `FsTest.Lsof`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/os/filesystem_tests.cpp 
> 29f06f3dd126007d6b3a81f31795270f0654b847 
> 
> 
> Diff: https://reviews.apache.org/r/68991/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 69043: Disabled warnings-as-errors for cares build.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 69043 was successfully built and tested.

Reviews applied: `['69043']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2474/mesos-review-69043

- Mesos Reviewbot Windows


On Oct. 16, 2018, 12:03 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69043/
> ---
> 
> (Updated Oct. 16, 2018, 12:03 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-9302
> https://issues.apache.org/jira/browse/MESOS-9302
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Attempting to build Mesos with default flags using gcc 8
> failed because the third-party dependency `cares`, which
> is bundled with `grpc`, was adding `-Werror` to its default
> compiler flags, which triggered on two new warning that
> gcc gained.
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am c69eebeb7eed066cc1d27fd2eccfc8b770a3970d 
> 
> 
> Diff: https://reviews.apache.org/r/69043/diff/1/
> 
> 
> Testing
> ---
> 
> * Started internal CI run (#4626).
> * Started build on Fedora 28 machine and observed successful build of grpc 
> dependency.
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 68994: Logged request processing time for some endpoints.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 68994 was successfully built and tested.

Reviews applied: `['68992', '68993', '68994']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2473/mesos-review-68994

- Mesos Reviewbot Windows


On Oct. 16, 2018, 11:57 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68994/
> ---
> 
> (Updated Oct. 16, 2018, 11:57 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Benno Evers, Benjamin Mahler, 
> Gastón Kleiman, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch leverages `logResponse()` function to print response status
> code together with the request processing time for some endpoints on
> the master and agent. Not all endpoints are participating to avoid
> unnecessary log pollution; only these that are know to be "slow" in
> generating the response.
> 
> Note that requests are still logged when they are fetched from the
> actor mailbox, i.e., affected endpoints now log twice per request:
> when the processing starts and when the response is ready to be sent.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 06d769aeba16586a020729d454f4d00688b78c78 
>   src/slave/slave.cpp 9d84dcb52e5b7e507fc375f184c5f77be08d70a2 
> 
> 
> Diff: https://reviews.apache.org/r/68994/diff/2/
> 
> 
> Testing
> ---
> 
> `make check` on Mac OS 10.13.6 and various Linux distros.
> 
> Manual testing
> ==
> Started a mesos-master process locally: `./bin/mesos-master.sh --work_dir=m`
> 
> 1. `/state` request: `http http://192.168.1.3:5050/state`
> Relevant snippet from the master log:
> ```
> I1011 16:07:05.978631 10211328 http.cpp:1178] HTTP GET for /master/state from 
> 192.168.1.3:56510 with User-Agent='HTTPie/0.9.3'
> I1011 16:07:05.981534 13430784 http.cpp:1195] HTTP GET for /master/state from 
> 192.168.1.3:56510: '200 OK' after 3.260928ms
> ```
> 
> 2. `/flags` request: `http http://192.168.1.3:5050/flags`
> Relevant snippet from the master log:
> ```
> I1011 16:08:51.012370 63365120 http.cpp:1178] HTTP GET for /master/flags from 
> 192.168.1.3:56629 with User-Agent='HTTPie/0.9.3'
> ```
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 68977: Added Record-IO encoder and decoder to Python library.

[Armand Grillet]

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

(Updated Okt. 16, 2018, 12:35 nachm.)


Review request for mesos and Kevin Klues.


Changes
---

Updated test instructions


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


Repository: mesos


Description
---

Added Record-IO encoder and decoder to Python library.

This code was pulled directly from:
https://github.com/dcos/dcos-core-cli/blob/7fd55421939a7782c237e2b8719c0fe2f543acd7/python/lib/dcos/dcos/recordio.py
https://github.com/dcos/dcos-core-cli/blob/7fd55421939a7782c237e2b8719c0fe2f543acd7/python/lib/dcos/tests/test_recordio.py

It will be used by the new CLI for commands such as `task exec`.


Diffs
-

  src/python/lib/mesos/__init__.py 40219c5e03915754b61fc5d7263a063f0d18cca1 
  src/python/lib/mesos/recordio.py PRE-CREATION 
  src/python/lib/tests/test_recordio.py PRE-CREATION 


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


Testing (updated)
---

```
$ tox -e py3-test -- -vvv -k test_recordio
py3-test installed: 
atomicwrites==1.2.1,attrs==18.2.0,certifi==2018.10.15,chardet==3.0.4,coverage==4.5.1,idna==2.7,mock==2.0.0,more-itertools==4.3.0,pbr==4.3.0,pluggy==0.8.0,py==1.7.0,pytest==3.7.3,pytest-cov==2.5.1,requests==2.19.1,six==1.11.0,tenacity==4.12.0,ujson==1.35,urllib3==1.23
py3-test runtests: PYTHONHASHSEED='3750347574'
py3-test runtests: commands[0] | py.test -vvv -k test_recordio
 test 
session starts 

platform linux -- Python 3.6.2, pytest-3.7.3, py-1.7.0, pluggy-0.8.0 -- 
/home/mesosphere.com/klueska/mesos/src/python/lib/.tox/py3-test/bin/python3
cachedir: .pytest_cache
rootdir: /home/mesosphere.com/klueska/mesos/src/python/lib, inifile:
plugins: cov-2.5.1
collected 26 items / 24 deselected  


tests/test_recordio.py::test_encode PASSED  
  [ 50%]
tests/test_recordio.py::test_encode_decode PASSED   
  [100%]

== 2 passed, 24 
deselected in 0.18 seconds 
==
__ 
summary 
__
  py3-test: commands succeeded
  congratulations :)
```


Thanks,

Armand Grillet

Re: Review Request 68977: Added Record-IO encoder and decoder to Python library.

[Armand Grillet]

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

(Updated Okt. 16, 2018, 12:16 nachm.)


Review request for mesos and Kevin Klues.


Changes
---

Changed description


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


Repository: mesos


Description (updated)
---

Added Record-IO encoder and decoder to Python library.

This code was pulled directly from:
https://github.com/dcos/dcos-core-cli/blob/7fd55421939a7782c237e2b8719c0fe2f543acd7/python/lib/dcos/dcos/recordio.py
https://github.com/dcos/dcos-core-cli/blob/7fd55421939a7782c237e2b8719c0fe2f543acd7/python/lib/dcos/tests/test_recordio.py

It will be used by the new CLI for commands such as `task exec`.


Diffs
-

  src/python/lib/mesos/__init__.py 40219c5e03915754b61fc5d7263a063f0d18cca1 
  src/python/lib/mesos/recordio.py PRE-CREATION 
  src/python/lib/tests/test_recordio.py PRE-CREATION 


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


Testing
---

```
(mesos-cli) ?  python (MESOS-6551) ? mesos-cli-tests
Running the Mesos CLI unit tests

TestAgentPlugin
test_list (cli.tests.agent.TestAgentPlugin) ... ok

TestInfrastructure
test_capture_output (cli.tests.tests.TestInfrastructure) ... ok
test_launch_binaries (cli.tests.tests.TestInfrastructure) ... ok

TestRecordIO
test_encode (cli.tests.recordio.TestRecordIO) ... ok
test_encode_decode (cli.tests.recordio.TestRecordIO) ... ok

TestTaskPlugin
test_list (cli.tests.task.TestTaskPlugin) ... ok

--
Ran 6 tests in 5.008s

OK
```


Thanks,

Armand Grillet

Review Request 69043: Disabled warnings-as-errors for cares build.

[Benno Evers]

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

Review request for mesos.


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


Repository: mesos


Description
---

Attempting to build Mesos with default flags using gcc 8
failed because the third-party dependency `cares`, which
is bundled with `grpc`, was adding `-Werror` to its default
compiler flags, which triggered on two new warning that
gcc gained.


Diffs
-

  3rdparty/Makefile.am c69eebeb7eed066cc1d27fd2eccfc8b770a3970d 


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


Testing
---

* Started internal CI run (#4626).
* Started build on Fedora 28 machine and observed successful build of grpc 
dependency.


Thanks,

Benno Evers

Re: Review Request 68971: Moved import of '../lib' from new CLI bootstrap to pip-requirements.txt.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On Okt. 16, 2018, 11:33 vorm., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68971/
> ---
> 
> (Updated Okt. 16, 2018, 11:33 vorm.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-8795
> https://issues.apache.org/jira/browse/MESOS-8795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds the Mesos Python package to the requirements needed by the
> new CLI. We were previously adding the path to the package when setting
> up the CLI virtual environment but this does not work with Pylint when
> it is run from the linters virtual environment.
> 
> By setting things in `pip-requirements.txt`, Pylint does not report
> inexisting linting errors when doing imports such as `import mesos`.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/bootstrap a6183d4c28281bf6d29c8b7f825ae474056f027b 
>   src/python/cli_new/pip-requirements.txt 
> d1822bf752ce76aa5da5999057fe1efb83747fd0 
> 
> 
> Diff: https://reviews.apache.org/r/68971/diff/3/
> 
> 
> Testing
> ---
> 
> Updated the agent plugin to do `from mesos.exceptions import MesosException` 
> and use `MesosException`. Commited to see the git hook running and saw the 
> error `E0401: Unable to import 'mesos.exceptions' (import-error).`. Updated 
> `support/pylint.config`, run the git hook again and saw that the error was 
> gone.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 68993: Introduced `logResponse` for http handlers.

[Alexander Rukletsov]

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

(Updated Oct. 16, 2018, 11:57 a.m.)


Review request for mesos, Benjamin Hindman, Benno Evers, Benjamin Mahler, 
Gastón Kleiman, and Till Toenshoff.


Repository: mesos


Description
---

Currently we use `logRequest` function to log requests to endpoints
in Mesos `MasterProcess`, `SlaveProcess`, and `FilesProcess`. Each
request is logged when it is first fetched from the actor mailbox.
However this obviously does not allow for logging the request
processing time.

This patch introduces a `logResponse` function which logs the request
together with the corresponding response status and the time spent
generating the response.


Diffs (updated)
-

  src/common/http.hpp 4f994a0744f098363327b785df56e877c9624e2a 
  src/common/http.cpp 9070071bbb42703b3f62e0cf50b31da943da015c 


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

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


Testing
---

See https://reviews.apache.org/r/68994/


Thanks,

Alexander Rukletsov

Re: Review Request 68994: Logged request processing time for some endpoints.

[Alexander Rukletsov]

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

(Updated Oct. 16, 2018, 11:57 a.m.)


Review request for mesos, Benjamin Hindman, Benno Evers, Benjamin Mahler, 
Gastón Kleiman, and Till Toenshoff.


Repository: mesos


Description
---

This patch leverages `logResponse()` function to print response status
code together with the request processing time for some endpoints on
the master and agent. Not all endpoints are participating to avoid
unnecessary log pollution; only these that are know to be "slow" in
generating the response.

Note that requests are still logged when they are fetched from the
actor mailbox, i.e., affected endpoints now log twice per request:
when the processing starts and when the response is ready to be sent.


Diffs (updated)
-

  src/master/master.cpp 06d769aeba16586a020729d454f4d00688b78c78 
  src/slave/slave.cpp 9d84dcb52e5b7e507fc375f184c5f77be08d70a2 


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

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


Testing
---

`make check` on Mac OS 10.13.6 and various Linux distros.

Manual testing
==
Started a mesos-master process locally: `./bin/mesos-master.sh --work_dir=m`

1. `/state` request: `http http://192.168.1.3:5050/state`
Relevant snippet from the master log:
```
I1011 16:07:05.978631 10211328 http.cpp:1178] HTTP GET for /master/state from 
192.168.1.3:56510 with User-Agent='HTTPie/0.9.3'
I1011 16:07:05.981534 13430784 http.cpp:1195] HTTP GET for /master/state from 
192.168.1.3:56510: '200 OK' after 3.260928ms
```

2. `/flags` request: `http http://192.168.1.3:5050/flags`
Relevant snippet from the master log:
```
I1011 16:08:51.012370 63365120 http.cpp:1178] HTTP GET for /master/flags from 
192.168.1.3:56629 with User-Agent='HTTPie/0.9.3'
```


Thanks,

Alexander Rukletsov

Re: Review Request 68992: Added 'received' timestamp into `process::Request`.

[Alexander Rukletsov]

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

(Updated Oct. 16, 2018, 11:56 a.m.)


Review request for mesos, Benjamin Hindman, Benno Evers, Benjamin Mahler, 
Gastón Kleiman, and Till Toenshoff.


Repository: mesos


Description
---

This allows us to note when a request comes in and
hence calculate request processing time.


Diffs (updated)
-

  3rdparty/libprocess/include/process/http.hpp 
bbcd0bac8bab51da2dae6c052896d11a86753744 


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

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


Testing
---

See https://reviews.apache.org/r/68994/


Thanks,

Alexander Rukletsov

Re: Review Request 69042: Enabled more constructors for master `RegistryOperation`.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 69042 was successfully built and tested.

Reviews applied: `['69041', '69042']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2472/mesos-review-69042

- Mesos Reviewbot Windows


On Oct. 16, 2018, 12:23 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69042/
> ---
> 
> (Updated Oct. 16, 2018, 12:23 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Chun-Hung Hsiao.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since this class was explicitly declaring a constructor, no other
> constructors such as copy or move constructors were generated.
> 
> This patch removes the custom constructor so all default constructors
> are generated automatically. This e.g., enables move construction of
> `RegistryOperation` values.
> 
> 
> Diffs
> -
> 
>   src/master/registrar.hpp 4f7c7ce2b3dea9f19eae67494f9767dc1c61e121 
> 
> 
> Diff: https://reviews.apache.org/r/69042/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Without this patch a clang close to trunk would warn
> ```
> ../src/resource_provider/registrar.cpp:387:5: warning: explicitly defaulted 
> move constructor is implicitly deleted [-Wdefaulted-function-deleted]
> AdaptedOperation(AdaptedOperation&&) = default;
> ^
> ../src/resource_provider/registrar.cpp:374:28: note: move constructor of 
> 'AdaptedOperation' is implicitly deleted because base class 
> 'master::RegistryOperation' has a deleted move constructor
>   class AdaptedOperation : public master::RegistryOperation
>^
> ../src/master/registrar.hpp:45:27: note: copy constructor of 
> 'RegistryOperation' is implicitly deleted because base class 
> 'process::Promise' has an inaccessible copy constructor
> class RegistryOperation : public process::Promise
>   ^
> ../src/resource_provider/registrar.cpp:389:23: warning: explicitly defaulted 
> move assignment operator is implicitly deleted [-Wdefaulted-function-deleted]
> AdaptedOperation& operator=(AdaptedOperation&&) = default;
>   ^
> ../src/resource_provider/registrar.cpp:374:28: note: move assignment operator 
> of 'AdaptedOperation' is implicitly deleted because base class 
> 'master::RegistryOperation' has a deleted move assignment operator
>   class AdaptedOperation : public master::RegistryOperation
>^
> ../src/master/registrar.hpp:45:27: note: copy assignment operator of 
> 'RegistryOperation' is implicitly deleted because base class 
> 'process::Promise' has an inaccessible copy assignment operator
> class RegistryOperation : public process::Promise
>   ^
> 2 warnings generated.
> ```
> 
> No warning is emitted with this patch.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 68992: Added 'received' timestamp into `process::Request`.

[Alexander Rukletsov]

> On Oct. 16, 2018, 11:41 a.m., Till Toenshoff wrote:
> > 3rdparty/libprocess/include/process/http.hpp
> > Lines 568-569 (patched)
> > 
> >
> > As discussed out of band, let's come up with a proper story here in a 
> > JIRA first and then get back to it.

Agreed — this suggests one possible solution, I'll remove the TODO.


- Alexander


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


On Oct. 11, 2018, 2:16 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68992/
> ---
> 
> (Updated Oct. 11, 2018, 2:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Benno Evers, Benjamin Mahler, 
> Gastón Kleiman, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This allows us to note when a request comes in and
> hence calculate request processing time.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> bbcd0bac8bab51da2dae6c052896d11a86753744 
> 
> 
> Diff: https://reviews.apache.org/r/68992/diff/1/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/68994/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 69047: Updated Python library to be easier to handle as a Python module.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On Okt. 16, 2018, 11:31 vorm., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69047/
> ---
> 
> (Updated Okt. 16, 2018, 11:31 vorm.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-8795
> https://issues.apache.org/jira/browse/MESOS-8795
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated Python library to be easier to handle as a Python module.
> 
> 
> Diffs
> -
> 
>   src/python/lib/mesos/__init__.py 40219c5e03915754b61fc5d7263a063f0d18cca1 
>   src/python/lib/setup.py 08f854f43681d1f694bb48604773256be7ce927b 
> 
> 
> Diff: https://reviews.apache.org/r/69047/diff/1/
> 
> 
> Testing
> ---
> 
> Tested manually using the Mesos CLI later in the chain.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 68992: Added 'received' timestamp into `process::Request`.

[Till Toenshoff via Review Board]

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


Fix it, then Ship it!





3rdparty/libprocess/include/process/http.hpp
Lines 568-569 (patched)


As discussed out of band, let's come up with a proper story here in a JIRA 
first and then get back to it.


- Till Toenshoff


On Oct. 11, 2018, 2:16 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68992/
> ---
> 
> (Updated Oct. 11, 2018, 2:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Benno Evers, Benjamin Mahler, 
> Gastón Kleiman, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This allows us to note when a request comes in and
> hence calculate request processing time.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/http.hpp 
> bbcd0bac8bab51da2dae6c052896d11a86753744 
> 
> 
> Diff: https://reviews.apache.org/r/68992/diff/1/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/68994/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 68993: Introduced `logResponse` for http handlers.

[Till Toenshoff via Review Board]

> On Oct. 11, 2018, 3:11 p.m., Benno Evers wrote:
> > src/common/http.cpp
> > Lines 1200 (patched)
> > 
> >
> > Since the outputs of this log line are likely to be used for 
> > rudimentary analysis using shell tools like `grep`, `cut`, `awk`, etc., I 
> > think it would be good to pick a unit (seconds or milliseconds) and fix the 
> > output to that.
> > 
> > This way, tools could immediately process and compare the printed 
> > durations without having to implement a small parser to convert to a common 
> > unit.

Very good point indeed.


- Till


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


On Oct. 11, 2018, 2:16 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68993/
> ---
> 
> (Updated Oct. 11, 2018, 2:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Benno Evers, Benjamin Mahler, 
> Gastón Kleiman, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently we use `logRequest` function to log requests to endpoints
> in Mesos `MasterProcess`, `SlaveProcess`, and `FilesProcess`. Each
> request is logged when it is first fetched from the actor mailbox.
> However this obviously does not allow for logging the request
> processing time.
> 
> This patch introduces a `logResponse` function which logs the request
> together with the corresponding response status and the time spent
> generating the response.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 4f994a0744f098363327b785df56e877c9624e2a 
>   src/common/http.cpp 9070071bbb42703b3f62e0cf50b31da943da015c 
> 
> 
> Diff: https://reviews.apache.org/r/68993/diff/1/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/68994/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 68993: Introduced `logResponse` for http handlers.

[Till Toenshoff via Review Board]

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


Ship it!




Modulo Benno's comment.


src/common/http.hpp
Lines 353-354 (patched)


Thanks for the TODO.


- Till Toenshoff


On Oct. 11, 2018, 2:16 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68993/
> ---
> 
> (Updated Oct. 11, 2018, 2:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Benno Evers, Benjamin Mahler, 
> Gastón Kleiman, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently we use `logRequest` function to log requests to endpoints
> in Mesos `MasterProcess`, `SlaveProcess`, and `FilesProcess`. Each
> request is logged when it is first fetched from the actor mailbox.
> However this obviously does not allow for logging the request
> processing time.
> 
> This patch introduces a `logResponse` function which logs the request
> together with the corresponding response status and the time spent
> generating the response.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 4f994a0744f098363327b785df56e877c9624e2a 
>   src/common/http.cpp 9070071bbb42703b3f62e0cf50b31da943da015c 
> 
> 
> Diff: https://reviews.apache.org/r/68993/diff/1/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/68994/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 68994: Logged request processing time for some endpoints.

[Till Toenshoff via Review Board]

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


Ship it!




Ship It!

- Till Toenshoff


On Oct. 11, 2018, 2:16 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68994/
> ---
> 
> (Updated Oct. 11, 2018, 2:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Benno Evers, Benjamin Mahler, 
> Gastón Kleiman, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch leverages `logResponse()` function to print response status
> code together with the request processing time for some endpoints on
> the master and agent. Not all endpoints are participating to avoid
> unnecessary log pollution; only these that are know to be "slow" in
> generating the response.
> 
> Note that requests are still logged when they are fetched from the
> actor mailbox, i.e., affected endpoints now log twice per request:
> when the processing starts and when the response is ready to be sent.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 06d769aeba16586a020729d454f4d00688b78c78 
>   src/slave/slave.cpp 9d84dcb52e5b7e507fc375f184c5f77be08d70a2 
> 
> 
> Diff: https://reviews.apache.org/r/68994/diff/1/
> 
> 
> Testing
> ---
> 
> `make check` on Mac OS 10.13.6 and various Linux distros.
> 
> Manual testing
> ==
> Started a mesos-master process locally: `./bin/mesos-master.sh --work_dir=m`
> 
> 1. `/state` request: `http http://192.168.1.3:5050/state`
> Relevant snippet from the master log:
> ```
> I1011 16:07:05.978631 10211328 http.cpp:1178] HTTP GET for /master/state from 
> 192.168.1.3:56510 with User-Agent='HTTPie/0.9.3'
> I1011 16:07:05.981534 13430784 http.cpp:1195] HTTP GET for /master/state from 
> 192.168.1.3:56510: '200 OK' after 3.260928ms
> ```
> 
> 2. `/flags` request: `http http://192.168.1.3:5050/flags`
> Relevant snippet from the master log:
> ```
> I1011 16:08:51.012370 63365120 http.cpp:1178] HTTP GET for /master/flags from 
> 192.168.1.3:56629 with User-Agent='HTTPie/0.9.3'
> ```
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 68994: Logged request processing time for some endpoints.

[Till Toenshoff via Review Board]

> On Oct. 12, 2018, 10:32 p.m., Benno Evers wrote:
> > As we already discussed offline, it's a bit unfortunate to have some code 
> > duplication here and only have the statistics enabled for selected 
> > endpoints. (for those unaware, this is done in order to avoid spamming the 
> > logs with request statistics for every requested static html page, 
> > javascript file, css template, etc.)
> > 
> > However, with the choices being between committing this, delaying the patch 
> > series to implement an alternative approach, or dropping the idea entirely, 
> > I still think the trade-offs favor committing this since it will help to 
> > solve some real issues for Mesos users, and doesn't add a big maintenance 
> > burden for the future. So I'm giving it a 'Ship It' as is.

Excellent reviewer comment!


- Till


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


On Oct. 11, 2018, 2:16 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68994/
> ---
> 
> (Updated Oct. 11, 2018, 2:16 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Benno Evers, Benjamin Mahler, 
> Gastón Kleiman, and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch leverages `logResponse()` function to print response status
> code together with the request processing time for some endpoints on
> the master and agent. Not all endpoints are participating to avoid
> unnecessary log pollution; only these that are know to be "slow" in
> generating the response.
> 
> Note that requests are still logged when they are fetched from the
> actor mailbox, i.e., affected endpoints now log twice per request:
> when the processing starts and when the response is ready to be sent.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 06d769aeba16586a020729d454f4d00688b78c78 
>   src/slave/slave.cpp 9d84dcb52e5b7e507fc375f184c5f77be08d70a2 
> 
> 
> Diff: https://reviews.apache.org/r/68994/diff/1/
> 
> 
> Testing
> ---
> 
> `make check` on Mac OS 10.13.6 and various Linux distros.
> 
> Manual testing
> ==
> Started a mesos-master process locally: `./bin/mesos-master.sh --work_dir=m`
> 
> 1. `/state` request: `http http://192.168.1.3:5050/state`
> Relevant snippet from the master log:
> ```
> I1011 16:07:05.978631 10211328 http.cpp:1178] HTTP GET for /master/state from 
> 192.168.1.3:56510 with User-Agent='HTTPie/0.9.3'
> I1011 16:07:05.981534 13430784 http.cpp:1195] HTTP GET for /master/state from 
> 192.168.1.3:56510: '200 OK' after 3.260928ms
> ```
> 
> 2. `/flags` request: `http http://192.168.1.3:5050/flags`
> Relevant snippet from the master log:
> ```
> I1011 16:08:51.012370 63365120 http.cpp:1178] HTTP GET for /master/flags from 
> 192.168.1.3:56629 with User-Agent='HTTPie/0.9.3'
> ```
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 68971: Moved import of '../lib' from new CLI bootstrap to pip-requirements.txt.

[Armand Grillet]

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

(Updated Oct. 16, 2018, 1:33 p.m.)


Review request for mesos and Kevin Klues.


Changes
---

Rebased.


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


Repository: mesos


Description (updated)
---

This adds the Mesos Python package to the requirements needed by the
new CLI. We were previously adding the path to the package when setting
up the CLI virtual environment but this does not work with Pylint when
it is run from the linters virtual environment.

By setting things in `pip-requirements.txt`, Pylint does not report
inexisting linting errors when doing imports such as `import mesos`.


Diffs (updated)
-

  src/python/cli_new/bootstrap a6183d4c28281bf6d29c8b7f825ae474056f027b 
  src/python/cli_new/pip-requirements.txt 
d1822bf752ce76aa5da5999057fe1efb83747fd0 


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

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


Testing
---

Updated the agent plugin to do `from mesos.exceptions import MesosException` 
and use `MesosException`. Commited to see the git hook running and saw the 
error `E0401: Unable to import 'mesos.exceptions' (import-error).`. Updated 
`support/pylint.config`, run the git hook again and saw that the error was gone.


Thanks,

Armand Grillet

Review Request 69047: Updated Python library to be easier to handle as a Python module.

[Armand Grillet]

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

Review request for mesos and Kevin Klues.


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


Repository: mesos


Description
---

Updated Python library to be easier to handle as a Python module.


Diffs
-

  src/python/lib/mesos/__init__.py 40219c5e03915754b61fc5d7263a063f0d18cca1 
  src/python/lib/setup.py 08f854f43681d1f694bb48604773256be7ce927b 


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


Testing
---


Thanks,

Armand Grillet

Review Request 69042: Enabled more constructors for master `RegistryOperation`.

[Benjamin Bannier]

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

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


Repository: mesos


Description
---

Since this class was explicitly declaring a constructor, no other
constructors such as copy or move constructors were generated.

This patch removes the custom constructor so all default constructors
are generated automatically. This e.g., enables move construction of
`RegistryOperation` values.


Diffs
-

  src/master/registrar.hpp 4f7c7ce2b3dea9f19eae67494f9767dc1c61e121 


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


Testing
---

`make check`

Without this patch a clang close to trunk would warn
```
../src/resource_provider/registrar.cpp:387:5: warning: explicitly defaulted 
move constructor is implicitly deleted [-Wdefaulted-function-deleted]
AdaptedOperation(AdaptedOperation&&) = default;
^
../src/resource_provider/registrar.cpp:374:28: note: move constructor of 
'AdaptedOperation' is implicitly deleted because base class 
'master::RegistryOperation' has a deleted move constructor
  class AdaptedOperation : public master::RegistryOperation
   ^
../src/master/registrar.hpp:45:27: note: copy constructor of 
'RegistryOperation' is implicitly deleted because base class 
'process::Promise' has an inaccessible copy constructor
class RegistryOperation : public process::Promise
  ^
../src/resource_provider/registrar.cpp:389:23: warning: explicitly defaulted 
move assignment operator is implicitly deleted [-Wdefaulted-function-deleted]
AdaptedOperation& operator=(AdaptedOperation&&) = default;
  ^
../src/resource_provider/registrar.cpp:374:28: note: move assignment operator 
of 'AdaptedOperation' is implicitly deleted because base class 
'master::RegistryOperation' has a deleted move assignment operator
  class AdaptedOperation : public master::RegistryOperation
   ^
../src/master/registrar.hpp:45:27: note: copy assignment operator of 
'RegistryOperation' is implicitly deleted because base class 
'process::Promise' has an inaccessible copy assignment operator
class RegistryOperation : public process::Promise
  ^
2 warnings generated.
```

No warning is emitted with this patch.


Thanks,

Benjamin Bannier

Review Request 69041: Relaxed `Promise` constructor and assignment operator requirements.

[Benjamin Bannier]

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

Review request for mesos, Benjamin Mahler and Meng Zhu.


Repository: mesos


Description
---

This patch enables construction and assignment from rvalues for
`Promise` values.


Diffs
-

  3rdparty/libprocess/include/process/future.hpp 
ba2120190ad6e6814ab268f3e00911904e739e8e 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 63654: Added a `net::socketpair` helper to stout.

[Benjamin Bannier]

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


Fix it, then Ship it!





3rdparty/stout/include/stout/os/posix/socket.hpp
Lines 97 (patched)


Fits on a single line.



3rdparty/stout/include/stout/os/posix/socket.hpp
Lines 142-146 (patched)


Indent by two spaces less,
```
if (::setsockopt(
  result[0],
  ...
```



3rdparty/stout/include/stout/os/posix/socket.hpp
Lines 146 (patched)


Nit: I believe `sizeof(enable)` would be marginally clearer.



3rdparty/stout/include/stout/os/posix/socket.hpp
Lines 152-156 (patched)


Ditto.



3rdparty/stout/include/stout/os/posix/socket.hpp
Lines 156 (patched)


Ditto.


- Benjamin Bannier


On Oct. 1, 2018, 7:36 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63654/
> ---
> 
> (Updated Oct. 1, 2018, 7:36 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8156
> https://issues.apache.org/jira/browse/MESOS-8156
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a `net::socketpair` helper to stout that deals with
> automatically setting O_CLOEXEC and disabling SIGPIPE where
> necessary.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/posix/socket.hpp 
> 5557394e9ce8e4addc07d328d55274a9d573dfcd 
>   3rdparty/stout/tests/os/sendfile_tests.cpp 
> e334f489756e6f3352e7b1e84aabb9d40b734138 
> 
> 
> Diff: https://reviews.apache.org/r/63654/diff/4/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26, macOS)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 63655: Switched to `net::socketpair` in `ns::clone`.

[Benjamin Bannier]

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


Fix it, then Ship it!





src/linux/ns.cpp
Lines 274 (patched)


Use `template <` instead.



src/linux/ns.cpp
Lines 277 (patched)


Remove two spaces.



src/linux/ns.cpp
Line 356 (original), 368 (patched)


Fits on a single line.


- Benjamin Bannier


On Oct. 1, 2018, 7:37 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63655/
> ---
> 
> (Updated Oct. 1, 2018, 7:37 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8156
> https://issues.apache.org/jira/browse/MESOS-8156
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Started using to `net::socketpair` in `ns::clone` so that we don't
> have to deal with `SOCK_CLOEXEC` portability.
> 
> 
> Diffs
> -
> 
>   src/linux/ns.cpp ffa9b65b7caf891a5bee1e20be128c37487335e4 
> 
> 
> Diff: https://reviews.apache.org/r/63655/diff/3/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26, macOS)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 63655: Switched to `net::socketpair` in `ns::clone`.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Oct. 1, 2018, 10:37 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63655/
> ---
> 
> (Updated Oct. 1, 2018, 10:37 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8156
> https://issues.apache.org/jira/browse/MESOS-8156
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Started using to `net::socketpair` in `ns::clone` so that we don't
> have to deal with `SOCK_CLOEXEC` portability.
> 
> 
> Diffs
> -
> 
>   src/linux/ns.cpp ffa9b65b7caf891a5bee1e20be128c37487335e4 
> 
> 
> Diff: https://reviews.apache.org/r/63655/diff/3/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26, macOS)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 63654: Added a `net::socketpair` helper to stout.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Oct. 1, 2018, 10:36 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63654/
> ---
> 
> (Updated Oct. 1, 2018, 10:36 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8156
> https://issues.apache.org/jira/browse/MESOS-8156
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a `net::socketpair` helper to stout that deals with
> automatically setting O_CLOEXEC and disabling SIGPIPE where
> necessary.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/posix/socket.hpp 
> 5557394e9ce8e4addc07d328d55274a9d573dfcd 
>   3rdparty/stout/tests/os/sendfile_tests.cpp 
> e334f489756e6f3352e7b1e84aabb9d40b734138 
> 
> 
> Diff: https://reviews.apache.org/r/63654/diff/4/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26, macOS)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 69037: WIP: Added an optional `vendor` field to `DiskInfo.Source`.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 69037 was successfully built and tested.

Reviews applied: `['69035', '69036', '69037']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2470/mesos-review-69037

- Mesos Reviewbot Windows


On Oct. 16, 2018, 4:41 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69037/
> ---
> 
> (Updated Oct. 16, 2018, 4:41 a.m.)
> 
> 
> Review request for Benjamin Bannier, James DeFelice and Jie Yu.
> 
> 
> Bugs: MESOS-9321
> https://issues.apache.org/jira/browse/MESOS-9321
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added an optional `vendor` field to `DiskInfo.Source`.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5a985fca39cdfb7e9b4775650a7e5dbe68c3b8ae 
>   include/mesos/v1/mesos.proto a5ebb786a98bd3fd34745ddc553aa7a751c0e337 
> 
> 
> Diff: https://reviews.apache.org/r/69037/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 69010: Synced SLRP checkpoints to the filesystem.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 69010 was successfully built and tested.

Reviews applied: `['69009', '69010']`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2471/mesos-review-69010

- Mesos Reviewbot Windows


On Oct. 16, 2018, 4:46 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69010/
> ---
> 
> (Updated Oct. 16, 2018, 4:46 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-9281
> https://issues.apache.org/jira/browse/MESOS-9281
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently if a system crashes, SLRP checkpoints might not be synced to
> the filesystem, so it is possible that an old or empty checkpoint will
> be read upon recovery. Moreover, if a CSI call has been issued right
> before the crash, the recovered state may be inconsistent with the
> actual state reported by the plugin. For example, the plugin might have
> created a volume but the checkpointed state does not know about it.
> 
> To avoid this inconsistency, we always call fsync()  when checkpointing
> SLRP states.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/provider.cpp 
> db783b53558811081fb2671e005e8bbbd9edbede 
>   src/slave/state.hpp 003211e4670c1092acb1634220d76bafd39e3a20 
> 
> 
> Diff: https://reviews.apache.org/r/69010/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 68706: Added master failover reregistration progress metrics.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['68706']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2469/mesos-review-68706

Relevant logs:

- 
[mesos-tests.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2469/mesos-review-68706/logs/mesos-tests.log):

```
I1016 06:34:30.897042  8800 executor.cpp:805] Shutting down
I1016 06:34:30.897042  8800 executor.cpp:918] Sending SIGTERM to process tree 
at pid 400xecutor(1)@192.10.1.5:62750
I1016 06:34:30.896052  1308 slave.cpp:909] Agent terminating
I1016 06:34:30.896052  8440 master.cpp:11082] Removing task 
dd62b7a3-f017-44fb-a6fc-b973be640983 with resources cpus(allocated: *):4; 
mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
*):[31000-32000] of framework 9f48f4c4-e51d-4f12-b338-ca5873a9eed6- on 
agent 9f48f4c4-e51d-4f12-b338-ca5873a9eed6-S0 at slave(461)@192.10.1.5:60955 
(windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net)
W1016 06:34:30.896052  1308 slave.cpp:3917] Ignoring shutdown framework 
9f48f4c4-e51d-4f12-b338-ca5873a9eed6- because it is terminating
I1016 06:34:30.899040  8440 master.cpp:1251] Agent 
9f48f4c4-e51d-4f12-b338-ca5873a9eed6-S0 at slave(461)@192.10.1.5:60955 
(windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net) disconnected
I1016 06:34:30.899040  8440 master.cpp:3317] Disconnecting agent 
9f48f4c4-e51d-4f12-b338-ca5873a9eed6-S0 at slave(461)@192.10.1.5:60955 
(windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net)
I1016 06:34:30.899040  8440 master.cpp:3336] Deactivating agent 
9f48f4c4-e51d-4f12-b338-ca5873a9eed6-S0 at slave(461)@192.10.1.5:60955 
(windows-02.aa0q4n2kgcyefckmv0xukjvy4f.xx.internal.cloudapp.net)
I1016 06:34:30.900039  5924 hierarchical.cpp:359] Removed framework 
9f48f4c4-e51d-4f12-b338-ca5873a9eed6-
I1016 06:34:30.900039  5924 hierarchical.cpp:803] Agent 
9f48f4c4-e51d-4f12-b338-ca5873a9eed6-S0 deactivated
I1016 06:34:30.900039  8604 containerizer.cpp:2455] Destroying container 
1443f5fd-7561-4184-a35d-e5eb03ace98a in RUNNING state
I1016 06:34:30.901036  8604 containerizer.cpp:3122] Transitioning the state of 
container 1443f5fd-7561-4184-a35d-e5eb03ace98a from RUNNING to DESTROYING
I1016 06:34:30.901036  8604 launcher.cpp:166] Asked to destroy container 
1443f5fd-7561-4184-a35d-e5eb03ace98a
I1016 06:34:30.935099  8440 containerizer.cpp:2961] Container 
1443f5fd-7561-4184-a35d-e5eb03ace98a has exited
I1016 06:34:30.967039 11088 master.cpp:1093] Master terminating
I1016 06:34:30.9690[   OK ] 
IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (584 ms)
[--] 1 test from IsolationFlag/MemoryIsolatorTest (601 ms total)

[--] Global test environment tear-down
[==] 1051 tests from 103 test cases ran. (488984 ms total)
[  PASSED  ] 1050 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] DockerFetcherPluginTest.INTERNET_CURL_FetchBlob

 1 FAILED TEST
  YOU HAVE 232 DISABLED TESTS

55  7268 hierarchical.cpp:645] Removed agent 
9f48f4c4-e51d-4f12-b338-ca5873a9eed6-S0
I1016 06:34:31.236047  6856 process.cpp:926] Stopped the socket accept loop
```

- Mesos Reviewbot Windows


On Oct. 16, 2018, 9:49 a.m., Xudong Ni wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68706/
> ---
> 
> (Updated Oct. 16, 2018, 9:49 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, James Peach, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-9178
> https://issues.apache.org/jira/browse/MESOS-9178
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> During the master failover, the time that the master elected is
> considered as the start of failover. In the progress of
> reregistration, the percentile represents the time when such
> percentile of agents finished registration again; The percentile of
> these data as in this metrics can represent overall reregistration
> progress; In case of degradation towards to the end of reregistration,
> the high percentile can reflect it; In the case there are unreachable
> agents in the failover, if certain percentile recovery couldn't be
> reached, the intiail value of that percentile will not be updated.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md 00c6ea94bcb73746aef740236632ede123f5b534 
>   src/master/master.hpp ea7e9242b62fe6c2cc0e717f9a9f2f0c1cc0a390 
>   src/master/master.cpp 06d769aeba16586a020729d454f4d00688b78c78 
>   src/master/metrics.hpp e1da18e6ba2737f729e1e30653020538150ae898 
>   src/master/metrics.cpp