Re: Review Request 69146: Made fetcher tests more robust by using the test sandbox.

[Benjamin Bannier]

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




src/tests/fetcher_tests.cpp
Lines 98-109 (patched)


We can avoid duplication here by making `FetcherTest` a `MesosTest` (that 
one would already today take care of `launcher_dir` and `fetcher_cache_dir`). 
This function would than at most need to invoke `MesosTest::CreateSlaveFlags` 
and make its non-general additions.

It probably even make sense to set `frameworks_home` in `CreateSlaveFlags` 
as well. That would remove the need for this function completely.


- Benjamin Bannier


On Oct. 25, 2018, 1:12 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69146/
> ---
> 
> (Updated Oct. 25, 2018, 1:12 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gilbert Song, 
> and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fetcher tests currently rely on some hard-coded paths,
> for example, `/tmp/mesos/fetcher` and `/tmp/frameworks`.
> Thus fetcher tests could fail if these directories already
> exit. This patch adds a helper function `createSlaveFlags()`
> to make use of the test sandboxes. The above paths will be
> replaced by `fetch/` and `frameworks/` under the sandbox.
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_tests.cpp 283238cdda17a94e034baa195bd9d4b57e363b8a 
> 
> 
> Diff: https://reviews.apache.org/r/69146/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 69146: Made fetcher tests more robust by using the test sandbox.

[Chun-Hung Hsiao]

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




src/tests/fetcher_tests.cpp
Line 120 (original), 132 (patched)


How about doing this:
```
Fetcher fetcher(CreateSlaveFlags());
```
Then we can get rid of the `flags` local variable?
I'm not opening an issue since this is minor. 

Ditto below.


- Chun-Hung Hsiao


On Oct. 24, 2018, 11:12 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69146/
> ---
> 
> (Updated Oct. 24, 2018, 11:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gilbert Song, 
> and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fetcher tests currently rely on some hard-coded paths,
> for example, `/tmp/mesos/fetcher` and `/tmp/frameworks`.
> Thus fetcher tests could fail if these directories already
> exit. This patch adds a helper function `createSlaveFlags()`
> to make use of the test sandboxes. The above paths will be
> replaced by `fetch/` and `frameworks/` under the sandbox.
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_tests.cpp 283238cdda17a94e034baa195bd9d4b57e363b8a 
> 
> 
> Diff: https://reviews.apache.org/r/69146/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 69146: Made fetcher tests more robust by using the test sandbox.

[Chun-Hung Hsiao]

> On Oct. 24, 2018, 11:31 p.m., Till Toenshoff wrote:
> > src/tests/fetcher_tests.cpp
> > Lines 105-106 (patched)
> > 
> >
> > While this solution appears to be the most efficient it also is not 
> > entirely consistent with how we do it in other places; 
> > Note that `getcwd()` returns the test-sandbox directory after the 
> > fixture did its job.
> > ```
> > path::join(os::getcwd(), "fetcher");
> > path::join(os::getcwd(), "frameworks");
> > ```
> > would be consistent.
> > 
> > Alternatively, we could at least do away with one `CHECK_NOTNONE` and 
> > pull it in front of these two uses of `sandbox`.
> 
> Joseph Wu wrote:
> A good chunk of tests have the following pattern:
> ```
> path::join(sandbox.get(), ...);
> ```
> 
> I've been leaning towards this because it forgoes the syscall.
> 
> Till Toenshoff wrote:
> Definitely more efficient, yes - wish we could "fix" that in one swoop - 
> but am not attached to the first part of my comment due to these reasons.
> 
> Meng Zhu wrote:
> For the second part, I think using two `CHECK_NOTNONE` is more readable 
> to me. Otherwise, we will end up with two unguarded `get()` (I am a little 
> bit allergic to unguarded `get()` :) )
> 
> Till Toenshoff wrote:
> How about 
> ```
> CHECK_NOTNONE(sandbox);
> flags.fetcher_cache_dir = path::join(sandbox.get(), "fetcher");
> flags.frameworks_home = path::join(sandbox.get(), "frameworks");
> ```
> 
> Meng Zhu wrote:
> I am not sure why it is preferred over the current code. I view 
> `CHECK_NOTNONE` more as a safe "get()" operation rather than a pure check.
> 
> If we want to save one check, we can do
> 
> ```
> ASSERT_SOME(sandbox); 
> flags.fetcher_cache_dir = path::join(sandbox.get(), "fetcher");
> flags.frameworks_home = path::join(sandbox.get(), "frameworks");
> 
> ```
> 
> However, the reason I prefer inline `CHECK_NOTNONE` is that it removes 
> order dependencies between statements. Otherwise when we do `get()`, we need 
> to carry the mental burden that it has to be done after the `ASSERT`.
>  
> I feel inline `CHECK_NOTNONE` is both easy to maintain (no order 
> dependency) and read (clearly express the intent of "can't be none" inline).
> 
> I personally prefer to never use naked `get()` given `CHECK_NOTNONE`.

Note that `Option::get` already has an assertion. The purpose of 
`CHECK_NOTNONE` to me is just to keep the line number so the check failure can 
be more easily located.

In general I'd prefer avoiding the `CHECK*` macros in tests as it would cause 
the test program to crash, thus no test cleanup would be performed. So an 
`ASSERT_SOME` is "safer" in this sense. That said, this check do seems a bit 
unnecessary because it's (practically) guaranteed that it is not none. Also you 
cannot use `ASSERT_*` in a non-void function. You have to do things like this: 
https://github.com/apache/mesos/blob/master/src/tests/storage_local_resource_provider_tests.cpp#L299-L301
And this limitation makes me incline to not do the assertion a bit.


- Chun-Hung


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


On Oct. 24, 2018, 11:12 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69146/
> ---
> 
> (Updated Oct. 24, 2018, 11:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gilbert Song, 
> and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fetcher tests currently rely on some hard-coded paths,
> for example, `/tmp/mesos/fetcher` and `/tmp/frameworks`.
> Thus fetcher tests could fail if these directories already
> exit. This patch adds a helper function `createSlaveFlags()`
> to make use of the test sandboxes. The above paths will be
> replaced by `fetch/` and `frameworks/` under the sandbox.
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_tests.cpp 283238cdda17a94e034baa195bd9d4b57e363b8a 
> 
> 
> Diff: https://reviews.apache.org/r/69146/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 69096: Moved a few allocator test helpers to `tests/allocator.hpp`.

[Meng Zhu]

> On Oct. 24, 2018, 5:22 p.m., Benjamin Mahler wrote:
> > src/tests/allocator.hpp
> > Lines 40-45 (patched)
> > 
> >
> > I think we some other create helpers lying around, e.g. createTask. Is 
> > this where these belong?
> > 
> > Do you think we need them?

I do not have use cases of other helpers in mind atm. Thus I prefer to pull in 
only these two.


- Meng


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


On Oct. 19, 2018, 6:28 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69096/
> ---
> 
> (Updated Oct. 19, 2018, 6:28 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Gastón Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This helps to share the helpers between
> `hierarchical_allocator_tests.cpp` and
> `hierarchical_allocator_benchmarks.cpp`.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 5795c7097a9ed1f659e169ad81a9f2c09481aa81 
>   src/tests/CMakeLists.txt 00dbdee1c06e7571fa799850cdaf7f2ccadc8bea 
>   src/tests/allocator.hpp 15a5396986ea6939f25b8d23b87a91c27338fc7b 
>   src/tests/allocator.cpp PRE-CREATION 
>   src/tests/hierarchical_allocator_tests.cpp 
> 27fbd9cf0c4442e7675362a806d35bad141ffb6d 
> 
> 
> Diff: https://reviews.apache.org/r/69096/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 69093: Removed `used` argument in `AgentProfile` in the allocator benchmark.

[Meng Zhu]

> On Oct. 24, 2018, 5:20 p.m., Benjamin Mahler wrote:
> > src/tests/hierarchical_allocator_benchmarks.cpp
> > Line 234 (original), 234 (patched)
> > 
> >
> > Hm.. how were you able to change this while it remained correct? Was 
> > this just an empty map always?

Yeah, it is currently not used. (So far there is only one benchmark in the 
suite and it does not utilize it). During the fixture code review, I 
recommended Kapil to add the option to specify used resources because some of 
the existing benchmarks in `hierarchical_allocator_test.cpp ` setup agents with 
pre-allocated resources.

Now I realize that we can't easily do this during our cluster setup. Because 
when we specify the `agentProfile`, no framework has been created yet. I am 
actually not quite sure why we need to specify `used`. In the future if 
necessary, we can always create a bunch of frameworks with their own quota 
roles to simulate this right after the cluster initialization.


- Meng


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


On Oct. 19, 2018, 6:27 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69093/
> ---
> 
> (Updated Oct. 19, 2018, 6:27 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Gastón Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently in the `HierarchicalAllocations_BENCHMARK_TestBase`,
> it is not easy to add agents with used resources because
> frameworks are created during the initialization which is
> after the agent profiles creation.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_benchmarks.cpp 
> bf9167b63747f7b8a402d950947028436307082a 
> 
> 
> Diff: https://reviews.apache.org/r/69093/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 69110: Added task health check definitions to master API responses.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 69110 was successfully built and tested.

Reviews applied: `['69110']`

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

- Mesos Reviewbot Windows


On Oct. 24, 2018, 5:51 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69110/
> ---
> 
> (Updated Oct. 24, 2018, 5:51 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Gastón Kleiman, and Vinod Kone.
> 
> 
> Bugs: MESOS-8780
> https://issues.apache.org/jira/browse/MESOS-8780
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Task protobuf message is updated to include the health check
> definition of a task when it is specified. Associated helpers are
> also updated along with a test which verifies that this field is
> reflected in master API responses.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 72966da75bc90d4b5d891e209e786e5326155d17 
>   include/mesos/v1/mesos.proto c6c1dae5494d9270a7a43a2d838769574ae47872 
>   src/common/http.cpp 80848aafe27eb0c35222ad236e671a32c4ab10aa 
>   src/common/protobuf_utils.cpp 77139d8a3931dc1e9e00fcea83d77d96244a34f3 
>   src/tests/api_tests.cpp c681b9b0b83f7472312083730a5433e3d6f0efc0 
>   src/tests/health_check_tests.cpp c972b9003196b4b4272d7c34311c84c117b1ae3d 
> 
> 
> Diff: https://reviews.apache.org/r/69110/diff/4/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 69146: Made fetcher tests more robust by using the test sandbox.

[Meng Zhu]

> On Oct. 24, 2018, 4:31 p.m., Till Toenshoff wrote:
> > src/tests/fetcher_tests.cpp
> > Lines 105-106 (patched)
> > 
> >
> > While this solution appears to be the most efficient it also is not 
> > entirely consistent with how we do it in other places; 
> > Note that `getcwd()` returns the test-sandbox directory after the 
> > fixture did its job.
> > ```
> > path::join(os::getcwd(), "fetcher");
> > path::join(os::getcwd(), "frameworks");
> > ```
> > would be consistent.
> > 
> > Alternatively, we could at least do away with one `CHECK_NOTNONE` and 
> > pull it in front of these two uses of `sandbox`.
> 
> Joseph Wu wrote:
> A good chunk of tests have the following pattern:
> ```
> path::join(sandbox.get(), ...);
> ```
> 
> I've been leaning towards this because it forgoes the syscall.
> 
> Till Toenshoff wrote:
> Definitely more efficient, yes - wish we could "fix" that in one swoop - 
> but am not attached to the first part of my comment due to these reasons.
> 
> Meng Zhu wrote:
> For the second part, I think using two `CHECK_NOTNONE` is more readable 
> to me. Otherwise, we will end up with two unguarded `get()` (I am a little 
> bit allergic to unguarded `get()` :) )
> 
> Till Toenshoff wrote:
> How about 
> ```
> CHECK_NOTNONE(sandbox);
> flags.fetcher_cache_dir = path::join(sandbox.get(), "fetcher");
> flags.frameworks_home = path::join(sandbox.get(), "frameworks");
> ```

I am not sure why it is preferred over the current code. I view `CHECK_NOTNONE` 
more as a safe "get()" operation rather than a pure check.

If we want to save one check, we can do

```
ASSERT_SOME(sandbox); 
flags.fetcher_cache_dir = path::join(sandbox.get(), "fetcher");
flags.frameworks_home = path::join(sandbox.get(), "frameworks");

```

However, the reason I prefer inline `CHECK_NOTNONE` is that it removes order 
dependencies between statements. Otherwise when we do `get()`, we need to carry 
the mental burden that it has to be done after the `ASSERT`.
 
I feel inline `CHECK_NOTNONE` is both easy to maintain (no order dependency) 
and read (clearly express the intent of "can't be none" inline).

I personally prefer to never use naked `get()` given `CHECK_NOTNONE`.


- Meng


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


On Oct. 24, 2018, 4:12 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69146/
> ---
> 
> (Updated Oct. 24, 2018, 4:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gilbert Song, 
> and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fetcher tests currently rely on some hard-coded paths,
> for example, `/tmp/mesos/fetcher` and `/tmp/frameworks`.
> Thus fetcher tests could fail if these directories already
> exit. This patch adds a helper function `createSlaveFlags()`
> to make use of the test sandboxes. The above paths will be
> replaced by `fetch/` and `frameworks/` under the sandbox.
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_tests.cpp 283238cdda17a94e034baa195bd9d4b57e363b8a 
> 
> 
> Diff: https://reviews.apache.org/r/69146/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 69149: Automatically remounted read-only bind mounts.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 69149 was successfully built and tested.

Reviews applied: `['69149']`

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

- Mesos Reviewbot Windows


On Oct. 24, 2018, 11:49 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69149/
> ---
> 
> (Updated Oct. 24, 2018, 11:49 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Ilya Pronin, Jie Yu, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-9354
> https://issues.apache.org/jira/browse/MESOS-9354
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To make a bind mount read-only, you have to first make the bind mount,
> then remount it with the read-only flag. This is a bit arcane, which is
> why `mount(8)` does it automatically.
> 
> This change updates `fs::mount()` to do the read-only remount
> automatically when it is making a read-only bind mount so that every
> caller doesn't have to carry special code to make it work correctly. All
> the callers that make an explicit remount are updated to simply pass
> the `MS_READONLY` flag if necessary.
> 
> 
> Diffs
> -
> 
>   src/examples/test_csi_plugin.cpp 7fa325e94304aec8927e345ebdac380105f624a8 
>   src/linux/fs.hpp 502f85c4a32d8658bdd701975dd5ac3d802d308e 
>   src/linux/fs.cpp 9055ef42edd1fb90e1026d1d603a9ba902cfc1fd 
>   src/slave/containerizer/docker.cpp 192dc29576a99fdc671bb842c01f50cd30dc20e1 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> 24c9fd6beb9657b80b33dc31c2939083c1aa9110 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> a47899cb528eef103f299def3bd3466905ac5b51 
>   src/slave/containerizer/mesos/isolators/gpu/isolator.cpp 
> dbbf92ffbe4a46cedca5b53f6ba172bfb308100e 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> 64271dfbb5c074ad3ac8d2a64c3943d739c0fffa 
>   src/slave/containerizer/mesos/isolators/volume/host_path.cpp 
> 2e03ef50a290c046ae2b02b332d3d007b572429d 
>   src/slave/containerizer/mesos/isolators/volume/image.cpp 
> 53cbaefeef7a6e10149e241e07d6e9cb8d510fc9 
>   src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp 
> 21d9528c23d9142eccec456184b42d085b57d12c 
>   src/slave/containerizer/mesos/provisioner/backends/bind.cpp 
> 7d564dc7de3e3de8726c7fd42a301d979c4a2574 
>   src/tests/containerizer/fs_tests.cpp 
> 23cad35b5db81a70b43bec1c1dbafe008c8dd4da 
> 
> 
> Diff: https://reviews.apache.org/r/69149/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 28)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 68957: Add documentation for per-framework metrics flag.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 68957 was successfully built and tested.

Reviews applied: `['68953', '68956', '68957']`

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

- Mesos Reviewbot Windows


On Oct. 24, 2018, 11:46 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68957/
> ---
> 
> (Updated Oct. 24, 2018, 11:46 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.
> 
> 
> Bugs: MESOS-9301
> https://issues.apache.org/jira/browse/MESOS-9301
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add documentation for per-framework metrics flag.
> 
> 
> Diffs
> -
> 
>   docs/configuration/master.md f290e377be9d7424899c82e099be98fc88688de1 
> 
> 
> Diff: https://reviews.apache.org/r/68957/diff/13/
> 
> 
> Testing
> ---
> 
> make check on OSX
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 69110: Added task health check definitions to master API responses.

[Greg Mann]

> On Oct. 23, 2018, 8:56 a.m., Alexander Rukletsov wrote:
> > src/tests/health_check_tests.cpp
> > Lines 460-466 (patched)
> > 
> >
> > What do you think about moving this blob to `AgentAPITest::GetState`? 
> > It will be consistent with the `TASK_ADDED` test.

I went ahead and did this thing.


- Greg


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


On Oct. 25, 2018, 12:51 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69110/
> ---
> 
> (Updated Oct. 25, 2018, 12:51 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Gastón Kleiman, and Vinod Kone.
> 
> 
> Bugs: MESOS-8780
> https://issues.apache.org/jira/browse/MESOS-8780
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Task protobuf message is updated to include the health check
> definition of a task when it is specified. Associated helpers are
> also updated along with a test which verifies that this field is
> reflected in master API responses.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 72966da75bc90d4b5d891e209e786e5326155d17 
>   include/mesos/v1/mesos.proto c6c1dae5494d9270a7a43a2d838769574ae47872 
>   src/common/http.cpp 80848aafe27eb0c35222ad236e671a32c4ab10aa 
>   src/common/protobuf_utils.cpp 77139d8a3931dc1e9e00fcea83d77d96244a34f3 
>   src/tests/api_tests.cpp c681b9b0b83f7472312083730a5433e3d6f0efc0 
>   src/tests/health_check_tests.cpp c972b9003196b4b4272d7c34311c84c117b1ae3d 
> 
> 
> Diff: https://reviews.apache.org/r/69110/diff/4/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 69110: Added task health check definitions to master API responses.

[Greg Mann]

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

(Updated Oct. 25, 2018, 12:51 a.m.)


Review request for mesos, Alexander Rukletsov, Gastón Kleiman, and Vinod Kone.


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


Repository: mesos


Description
---

The Task protobuf message is updated to include the health check
definition of a task when it is specified. Associated helpers are
also updated along with a test which verifies that this field is
reflected in master API responses.


Diffs (updated)
-

  include/mesos/mesos.proto 72966da75bc90d4b5d891e209e786e5326155d17 
  include/mesos/v1/mesos.proto c6c1dae5494d9270a7a43a2d838769574ae47872 
  src/common/http.cpp 80848aafe27eb0c35222ad236e671a32c4ab10aa 
  src/common/protobuf_utils.cpp 77139d8a3931dc1e9e00fcea83d77d96244a34f3 
  src/tests/api_tests.cpp c681b9b0b83f7472312083730a5433e3d6f0efc0 
  src/tests/health_check_tests.cpp c972b9003196b4b4272d7c34311c84c117b1ae3d 


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

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


Testing
---

`make check`


Thanks,

Greg Mann

Re: Review Request 69096: Moved a few allocator test helpers to `tests/allocator.hpp`.

[Benjamin Mahler]

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


Fix it, then Ship it!





src/tests/allocator.hpp
Lines 40-45 (patched)


I think we some other create helpers lying around, e.g. createTask. Is this 
where these belong?

Do you think we need them?


- Benjamin Mahler


On Oct. 20, 2018, 1:28 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69096/
> ---
> 
> (Updated Oct. 20, 2018, 1:28 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Gastón Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This helps to share the helpers between
> `hierarchical_allocator_tests.cpp` and
> `hierarchical_allocator_benchmarks.cpp`.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 5795c7097a9ed1f659e169ad81a9f2c09481aa81 
>   src/tests/CMakeLists.txt 00dbdee1c06e7571fa799850cdaf7f2ccadc8bea 
>   src/tests/allocator.hpp 15a5396986ea6939f25b8d23b87a91c27338fc7b 
>   src/tests/allocator.cpp PRE-CREATION 
>   src/tests/hierarchical_allocator_tests.cpp 
> 27fbd9cf0c4442e7675362a806d35bad141ffb6d 
> 
> 
> Diff: https://reviews.apache.org/r/69096/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 69093: Removed `used` argument in `AgentProfile` in the allocator benchmark.

[Benjamin Mahler]

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




src/tests/hierarchical_allocator_benchmarks.cpp
Line 234 (original), 234 (patched)


Hm.. how were you able to change this while it remained correct? Was this 
just an empty map always?


- Benjamin Mahler


On Oct. 20, 2018, 1:27 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69093/
> ---
> 
> (Updated Oct. 20, 2018, 1:27 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Gastón Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently in the `HierarchicalAllocations_BENCHMARK_TestBase`,
> it is not easy to add agents with used resources because
> frameworks are created during the initialization which is
> after the agent profiles creation.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_benchmarks.cpp 
> bf9167b63747f7b8a402d950947028436307082a 
> 
> 
> Diff: https://reviews.apache.org/r/69093/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 69095: Added `hierarchical_allocator_benchmarks.cpp` to `CMakeLists.txt`.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On Oct. 20, 2018, 1:27 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69095/
> ---
> 
> (Updated Oct. 20, 2018, 1:27 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Gastón Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/CMakeLists.txt 00dbdee1c06e7571fa799850cdaf7f2ccadc8bea 
> 
> 
> Diff: https://reviews.apache.org/r/69095/diff/1/
> 
> 
> Testing
> ---
> 
> Build with cmake, verified that `hierarchical_allocator_benchmarks` is built.
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 69094: Renamed one allocator benchmark to be more descriptive.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On Oct. 20, 2018, 1:27 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69094/
> ---
> 
> (Updated Oct. 20, 2018, 1:27 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Gastón Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Renamed `Allocations` to `MultiFrameworkAllocations`.
> Also removed `_TestBase` from the fixture name.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_benchmarks.cpp 
> bf9167b63747f7b8a402d950947028436307082a 
> 
> 
> Diff: https://reviews.apache.org/r/69094/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 69092: Added default arguments to `FrameworkProfile` in allocator benchmark.

[Benjamin Mahler]

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



Hm.. I don't understand what this is doing. Is this setting default framework 
behavior if someone writing a benchmark doesn't want to have to pass the 
arguments?

- Benjamin Mahler


On Oct. 20, 2018, 1:27 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69092/
> ---
> 
> (Updated Oct. 20, 2018, 1:27 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Gastón Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For frameworks that do not care about launching tasks, we should
> provide some default task launch settings to simplify the benchmark
> settings.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_benchmarks.cpp 
> bf9167b63747f7b8a402d950947028436307082a 
> 
> 
> Diff: https://reviews.apache.org/r/69092/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 69146: Made fetcher tests more robust by using the test sandbox.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 69146 was successfully built and tested.

Reviews applied: `['69146']`

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

- Mesos Reviewbot Windows


On Oct. 24, 2018, 11:12 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69146/
> ---
> 
> (Updated Oct. 24, 2018, 11:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gilbert Song, 
> and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fetcher tests currently rely on some hard-coded paths,
> for example, `/tmp/mesos/fetcher` and `/tmp/frameworks`.
> Thus fetcher tests could fail if these directories already
> exit. This patch adds a helper function `createSlaveFlags()`
> to make use of the test sandboxes. The above paths will be
> replaced by `fetch/` and `frameworks/` under the sandbox.
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_tests.cpp 283238cdda17a94e034baa195bd9d4b57e363b8a 
> 
> 
> Diff: https://reviews.apache.org/r/69146/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Review Request 69149: Automatically remounted read-only bind mounts.

[James Peach]

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

Review request for mesos, Gilbert Song, Ilya Pronin, Jie Yu, and Jiang Yan Xu.


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


Repository: mesos


Description
---

To make a bind mount read-only, you have to first make the bind mount,
then remount it with the read-only flag. This is a bit arcane, which is
why `mount(8)` does it automatically.

This change updates `fs::mount()` to do the read-only remount
automatically when it is making a read-only bind mount so that every
caller doesn't have to carry special code to make it work correctly. All
the callers that make an explicit remount are updated to simply pass
the `MS_READONLY` flag if necessary.


Diffs
-

  src/examples/test_csi_plugin.cpp 7fa325e94304aec8927e345ebdac380105f624a8 
  src/linux/fs.hpp 502f85c4a32d8658bdd701975dd5ac3d802d308e 
  src/linux/fs.cpp 9055ef42edd1fb90e1026d1d603a9ba902cfc1fd 
  src/slave/containerizer/docker.cpp 192dc29576a99fdc671bb842c01f50cd30dc20e1 
  src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
24c9fd6beb9657b80b33dc31c2939083c1aa9110 
  src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
a47899cb528eef103f299def3bd3466905ac5b51 
  src/slave/containerizer/mesos/isolators/gpu/isolator.cpp 
dbbf92ffbe4a46cedca5b53f6ba172bfb308100e 
  src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
64271dfbb5c074ad3ac8d2a64c3943d739c0fffa 
  src/slave/containerizer/mesos/isolators/volume/host_path.cpp 
2e03ef50a290c046ae2b02b332d3d007b572429d 
  src/slave/containerizer/mesos/isolators/volume/image.cpp 
53cbaefeef7a6e10149e241e07d6e9cb8d510fc9 
  src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp 
21d9528c23d9142eccec456184b42d085b57d12c 
  src/slave/containerizer/mesos/provisioner/backends/bind.cpp 
7d564dc7de3e3de8726c7fd42a301d979c4a2574 
  src/tests/containerizer/fs_tests.cpp 23cad35b5db81a70b43bec1c1dbafe008c8dd4da 


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


Testing
---

sudo make check (Fedora 28)


Thanks,

James Peach

Re: Review Request 68957: Add documentation for per-framework metrics flag.

[Jacob Janco]

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

(Updated Oct. 24, 2018, 11:46 p.m.)


Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.


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


Repository: mesos


Description
---

Add documentation for per-framework metrics flag.


Diffs (updated)
-

  docs/configuration/master.md f290e377be9d7424899c82e099be98fc88688de1 


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

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


Testing
---

make check on OSX


Thanks,

Jacob Janco

Re: Review Request 69146: Made fetcher tests more robust by using the test sandbox.

[Till Toenshoff via Review Board]

> On Oct. 24, 2018, 11:31 p.m., Till Toenshoff wrote:
> > src/tests/fetcher_tests.cpp
> > Lines 105-106 (patched)
> > 
> >
> > While this solution appears to be the most efficient it also is not 
> > entirely consistent with how we do it in other places; 
> > Note that `getcwd()` returns the test-sandbox directory after the 
> > fixture did its job.
> > ```
> > path::join(os::getcwd(), "fetcher");
> > path::join(os::getcwd(), "frameworks");
> > ```
> > would be consistent.
> > 
> > Alternatively, we could at least do away with one `CHECK_NOTNONE` and 
> > pull it in front of these two uses of `sandbox`.
> 
> Joseph Wu wrote:
> A good chunk of tests have the following pattern:
> ```
> path::join(sandbox.get(), ...);
> ```
> 
> I've been leaning towards this because it forgoes the syscall.
> 
> Till Toenshoff wrote:
> Definitely more efficient, yes - wish we could "fix" that in one swoop - 
> but am not attached to the first part of my comment due to these reasons.
> 
> Meng Zhu wrote:
> For the second part, I think using two `CHECK_NOTNONE` is more readable 
> to me. Otherwise, we will end up with two unguarded `get()` (I am a little 
> bit allergic to unguarded `get()` :) )

How about 
```
CHECK_NOTNONE(sandbox);
flags.fetcher_cache_dir = path::join(sandbox.get(), "fetcher");
flags.frameworks_home = path::join(sandbox.get(), "frameworks");
```


- Till


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


On Oct. 24, 2018, 11:12 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69146/
> ---
> 
> (Updated Oct. 24, 2018, 11:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gilbert Song, 
> and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fetcher tests currently rely on some hard-coded paths,
> for example, `/tmp/mesos/fetcher` and `/tmp/frameworks`.
> Thus fetcher tests could fail if these directories already
> exit. This patch adds a helper function `createSlaveFlags()`
> to make use of the test sandboxes. The above paths will be
> replaced by `fetch/` and `frameworks/` under the sandbox.
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_tests.cpp 283238cdda17a94e034baa195bd9d4b57e363b8a 
> 
> 
> Diff: https://reviews.apache.org/r/69146/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 68956: Add a flag to toggle per-framework metrics.

[Jacob Janco]

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

(Updated Oct. 24, 2018, 11:46 p.m.)


Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.


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


Repository: mesos


Description
---

In clusters with high numbers of frameworks, it
can be necessary to control publishing of per
framework metrics. Metrics are still collected for
active frameworks, but will not be pulished if
this flag is set. This allows future extraction
of these metrics e.g. an API call or logs while
keeping the code change simple.


Diffs (updated)
-

  include/mesos/allocator/allocator.hpp 
61b2b843a10fd73bf876a86e9e18b2d78b70f9b1 
  src/master/allocator/mesos/hierarchical.hpp 
7b31c9d55dfae8a5da63f9d6e5db90bd4388bd92 
  src/master/allocator/mesos/hierarchical.cpp 
c89a63dd0e2beab64890cb2e4100d9fe903b86e9 
  src/master/allocator/mesos/metrics.hpp 
34cc16b3a4e0622d8ab1c39b093e58e9a37702c0 
  src/master/allocator/mesos/metrics.cpp 
73e68eb4ef53b56f2a764b0504e92d4688eb183c 
  src/master/flags.hpp 4a260155b32fada4ba7a6ae6de7aaecaa25839ff 
  src/master/flags.cpp 6ad53ed44d82e25c05548135b8f152d45ebf6629 
  src/master/framework.cpp 7cfe9f49cb407655984bdee16f7567576d553711 
  src/master/master.hpp ea7e9242b62fe6c2cc0e717f9a9f2f0c1cc0a390 
  src/master/master.cpp 0c95c438975efd949cbf86f7d8bfea940c20a43a 
  src/master/metrics.hpp e1da18e6ba2737f729e1e30653020538150ae898 
  src/master/metrics.cpp 56a7eef2d279ad3248092d37d19013d3ac110757 


Diff: https://reviews.apache.org/r/68956/diff/12/

Changes: https://reviews.apache.org/r/68956/diff/11-12/


Testing
---

make check on OSX

test-framework in local cluster with flag on/off

flag off: 

{
  "allocator/event_queue_dispatches": 0,
  "allocator/mesos/allocation_run_latency_ms": 0.048128,
  "allocator/mesos/allocation_run_latency_ms/count": 28,
  "allocator/mesos/allocation_run_latency_ms/max": 0.086784,
  "allocator/mesos/allocation_run_latency_ms/min": 0.02304,
  "allocator/mesos/allocation_run_latency_ms/p50": 0.048128,
  "allocator/mesos/allocation_run_latency_ms/p90": 0.0553472,
  "allocator/mesos/allocation_run_latency_ms/p95": 0.0572287996,
  "allocator/mesos/allocation_run_latency_ms/p99": 0.07897344,
  "allocator/mesos/allocation_run_latency_ms/p999": 0.0860029439997,
  "allocator/mesos/allocation_run_latency_ms/p": 0.0867058943998,
  "allocator/mesos/allocation_run_ms": 0.038144,
  "allocator/mesos/allocation_run_ms/count": 28,
  "allocator/mesos/allocation_run_ms/max": 0.326912,
  "allocator/mesos/allocation_run_ms/min": 0.01408,
  "allocator/mesos/allocation_run_ms/p50": 0.038144,
  "allocator/mesos/allocation_run_ms/p90": 0.0663552,
  "allocator/mesos/allocation_run_ms/p95": 0.133964786,
  "allocator/mesos/allocation_run_ms/p99": 0.284541443,
  "allocator/mesos/allocation_run_ms/p999": 0.3226749439985,
  "allocator/mesos/allocation_run_ms/p": 0.326488294398,
  "allocator/mesos/allocation_runs": 28,
  "allocator/mesos/event_queue_dispatches": 6,
  "allocator/mesos/resources/cpus/offered_or_allocated": 0,
  "allocator/mesos/resources/cpus/total": 8,
  "allocator/mesos/resources/disk/offered_or_allocated": 0,
  "allocator/mesos/resources/disk/total": 471782,
  "allocator/mesos/resources/mem/offered_or_allocated": 0,
  "allocator/mesos/resources/mem/total": 15360,
  "master/cpus_percent": 0,
  "master/cpus_revocable_percent": 0,
  "master/cpus_revocable_total": 0,
  "master/cpus_revocable_used": 0,
  "master/cpus_total": 8,
  "master/cpus_used": 0,
  "master/disk_percent": 0,
  "master/disk_revocable_percent": 0,
  "master/disk_revocable_total": 0,
  "master/disk_revocable_used": 0,
  "master/disk_total": 471782,
  "master/disk_used": 0,
  "master/dropped_messages": 0,
  "master/elected": 1,
  "master/event_queue_dispatches": 10,
  "master/event_queue_http_requests": 0,
  "master/event_queue_messages": 0,
  "master/frameworks_active": 0,
  "master/frameworks_connected": 0,
  "master/frameworks_disconnected": 0,
  "master/frameworks_inactive": 0,
  "master/gpus_percent": 0,
  "master/gpus_revocable_percent": 0,
  "master/gpus_revocable_total": 0,
  "master/gpus_revocable_used": 0,
  "master/gpus_total": 0,
  "master/gpus_used": 0,
  "master/invalid_executor_to_framework_messages": 0,
  "master/invalid_framework_to_executor_messages": 0,
  "master/invalid_operation_status_update_acknowledgements": 0,
  "master/invalid_status_update_acknowledgements": 0,
  "master/invalid_status_updates": 0,
  "master/mem_percent": 0,
  "master/mem_revocable_percent": 0,
  "master/mem_revocable_total": 0,
  "master/mem_revocable_used": 0,
  "master/mem_total": 15360,
  "master/mem_used": 0,
  "master/messages_authenticate": 0,
  "master/messages_deactivate_framework": 1,
  "master/messages_decline_offer

Re: Review Request 68953: Refactor allocator configuration into a struct.

[Jacob Janco]

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

(Updated Oct. 24, 2018, 11:46 p.m.)


Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.


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


Repository: mesos


Description
---

There has been a proliferation of configuration
passed into the allocator. This commit collects
that configuration into an `Options` struct.
Test fixes have been added as well.


Diffs (updated)
-

  include/mesos/allocator/allocator.hpp 
61b2b843a10fd73bf876a86e9e18b2d78b70f9b1 
  src/master/allocator/mesos/allocator.hpp 
a4d7f2be3e8ff71cc2c45cb8ed808b9dbb821aaf 
  src/master/allocator/mesos/hierarchical.hpp 
7b31c9d55dfae8a5da63f9d6e5db90bd4388bd92 
  src/master/allocator/mesos/hierarchical.cpp 
c89a63dd0e2beab64890cb2e4100d9fe903b86e9 
  src/master/master.cpp 0c95c438975efd949cbf86f7d8bfea940c20a43a 
  src/tests/allocator.hpp 15a5396986ea6939f25b8d23b87a91c27338fc7b 
  src/tests/api_tests.cpp c681b9b0b83f7472312083730a5433e3d6f0efc0 
  src/tests/hierarchical_allocator_benchmarks.cpp 
bf9167b63747f7b8a402d950947028436307082a 
  src/tests/hierarchical_allocator_tests.cpp 
27fbd9cf0c4442e7675362a806d35bad141ffb6d 
  src/tests/master_allocator_tests.cpp 88288ae8528900b017941db9b07f5f83649fa096 
  src/tests/master_quota_tests.cpp f669396569c92609ee75ea7ae427eb5f5769bfd9 
  src/tests/reservation_tests.cpp d6931220139d91620c886591d7916079b8541982 
  src/tests/resource_offers_tests.cpp 24800c2aa291431e4865e4104da62054b14e5eca 
  src/tests/slave_recovery_tests.cpp 5842ccffaf8c409aaa9c84720ba6c7b07ba6dc7c 


Diff: https://reviews.apache.org/r/68953/diff/11/

Changes: https://reviews.apache.org/r/68953/diff/10-11/


Testing
---

make check on OSX


Thanks,

Jacob Janco

Re: Review Request 69146: Made fetcher tests more robust by using the test sandbox.

[Meng Zhu]

> On Oct. 24, 2018, 4:31 p.m., Till Toenshoff wrote:
> > src/tests/fetcher_tests.cpp
> > Lines 105-106 (patched)
> > 
> >
> > While this solution appears to be the most efficient it also is not 
> > entirely consistent with how we do it in other places; 
> > Note that `getcwd()` returns the test-sandbox directory after the 
> > fixture did its job.
> > ```
> > path::join(os::getcwd(), "fetcher");
> > path::join(os::getcwd(), "frameworks");
> > ```
> > would be consistent.
> > 
> > Alternatively, we could at least do away with one `CHECK_NOTNONE` and 
> > pull it in front of these two uses of `sandbox`.
> 
> Joseph Wu wrote:
> A good chunk of tests have the following pattern:
> ```
> path::join(sandbox.get(), ...);
> ```
> 
> I've been leaning towards this because it forgoes the syscall.
> 
> Till Toenshoff wrote:
> Definitely more efficient, yes - wish we could "fix" that in one swoop - 
> but am not attached to the first part of my comment due to these reasons.

For the second part, I think using two `CHECK_NOTNONE` is more readable to me. 
Otherwise, we will end up with two unguarded `get()` (I am a little bit 
allergic to unguarded `get()` :) )


- Meng


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


On Oct. 24, 2018, 4:12 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69146/
> ---
> 
> (Updated Oct. 24, 2018, 4:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gilbert Song, 
> and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fetcher tests currently rely on some hard-coded paths,
> for example, `/tmp/mesos/fetcher` and `/tmp/frameworks`.
> Thus fetcher tests could fail if these directories already
> exit. This patch adds a helper function `createSlaveFlags()`
> to make use of the test sandboxes. The above paths will be
> replaced by `fetch/` and `frameworks/` under the sandbox.
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_tests.cpp 283238cdda17a94e034baa195bd9d4b57e363b8a 
> 
> 
> Diff: https://reviews.apache.org/r/69146/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 69146: Made fetcher tests more robust by using the test sandbox.

[Till Toenshoff via Review Board]

> On Oct. 24, 2018, 11:31 p.m., Till Toenshoff wrote:
> > src/tests/fetcher_tests.cpp
> > Lines 105-106 (patched)
> > 
> >
> > While this solution appears to be the most efficient it also is not 
> > entirely consistent with how we do it in other places; 
> > Note that `getcwd()` returns the test-sandbox directory after the 
> > fixture did its job.
> > ```
> > path::join(os::getcwd(), "fetcher");
> > path::join(os::getcwd(), "frameworks");
> > ```
> > would be consistent.
> > 
> > Alternatively, we could at least do away with one `CHECK_NOTNONE` and 
> > pull it in front of these two uses of `sandbox`.
> 
> Joseph Wu wrote:
> A good chunk of tests have the following pattern:
> ```
> path::join(sandbox.get(), ...);
> ```
> 
> I've been leaning towards this because it forgoes the syscall.

Definitely more efficient, yes - wish we could "fix" that in one swoop - but am 
not attached to the first part of my comment due to these reasons.


- Till


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


On Oct. 24, 2018, 11:12 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69146/
> ---
> 
> (Updated Oct. 24, 2018, 11:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gilbert Song, 
> and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fetcher tests currently rely on some hard-coded paths,
> for example, `/tmp/mesos/fetcher` and `/tmp/frameworks`.
> Thus fetcher tests could fail if these directories already
> exit. This patch adds a helper function `createSlaveFlags()`
> to make use of the test sandboxes. The above paths will be
> replaced by `fetch/` and `frameworks/` under the sandbox.
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_tests.cpp 283238cdda17a94e034baa195bd9d4b57e363b8a 
> 
> 
> Diff: https://reviews.apache.org/r/69146/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 69146: Made fetcher tests more robust by using the test sandbox.

[Joseph Wu]

> On Oct. 24, 2018, 4:31 p.m., Till Toenshoff wrote:
> > src/tests/fetcher_tests.cpp
> > Lines 105-106 (patched)
> > 
> >
> > While this solution appears to be the most efficient it also is not 
> > entirely consistent with how we do it in other places; 
> > Note that `getcwd()` returns the test-sandbox directory after the 
> > fixture did its job.
> > ```
> > path::join(os::getcwd(), "fetcher");
> > path::join(os::getcwd(), "frameworks");
> > ```
> > would be consistent.
> > 
> > Alternatively, we could at least do away with one `CHECK_NOTNONE` and 
> > pull it in front of these two uses of `sandbox`.

A good chunk of tests have the following pattern:
```
path::join(sandbox.get(), ...);
```

I've been leaning towards this because it forgoes the syscall.


- Joseph


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


On Oct. 24, 2018, 4:12 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69146/
> ---
> 
> (Updated Oct. 24, 2018, 4:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gilbert Song, 
> and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fetcher tests currently rely on some hard-coded paths,
> for example, `/tmp/mesos/fetcher` and `/tmp/frameworks`.
> Thus fetcher tests could fail if these directories already
> exit. This patch adds a helper function `createSlaveFlags()`
> to make use of the test sandboxes. The above paths will be
> replaced by `fetch/` and `frameworks/` under the sandbox.
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_tests.cpp 283238cdda17a94e034baa195bd9d4b57e363b8a 
> 
> 
> Diff: https://reviews.apache.org/r/69146/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 69146: Made fetcher tests more robust by using the test sandbox.

[Till Toenshoff via Review Board]

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


Fix it, then Ship it!





src/tests/fetcher_tests.cpp
Lines 105-106 (patched)


While this solution appears to be the most efficient it also is not 
entirely consistent with how we do it in other places; 
Note that `getcwd()` returns the test-sandbox directory after the fixture 
did its job.
```
path::join(os::getcwd(), "fetcher");
path::join(os::getcwd(), "frameworks");
```
would be consistent.

Alternatively, we could at least do away with one `CHECK_NOTNONE` and pull 
it in front of these two uses of `sandbox`.


- Till Toenshoff


On Oct. 24, 2018, 11:12 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69146/
> ---
> 
> (Updated Oct. 24, 2018, 11:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gilbert Song, 
> and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fetcher tests currently rely on some hard-coded paths,
> for example, `/tmp/mesos/fetcher` and `/tmp/frameworks`.
> Thus fetcher tests could fail if these directories already
> exit. This patch adds a helper function `createSlaveFlags()`
> to make use of the test sandboxes. The above paths will be
> replaced by `fetch/` and `frameworks/` under the sandbox.
> 
> 
> Diffs
> -
> 
>   src/tests/fetcher_tests.cpp 283238cdda17a94e034baa195bd9d4b57e363b8a 
> 
> 
> Diff: https://reviews.apache.org/r/69146/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 69146: Made fetcher tests more robust by using the test sandbox.

[Meng Zhu]

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

(Updated Oct. 24, 2018, 4:12 p.m.)


Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, Gilbert Song, and 
Till Toenshoff.


Changes
---

Added a helper function `CreateSlaveFlags()`.


Repository: mesos


Description (updated)
---

Fetcher tests currently rely on some hard-coded paths,
for example, `/tmp/mesos/fetcher` and `/tmp/frameworks`.
Thus fetcher tests could fail if these directories already
exit. This patch adds a helper function `createSlaveFlags()`
to make use of the test sandboxes. The above paths will be
replaced by `fetch/` and `frameworks/` under the sandbox.


Diffs (updated)
-

  src/tests/fetcher_tests.cpp 283238cdda17a94e034baa195bd9d4b57e363b8a 


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

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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 68956: Add a flag to toggle per-framework metrics.

[Jacob Janco]

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

(Updated Oct. 24, 2018, 11:08 p.m.)


Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.


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


Repository: mesos


Description
---

In clusters with high numbers of frameworks, it
can be necessary to control publishing of per
framework metrics. Metrics are still collected for
active frameworks, but will not be pulished if
this flag is set. This allows future extraction
of these metrics e.g. an API call or logs while
keeping the code change simple.


Diffs (updated)
-

  include/mesos/allocator/allocator.hpp 
61b2b843a10fd73bf876a86e9e18b2d78b70f9b1 
  src/master/allocator/mesos/hierarchical.hpp 
7b31c9d55dfae8a5da63f9d6e5db90bd4388bd92 
  src/master/allocator/mesos/hierarchical.cpp 
c89a63dd0e2beab64890cb2e4100d9fe903b86e9 
  src/master/allocator/mesos/metrics.hpp 
34cc16b3a4e0622d8ab1c39b093e58e9a37702c0 
  src/master/allocator/mesos/metrics.cpp 
73e68eb4ef53b56f2a764b0504e92d4688eb183c 
  src/master/flags.hpp 4a260155b32fada4ba7a6ae6de7aaecaa25839ff 
  src/master/flags.cpp 6ad53ed44d82e25c05548135b8f152d45ebf6629 
  src/master/framework.cpp 7cfe9f49cb407655984bdee16f7567576d553711 
  src/master/master.hpp ea7e9242b62fe6c2cc0e717f9a9f2f0c1cc0a390 
  src/master/master.cpp 0c95c438975efd949cbf86f7d8bfea940c20a43a 
  src/master/metrics.hpp e1da18e6ba2737f729e1e30653020538150ae898 
  src/master/metrics.cpp 56a7eef2d279ad3248092d37d19013d3ac110757 


Diff: https://reviews.apache.org/r/68956/diff/11/

Changes: https://reviews.apache.org/r/68956/diff/10-11/


Testing
---

make check on OSX

test-framework in local cluster with flag on/off

flag off: 

{
  "allocator/event_queue_dispatches": 0,
  "allocator/mesos/allocation_run_latency_ms": 0.048128,
  "allocator/mesos/allocation_run_latency_ms/count": 28,
  "allocator/mesos/allocation_run_latency_ms/max": 0.086784,
  "allocator/mesos/allocation_run_latency_ms/min": 0.02304,
  "allocator/mesos/allocation_run_latency_ms/p50": 0.048128,
  "allocator/mesos/allocation_run_latency_ms/p90": 0.0553472,
  "allocator/mesos/allocation_run_latency_ms/p95": 0.0572287996,
  "allocator/mesos/allocation_run_latency_ms/p99": 0.07897344,
  "allocator/mesos/allocation_run_latency_ms/p999": 0.0860029439997,
  "allocator/mesos/allocation_run_latency_ms/p": 0.0867058943998,
  "allocator/mesos/allocation_run_ms": 0.038144,
  "allocator/mesos/allocation_run_ms/count": 28,
  "allocator/mesos/allocation_run_ms/max": 0.326912,
  "allocator/mesos/allocation_run_ms/min": 0.01408,
  "allocator/mesos/allocation_run_ms/p50": 0.038144,
  "allocator/mesos/allocation_run_ms/p90": 0.0663552,
  "allocator/mesos/allocation_run_ms/p95": 0.133964786,
  "allocator/mesos/allocation_run_ms/p99": 0.284541443,
  "allocator/mesos/allocation_run_ms/p999": 0.3226749439985,
  "allocator/mesos/allocation_run_ms/p": 0.326488294398,
  "allocator/mesos/allocation_runs": 28,
  "allocator/mesos/event_queue_dispatches": 6,
  "allocator/mesos/resources/cpus/offered_or_allocated": 0,
  "allocator/mesos/resources/cpus/total": 8,
  "allocator/mesos/resources/disk/offered_or_allocated": 0,
  "allocator/mesos/resources/disk/total": 471782,
  "allocator/mesos/resources/mem/offered_or_allocated": 0,
  "allocator/mesos/resources/mem/total": 15360,
  "master/cpus_percent": 0,
  "master/cpus_revocable_percent": 0,
  "master/cpus_revocable_total": 0,
  "master/cpus_revocable_used": 0,
  "master/cpus_total": 8,
  "master/cpus_used": 0,
  "master/disk_percent": 0,
  "master/disk_revocable_percent": 0,
  "master/disk_revocable_total": 0,
  "master/disk_revocable_used": 0,
  "master/disk_total": 471782,
  "master/disk_used": 0,
  "master/dropped_messages": 0,
  "master/elected": 1,
  "master/event_queue_dispatches": 10,
  "master/event_queue_http_requests": 0,
  "master/event_queue_messages": 0,
  "master/frameworks_active": 0,
  "master/frameworks_connected": 0,
  "master/frameworks_disconnected": 0,
  "master/frameworks_inactive": 0,
  "master/gpus_percent": 0,
  "master/gpus_revocable_percent": 0,
  "master/gpus_revocable_total": 0,
  "master/gpus_revocable_used": 0,
  "master/gpus_total": 0,
  "master/gpus_used": 0,
  "master/invalid_executor_to_framework_messages": 0,
  "master/invalid_framework_to_executor_messages": 0,
  "master/invalid_operation_status_update_acknowledgements": 0,
  "master/invalid_status_update_acknowledgements": 0,
  "master/invalid_status_updates": 0,
  "master/mem_percent": 0,
  "master/mem_revocable_percent": 0,
  "master/mem_revocable_total": 0,
  "master/mem_revocable_used": 0,
  "master/mem_total": 15360,
  "master/mem_used": 0,
  "master/messages_authenticate": 0,
  "master/messages_deactivate_framework": 1,
  "master/messages_decline_offer

Re: Review Request 68957: Add documentation for per-framework metrics flag.

[Jacob Janco]

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

(Updated Oct. 24, 2018, 11:08 p.m.)


Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.


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


Repository: mesos


Description
---

Add documentation for per-framework metrics flag.


Diffs (updated)
-

  docs/configuration/master.md f290e377be9d7424899c82e099be98fc88688de1 


Diff: https://reviews.apache.org/r/68957/diff/12/

Changes: https://reviews.apache.org/r/68957/diff/11-12/


Testing
---

make check on OSX


Thanks,

Jacob Janco

Re: Review Request 68953: Refactor allocator configuration into a struct.

[Jacob Janco]

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

(Updated Oct. 24, 2018, 11:08 p.m.)


Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.


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


Repository: mesos


Description
---

There has been a proliferation of configuration
passed into the allocator. This commit collects
that configuration into an `Options` struct.
Test fixes have been added as well.


Diffs (updated)
-

  include/mesos/allocator/allocator.hpp 
61b2b843a10fd73bf876a86e9e18b2d78b70f9b1 
  src/master/allocator/mesos/allocator.hpp 
a4d7f2be3e8ff71cc2c45cb8ed808b9dbb821aaf 
  src/master/allocator/mesos/hierarchical.hpp 
7b31c9d55dfae8a5da63f9d6e5db90bd4388bd92 
  src/master/allocator/mesos/hierarchical.cpp 
c89a63dd0e2beab64890cb2e4100d9fe903b86e9 
  src/master/master.cpp 0c95c438975efd949cbf86f7d8bfea940c20a43a 
  src/tests/allocator.hpp 15a5396986ea6939f25b8d23b87a91c27338fc7b 
  src/tests/api_tests.cpp c681b9b0b83f7472312083730a5433e3d6f0efc0 
  src/tests/hierarchical_allocator_benchmarks.cpp 
bf9167b63747f7b8a402d950947028436307082a 
  src/tests/hierarchical_allocator_tests.cpp 
27fbd9cf0c4442e7675362a806d35bad141ffb6d 
  src/tests/master_allocator_tests.cpp 88288ae8528900b017941db9b07f5f83649fa096 
  src/tests/master_quota_tests.cpp f669396569c92609ee75ea7ae427eb5f5769bfd9 
  src/tests/reservation_tests.cpp d6931220139d91620c886591d7916079b8541982 
  src/tests/resource_offers_tests.cpp 24800c2aa291431e4865e4104da62054b14e5eca 
  src/tests/slave_recovery_tests.cpp 5842ccffaf8c409aaa9c84720ba6c7b07ba6dc7c 


Diff: https://reviews.apache.org/r/68953/diff/10/

Changes: https://reviews.apache.org/r/68953/diff/9-10/


Testing
---

make check on OSX


Thanks,

Jacob Janco

Re: Review Request 69085: Stout: Always `fsync` created directories in POSIX `mkdir`.

[Chun-Hung Hsiao]

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

(Updated Oct. 24, 2018, 11:02 p.m.)


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


Changes
---

Addressed Benjamin's comments.


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


Repository: mesos


Description
---

To ensure the directories created by `mkdir` are commited to their
filesystems, an `fsync` will be called on the parent of each created
directory.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/posix/mkdir.hpp 
418db9af310ed763a5ae4735c2ebdd1ca38738ba 


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

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


Testing
---

make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 69085: Stout: Always `fsync` created directories in POSIX `mkdir`.

[Chun-Hung Hsiao]

> On Oct. 24, 2018, 8:50 p.m., Benjamin Bannier wrote:
> > 3rdparty/stout/include/stout/os/posix/mkdir.hpp
> > Lines 63 (patched)
> > 
> >
> > We need to reset `errno` before invoking `::mkdir` to make sure we 
> > don't pick up a lingering preexisting `errno`.

Instead of relying on `errno` being zero, I've altered the logic a bit to only 
call `fsync` if `::mkdir` actually succeeds.


- Chun-Hung


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


On Oct. 19, 2018, 5:54 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69085/
> ---
> 
> (Updated Oct. 19, 2018, 5:54 p.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
> ---
> 
> To ensure the directories created by `mkdir` are commited to their
> filesystems, an `fsync` will be called on the parent of each created
> directory.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/posix/mkdir.hpp 
> 418db9af310ed763a5ae4735c2ebdd1ca38738ba 
> 
> 
> Diff: https://reviews.apache.org/r/69085/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 68957: Add documentation for per-framework metrics flag.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['68953', '68956', '68957']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

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

Review Request 69146: Made fetcher tests more robust by using the test sandbox.

[Meng Zhu]

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

Review request for mesos, Chun-Hung Hsiao and Gilbert Song.


Repository: mesos


Description
---

Fetcher tests currently rely on some hard-coded paths,
for example, `/tmp/mesos/fetcher` and `/tmp/frameworks`.
Thus fetcher tests could fail if these directories already
exit. This patch makes use of the test sandboxes.
The above paths will be replaced by `fetch/` and `frameworks/`
under the sandbox.


Diffs
-

  src/tests/fetcher_tests.cpp 283238cdda17a94e034baa195bd9d4b57e363b8a 


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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 68953: Refactor allocator configuration into a struct.

[Jacob Janco]

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

(Updated Oct. 24, 2018, 10:17 p.m.)


Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.


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


Repository: mesos


Description
---

There has been a proliferation of configuration
passed into the allocator. This commit collects
that configuration into an `Options` struct.
Test fixes have been added as well.


Diffs (updated)
-

  include/mesos/allocator/allocator.hpp 
61b2b843a10fd73bf876a86e9e18b2d78b70f9b1 
  src/master/allocator/mesos/allocator.hpp 
a4d7f2be3e8ff71cc2c45cb8ed808b9dbb821aaf 
  src/master/allocator/mesos/hierarchical.hpp 
7b31c9d55dfae8a5da63f9d6e5db90bd4388bd92 
  src/master/allocator/mesos/hierarchical.cpp 
c89a63dd0e2beab64890cb2e4100d9fe903b86e9 
  src/master/master.cpp 0c95c438975efd949cbf86f7d8bfea940c20a43a 
  src/tests/allocator.hpp 15a5396986ea6939f25b8d23b87a91c27338fc7b 
  src/tests/api_tests.cpp c681b9b0b83f7472312083730a5433e3d6f0efc0 
  src/tests/hierarchical_allocator_benchmarks.cpp 
bf9167b63747f7b8a402d950947028436307082a 
  src/tests/hierarchical_allocator_tests.cpp 
27fbd9cf0c4442e7675362a806d35bad141ffb6d 
  src/tests/master_allocator_tests.cpp 88288ae8528900b017941db9b07f5f83649fa096 
  src/tests/master_quota_tests.cpp f669396569c92609ee75ea7ae427eb5f5769bfd9 
  src/tests/reservation_tests.cpp d6931220139d91620c886591d7916079b8541982 
  src/tests/resource_offers_tests.cpp 24800c2aa291431e4865e4104da62054b14e5eca 
  src/tests/slave_recovery_tests.cpp 5842ccffaf8c409aaa9c84720ba6c7b07ba6dc7c 


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

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


Testing
---

make check on OSX


Thanks,

Jacob Janco

Re: Review Request 68957: Add documentation for per-framework metrics flag.

[Jacob Janco]

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

(Updated Oct. 24, 2018, 10:17 p.m.)


Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.


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


Repository: mesos


Description
---

Add documentation for per-framework metrics flag.


Diffs (updated)
-

  docs/configuration/master.md f290e377be9d7424899c82e099be98fc88688de1 


Diff: https://reviews.apache.org/r/68957/diff/11/

Changes: https://reviews.apache.org/r/68957/diff/10-11/


Testing
---

make check on OSX


Thanks,

Jacob Janco

Re: Review Request 68956: Add a flag to toggle per-framework metrics.

[Jacob Janco]

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

(Updated Oct. 24, 2018, 10:17 p.m.)


Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.


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


Repository: mesos


Description
---

In clusters with high numbers of frameworks, it
can be necessary to control publishing of per
framework metrics. Metrics are still collected for
active frameworks, but will not be pulished if
this flag is set. This allows future extraction
of these metrics e.g. an API call or logs while
keeping the code change simple.


Diffs (updated)
-

  include/mesos/allocator/allocator.hpp 
61b2b843a10fd73bf876a86e9e18b2d78b70f9b1 
  src/master/allocator/mesos/hierarchical.hpp 
7b31c9d55dfae8a5da63f9d6e5db90bd4388bd92 
  src/master/allocator/mesos/hierarchical.cpp 
c89a63dd0e2beab64890cb2e4100d9fe903b86e9 
  src/master/allocator/mesos/metrics.hpp 
34cc16b3a4e0622d8ab1c39b093e58e9a37702c0 
  src/master/allocator/mesos/metrics.cpp 
73e68eb4ef53b56f2a764b0504e92d4688eb183c 
  src/master/flags.hpp 4a260155b32fada4ba7a6ae6de7aaecaa25839ff 
  src/master/flags.cpp 6ad53ed44d82e25c05548135b8f152d45ebf6629 
  src/master/framework.cpp 7cfe9f49cb407655984bdee16f7567576d553711 
  src/master/master.hpp ea7e9242b62fe6c2cc0e717f9a9f2f0c1cc0a390 
  src/master/master.cpp 0c95c438975efd949cbf86f7d8bfea940c20a43a 
  src/master/metrics.hpp e1da18e6ba2737f729e1e30653020538150ae898 
  src/master/metrics.cpp 56a7eef2d279ad3248092d37d19013d3ac110757 


Diff: https://reviews.apache.org/r/68956/diff/10/

Changes: https://reviews.apache.org/r/68956/diff/9-10/


Testing
---

make check on OSX

test-framework in local cluster with flag on/off

flag off: 

{
  "allocator/event_queue_dispatches": 0,
  "allocator/mesos/allocation_run_latency_ms": 0.048128,
  "allocator/mesos/allocation_run_latency_ms/count": 28,
  "allocator/mesos/allocation_run_latency_ms/max": 0.086784,
  "allocator/mesos/allocation_run_latency_ms/min": 0.02304,
  "allocator/mesos/allocation_run_latency_ms/p50": 0.048128,
  "allocator/mesos/allocation_run_latency_ms/p90": 0.0553472,
  "allocator/mesos/allocation_run_latency_ms/p95": 0.0572287996,
  "allocator/mesos/allocation_run_latency_ms/p99": 0.07897344,
  "allocator/mesos/allocation_run_latency_ms/p999": 0.0860029439997,
  "allocator/mesos/allocation_run_latency_ms/p": 0.0867058943998,
  "allocator/mesos/allocation_run_ms": 0.038144,
  "allocator/mesos/allocation_run_ms/count": 28,
  "allocator/mesos/allocation_run_ms/max": 0.326912,
  "allocator/mesos/allocation_run_ms/min": 0.01408,
  "allocator/mesos/allocation_run_ms/p50": 0.038144,
  "allocator/mesos/allocation_run_ms/p90": 0.0663552,
  "allocator/mesos/allocation_run_ms/p95": 0.133964786,
  "allocator/mesos/allocation_run_ms/p99": 0.284541443,
  "allocator/mesos/allocation_run_ms/p999": 0.3226749439985,
  "allocator/mesos/allocation_run_ms/p": 0.326488294398,
  "allocator/mesos/allocation_runs": 28,
  "allocator/mesos/event_queue_dispatches": 6,
  "allocator/mesos/resources/cpus/offered_or_allocated": 0,
  "allocator/mesos/resources/cpus/total": 8,
  "allocator/mesos/resources/disk/offered_or_allocated": 0,
  "allocator/mesos/resources/disk/total": 471782,
  "allocator/mesos/resources/mem/offered_or_allocated": 0,
  "allocator/mesos/resources/mem/total": 15360,
  "master/cpus_percent": 0,
  "master/cpus_revocable_percent": 0,
  "master/cpus_revocable_total": 0,
  "master/cpus_revocable_used": 0,
  "master/cpus_total": 8,
  "master/cpus_used": 0,
  "master/disk_percent": 0,
  "master/disk_revocable_percent": 0,
  "master/disk_revocable_total": 0,
  "master/disk_revocable_used": 0,
  "master/disk_total": 471782,
  "master/disk_used": 0,
  "master/dropped_messages": 0,
  "master/elected": 1,
  "master/event_queue_dispatches": 10,
  "master/event_queue_http_requests": 0,
  "master/event_queue_messages": 0,
  "master/frameworks_active": 0,
  "master/frameworks_connected": 0,
  "master/frameworks_disconnected": 0,
  "master/frameworks_inactive": 0,
  "master/gpus_percent": 0,
  "master/gpus_revocable_percent": 0,
  "master/gpus_revocable_total": 0,
  "master/gpus_revocable_used": 0,
  "master/gpus_total": 0,
  "master/gpus_used": 0,
  "master/invalid_executor_to_framework_messages": 0,
  "master/invalid_framework_to_executor_messages": 0,
  "master/invalid_operation_status_update_acknowledgements": 0,
  "master/invalid_status_update_acknowledgements": 0,
  "master/invalid_status_updates": 0,
  "master/mem_percent": 0,
  "master/mem_revocable_percent": 0,
  "master/mem_revocable_total": 0,
  "master/mem_revocable_used": 0,
  "master/mem_total": 15360,
  "master/mem_used": 0,
  "master/messages_authenticate": 0,
  "master/messages_deactivate_framework": 1,
  "master/messages_decline_offers

Re: Review Request 68957: Add documentation for per-framework metrics flag.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the dependent review: 68956.

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

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

Relevant logs:

- 
[apply-review-68956.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2522/mesos-review-68957/logs/apply-review-68956.log):

```
error: missing binary patch data for 
'src/tests/.hierarchical_allocator_tests.cpp.swp'
error: binary patch does not apply to 
'src/tests/.hierarchical_allocator_tests.cpp.swp'
error: src/tests/.hierarchical_allocator_tests.cpp.swp: patch does not apply
```

- Mesos Reviewbot Windows


On Oct. 24, 2018, 9:38 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68957/
> ---
> 
> (Updated Oct. 24, 2018, 9:38 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.
> 
> 
> Bugs: MESOS-9301
> https://issues.apache.org/jira/browse/MESOS-9301
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add documentation for per-framework metrics flag.
> 
> 
> Diffs
> -
> 
>   docs/configuration/master.md f290e377be9d7424899c82e099be98fc88688de1 
> 
> 
> Diff: https://reviews.apache.org/r/68957/diff/10/
> 
> 
> Testing
> ---
> 
> make check on OSX
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 69082: Correctly propagated `close` failures in some instances.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 69082 was successfully built and tested.

Reviews applied: `['69082']`

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

- Mesos Reviewbot Windows


On Oct. 24, 2018, 8:22 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69082/
> ---
> 
> (Updated Oct. 24, 2018, 8:22 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Alex Clemmer.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When e.g., writing to disk, errors from write might only manifest at
> `::close` time. We update some instances to correctly propagate these
> errors instead of dropping them silently. We only propagate the
> `::close` error if the write operation succeeded, otherwise we just
> propagate the error from the write operation.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/posix/mktemp.hpp 
> 63b3d1a7720d07f877fa1d4eb7f32a548916637a 
>   3rdparty/stout/include/stout/os/write.hpp 
> cd35285a9595004bac627abf687588050aef8161 
>   3rdparty/stout/include/stout/protobuf.hpp 
> 1d03e1e3a8dd642f7239d777fb04759caf100a8b 
> 
> 
> Diff: https://reviews.apache.org/r/69082/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 68956: Add a flag to toggle per-framework metrics.

[Jacob Janco]

> On Oct. 23, 2018, 11:35 p.m., James Peach wrote:
> > src/master/metrics.hpp
> > Line 223 (original), 223 (patched)
> > 
> >
> > We don't need the `explicit` here anymore.

Fixed.


- Jacob


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


On Oct. 24, 2018, 9:38 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68956/
> ---
> 
> (Updated Oct. 24, 2018, 9:38 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.
> 
> 
> Bugs: MESOS-9301
> https://issues.apache.org/jira/browse/MESOS-9301
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In clusters with high numbers of frameworks, it
> can be necessary to control publishing of per
> framework metrics. Metrics are still collected for
> active frameworks, but will not be pulished if
> this flag is set. This allows future extraction
> of these metrics e.g. an API call or logs while
> keeping the code change simple.
> 
> 
> Diffs
> -
> 
>   include/mesos/allocator/allocator.hpp 
> 61b2b843a10fd73bf876a86e9e18b2d78b70f9b1 
>   src/master/allocator/mesos/hierarchical.hpp 
> 7b31c9d55dfae8a5da63f9d6e5db90bd4388bd92 
>   src/master/allocator/mesos/hierarchical.cpp 
> c89a63dd0e2beab64890cb2e4100d9fe903b86e9 
>   src/master/allocator/mesos/metrics.hpp 
> 34cc16b3a4e0622d8ab1c39b093e58e9a37702c0 
>   src/master/allocator/mesos/metrics.cpp 
> 73e68eb4ef53b56f2a764b0504e92d4688eb183c 
>   src/master/flags.hpp 4a260155b32fada4ba7a6ae6de7aaecaa25839ff 
>   src/master/flags.cpp 6ad53ed44d82e25c05548135b8f152d45ebf6629 
>   src/master/framework.cpp 7cfe9f49cb407655984bdee16f7567576d553711 
>   src/master/master.hpp ea7e9242b62fe6c2cc0e717f9a9f2f0c1cc0a390 
>   src/master/master.cpp 0c95c438975efd949cbf86f7d8bfea940c20a43a 
>   src/master/metrics.hpp e1da18e6ba2737f729e1e30653020538150ae898 
>   src/master/metrics.cpp 56a7eef2d279ad3248092d37d19013d3ac110757 
>   src/tests/.hierarchical_allocator_tests.cpp.swp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68956/diff/9/
> 
> 
> Testing
> ---
> 
> make check on OSX
> 
> test-framework in local cluster with flag on/off
> 
> flag off: 
> 
> {
>   "allocator/event_queue_dispatches": 0,
>   "allocator/mesos/allocation_run_latency_ms": 0.048128,
>   "allocator/mesos/allocation_run_latency_ms/count": 28,
>   "allocator/mesos/allocation_run_latency_ms/max": 0.086784,
>   "allocator/mesos/allocation_run_latency_ms/min": 0.02304,
>   "allocator/mesos/allocation_run_latency_ms/p50": 0.048128,
>   "allocator/mesos/allocation_run_latency_ms/p90": 0.0553472,
>   "allocator/mesos/allocation_run_latency_ms/p95": 0.0572287996,
>   "allocator/mesos/allocation_run_latency_ms/p99": 0.07897344,
>   "allocator/mesos/allocation_run_latency_ms/p999": 0.0860029439997,
>   "allocator/mesos/allocation_run_latency_ms/p": 0.0867058943998,
>   "allocator/mesos/allocation_run_ms": 0.038144,
>   "allocator/mesos/allocation_run_ms/count": 28,
>   "allocator/mesos/allocation_run_ms/max": 0.326912,
>   "allocator/mesos/allocation_run_ms/min": 0.01408,
>   "allocator/mesos/allocation_run_ms/p50": 0.038144,
>   "allocator/mesos/allocation_run_ms/p90": 0.0663552,
>   "allocator/mesos/allocation_run_ms/p95": 0.133964786,
>   "allocator/mesos/allocation_run_ms/p99": 0.284541443,
>   "allocator/mesos/allocation_run_ms/p999": 0.3226749439985,
>   "allocator/mesos/allocation_run_ms/p": 0.326488294398,
>   "allocator/mesos/allocation_runs": 28,
>   "allocator/mesos/event_queue_dispatches": 6,
>   "allocator/mesos/resources/cpus/offered_or_allocated": 0,
>   "allocator/mesos/resources/cpus/total": 8,
>   "allocator/mesos/resources/disk/offered_or_allocated": 0,
>   "allocator/mesos/resources/disk/total": 471782,
>   "allocator/mesos/resources/mem/offered_or_allocated": 0,
>   "allocator/mesos/resources/mem/total": 15360,
>   "master/cpus_percent": 0,
>   "master/cpus_revocable_percent": 0,
>   "master/cpus_revocable_total": 0,
>   "master/cpus_revocable_used": 0,
>   "master/cpus_total": 8,
>   "master/cpus_used": 0,
>   "master/disk_percent": 0,
>   "master/disk_revocable_percent": 0,
>   "master/disk_revocable_total": 0,
>   "master/disk_revocable_used": 0,
>   "master/disk_total": 471782,
>   "master/disk_used": 0,
>   "master/dropped_messages": 0,
>   "master/elected": 1,
>   "master/event_queue_dispatches": 10,
>   "master/event_queue_http_requests": 0,
>   "master/event_queue_messages": 0,
>   "master/frameworks_active": 0,
>   "master/frameworks_connected": 0,
>   "master/frameworks_disconnected": 0,
>   "m

Re: Review Request 68956: Add a flag to toggle per-framework metrics.

[Jacob Janco]

> On Oct. 24, 2018, 2:55 a.m., Greg Mann wrote:
> > src/master/flags.cpp
> > Lines 686-689 (patched)
> > 
> >
> > We generally use the same text here and in the markdown/html 
> > documentation. Could you update this patch to include the more verbose 
> > explanation given in the documentation patch?

Fixed.


- Jacob


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


On Oct. 24, 2018, 9:38 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68956/
> ---
> 
> (Updated Oct. 24, 2018, 9:38 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.
> 
> 
> Bugs: MESOS-9301
> https://issues.apache.org/jira/browse/MESOS-9301
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In clusters with high numbers of frameworks, it
> can be necessary to control publishing of per
> framework metrics. Metrics are still collected for
> active frameworks, but will not be pulished if
> this flag is set. This allows future extraction
> of these metrics e.g. an API call or logs while
> keeping the code change simple.
> 
> 
> Diffs
> -
> 
>   include/mesos/allocator/allocator.hpp 
> 61b2b843a10fd73bf876a86e9e18b2d78b70f9b1 
>   src/master/allocator/mesos/hierarchical.hpp 
> 7b31c9d55dfae8a5da63f9d6e5db90bd4388bd92 
>   src/master/allocator/mesos/hierarchical.cpp 
> c89a63dd0e2beab64890cb2e4100d9fe903b86e9 
>   src/master/allocator/mesos/metrics.hpp 
> 34cc16b3a4e0622d8ab1c39b093e58e9a37702c0 
>   src/master/allocator/mesos/metrics.cpp 
> 73e68eb4ef53b56f2a764b0504e92d4688eb183c 
>   src/master/flags.hpp 4a260155b32fada4ba7a6ae6de7aaecaa25839ff 
>   src/master/flags.cpp 6ad53ed44d82e25c05548135b8f152d45ebf6629 
>   src/master/framework.cpp 7cfe9f49cb407655984bdee16f7567576d553711 
>   src/master/master.hpp ea7e9242b62fe6c2cc0e717f9a9f2f0c1cc0a390 
>   src/master/master.cpp 0c95c438975efd949cbf86f7d8bfea940c20a43a 
>   src/master/metrics.hpp e1da18e6ba2737f729e1e30653020538150ae898 
>   src/master/metrics.cpp 56a7eef2d279ad3248092d37d19013d3ac110757 
>   src/tests/.hierarchical_allocator_tests.cpp.swp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68956/diff/9/
> 
> 
> Testing
> ---
> 
> make check on OSX
> 
> test-framework in local cluster with flag on/off
> 
> flag off: 
> 
> {
>   "allocator/event_queue_dispatches": 0,
>   "allocator/mesos/allocation_run_latency_ms": 0.048128,
>   "allocator/mesos/allocation_run_latency_ms/count": 28,
>   "allocator/mesos/allocation_run_latency_ms/max": 0.086784,
>   "allocator/mesos/allocation_run_latency_ms/min": 0.02304,
>   "allocator/mesos/allocation_run_latency_ms/p50": 0.048128,
>   "allocator/mesos/allocation_run_latency_ms/p90": 0.0553472,
>   "allocator/mesos/allocation_run_latency_ms/p95": 0.0572287996,
>   "allocator/mesos/allocation_run_latency_ms/p99": 0.07897344,
>   "allocator/mesos/allocation_run_latency_ms/p999": 0.0860029439997,
>   "allocator/mesos/allocation_run_latency_ms/p": 0.0867058943998,
>   "allocator/mesos/allocation_run_ms": 0.038144,
>   "allocator/mesos/allocation_run_ms/count": 28,
>   "allocator/mesos/allocation_run_ms/max": 0.326912,
>   "allocator/mesos/allocation_run_ms/min": 0.01408,
>   "allocator/mesos/allocation_run_ms/p50": 0.038144,
>   "allocator/mesos/allocation_run_ms/p90": 0.0663552,
>   "allocator/mesos/allocation_run_ms/p95": 0.133964786,
>   "allocator/mesos/allocation_run_ms/p99": 0.284541443,
>   "allocator/mesos/allocation_run_ms/p999": 0.3226749439985,
>   "allocator/mesos/allocation_run_ms/p": 0.326488294398,
>   "allocator/mesos/allocation_runs": 28,
>   "allocator/mesos/event_queue_dispatches": 6,
>   "allocator/mesos/resources/cpus/offered_or_allocated": 0,
>   "allocator/mesos/resources/cpus/total": 8,
>   "allocator/mesos/resources/disk/offered_or_allocated": 0,
>   "allocator/mesos/resources/disk/total": 471782,
>   "allocator/mesos/resources/mem/offered_or_allocated": 0,
>   "allocator/mesos/resources/mem/total": 15360,
>   "master/cpus_percent": 0,
>   "master/cpus_revocable_percent": 0,
>   "master/cpus_revocable_total": 0,
>   "master/cpus_revocable_used": 0,
>   "master/cpus_total": 8,
>   "master/cpus_used": 0,
>   "master/disk_percent": 0,
>   "master/disk_revocable_percent": 0,
>   "master/disk_revocable_total": 0,
>   "master/disk_revocable_used": 0,
>   "master/disk_total": 471782,
>   "master/disk_used": 0,
>   "master/dropped_messages": 0,
>   "master/elected": 1,
>   "master/event_queue_dispatches": 10,
>   "master/event_queue_http_requests": 0,
>   "master/event_queue_messa

Re: Review Request 68953: Refactor allocator configuration into a struct.

[Jacob Janco]

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

(Updated Oct. 24, 2018, 9:38 p.m.)


Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.


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


Repository: mesos


Description
---

There has been a proliferation of configuration
passed into the allocator. This commit collects
that configuration into an `Options` struct.
Test fixes have been added as well.


Diffs (updated)
-

  include/mesos/allocator/allocator.hpp 
61b2b843a10fd73bf876a86e9e18b2d78b70f9b1 
  src/master/allocator/mesos/allocator.hpp 
a4d7f2be3e8ff71cc2c45cb8ed808b9dbb821aaf 
  src/master/allocator/mesos/hierarchical.hpp 
7b31c9d55dfae8a5da63f9d6e5db90bd4388bd92 
  src/master/allocator/mesos/hierarchical.cpp 
c89a63dd0e2beab64890cb2e4100d9fe903b86e9 
  src/master/master.cpp 0c95c438975efd949cbf86f7d8bfea940c20a43a 
  src/tests/allocator.hpp 15a5396986ea6939f25b8d23b87a91c27338fc7b 
  src/tests/api_tests.cpp c681b9b0b83f7472312083730a5433e3d6f0efc0 
  src/tests/hierarchical_allocator_benchmarks.cpp 
bf9167b63747f7b8a402d950947028436307082a 
  src/tests/hierarchical_allocator_tests.cpp 
27fbd9cf0c4442e7675362a806d35bad141ffb6d 
  src/tests/master_allocator_tests.cpp 88288ae8528900b017941db9b07f5f83649fa096 
  src/tests/master_quota_tests.cpp f669396569c92609ee75ea7ae427eb5f5769bfd9 
  src/tests/reservation_tests.cpp d6931220139d91620c886591d7916079b8541982 
  src/tests/resource_offers_tests.cpp 24800c2aa291431e4865e4104da62054b14e5eca 
  src/tests/slave_recovery_tests.cpp 5842ccffaf8c409aaa9c84720ba6c7b07ba6dc7c 


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

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


Testing
---

make check on OSX


Thanks,

Jacob Janco

Re: Review Request 68956: Add a flag to toggle per-framework metrics.

[Jacob Janco]

> On Oct. 24, 2018, 2:45 a.m., Greg Mann wrote:
> > Just a couple style nits below, sorry for the late review.

Done.


> On Oct. 24, 2018, 2:45 a.m., Greg Mann wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Line 268 (original), 269-272 (patched)
> > 
> >
> > Nit: style guide would suggest:
> > ```
> >   frameworks.insert(
> >   {frameworkId, Framework(
> >   frameworkInfo,
> >   suppressedRoles,
> >   active,
> >   options.publishPerFrameworkMetrics)});
> > ```

Fixed.


> On Oct. 24, 2018, 2:45 a.m., Greg Mann wrote:
> > src/master/allocator/mesos/metrics.cpp
> > Lines 217-218 (original), 217-218 (patched)
> > 
> >
> > Nit: our style guide would suggest:
> > ```
> > FrameworkMetrics::FrameworkMetrics(
> > const FrameworkInfo& _frameworkInfo,
> > const bool _publishPerFrameworkMetrics)
> > ```

Fixed.


> On Oct. 24, 2018, 2:45 a.m., Greg Mann wrote:
> > src/master/allocator/mesos/metrics.cpp
> > Lines 220 (patched)
> > 
> >
> > Nit: indent two more spaces.

Fixed.


- Jacob


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


On Oct. 24, 2018, 9:38 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68956/
> ---
> 
> (Updated Oct. 24, 2018, 9:38 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.
> 
> 
> Bugs: MESOS-9301
> https://issues.apache.org/jira/browse/MESOS-9301
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In clusters with high numbers of frameworks, it
> can be necessary to control publishing of per
> framework metrics. Metrics are still collected for
> active frameworks, but will not be pulished if
> this flag is set. This allows future extraction
> of these metrics e.g. an API call or logs while
> keeping the code change simple.
> 
> 
> Diffs
> -
> 
>   include/mesos/allocator/allocator.hpp 
> 61b2b843a10fd73bf876a86e9e18b2d78b70f9b1 
>   src/master/allocator/mesos/hierarchical.hpp 
> 7b31c9d55dfae8a5da63f9d6e5db90bd4388bd92 
>   src/master/allocator/mesos/hierarchical.cpp 
> c89a63dd0e2beab64890cb2e4100d9fe903b86e9 
>   src/master/allocator/mesos/metrics.hpp 
> 34cc16b3a4e0622d8ab1c39b093e58e9a37702c0 
>   src/master/allocator/mesos/metrics.cpp 
> 73e68eb4ef53b56f2a764b0504e92d4688eb183c 
>   src/master/flags.hpp 4a260155b32fada4ba7a6ae6de7aaecaa25839ff 
>   src/master/flags.cpp 6ad53ed44d82e25c05548135b8f152d45ebf6629 
>   src/master/framework.cpp 7cfe9f49cb407655984bdee16f7567576d553711 
>   src/master/master.hpp ea7e9242b62fe6c2cc0e717f9a9f2f0c1cc0a390 
>   src/master/master.cpp 0c95c438975efd949cbf86f7d8bfea940c20a43a 
>   src/master/metrics.hpp e1da18e6ba2737f729e1e30653020538150ae898 
>   src/master/metrics.cpp 56a7eef2d279ad3248092d37d19013d3ac110757 
>   src/tests/.hierarchical_allocator_tests.cpp.swp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68956/diff/9/
> 
> 
> Testing
> ---
> 
> make check on OSX
> 
> test-framework in local cluster with flag on/off
> 
> flag off: 
> 
> {
>   "allocator/event_queue_dispatches": 0,
>   "allocator/mesos/allocation_run_latency_ms": 0.048128,
>   "allocator/mesos/allocation_run_latency_ms/count": 28,
>   "allocator/mesos/allocation_run_latency_ms/max": 0.086784,
>   "allocator/mesos/allocation_run_latency_ms/min": 0.02304,
>   "allocator/mesos/allocation_run_latency_ms/p50": 0.048128,
>   "allocator/mesos/allocation_run_latency_ms/p90": 0.0553472,
>   "allocator/mesos/allocation_run_latency_ms/p95": 0.0572287996,
>   "allocator/mesos/allocation_run_latency_ms/p99": 0.07897344,
>   "allocator/mesos/allocation_run_latency_ms/p999": 0.0860029439997,
>   "allocator/mesos/allocation_run_latency_ms/p": 0.0867058943998,
>   "allocator/mesos/allocation_run_ms": 0.038144,
>   "allocator/mesos/allocation_run_ms/count": 28,
>   "allocator/mesos/allocation_run_ms/max": 0.326912,
>   "allocator/mesos/allocation_run_ms/min": 0.01408,
>   "allocator/mesos/allocation_run_ms/p50": 0.038144,
>   "allocator/mesos/allocation_run_ms/p90": 0.0663552,
>   "allocator/mesos/allocation_run_ms/p95": 0.133964786,
>   "allocator/mesos/allocation_run_ms/p99": 0.284541443,
>   "allocator/mesos/allocation_run_ms/p999": 0.3226749439985,
>   "allocator/mesos/allocation_run_ms/p": 0.326488294398,
>   "allocator/mesos/allocation_runs": 28,
>   "allocator/mesos/

Re: Review Request 68957: Add documentation for per-framework metrics flag.

[Jacob Janco]

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

(Updated Oct. 24, 2018, 9:38 p.m.)


Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.


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


Repository: mesos


Description
---

Add documentation for per-framework metrics flag.


Diffs (updated)
-

  docs/configuration/master.md f290e377be9d7424899c82e099be98fc88688de1 


Diff: https://reviews.apache.org/r/68957/diff/10/

Changes: https://reviews.apache.org/r/68957/diff/9-10/


Testing
---

make check on OSX


Thanks,

Jacob Janco

Re: Review Request 68956: Add a flag to toggle per-framework metrics.

[Jacob Janco]

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

(Updated Oct. 24, 2018, 9:38 p.m.)


Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.


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


Repository: mesos


Description
---

In clusters with high numbers of frameworks, it
can be necessary to control publishing of per
framework metrics. Metrics are still collected for
active frameworks, but will not be pulished if
this flag is set. This allows future extraction
of these metrics e.g. an API call or logs while
keeping the code change simple.


Diffs (updated)
-

  include/mesos/allocator/allocator.hpp 
61b2b843a10fd73bf876a86e9e18b2d78b70f9b1 
  src/master/allocator/mesos/hierarchical.hpp 
7b31c9d55dfae8a5da63f9d6e5db90bd4388bd92 
  src/master/allocator/mesos/hierarchical.cpp 
c89a63dd0e2beab64890cb2e4100d9fe903b86e9 
  src/master/allocator/mesos/metrics.hpp 
34cc16b3a4e0622d8ab1c39b093e58e9a37702c0 
  src/master/allocator/mesos/metrics.cpp 
73e68eb4ef53b56f2a764b0504e92d4688eb183c 
  src/master/flags.hpp 4a260155b32fada4ba7a6ae6de7aaecaa25839ff 
  src/master/flags.cpp 6ad53ed44d82e25c05548135b8f152d45ebf6629 
  src/master/framework.cpp 7cfe9f49cb407655984bdee16f7567576d553711 
  src/master/master.hpp ea7e9242b62fe6c2cc0e717f9a9f2f0c1cc0a390 
  src/master/master.cpp 0c95c438975efd949cbf86f7d8bfea940c20a43a 
  src/master/metrics.hpp e1da18e6ba2737f729e1e30653020538150ae898 
  src/master/metrics.cpp 56a7eef2d279ad3248092d37d19013d3ac110757 
  src/tests/.hierarchical_allocator_tests.cpp.swp PRE-CREATION 


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

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


Testing
---

make check on OSX

test-framework in local cluster with flag on/off

flag off: 

{
  "allocator/event_queue_dispatches": 0,
  "allocator/mesos/allocation_run_latency_ms": 0.048128,
  "allocator/mesos/allocation_run_latency_ms/count": 28,
  "allocator/mesos/allocation_run_latency_ms/max": 0.086784,
  "allocator/mesos/allocation_run_latency_ms/min": 0.02304,
  "allocator/mesos/allocation_run_latency_ms/p50": 0.048128,
  "allocator/mesos/allocation_run_latency_ms/p90": 0.0553472,
  "allocator/mesos/allocation_run_latency_ms/p95": 0.0572287996,
  "allocator/mesos/allocation_run_latency_ms/p99": 0.07897344,
  "allocator/mesos/allocation_run_latency_ms/p999": 0.0860029439997,
  "allocator/mesos/allocation_run_latency_ms/p": 0.0867058943998,
  "allocator/mesos/allocation_run_ms": 0.038144,
  "allocator/mesos/allocation_run_ms/count": 28,
  "allocator/mesos/allocation_run_ms/max": 0.326912,
  "allocator/mesos/allocation_run_ms/min": 0.01408,
  "allocator/mesos/allocation_run_ms/p50": 0.038144,
  "allocator/mesos/allocation_run_ms/p90": 0.0663552,
  "allocator/mesos/allocation_run_ms/p95": 0.133964786,
  "allocator/mesos/allocation_run_ms/p99": 0.284541443,
  "allocator/mesos/allocation_run_ms/p999": 0.3226749439985,
  "allocator/mesos/allocation_run_ms/p": 0.326488294398,
  "allocator/mesos/allocation_runs": 28,
  "allocator/mesos/event_queue_dispatches": 6,
  "allocator/mesos/resources/cpus/offered_or_allocated": 0,
  "allocator/mesos/resources/cpus/total": 8,
  "allocator/mesos/resources/disk/offered_or_allocated": 0,
  "allocator/mesos/resources/disk/total": 471782,
  "allocator/mesos/resources/mem/offered_or_allocated": 0,
  "allocator/mesos/resources/mem/total": 15360,
  "master/cpus_percent": 0,
  "master/cpus_revocable_percent": 0,
  "master/cpus_revocable_total": 0,
  "master/cpus_revocable_used": 0,
  "master/cpus_total": 8,
  "master/cpus_used": 0,
  "master/disk_percent": 0,
  "master/disk_revocable_percent": 0,
  "master/disk_revocable_total": 0,
  "master/disk_revocable_used": 0,
  "master/disk_total": 471782,
  "master/disk_used": 0,
  "master/dropped_messages": 0,
  "master/elected": 1,
  "master/event_queue_dispatches": 10,
  "master/event_queue_http_requests": 0,
  "master/event_queue_messages": 0,
  "master/frameworks_active": 0,
  "master/frameworks_connected": 0,
  "master/frameworks_disconnected": 0,
  "master/frameworks_inactive": 0,
  "master/gpus_percent": 0,
  "master/gpus_revocable_percent": 0,
  "master/gpus_revocable_total": 0,
  "master/gpus_revocable_used": 0,
  "master/gpus_total": 0,
  "master/gpus_used": 0,
  "master/invalid_executor_to_framework_messages": 0,
  "master/invalid_framework_to_executor_messages": 0,
  "master/invalid_operation_status_update_acknowledgements": 0,
  "master/invalid_status_update_acknowledgements": 0,
  "master/invalid_status_updates": 0,
  "master/mem_percent": 0,
  "master/mem_revocable_percent": 0,
  "master/mem_revocable_total": 0,
  "master/mem_revocable_used": 0,
  "master/mem_total": 15360,
  "master/mem_used": 0,
  "master/messages_authenticate": 0,
  "master/message

Re: Review Request 68953: Refactor allocator configuration into a struct.

[Jacob Janco]

> On Oct. 24, 2018, 2:51 a.m., Greg Mann wrote:
> > src/tests/hierarchical_allocator_tests.cpp
> > Lines 188 (patched)
> > 
> >
> > Nit: indented too far.

Fixed.


> On Oct. 24, 2018, 2:51 a.m., Greg Mann wrote:
> > src/tests/hierarchical_allocator_tests.cpp
> > Lines 189 (patched)
> > 
> >
> > Is the `Some()` here really necessary?

Nope.


- Jacob


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


On Oct. 24, 2018, 9:38 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68953/
> ---
> 
> (Updated Oct. 24, 2018, 9:38 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.
> 
> 
> Bugs: MESOS-9301
> https://issues.apache.org/jira/browse/MESOS-9301
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> There has been a proliferation of configuration
> passed into the allocator. This commit collects
> that configuration into an `Options` struct.
> Test fixes have been added as well.
> 
> 
> Diffs
> -
> 
>   include/mesos/allocator/allocator.hpp 
> 61b2b843a10fd73bf876a86e9e18b2d78b70f9b1 
>   src/master/allocator/mesos/allocator.hpp 
> a4d7f2be3e8ff71cc2c45cb8ed808b9dbb821aaf 
>   src/master/allocator/mesos/hierarchical.hpp 
> 7b31c9d55dfae8a5da63f9d6e5db90bd4388bd92 
>   src/master/allocator/mesos/hierarchical.cpp 
> c89a63dd0e2beab64890cb2e4100d9fe903b86e9 
>   src/master/master.cpp 0c95c438975efd949cbf86f7d8bfea940c20a43a 
>   src/tests/allocator.hpp 15a5396986ea6939f25b8d23b87a91c27338fc7b 
>   src/tests/api_tests.cpp c681b9b0b83f7472312083730a5433e3d6f0efc0 
>   src/tests/hierarchical_allocator_benchmarks.cpp 
> bf9167b63747f7b8a402d950947028436307082a 
>   src/tests/hierarchical_allocator_tests.cpp 
> 27fbd9cf0c4442e7675362a806d35bad141ffb6d 
>   src/tests/master_allocator_tests.cpp 
> 88288ae8528900b017941db9b07f5f83649fa096 
>   src/tests/master_quota_tests.cpp f669396569c92609ee75ea7ae427eb5f5769bfd9 
>   src/tests/reservation_tests.cpp d6931220139d91620c886591d7916079b8541982 
>   src/tests/resource_offers_tests.cpp 
> 24800c2aa291431e4865e4104da62054b14e5eca 
>   src/tests/slave_recovery_tests.cpp 5842ccffaf8c409aaa9c84720ba6c7b07ba6dc7c 
> 
> 
> Diff: https://reviews.apache.org/r/68953/diff/8/
> 
> 
> Testing
> ---
> 
> make check on OSX
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 68957: Add documentation for per-framework metrics flag.

[Jacob Janco]

> On Oct. 23, 2018, 11:42 p.m., James Peach wrote:
> > docs/configuration/master.md
> > Lines 667 (patched)
> > 
> >
> > This should be in alphabetical order.

Fixed.


- Jacob


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


On Oct. 24, 2018, 9:38 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68957/
> ---
> 
> (Updated Oct. 24, 2018, 9:38 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Greg Mann, and James Peach.
> 
> 
> Bugs: MESOS-9301
> https://issues.apache.org/jira/browse/MESOS-9301
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add documentation for per-framework metrics flag.
> 
> 
> Diffs
> -
> 
>   docs/configuration/master.md f290e377be9d7424899c82e099be98fc88688de1 
> 
> 
> Diff: https://reviews.apache.org/r/68957/diff/10/
> 
> 
> Testing
> ---
> 
> make check on OSX
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 69109: Updated libprocess to log all socket errors consistently.

[Gastón Kleiman]

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


Ship it!




Ship It!

- Gastón Kleiman


On Oct. 21, 2018, 4:40 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69109/
> ---
> 
> (Updated Oct. 21, 2018, 4:40 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, Jie Yu, and Till Toenshoff.
> 
> 
> Bugs: MESOS-9340
> https://issues.apache.org/jira/browse/MESOS-9340
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Some of these errors were getting logged at VLOG(1), INFO, or
> not at all. Notably, recv errors were either VLOG(1) or not
> logged. Send errors were not logged.
> 
> Now, we log the socket fd, peer address, and error message at
> the warning level whenever a socket error occurs.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> fe89f557b763390fbb05b52b3ad18c4cc886e737 
> 
> 
> Diff: https://reviews.apache.org/r/69109/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 69085: Stout: Always `fsync` created directories in POSIX `mkdir`.

[Benjamin Bannier]

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




3rdparty/stout/include/stout/os/posix/mkdir.hpp
Lines 63 (patched)


We need to reset `errno` before invoking `::mkdir` to make sure we don't 
pick up a lingering preexisting `errno`.


- Benjamin Bannier


On Oct. 19, 2018, 7:54 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69085/
> ---
> 
> (Updated Oct. 19, 2018, 7:54 p.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
> ---
> 
> To ensure the directories created by `mkdir` are commited to their
> filesystems, an `fsync` will be called on the parent of each created
> directory.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/posix/mkdir.hpp 
> 418db9af310ed763a5ae4735c2ebdd1ca38738ba 
> 
> 
> Diff: https://reviews.apache.org/r/69085/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 69085: Stout: Always `fsync` created directories in POSIX `mkdir`.

[Benjamin Bannier]

> On Oct. 22, 2018, 8:05 p.m., Benjamin Bannier wrote:
> > 3rdparty/stout/include/stout/os/posix/mkdir.hpp
> > Lines 75 (patched)
> > 
> >
> > I am not sure we should abort on the first error here. Many documented 
> > failure scenarios of `::fsync` are either very unlikley in this scope or 
> > retryable.
> > 
> > To get to the error code we might need to invoke `::fsync` directly.
> 
> Chun-Hung Hsiao wrote:
> It seems to me that we should abort on any of the errors listed in 
> http://pubs.opengroup.org/onlinepubs/9699919799/functions/fsync.html except 
> for `EBADF`, which cannot happen.

Reopening this for some further discussion.

Thanks for looking, these most errors there do indeed look rather non-retryable 
(e.g., those related to disk space). This leaves us in a situation where 
`os::mkdir` could return an error, but we are not certain (1) which directories 
were actually created (preexisting), and (2) which directories actually have 
reached the disk (buffer). Additionally, we leave the responsibility for 
cleanup to the caller, but give no indication how far we got.

While (1) is already pretty bad, it seems that a user explicitly requesting 
syncing (2) makes it very hard to promise good, consistent state.

As a compromise we could get rid of `dirs_to_sync` altogether, and invoke 
`fsync` right after `mkdir`. That might provide a slightly more consistent 
picture.


- Benjamin


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


On Oct. 19, 2018, 7:54 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69085/
> ---
> 
> (Updated Oct. 19, 2018, 7:54 p.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
> ---
> 
> To ensure the directories created by `mkdir` are commited to their
> filesystems, an `fsync` will be called on the parent of each created
> directory.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/posix/mkdir.hpp 
> 418db9af310ed763a5ae4735c2ebdd1ca38738ba 
> 
> 
> Diff: https://reviews.apache.org/r/69085/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

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

[Benjamin Bannier]

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


Ship it!




Ship It!

- Benjamin Bannier


On Oct. 18, 2018, 4:01 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69010/
> ---
> 
> (Updated Oct. 18, 2018, 4:01 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/6/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Review Request 69082: Correctly propagated `close` failures in some instances.

[Benjamin Bannier]

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

Review request for mesos, Chun-Hung Hsiao and Alex Clemmer.


Repository: mesos


Description
---

When e.g., writing to disk, errors from write might only manifest at
`::close` time. We update some instances to correctly propagate these
errors instead of dropping them silently. We only propagate the
`::close` error if the write operation succeeded, otherwise we just
propagate the error from the write operation.


Diffs
-

  3rdparty/stout/include/stout/os/posix/mktemp.hpp 
63b3d1a7720d07f877fa1d4eb7f32a548916637a 
  3rdparty/stout/include/stout/os/write.hpp 
cd35285a9595004bac627abf687588050aef8161 
  3rdparty/stout/include/stout/protobuf.hpp 
1d03e1e3a8dd642f7239d777fb04759caf100a8b 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 69138: Updated 'CLI_FILES' in 'cli_new/CmakeLists.txt'.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [69138]

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. 24, 2018, 2:30 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69138/
> ---
> 
> (Updated Oct. 24, 2018, 2:30 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Kevin Klues.
> 
> 
> Bugs: MESOS-9350
> https://issues.apache.org/jira/browse/MESOS-9350
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated 'CLI_FILES' in 'cli_new/CmakeLists.txt'.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/CMakeLists.txt 9eda91778fa174709812f9830a81c3e6d952eed2 
> 
> 
> Diff: https://reviews.apache.org/r/69138/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 69140: Allowed for unbundled libarchive on cmake builds.

[Joseph Wu]

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



We could probably do away with some of the double-negatives (i.e. `not 
UNBUNDLED` -> `BUNDLED`) in the new variable names and logic.


3rdparty/cmake/FindLIBARCHIVE.cmake
Lines 21-24 (patched)


Prefix this with:
```
# NOTE: If this fails, stderr is ignored, and the output variable is empty.
# This has no deleterious effect on our path search.
```
To note that non-OSX builds are unaffected by this command not existing.



3rdparty/cmake/FindLIBARCHIVE.cmake
Lines 26-29 (patched)


You might get some unexpected results on OSX if you don't reset the 
`POSSIBLE_*` variables outside the conditional.
```
  set(POSSIBLE_LIBARCHIVE_INCLUDE_DIRS "")
  set(POSSIBLE_LIBARCHIVE_LIB_DIRS "")

  if (NOT "${LIBARCHIVE_PREFIX}" STREQUAL "")
list(APPEND POSSIBLE_LIBARCHIVE_INCLUDE_DIRS 
${LIBARCHIVE_PREFIX}/include)
list(APPEND POSSIBLE_LIBARCHIVE_LIB_DIRS ${LIBARCHIVE_PREFIX}/lib)
  endif()
```



cmake/CompilationConfigure.cmake
Lines 82-84 (patched)


The `LIBARCHIVE_ROOT_DIR` variable doesn't need to be inside this 
conditional.  The variable would have no effect when we are using a bundled 
library, and would still be set/read on using an installed library.


- Joseph Wu


On Oct. 24, 2018, 5:25 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69140/
> ---
> 
> (Updated Oct. 24, 2018, 5:25 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, James 
> Peach, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allowed for unbundled libarchive on cmake builds.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 9584f537cc2a862ce17037224fd0628ffda54b2b 
>   3rdparty/cmake/FindLIBARCHIVE.cmake PRE-CREATION 
>   cmake/CompilationConfigure.cmake 5d2be0adc55ac5302c2e0e62131feb65657466be 
> 
> 
> Diff: https://reviews.apache.org/r/69140/diff/1/
> 
> 
> Testing
> ---
> 
> cmake .. -DUNBUNDLED_LIBARCHIVE=ON
> cmake .. -DUNBUNDLED_LIBARCHIVE=OFF
> cmake .. -DUNBUNDLED_LIBARCHIVE=ON 
> -DLIBARCHIVE_ROOT_DIR=/usr/local/opt/libarchive
> cmake ..
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 69132: Used mount(8) to make XFS loop device mounts.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 69132 was successfully built and tested.

Reviews applied: `['69132']`

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

- Mesos Reviewbot Windows


On Oct. 24, 2018, 3:41 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69132/
> ---
> 
> (Updated Oct. 24, 2018, 3:41 p.m.)
> 
> 
> Review request for mesos, Ilya Pronin, Jacob Janco, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When we added the `filesystem/linux` isolator to the XFS isolation
> tests, we broke them for older Linux distributions (specifically
> CentOS 6). The `filesystem/linux` isolator needs to remount the
> work directory in shared mode using `mount(8)`, but `mount(8)`
> requires the mount point it is manipulating to be present in
> `mtab(5)`. Since the XFS tests simply issued a `mount(2)` system
> call, `mtab(5)` wasn't updated and the remount would fail. Modern
> Linux distributions don't see this problem because `/` is mounted
> shared by default, and `mtab(5)` is a symlink to `/proc/self/mounts`
> (i.e. always in sync).
> 
> The straightforward fix is to run `mount(8)` to make the XFS test
> fixture mounts, so that `mount(8)` always sees a consistent view of
> `mtab(5)`.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/xfs_quota_tests.cpp 
> 084df067a8d97efdc86fc0ed878bc3f2edca06e5 
> 
> 
> Diff: https://reviews.apache.org/r/69132/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 28, CentOS 6)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 68812: Added example framework for inverse-offers.

[Alexander Rukletsov]

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


Fix it, then Ship it!




Please also add it to CMakeLists!


src/examples/inverse_offer_framework.cpp
Lines 495 (patched)


Let's extract it into `constexpr char FRAMEWORK_METRICS_PREFIX[]`.



src/examples/inverse_offer_framework.cpp
Lines 598 (patched)


How about pulling it into a constant, e.g., `constexpr char 
FRAMEWORK_NAME[]`?


- Alexander Rukletsov


On Sept. 23, 2018, 12:08 a.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68812/
> ---
> 
> (Updated Sept. 23, 2018, 12:08 a.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Bugs: MESOS-5827
> https://issues.apache.org/jira/browse/MESOS-5827
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds an example framework displaying how to handle inverse offers.
> This example is based on the original review request
> https://reviews.apache.org/r/50010/ by Joseph Wu.
> Some changes were applied adding framework authentication
> capabilites, updated PullGauge metrics and other minor adaptations
> following the other example frameworks.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 5795c7097a9ed1f659e169ad81a9f2c09481aa81 
>   src/examples/inverse_offer_framework.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/68812/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 69132: Used mount(8) to make XFS loop device mounts.

[James Peach]

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

(Updated Oct. 24, 2018, 3:41 p.m.)


Review request for mesos, Ilya Pronin, Jacob Janco, and Jiang Yan Xu.


Repository: mesos


Description
---

When we added the `filesystem/linux` isolator to the XFS isolation
tests, we broke them for older Linux distributions (specifically
CentOS 6). The `filesystem/linux` isolator needs to remount the
work directory in shared mode using `mount(8)`, but `mount(8)`
requires the mount point it is manipulating to be present in
`mtab(5)`. Since the XFS tests simply issued a `mount(2)` system
call, `mtab(5)` wasn't updated and the remount would fail. Modern
Linux distributions don't see this problem because `/` is mounted
shared by default, and `mtab(5)` is a symlink to `/proc/self/mounts`
(i.e. always in sync).

The straightforward fix is to run `mount(8)` to make the XFS test
fixture mounts, so that `mount(8)` always sees a consistent view of
`mtab(5)`.


Diffs (updated)
-

  src/tests/containerizer/xfs_quota_tests.cpp 
084df067a8d97efdc86fc0ed878bc3f2edca06e5 


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

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


Testing
---

sudo make check (Fedora 28, CentOS 6)


Thanks,

James Peach

Re: Review Request 69132: Used mount(8) to make XFS loop device mounts.

[James Peach]

> On Oct. 24, 2018, 12:59 a.m., Ilya Pronin wrote:
> > src/tests/containerizer/xfs_quota_tests.cpp
> > Lines 147-152 (original), 147-164 (patched)
> > 
> >
> > The command looks the same in both cases, the only difference is 
> > options. Maybe avoid duplicating code by constructing it? Like
> > ```c++
> > string mountCmd = strings::format(
> > "mount -t xfs%s %s %s",
> > mountOptions.isNone() ? " " : " -o " + mountOptions.get(),
> > loopDevice.get(),
> > mntPath);
> > ```

Good point, I'll fix that. I think this looks a little cleaner:

```C
   const string mountCmd = mountOptions.isNone()
 ? strings::format("mount -t xfs %s %s", loopDevice.get(), mntPath),
 : strings::format("mount -t xfs -o %s %s %s",
  mountOptions.get(), loopDevice.get(), mntPath);
```


- James


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


On Oct. 23, 2018, 11:17 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69132/
> ---
> 
> (Updated Oct. 23, 2018, 11:17 p.m.)
> 
> 
> Review request for mesos, Ilya Pronin, Jacob Janco, and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When we added the `filesystem/linux` isolator to the XFS isolation
> tests, we broke them for older Linux distributions (specifically
> CentOS 6). The `filesystem/linux` isolator needs to remount the
> work directory in shared mode using `mount(8)`, but `mount(8)`
> requires the mount point it is manipulating to be present in
> `mtab(5)`. Since the XFS tests simply issued a `mount(2)` system
> call, `mtab(5)` wasn't updated and the remount would fail. Modern
> Linux distributions don't see this problem because `/` is mounted
> shared by default, and `mtab(5)` is a symlink to `/proc/self/mounts`
> (i.e. always in sync).
> 
> The straightforward fix is to run `mount(8)` to make the XFS test
> fixture mounts, so that `mount(8)` always sees a consistent view of
> `mtab(5)`.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/xfs_quota_tests.cpp 
> 084df067a8d97efdc86fc0ed878bc3f2edca06e5 
> 
> 
> Diff: https://reviews.apache.org/r/69132/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 28, CentOS 6)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 69140: Allowed for unbundled libarchive on cmake builds.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 69140 was successfully built and tested.

Reviews applied: `['69140']`

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

- Mesos Reviewbot Windows


On Oct. 24, 2018, 12:25 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69140/
> ---
> 
> (Updated Oct. 24, 2018, 12:25 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, James 
> Peach, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allowed for unbundled libarchive on cmake builds.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 9584f537cc2a862ce17037224fd0628ffda54b2b 
>   3rdparty/cmake/FindLIBARCHIVE.cmake PRE-CREATION 
>   cmake/CompilationConfigure.cmake 5d2be0adc55ac5302c2e0e62131feb65657466be 
> 
> 
> Diff: https://reviews.apache.org/r/69140/diff/1/
> 
> 
> Testing
> ---
> 
> cmake .. -DUNBUNDLED_LIBARCHIVE=ON
> cmake .. -DUNBUNDLED_LIBARCHIVE=OFF
> cmake .. -DUNBUNDLED_LIBARCHIVE=ON 
> -DLIBARCHIVE_ROOT_DIR=/usr/local/opt/libarchive
> cmake ..
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 69140: Allowed for unbundled libarchive on cmake builds.

[Till Toenshoff via Review Board]

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




3rdparty/cmake/FindLIBARCHIVE.cmake
Lines 19 (patched)


Created https://issues.apache.org/jira/browse/MESOS-9351 for tracking this 
effort. Will tackle that soonish...


- Till Toenshoff


On Oct. 24, 2018, 12:25 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69140/
> ---
> 
> (Updated Oct. 24, 2018, 12:25 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, James 
> Peach, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allowed for unbundled libarchive on cmake builds.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 9584f537cc2a862ce17037224fd0628ffda54b2b 
>   3rdparty/cmake/FindLIBARCHIVE.cmake PRE-CREATION 
>   cmake/CompilationConfigure.cmake 5d2be0adc55ac5302c2e0e62131feb65657466be 
> 
> 
> Diff: https://reviews.apache.org/r/69140/diff/1/
> 
> 
> Testing
> ---
> 
> cmake .. -DUNBUNDLED_LIBARCHIVE=ON
> cmake .. -DUNBUNDLED_LIBARCHIVE=OFF
> cmake .. -DUNBUNDLED_LIBARCHIVE=ON 
> -DLIBARCHIVE_ROOT_DIR=/usr/local/opt/libarchive
> cmake ..
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 69140: Allowed for unbundled libarchive on cmake builds.

[Till Toenshoff via Review Board]

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




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


Am a bit unsure about this specific change and would appreciate a 
windows-feedback :)


- Till Toenshoff


On Oct. 24, 2018, 12:25 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69140/
> ---
> 
> (Updated Oct. 24, 2018, 12:25 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, James 
> Peach, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Allowed for unbundled libarchive on cmake builds.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 9584f537cc2a862ce17037224fd0628ffda54b2b 
>   3rdparty/cmake/FindLIBARCHIVE.cmake PRE-CREATION 
>   cmake/CompilationConfigure.cmake 5d2be0adc55ac5302c2e0e62131feb65657466be 
> 
> 
> Diff: https://reviews.apache.org/r/69140/diff/1/
> 
> 
> Testing
> ---
> 
> cmake .. -DUNBUNDLED_LIBARCHIVE=ON
> cmake .. -DUNBUNDLED_LIBARCHIVE=OFF
> cmake .. -DUNBUNDLED_LIBARCHIVE=ON 
> -DLIBARCHIVE_ROOT_DIR=/usr/local/opt/libarchive
> cmake ..
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Review Request 69140: Allowed for unbundled libarchive on cmake builds.

[Till Toenshoff via Review Board]

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

Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, James Peach, 
and Joseph Wu.


Repository: mesos


Description
---

Allowed for unbundled libarchive on cmake builds.


Diffs
-

  3rdparty/CMakeLists.txt 9584f537cc2a862ce17037224fd0628ffda54b2b 
  3rdparty/cmake/FindLIBARCHIVE.cmake PRE-CREATION 
  cmake/CompilationConfigure.cmake 5d2be0adc55ac5302c2e0e62131feb65657466be 


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


Testing
---

cmake .. -DUNBUNDLED_LIBARCHIVE=ON
cmake .. -DUNBUNDLED_LIBARCHIVE=OFF
cmake .. -DUNBUNDLED_LIBARCHIVE=ON 
-DLIBARCHIVE_ROOT_DIR=/usr/local/opt/libarchive
cmake ..


Thanks,

Till Toenshoff

Re: Review Request 69138: Updated 'CLI_FILES' in 'cli_new/CmakeLists.txt'.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 69138 was successfully built and tested.

Reviews applied: `['69138']`

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

- Mesos Reviewbot Windows


On Oct. 24, 2018, 2:30 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69138/
> ---
> 
> (Updated Oct. 24, 2018, 2:30 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Kevin Klues.
> 
> 
> Bugs: MESOS-9350
> https://issues.apache.org/jira/browse/MESOS-9350
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated 'CLI_FILES' in 'cli_new/CmakeLists.txt'.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/CMakeLists.txt 9eda91778fa174709812f9830a81c3e6d952eed2 
> 
> 
> Diff: https://reviews.apache.org/r/69138/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Review Request 69138: Updated 'CLI_FILES' in 'cli_new/CmakeLists.txt'.

[Armand Grillet]

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

Review request for mesos, Benjamin Bannier and Kevin Klues.


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


Repository: mesos


Description
---

Updated 'CLI_FILES' in 'cli_new/CmakeLists.txt'.


Diffs
-

  src/python/cli_new/CMakeLists.txt 9eda91778fa174709812f9830a81c3e6d952eed2 


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


Testing
---


Thanks,

Armand Grillet