Re: Review Request 66314: Fix 3rdparty build commands for FreeBSD.

[David Forsythe]

> On March 28, 2018, 8:06 a.m., Benjamin Bannier wrote:
> > 3rdparty/CMakeLists.txt
> > Lines 58-63 (patched)
> > 
> >
> > Like discussed offline, I don't think there is a reason we need to bolt 
> > such logic on the used system, at least currently.
> > 
> > Let's instead use `find_program` to find a `make` and use it to set 
> > this variable.
> > 
> > Unless I miss something, we do not depend on GNU make here, so let's 
> > maybe reflect that in a more general name, e.g., just `MAKE` if it is 
> > available.
> 
> David Forsythe wrote:
> I took a look at find_program, but I don't think that will solve the 
> problem.
> 
> We actually *do* depend on gmake here (even on darwin). On FreeBSD GHU 
> make is a 3rd party application and is actually called and installed as 
> `gmake`, whereas the system make is actually Pmake (which is incompatible).

I should clarify that there is incompability in a 3rdparty Makefile, not that 
the two makes are completely incompatible.

Particularly there are three issues - LevelDB, libev, and glog.

LevelDB is always going to fail without `gmake`, because it does not use 
autotools. I don't know how we can get around that.

libev and glog do use autotools. They have some strange behavior when building, 
though --
If I use `cmake --build` they both fail on a bad target (because of an 
incompatible Makefile).
If I use (bsd) `make`, they build fine.
If I use `gmake` they both fail (bad target).

For both the first and third scenario, I can run the commands that are failing 
outside of the build and they succeed.

Accordng to the cache, cmake is using `gmake` as `CMAKE_MAKE_PROGRAM` in these 
cases, so it's not clear to me why anything would fail when I do the whole 
build with `gmake`. It seems like the build can go make -> gmake, but not gmake 
-> make.

I think the baseline should be having `cmake --build` work, and the only way I 
have gotten everything to work as expected is by forcing `gmake` everywhere. 
I'm learning Cmake as I go so I could be missing something obvious to fix this.


- David


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


On March 27, 2018, 7:10 p.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66314/
> ---
> 
> (Updated March 27, 2018, 7:10 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-4176
> https://issues.apache.org/jira/browse/MESOS-4176
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix 3rdparty build commands for FreeBSD.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 2b63b58f7d6a88c9986b746283dcfa79b7bcb270 
>   cmake/CompilationConfigure.cmake 64cc56ee4208afe05df0f28af5890157e4c7d82c 
> 
> 
> Diff: https://reviews.apache.org/r/66314/diff/1/
> 
> 
> Testing
> ---
> 
> cmake --build on FreeBSD
> 
> 
> Thanks,
> 
> David Forsythe
> 
>

Re: Review Request 66144: Enforced task launch order on the agent.

[Greg Mann]

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




src/slave/slave.cpp
Lines 2224 (patched)


s/refer/referred/



src/slave/slave.cpp
Lines 2225 (patched)


s/"seqFuture"/`seqFuture`/



src/slave/slave.cpp
Lines 2234-2235 (patched)


Is unschedule GC the only failure which could lead to this?

If not, then maybe say "due to unschedule GC or some other failure".



src/slave/slave.cpp
Lines 2241 (patched)


s/tasks'/task's/



src/slave/slave.cpp
Lines 2244 (patched)


s/sequence/the sequence/



src/slave/slave.cpp
Lines 2261-2268 (patched)


I think we want to `return;` at the end of this conditional block? And if 
you make this lambda accept a string parameter, you could include the Failure 
message in this logging.

Also, it looks like the local variable `error` is not really needed?


- Greg Mann


On March 28, 2018, 7:05 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66144/
> ---
> 
> (Updated March 28, 2018, 7:05 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-8617 and MESOS-8624
> https://issues.apache.org/jira/browse/MESOS-8617
> https://issues.apache.org/jira/browse/MESOS-8624
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Up until now, Mesos does not guarantee in-order
> task launch on the agent. There are two asynchronous
> steps (unschedule GC and task authorization) in the
> agent task launch path. Depending on the CPU scheduling
> order, a later task launch may finish these two steps earlier
> than its predecessors and get to the launch executor stage
> earlier, resulting in out-of-order task delivery.
> 
> This patch enforces the task delivery order by sequencing
> task launch after the two asynchronous steps, specifically
> right before entering `__run()`.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb 
>   src/slave/slave.cpp e76daabad0d2d68aa42d1da809d4a23459eaaacb 
> 
> 
> Diff: https://reviews.apache.org/r/66144/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 65876: Enabled `--fetch_stall_timeout` in curl-based URI fetcher plugins.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On March 28, 2018, 5:07 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65876/
> ---
> 
> (Updated March 28, 2018, 5:07 p.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Bugs: MESOS-8620
> https://issues.apache.org/jira/browse/MESOS-8620
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch passes the `--fetch_stall_timeout` agent flag into
> `DockerFetcherPlugin` (through setting flag `docker_stall_timeout` in
> the Docker store) and `CurlFetcherPlugin` (through setting flag
> `curl_stall_timeout` in the Appc store).
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> c1f9661906eae019e43e1c3df7c28cccad3a6bb0 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> c308bb04773fa81265d91a1247b3db3434b5746b 
>   src/uri/fetchers/curl.hpp 909c2eb450630912c13831c3aa180fbccc8e9e4b 
>   src/uri/fetchers/curl.cpp 43584aaa424dc57f00880d5c89cdb73f23685800 
>   src/uri/fetchers/docker.hpp 6a89c0a69dd6f454cc22d025644900416e55ad50 
>   src/uri/fetchers/docker.cpp 21f8eb15903d505ba1cabe1f77a8f5d7043964d5 
> 
> 
> Diff: https://reviews.apache.org/r/65876/diff/3/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65856: Added `--fetcher_stall_timeout` to abort stalled artifact fetching.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On March 28, 2018, 4:51 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65856/
> ---
> 
> (Updated March 28, 2018, 4:51 p.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Bugs: MESOS-8620
> https://issues.apache.org/jira/browse/MESOS-8620
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This flag specifies a timeout for `mesos-fetcher` to wait before
> aborting if the download speed keeps below 1 bytes/sec. This would avoid
> containers to get stuck at FETCHING.
> 
> 
> Diffs
> -
> 
>   docs/configuration/agent.md 13e4c551b8b0ba47190b4016220e48c3a4c391fb 
>   include/mesos/fetcher/fetcher.proto 
> 6a5d807221681853444ffd3ab42a23827604e550 
>   src/launcher/fetcher.cpp 2f42fa221a42fdc131a8a97ded4e9433ce51ea77 
>   src/slave/constants.hpp f1fc2bfcb9e093ab39a550d8fc7daa8fadee6f64 
>   src/slave/containerizer/fetcher.cpp 
> f9ab55404801e27900dc82316c1ca595fd65b942 
>   src/slave/flags.hpp 949a4783caf8aac9a246a98525a5287b0f8256d8 
>   src/slave/flags.cpp 962b07c1d701f4ab819b14730fbc116b981433bb 
> 
> 
> Diff: https://reviews.apache.org/r/65856/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> Manually tested with Nginx servers with the following cases:
> 1. Sleeps for 59 seconds before serving artifacts (successful)
> 2. Sleeps for 1 mintue before serving artifacts (failed)
> 3. Sleeps for 55 seconds and then serve a 640B artifact with 12 bytes/second 
> (successful)
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66347: Added a test for killing executor during task launch.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [66118, 66119, 66120, 65679, 66126, 66143, 66322, 66144, 
66145, 66178, 66323, 66346, 66347]

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 March 28, 2018, 10:46 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66347/
> ---
> 
> (Updated March 28, 2018, 10:46 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-8617 and MESOS-8624
> https://issues.apache.org/jira/browse/MESOS-8617
> https://issues.apache.org/jira/browse/MESOS-8624
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test verifies that when agent shuts down a running
> executor, launching tasks on the agent that use the same
> executor will be dropped properly.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353 
> 
> 
> Diff: https://reviews.apache.org/r/66347/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 66327: WIP: Added the `LIST_RESOURCE_PROVIDER_CONFIGS` agent API call.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66327 was successfully built and tested.

Reviews applied: `['66318', '66325', '66326', '66327']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66327

- Mesos Reviewbot Windows


On March 28, 2018, 3:22 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66327/
> ---
> 
> (Updated March 28, 2018, 3:22 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James DeFelice, and Jie Yu.
> 
> 
> Bugs: MESOS-8745
> https://issues.apache.org/jira/browse/MESOS-8745
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `LIST_RESOURCE_PROVIDER_CONFIGS` returns a list of all valid resource
> provider configs from the resource provider config directory, except
> for the ones that are placed out-of-band.
> 
> 
> Diffs
> -
> 
>   include/mesos/agent/agent.proto adaccb3509fdfc1e7cac9482e93a91e83bc5625d 
>   include/mesos/v1/agent/agent.proto 71352a79fe7d28d633a4badceafe18086c1e5ab7 
> 
> 
> Diff: https://reviews.apache.org/r/66327/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66343: Added test for difference operator of hashset.

[Jason Lai]

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


Ship it!




Ship It!

- Jason Lai


On March 28, 2018, 6:23 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66343/
> ---
> 
> (Updated March 28, 2018, 6:23 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Chun-Hung Hsiao, and Jason Lai.
> 
> 
> Bugs: MESOS-8746
> https://issues.apache.org/jira/browse/MESOS-8746
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added test for difference operator of hashset.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/hashset_tests.cpp 
> d2b7bf248647c63c00c8c82b8176130c87c50eb4 
> 
> 
> Diff: https://reviews.apache.org/r/66343/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66342: Added difference operator overload for hashset.

[Jason Lai]

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


Ship it!




Ship It!

- Jason Lai


On March 28, 2018, 6:23 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66342/
> ---
> 
> (Updated March 28, 2018, 6:23 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Chun-Hung Hsiao, and Jason Lai.
> 
> 
> Bugs: MESOS-8746
> https://issues.apache.org/jira/browse/MESOS-8746
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added difference operator overload for hashset.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/hashset.hpp 
> 6af209c53185207b53396e7687e3bd7357e57bf1 
> 
> 
> Diff: https://reviews.apache.org/r/66342/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 65876: Enabled `--fetch_stall_timeout` in curl-based URI fetcher plugins.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65876 was successfully built and tested.

Reviews applied: `['65855', '65856', '65876']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65876

- Mesos Reviewbot Windows


On March 28, 2018, 5:07 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65876/
> ---
> 
> (Updated March 28, 2018, 5:07 p.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Bugs: MESOS-8620
> https://issues.apache.org/jira/browse/MESOS-8620
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch passes the `--fetch_stall_timeout` agent flag into
> `DockerFetcherPlugin` (through setting flag `docker_stall_timeout` in
> the Docker store) and `CurlFetcherPlugin` (through setting flag
> `curl_stall_timeout` in the Appc store).
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> c1f9661906eae019e43e1c3df7c28cccad3a6bb0 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> c308bb04773fa81265d91a1247b3db3434b5746b 
>   src/uri/fetchers/curl.hpp 909c2eb450630912c13831c3aa180fbccc8e9e4b 
>   src/uri/fetchers/curl.cpp 43584aaa424dc57f00880d5c89cdb73f23685800 
>   src/uri/fetchers/docker.hpp 6a89c0a69dd6f454cc22d025644900416e55ad50 
>   src/uri/fetchers/docker.cpp 21f8eb15903d505ba1cabe1f77a8f5d7043964d5 
> 
> 
> Diff: https://reviews.apache.org/r/65876/diff/3/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65876: Enabled `--fetch_stall_timeout` in curl-based URI fetcher plugins.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [65855, 65856, 65876]

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 March 29, 2018, 12:07 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65876/
> ---
> 
> (Updated March 29, 2018, 12:07 a.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Bugs: MESOS-8620
> https://issues.apache.org/jira/browse/MESOS-8620
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch passes the `--fetch_stall_timeout` agent flag into
> `DockerFetcherPlugin` (through setting flag `docker_stall_timeout` in
> the Docker store) and `CurlFetcherPlugin` (through setting flag
> `curl_stall_timeout` in the Appc store).
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> c1f9661906eae019e43e1c3df7c28cccad3a6bb0 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> c308bb04773fa81265d91a1247b3db3434b5746b 
>   src/uri/fetchers/curl.hpp 909c2eb450630912c13831c3aa180fbccc8e9e4b 
>   src/uri/fetchers/curl.cpp 43584aaa424dc57f00880d5c89cdb73f23685800 
>   src/uri/fetchers/docker.hpp 6a89c0a69dd6f454cc22d025644900416e55ad50 
>   src/uri/fetchers/docker.cpp 21f8eb15903d505ba1cabe1f77a8f5d7043964d5 
> 
> 
> Diff: https://reviews.apache.org/r/65876/diff/3/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66325: Implemented idempotency for agent resource provider config API calls.

[Chun-Hung Hsiao]

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

(Updated March 29, 2018, 2:10 a.m.)


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


Changes
---

Addressed Jie's comment.


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


Repository: mesos


Description
---

`ADD_RESOURCE_PROVIDER_CONFIG`, `UPDATE_RESOURCE_PROVIDER_CONFIG` and
`REMOVE_RESOURCE_PROVIDER_CONFIG` now return 200 for identical
repetivite calls.


Diffs (updated)
-

  src/resource_provider/daemon.hpp a6d0013fa3645fd2b705351a86679f7fba13e7e3 
  src/resource_provider/daemon.cpp d0a8186da80a1bffbb71f524e639bc7d75ef6243 
  src/resource_provider/storage/provider.cpp 
8db1ce0589be2885cd970f029027fb73c5cbc09f 
  src/slave/http.cpp 65081c95c888a8236aafdfc627a7aa4e9a72b65d 


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

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


Testing
---

make

NOTE: Test `*/AgentResourceProviderConfigApiTest.RemoveNotFound` won't pass 
since the semantics of `REMOVE_RESOURCE_PROVIDER_CONFIG` is changed.
Tests are added in the next patch.


Thanks,

Chun-Hung Hsiao

Re: Review Request 66347: Added a test for killing executor during task launch.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66347 was successfully built and tested.

Reviews applied: `['66118', '66119', '66120', '65679', '66126', '66143', 
'66322', '66144', '66145', '66178', '66323', '66346', '66347']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66347

- Mesos Reviewbot Windows


On March 28, 2018, 10:46 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66347/
> ---
> 
> (Updated March 28, 2018, 10:46 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-8617 and MESOS-8624
> https://issues.apache.org/jira/browse/MESOS-8617
> https://issues.apache.org/jira/browse/MESOS-8624
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test verifies that when agent shuts down a running
> executor, launching tasks on the agent that use the same
> executor will be dropped properly.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353 
> 
> 
> Diff: https://reviews.apache.org/r/66347/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 66326: Added tests for agent resource provider API idempotency.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On March 28, 2018, 3:09 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66326/
> ---
> 
> (Updated March 28, 2018, 3:09 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James DeFelice, and Jie Yu.
> 
> 
> Bugs: MESOS-8742
> https://issues.apache.org/jira/browse/MESOS-8742
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patche adds tests to verify that adding, updating and removing the
> same resource provider config will return 200 OK without triggering any
> resource provider launch/termination.
> 
> 
> Diffs
> -
> 
>   src/tests/agent_resource_provider_config_api_tests.cpp 
> 53b8cb8f9463dec5fa684e64c61a3ad5b6c88104 
> 
> 
> Diff: https://reviews.apache.org/r/66326/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66323: Added tests for failed task launch on agent.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66323 was successfully built and tested.

Reviews applied: `['66118', '66119', '66120', '65679', '66126', '66143', 
'66322', '66144', '66145', '66178', '66323']`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66323

- Mesos Reviewbot Windows


On March 28, 2018, 8:30 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66323/
> ---
> 
> (Updated March 28, 2018, 8:30 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-8617 and MESOS-8624
> https://issues.apache.org/jira/browse/MESOS-8617
> https://issues.apache.org/jira/browse/MESOS-8624
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test verifies the agent behavior of launching
> two task(group)s using the same executor. When both
> tasks are launching on the agent (before creating
> any executor), if the first task (in the agent receiving
> order) fails to launch, the later task will get dropped.
> If the later task fails to launch, the first task
> should still launch successfully.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353 
> 
> 
> Diff: https://reviews.apache.org/r/66323/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 65876: Enabled `--fetch_stall_timeout` in curl-based URI fetcher plugins.

[Chun-Hung Hsiao]

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

(Updated March 29, 2018, 12:07 a.m.)


Review request for mesos and Gilbert Song.


Changes
---

Updated comments to call out the default low speed limit for curl is 1b/sec.


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


Repository: mesos


Description
---

This patch passes the `--fetch_stall_timeout` agent flag into
`DockerFetcherPlugin` (through setting flag `docker_stall_timeout` in
the Docker store) and `CurlFetcherPlugin` (through setting flag
`curl_stall_timeout` in the Appc store).


Diffs (updated)
-

  src/slave/containerizer/mesos/provisioner/appc/store.cpp 
c1f9661906eae019e43e1c3df7c28cccad3a6bb0 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
c308bb04773fa81265d91a1247b3db3434b5746b 
  src/uri/fetchers/curl.hpp 909c2eb450630912c13831c3aa180fbccc8e9e4b 
  src/uri/fetchers/curl.cpp 43584aaa424dc57f00880d5c89cdb73f23685800 
  src/uri/fetchers/docker.hpp 6a89c0a69dd6f454cc22d025644900416e55ad50 
  src/uri/fetchers/docker.cpp 21f8eb15903d505ba1cabe1f77a8f5d7043964d5 


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

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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 65856: Added `--fetcher_stall_timeout` to abort stalled artifact fetching.

[Chun-Hung Hsiao]

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

(Updated March 28, 2018, 11:51 p.m.)


Review request for mesos and Gilbert Song.


Changes
---

Addressed Gilbert's and James' comment.


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


Repository: mesos


Description (updated)
---

This flag specifies a timeout for `mesos-fetcher` to wait before
aborting if the download speed keeps below 1 bytes/sec. This would avoid
containers to get stuck at FETCHING.


Diffs (updated)
-

  docs/configuration/agent.md 13e4c551b8b0ba47190b4016220e48c3a4c391fb 
  include/mesos/fetcher/fetcher.proto 6a5d807221681853444ffd3ab42a23827604e550 
  src/launcher/fetcher.cpp 2f42fa221a42fdc131a8a97ded4e9433ce51ea77 
  src/slave/constants.hpp f1fc2bfcb9e093ab39a550d8fc7daa8fadee6f64 
  src/slave/containerizer/fetcher.cpp f9ab55404801e27900dc82316c1ca595fd65b942 
  src/slave/flags.hpp 949a4783caf8aac9a246a98525a5287b0f8256d8 
  src/slave/flags.cpp 962b07c1d701f4ab819b14730fbc116b981433bb 


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

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


Testing
---

sudo make check

Manually tested with Nginx servers with the following cases:
1. Sleeps for 59 seconds before serving artifacts (successful)
2. Sleeps for 1 mintue before serving artifacts (failed)
3. Sleeps for 55 seconds and then serve a 640B artifact with 12 bytes/second 
(successful)


Thanks,

Chun-Hung Hsiao

Re: Review Request 65876: Enabled `--fetch_stall_timeout` in curl-based URI fetcher plugins.

[Chun-Hung Hsiao]

> On March 22, 2018, 11:08 p.m., Gilbert Song wrote:
> >

As we discussed, the motivation to remove the default values is to make it 
explicit at the call site to improve readability.


- Chun-Hung


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


On March 2, 2018, 7:20 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65876/
> ---
> 
> (Updated March 2, 2018, 7:20 p.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Bugs: MESOS-8620
> https://issues.apache.org/jira/browse/MESOS-8620
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch passes the `--fetch_stall_timeout` agent flag into
> `DockerFetcherPlugin` (through setting flag `docker_stall_timeout` in
> the Docker store) and `CurlFetcherPlugin` (through setting flag
> `curl_stall_timeout` in the Appc store).
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/appc/store.cpp 
> c1f9661906eae019e43e1c3df7c28cccad3a6bb0 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> c308bb04773fa81265d91a1247b3db3434b5746b 
>   src/uri/fetchers/curl.hpp 909c2eb450630912c13831c3aa180fbccc8e9e4b 
>   src/uri/fetchers/curl.cpp 43584aaa424dc57f00880d5c89cdb73f23685800 
>   src/uri/fetchers/docker.hpp 6a89c0a69dd6f454cc22d025644900416e55ad50 
>   src/uri/fetchers/docker.cpp 21f8eb15903d505ba1cabe1f77a8f5d7043964d5 
> 
> 
> Diff: https://reviews.apache.org/r/65876/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66325: Implemented idempotency for agent resource provider config API calls.

[Jie Yu]

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


Fix it, then Ship it!





src/resource_provider/daemon.cpp
Line 274 (original), 285 (patched)


Why we need to return `bool`?


- Jie Yu


On March 28, 2018, 10:20 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66325/
> ---
> 
> (Updated March 28, 2018, 10:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James DeFelice, and Jie Yu.
> 
> 
> Bugs: MESOS-8742
> https://issues.apache.org/jira/browse/MESOS-8742
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `ADD_RESOURCE_PROVIDER_CONFIG`, `UPDATE_RESOURCE_PROVIDER_CONFIG` and
> `REMOVE_RESOURCE_PROVIDER_CONFIG` now return 200 for identical
> repetivite calls.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/daemon.cpp d0a8186da80a1bffbb71f524e639bc7d75ef6243 
>   src/resource_provider/storage/provider.cpp 
> 8db1ce0589be2885cd970f029027fb73c5cbc09f 
>   src/slave/http.cpp 65081c95c888a8236aafdfc627a7aa4e9a72b65d 
> 
> 
> Diff: https://reviews.apache.org/r/66325/diff/2/
> 
> 
> Testing
> ---
> 
> make
> 
> NOTE: Test `*/AgentResourceProviderConfigApiTest.RemoveNotFound` won't pass 
> since the semantics of `REMOVE_RESOURCE_PROVIDER_CONFIG` is changed.
> Tests are added in the next patch.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66318: Made agent resource provider config API calls idempotent.

[Jie Yu]

> On March 28, 2018, 2:36 a.m., James DeFelice wrote:
> > include/mesos/agent/agent.proto
> > Line 332 (original), 338 (patched)
> > 
> >
> > Maybe return 409 instead of 404 here, if the caller tries to update a 
> > config that does not exist? Because, you know, 404s can be misleading given 
> > the convo we had earlier...
> 
> Chun-Hung Hsiao wrote:
> Yeah I was aware of this issue and discussed with Jie but we still 
> thought 404 is semantically more correct here. Will probably raise a general 
> question related to this on the next API WG.

I think 409 is not the right code for this case. The best code is 404. I think 
we probably need to fix libprocess to return 50x during initialization.


- Jie


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


On March 28, 2018, 3:22 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66318/
> ---
> 
> (Updated March 28, 2018, 3:22 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James DeFelice, and Jie Yu.
> 
> 
> Bugs: MESOS-8742
> https://issues.apache.org/jira/browse/MESOS-8742
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds descriptions to declare the following agent API calls
> idempotent:
>   - `ADD_RESOURCE_PROVIDER_CONFIG`
>   - `UPDATE_RESOURCE_PROVIDER_CONFIG`
>   - `REMOVE_RESOURCE_PROVIDER_CONFIG`
> 
> 
> Diffs
> -
> 
>   include/mesos/agent/agent.proto adaccb3509fdfc1e7cac9482e93a91e83bc5625d 
>   include/mesos/v1/agent/agent.proto 71352a79fe7d28d633a4badceafe18086c1e5ab7 
> 
> 
> Diff: https://reviews.apache.org/r/66318/diff/2/
> 
> 
> Testing
> ---
> 
> N/A
> 
> The actual implementation will be in the next patch.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66318: Made agent resource provider config API calls idempotent.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On March 28, 2018, 3:22 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66318/
> ---
> 
> (Updated March 28, 2018, 3:22 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James DeFelice, and Jie Yu.
> 
> 
> Bugs: MESOS-8742
> https://issues.apache.org/jira/browse/MESOS-8742
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds descriptions to declare the following agent API calls
> idempotent:
>   - `ADD_RESOURCE_PROVIDER_CONFIG`
>   - `UPDATE_RESOURCE_PROVIDER_CONFIG`
>   - `REMOVE_RESOURCE_PROVIDER_CONFIG`
> 
> 
> Diffs
> -
> 
>   include/mesos/agent/agent.proto adaccb3509fdfc1e7cac9482e93a91e83bc5625d 
>   include/mesos/v1/agent/agent.proto 71352a79fe7d28d633a4badceafe18086c1e5ab7 
> 
> 
> Diff: https://reviews.apache.org/r/66318/diff/2/
> 
> 
> Testing
> ---
> 
> N/A
> 
> The actual implementation will be in the next patch.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66344: Supported non-speculative operations on agent default resources.

[Greg Mann]

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



- Greg Mann


On March 28, 2018, 9:41 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66344/
> ---
> 
> (Updated March 28, 2018, 9:41 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We use `getResourceConversions` to calculate `conversions`,
> apply that to agent default resources if not on resource provider,
> and send `UpdateOperationStatusMessage` message back to master.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp e76daabad0d2d68aa42d1da809d4a23459eaaacb 
> 
> 
> Diff: https://reviews.apache.org/r/66344/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66344: Supported non-speculative operations on agent default resources.

[Greg Mann]

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




src/slave/slave.cpp
Line 4311 (original), 4308 (patched)


s/Calculated/Calculate/



src/slave/slave.cpp
Lines 4314 (patched)


Perhaps we should leave a comment here explaining the validity of this 
CHECK? Something like "Operation validation guarantees that we can generate 
resource conversions from all operations on agent default resources."

Currently, we rely on the fact that operations like CREATE_VOLUME (which 
cannot be used to generate resource conversions) are invalidated if they have a 
resource provider ID set. It might be nice to validate this case more 
explicitly in the future, but since we have separate validation functions for 
each operation type that would be cumbersome right now.



src/slave/slave.cpp
Lines 4321-4323 (patched)


Indented too far.



src/slave/slave.cpp
Lines 4339 (patched)


s/updateOperation/`updateOperation`/
s/operation/operations/


- Greg Mann


On March 28, 2018, 9:41 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66344/
> ---
> 
> (Updated March 28, 2018, 9:41 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We use `getResourceConversions` to calculate `conversions`,
> apply that to agent default resources if not on resource provider,
> and send `UpdateOperationStatusMessage` message back to master.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp e76daabad0d2d68aa42d1da809d4a23459eaaacb 
> 
> 
> Diff: https://reviews.apache.org/r/66344/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66344: Supported non-speculative operations on agent default resources.

[Greg Mann]

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




src/slave/slave.cpp
Line 4311 (original), 4308 (patched)


s/Calculated/Calculate/



src/slave/slave.cpp
Lines 4314 (patched)


Perhaps we should leave a comment here explaining the validity of this 
CHECK? Something like "Operation validation guarantees that we can generate 
resource conversions from all operations on agent default resources."

Currently, we rely on the fact that operations like CREATE_VOLUME (which 
cannot be used to generate resource conversions) are invalidated if they have a 
resource provider ID set. It might be nice to validate this case more 
explicitly in the future, but since we have separate validation functions for 
each operation type that would be cumbersome right now.



src/slave/slave.cpp
Lines 4321-4323 (patched)


Indented too far.



src/slave/slave.cpp
Lines 4339 (patched)


s/updateOperation/`updateOperation`/
s/operation/operations/


- Greg Mann


On March 28, 2018, 9:41 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66344/
> ---
> 
> (Updated March 28, 2018, 9:41 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We use `getResourceConversions` to calculate `conversions`,
> apply that to agent default resources if not on resource provider,
> and send `UpdateOperationStatusMessage` message back to master.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp e76daabad0d2d68aa42d1da809d4a23459eaaacb 
> 
> 
> Diff: https://reviews.apache.org/r/66344/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Review Request 66347: Added a test for killing executor during task launch.

[Meng Zhu]

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

Review request for mesos and Greg Mann.


Bugs: MESOS-8617 and MESOS-8624
https://issues.apache.org/jira/browse/MESOS-8617
https://issues.apache.org/jira/browse/MESOS-8624


Repository: mesos


Description
---

This test verifies that when agent shuts down a running
executor, launching tasks on the agent that use the same
executor will be dropped properly.


Diffs
-

  src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353 


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


Testing
---

make check


Thanks,

Meng Zhu

Review Request 66346: Added two matchers for TaskInfo and TaskGroup.

[Meng Zhu]

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

Review request for mesos and Greg Mann.


Repository: mesos


Description
---

Added a matcher to match the taskID of `Option`.
Added a matcher to match an `Option` which
contains a task with the specified taskID.


Diffs
-

  src/tests/mesos.hpp 46c271b5c5bedbdabd58b3cdbb82216d55c846bd 


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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 66211: Added the fields `ExposedPorts` and `Volumes` into Docker v1 image spec.

[Gilbert Song]

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



We were planning to land these patches when we need them. For example, the 
volume defined in docker images may not fit in Mesos' model.

- Gilbert Song


On March 22, 2018, 2:47 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66211/
> ---
> 
> (Updated March 22, 2018, 2:47 a.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Bugs: MESOS-8718
> https://issues.apache.org/jira/browse/MESOS-8718
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added the fields `ExposedPorts` and `Volumes` into Docker v1 image spec.
> 
> 
> Diffs
> -
> 
>   include/mesos/docker/v1.proto 338675490b7e624196bbb745173daa028a13280c 
> 
> 
> Diff: https://reviews.apache.org/r/66211/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 66207: Used native protobuf map in OCI v1 image spec.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On March 21, 2018, 11:59 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66207/
> ---
> 
> (Updated March 21, 2018, 11:59 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Chun-Hung Hsiao, and Gilbert Song.
> 
> 
> Bugs: MESOS-8702
> https://issues.apache.org/jira/browse/MESOS-8702
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used native protobuf map in OCI v1 image spec.
> 
> 
> Diffs
> -
> 
>   include/mesos/oci/spec.proto f7f197909ebd5517941e063d8c80b0a7f745bb8d 
>   src/oci/spec.cpp ad5b8bea54e000752c681a9bfd900b25a4158544 
>   src/tests/containerizer/oci_spec_tests.cpp 
> f9a98f635327eb0896f8d9719c8be91e5b5c69d3 
> 
> 
> Diff: https://reviews.apache.org/r/66207/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 66327: WIP: Added the `LIST_RESOURCE_PROVIDER_CONFIGS` agent API call.

[Chun-Hung Hsiao]

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

(Updated March 28, 2018, 10:22 p.m.)


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


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


Repository: mesos


Description
---

`LIST_RESOURCE_PROVIDER_CONFIGS` returns a list of all valid resource
provider configs from the resource provider config directory, except
for the ones that are placed out-of-band.


Diffs (updated)
-

  include/mesos/agent/agent.proto adaccb3509fdfc1e7cac9482e93a91e83bc5625d 
  include/mesos/v1/agent/agent.proto 71352a79fe7d28d633a4badceafe18086c1e5ab7 


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

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


Testing
---


Thanks,

Chun-Hung Hsiao

Re: Review Request 66325: Implemented idempotency for agent resource provider config API calls.

[Chun-Hung Hsiao]

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

(Updated March 28, 2018, 10:20 p.m.)


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


Changes
---

Added validations for `info.id` and made the quoting marks in error messages 
consistent.


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


Repository: mesos


Description
---

`ADD_RESOURCE_PROVIDER_CONFIG`, `UPDATE_RESOURCE_PROVIDER_CONFIG` and
`REMOVE_RESOURCE_PROVIDER_CONFIG` now return 200 for identical
repetivite calls.


Diffs (updated)
-

  src/resource_provider/daemon.cpp d0a8186da80a1bffbb71f524e639bc7d75ef6243 
  src/resource_provider/storage/provider.cpp 
8db1ce0589be2885cd970f029027fb73c5cbc09f 
  src/slave/http.cpp 65081c95c888a8236aafdfc627a7aa4e9a72b65d 


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

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


Testing
---

make

NOTE: Test `*/AgentResourceProviderConfigApiTest.RemoveNotFound` won't pass 
since the semantics of `REMOVE_RESOURCE_PROVIDER_CONFIG` is changed.
Tests are added in the next patch.


Thanks,

Chun-Hung Hsiao

Re: Review Request 66206: Used native protobuf map in Docker v1 image spec.

[Gilbert Song]

> On March 22, 2018, 11:39 a.m., Chun-Hung Hsiao wrote:
> > include/mesos/docker/v1.proto
> > Line 67 (original), 57 (patched)
> > 
> >
> > Since this is a public proto file, is there any endpoint or API that 
> > enables a user to directly use this proto message? If yes, then this seems 
> > a breaking change since the user may be providing the following json:
> > ```
> > {
> >   "labels": [
> > {
> >   "key": "SOME_KEY",
> >   "value": "SOME_VALUE"
> > }
> >   ]
> > }
> > ```
> 
> Qian Zhang wrote:
> It is used to parse the image manifest of the Docker image pulled by 
> Docker image store in UCR, so I do not think the user will use it directly. 
> And based on Docker v1 image spec, the field name is `Labels` rather than 
> `labels` which is just our own workaround in Mesos code, so I think it should 
> not be possible for a Docker image manifest having labels like below:
> ```
> "labels": [
> {
>   "key": "SOME_KEY",
>   "value": "SOME_VALUE"
> }
>   ]
> ```

@Chun, this is not a publi facing API. Should be fine.


- Gilbert


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


On March 21, 2018, 11:56 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66206/
> ---
> 
> (Updated March 21, 2018, 11:56 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Chun-Hung Hsiao, and Gilbert Song.
> 
> 
> Bugs: MESOS-8702
> https://issues.apache.org/jira/browse/MESOS-8702
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used native protobuf map in Docker v1 image spec.
> 
> 
> Diffs
> -
> 
>   include/mesos/docker/v1.proto 338675490b7e624196bbb745173daa028a13280c 
>   src/docker/spec.cpp 15224f90f965079ff83dfedff0182a97a5ff0360 
>   src/slave/containerizer/mesos/isolators/gpu/volume.cpp 
> 536a3c711faf3c165bebaaa3c92717737d67b6f3 
>   src/tests/containerizer/docker_spec_tests.cpp 
> 5fde49a495c2a2fa880e7cb1bae2f636edecd480 
> 
> 
> Diff: https://reviews.apache.org/r/66206/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 66049: Added offer operation to grow and shrink persistent volumes.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On March 28, 2018, 6:24 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66049/
> ---
> 
> (Updated March 28, 2018, 6:24 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added offer operation to grow and shrink persistent volumes.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 676f0b090cad7ebf59eb32556f17ff8b5f247907 
>   include/mesos/v1/mesos.proto 10d506517c9f098374ab0c8f4dcfda42e1896c95 
> 
> 
> Diff: https://reviews.apache.org/r/66049/diff/5/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66206: Used native protobuf map in Docker v1 image spec.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On March 21, 2018, 11:56 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66206/
> ---
> 
> (Updated March 21, 2018, 11:56 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Chun-Hung Hsiao, and Gilbert Song.
> 
> 
> Bugs: MESOS-8702
> https://issues.apache.org/jira/browse/MESOS-8702
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used native protobuf map in Docker v1 image spec.
> 
> 
> Diffs
> -
> 
>   include/mesos/docker/v1.proto 338675490b7e624196bbb745173daa028a13280c 
>   src/docker/spec.cpp 15224f90f965079ff83dfedff0182a97a5ff0360 
>   src/slave/containerizer/mesos/isolators/gpu/volume.cpp 
> 536a3c711faf3c165bebaaa3c92717737d67b6f3 
>   src/tests/containerizer/docker_spec_tests.cpp 
> 5fde49a495c2a2fa880e7cb1bae2f636edecd480 
> 
> 
> Diff: https://reviews.apache.org/r/66206/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 66344: Supported non-speculative operations on agent default resources.

[Zhitao Li]

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

(Updated March 28, 2018, 2:41 p.m.)


Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.


Changes
---

Update logging and summary.


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


Repository: mesos


Description (updated)
---

We use `getResourceConversions` to calculate `conversions`,
apply that to agent default resources if not on resource provider,
and send `UpdateOperationStatusMessage` message back to master.


Diffs (updated)
-

  src/slave/slave.cpp e76daabad0d2d68aa42d1da809d4a23459eaaacb 


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

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66344: Supported non-speculative operations on agent default resources.

[Zhitao Li]

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

(Updated March 28, 2018, 2:29 p.m.)


Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.


Changes
---

Review comments from https://reviews.apache.org/r/66050/#comment280724


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


Repository: mesos


Description
---

Supported non-speculative operations on agent default resources.


Diffs (updated)
-

  src/slave/slave.cpp e76daabad0d2d68aa42d1da809d4a23459eaaacb 


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

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66050: Implemented grow and shrink of persistent volumes.

[Zhitao Li]

> On March 27, 2018, 3:24 p.m., Greg Mann wrote:
> > src/slave/slave.cpp
> > Lines 8054-8056 (patched)
> > 
> >
> > Looks like we will now run `checkpointResources()` on resource provider 
> > resources which are contained in a speculative operation - this will 
> > violate a check in the agent: 
> > https://github.com/apache/mesos/blob/21e4b45b388d0b272236b1c58313569f8a1d1fc8/src/slave/slave.cpp#L4088
> 
> Chun-Hung Hsiao wrote:
> I don't think this is true since `needCheckpointing` filters out 
> resources with providers: 
> https://github.com/apache/mesos/blob/master/src/common/resources_utils.cpp#L30

I'm going to only checkpoint agent part when upon `else` of 
`resourceProviderId.isSome()` check below.


- Zhitao


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


On March 28, 2018, 11:26 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66050/
> ---
> 
> (Updated March 28, 2018, 11:26 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new offer operations are implemented as non-speculative operations,
> which means they cannot be chained with previous offer operations which
> depend on results of each other.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.cpp 141a444534b776a2c90e2a0daf9727cd21e39080 
>   src/common/resources_utils.cpp 9be01c1abd48264e308960f35cc7c2ee8a367518 
>   src/master/master.cpp 20692c36c021fdae91de0b156f26fc56cf7c4f45 
>   src/master/validation.hpp 7c129ceb929596acbb64d37025e055661277e6bf 
>   src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 
>   src/tests/mesos.hpp 46c271b5c5bedbdabd58b3cdbb82216d55c846bd 
>   src/tests/persistent_volume_tests.cpp 
> 4edf781711d9efdb994114aeb6289b6af750b87a 
>   src/tests/reservation_tests.cpp 5570df2e0b208512d0a0a3079a180e1acfe08f3d 
> 
> 
> Diff: https://reviews.apache.org/r/66050/diff/7/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66050: Implemented grow and shrink of persistent volumes.

[Zhitao Li]

> On March 26, 2018, 5:37 p.m., Chun-Hung Hsiao wrote:
> > src/slave/slave.cpp
> > Lines 8053-8059 (patched)
> > 
> >
> > If we checkpoint resources after sending status updates, then say if 
> > the agent crashes before checkpointing, the master would have assumed that 
> > the operation is completed but the agent will recover to its old states. So 
> > let's checkpoint resources before sending status updates, as I suggested 
> > above.

That's already the case.


- Zhitao


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


On March 28, 2018, 11:26 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66050/
> ---
> 
> (Updated March 28, 2018, 11:26 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new offer operations are implemented as non-speculative operations,
> which means they cannot be chained with previous offer operations which
> depend on results of each other.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.cpp 141a444534b776a2c90e2a0daf9727cd21e39080 
>   src/common/resources_utils.cpp 9be01c1abd48264e308960f35cc7c2ee8a367518 
>   src/master/master.cpp 20692c36c021fdae91de0b156f26fc56cf7c4f45 
>   src/master/validation.hpp 7c129ceb929596acbb64d37025e055661277e6bf 
>   src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 
>   src/tests/mesos.hpp 46c271b5c5bedbdabd58b3cdbb82216d55c846bd 
>   src/tests/persistent_volume_tests.cpp 
> 4edf781711d9efdb994114aeb6289b6af750b87a 
>   src/tests/reservation_tests.cpp 5570df2e0b208512d0a0a3079a180e1acfe08f3d 
> 
> 
> Diff: https://reviews.apache.org/r/66050/diff/7/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66050: Implemented grow and shrink of persistent volumes.

[Zhitao Li]

> On March 26, 2018, 5:37 p.m., Chun-Hung Hsiao wrote:
> > src/slave/slave.cpp
> > Lines 4292-4324 (original), 4292-4342 (patched)
> > 
> >
> > Let's do some refactoring here. There are 3 cases:
> > 
> > 1. For speculative RP operations, the agent needs to apply them so it 
> > can proceed, but the status update is delegated to RP themselves so no need 
> > to generate state updates here.
> > 2. For non-speculative RP operations, the agent can completely delegate 
> > the handling to RP.
> > 3. For non-RP operations, the agent needs to apply, checkpoint, and 
> > update the statuses.
> > 
> > One option I can think of for refactoring is:
> > 1. Make `apply` handle `GROW_VOLUME` and `SHRINK_VOLUME` and return an 
> > `OperationStatus`. For speculative operation, do the actual resource change 
> > and return an update with an empty conversion; for non-speculative 
> > operation, return one with the actual conversion.
> > 2. Do the following here:
> > ```
> > if (resourceProviderId.isSome()) {
> >   if (protobuf::isSpeculativeOperation(operation.info())) {
> > apply(operation);
> >   }
> >   return resourceProviderManager.applyOperation(message);
> > }
> > 
> > const OperationStatus status = apply(operation);
> > 
> > ... // The checkpointing logic.
> > 
> > UpdateOperationStatusMessage update =
> >   protobuf::createUpdateOperationStatusMessage(... status ...);
> >   
> > updateOperation(operation, update);
> > ```
> > Other thoughts on refactoring/improving the readability are also 
> > welcome.
> > 
> > Also, the current agent code doesn't checkpoint operations, which might 
> > cause some reconciliation problem. But I think we can fix this later, just 
> > need to think through the potential problems first.

I'm separating this to a different patch. After we confirm the logic is sound, 
let's discuss refactoring.


- Zhitao


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


On March 28, 2018, 11:26 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66050/
> ---
> 
> (Updated March 28, 2018, 11:26 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new offer operations are implemented as non-speculative operations,
> which means they cannot be chained with previous offer operations which
> depend on results of each other.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.cpp 141a444534b776a2c90e2a0daf9727cd21e39080 
>   src/common/resources_utils.cpp 9be01c1abd48264e308960f35cc7c2ee8a367518 
>   src/master/master.cpp 20692c36c021fdae91de0b156f26fc56cf7c4f45 
>   src/master/validation.hpp 7c129ceb929596acbb64d37025e055661277e6bf 
>   src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 
>   src/tests/mesos.hpp 46c271b5c5bedbdabd58b3cdbb82216d55c846bd 
>   src/tests/persistent_volume_tests.cpp 
> 4edf781711d9efdb994114aeb6289b6af750b87a 
>   src/tests/reservation_tests.cpp 5570df2e0b208512d0a0a3079a180e1acfe08f3d 
> 
> 
> Diff: https://reviews.apache.org/r/66050/diff/7/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66050: Implemented grow and shrink of persistent volumes.

[Chun-Hung Hsiao]

> On March 27, 2018, 10:24 p.m., Greg Mann wrote:
> > src/slave/slave.cpp
> > Lines 8054-8056 (patched)
> > 
> >
> > Looks like we will now run `checkpointResources()` on resource provider 
> > resources which are contained in a speculative operation - this will 
> > violate a check in the agent: 
> > https://github.com/apache/mesos/blob/21e4b45b388d0b272236b1c58313569f8a1d1fc8/src/slave/slave.cpp#L4088

I don't think this is true since `needCheckpointing` filters out resources with 
providers: 
https://github.com/apache/mesos/blob/master/src/common/resources_utils.cpp#L30


- Chun-Hung


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


On March 28, 2018, 6:26 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66050/
> ---
> 
> (Updated March 28, 2018, 6:26 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new offer operations are implemented as non-speculative operations,
> which means they cannot be chained with previous offer operations which
> depend on results of each other.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.cpp 141a444534b776a2c90e2a0daf9727cd21e39080 
>   src/common/resources_utils.cpp 9be01c1abd48264e308960f35cc7c2ee8a367518 
>   src/master/master.cpp 20692c36c021fdae91de0b156f26fc56cf7c4f45 
>   src/master/validation.hpp 7c129ceb929596acbb64d37025e055661277e6bf 
>   src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 
>   src/tests/mesos.hpp 46c271b5c5bedbdabd58b3cdbb82216d55c846bd 
>   src/tests/persistent_volume_tests.cpp 
> 4edf781711d9efdb994114aeb6289b6af750b87a 
>   src/tests/reservation_tests.cpp 5570df2e0b208512d0a0a3079a180e1acfe08f3d 
> 
> 
> Diff: https://reviews.apache.org/r/66050/diff/7/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Review Request 66323: Added tests for failed task launch on agent.

[Meng Zhu]

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

Review request for mesos and Greg Mann.


Bugs: MESOS-8617 and MESOS-8624
https://issues.apache.org/jira/browse/MESOS-8617
https://issues.apache.org/jira/browse/MESOS-8624


Repository: mesos


Description
---

This test verifies the agent behavior of launching
two task(group)s using the same executor. When both
tasks are launching on the agent (before creating
any executor), if the first task (in the agent receiving
order) fails to launch, the later task will get dropped.
If the later task fails to launch, the first task
should still launch successfully.


Diffs
-

  src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353 


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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 66050: Implemented grow and shrink of persistent volumes.

[Zhitao Li]

> On March 27, 2018, 12:14 p.m., Greg Mann wrote:
> > src/common/resources_utils.cpp
> > Lines 203-205 (patched)
> > 
> >
> > This is already done in the validation code - do we need to do it again 
> > here? Ditto below.
> 
> Chun-Hung Hsiao wrote:
> IMO from the perspective of designing a utility function, it seems 
> appropriate to me to return an error here, and we do a CHECK at the call 
> site. It is also consistent with other operations such as `LAUNCH`.

I agreew with Chun on this one: if someone incorrectly used this function for a 
volume on RP, this check can still catch the issue.


- Zhitao


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


On March 28, 2018, 11:26 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66050/
> ---
> 
> (Updated March 28, 2018, 11:26 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new offer operations are implemented as non-speculative operations,
> which means they cannot be chained with previous offer operations which
> depend on results of each other.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.cpp 141a444534b776a2c90e2a0daf9727cd21e39080 
>   src/common/resources_utils.cpp 9be01c1abd48264e308960f35cc7c2ee8a367518 
>   src/master/master.cpp 20692c36c021fdae91de0b156f26fc56cf7c4f45 
>   src/master/validation.hpp 7c129ceb929596acbb64d37025e055661277e6bf 
>   src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 
>   src/tests/mesos.hpp 46c271b5c5bedbdabd58b3cdbb82216d55c846bd 
>   src/tests/persistent_volume_tests.cpp 
> 4edf781711d9efdb994114aeb6289b6af750b87a 
>   src/tests/reservation_tests.cpp 5570df2e0b208512d0a0a3079a180e1acfe08f3d 
> 
> 
> Diff: https://reviews.apache.org/r/66050/diff/7/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 65350: Modified `cgroups::prepare` to check nested cgroups support only once.

[Gilbert Song]

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



Thanks for all efforts on this issue, Andrei. However, adding a mutex lock does 
not seem a clean fix to me. Please see my comment for details 
https://issues.apache.org/jira/browse/MESOS-8489?focusedCommentId=16417999&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16417999

- Gilbert Song


On March 12, 2018, 7:50 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65350/
> ---
> 
> (Updated March 12, 2018, 7:50 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Gilbert Song, Jie Yu, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-8489
> https://issues.apache.org/jira/browse/MESOS-8489
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch modifies `cgroup::prepare` function to save the results of
> a test for a nested cgroups support in a hashmap to reuse it in the
> future.
> 
> Besides optimization, this patch fixes flaky
> `LinuxCapabilitiesIsolatorFlagsTest.ROOT_IsolatorFlags` test. There
> was a race condition between two starting agents, where the first
> agent could detect `test` orphaned container while iterating over
> `/sys/fs/cgroup/freezer/mesos/` cgroup, because the second agent
> is calling `cgroup::prepare` during its initialization. This patch
> fixes the test, because it won't create a `test` cgroup in
> `cgroups::prepare` as this test has already been passed during the
> initialization of the first agent and the result is stored in hashmap.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp 682b288da8d56a623ef32cf6f0beecd9fc327622 
> 
> 
> Diff: https://reviews.apache.org/r/65350/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 25)
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 66255: Temporarily disabled CSI proto compilation when gRPC is disabled.

[Zhitao Li]

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


Ship it!




Ship It!

- Zhitao Li


On March 23, 2018, 1:44 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66255/
> ---
> 
> (Updated March 23, 2018, 1:44 p.m.)
> 
> 
> Review request for mesos, Benno Evers, Jie Yu, James Peach, Kapil Arya, and 
> Zhitao Li.
> 
> 
> Bugs: MESOS-8724
> https://issues.apache.org/jira/browse/MESOS-8724
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `major` and `minor` macros defined on some systems conflict with
> field names in the CSI spec proto, so its compilation is temporarily
> disabled for now until CSI is bumped to 0.2, where those field are
> removed.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt 3b04e6353979c05a38f1b032704dce566d8fd561 
>   src/Makefile.am 56ce55480553c08450830987f217d0abedd5b2b8 
>   src/slave/flags.cpp dd8dfb7a8a9f7c6030939c9eea841eb47deadfc4 
>   src/slave/slave.cpp e76daabad0d2d68aa42d1da809d4a23459eaaacb 
> 
> 
> Diff: https://reviews.apache.org/r/66255/diff/2/
> 
> 
> Testing
> ---
> 
> `make check` on CentOS (w/ and w/o gRPC) and Mac (w/o gRPC)
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66144: Enforced task launch order on the agent.

[Meng Zhu]

> On March 23, 2018, 1:12 a.m., Greg Mann wrote:
> > src/slave/slave.cpp
> > Lines 2906-2910 (patched)
> > 
> >
> > In this case, the executor struct exists - do we really want to erase 
> > the sequence here?

ah, thanks for catching this!


- Meng


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


On March 28, 2018, 12:05 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66144/
> ---
> 
> (Updated March 28, 2018, 12:05 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-8617 and MESOS-8624
> https://issues.apache.org/jira/browse/MESOS-8617
> https://issues.apache.org/jira/browse/MESOS-8624
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Up until now, Mesos does not guarantee in-order
> task launch on the agent. There are two asynchronous
> steps (unschedule GC and task authorization) in the
> agent task launch path. Depending on the CPU scheduling
> order, a later task launch may finish these two steps earlier
> than its predecessors and get to the launch executor stage
> earlier, resulting in out-of-order task delivery.
> 
> This patch enforces the task delivery order by sequencing
> task launch after the two asynchronous steps, specifically
> right before entering `__run()`.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb 
>   src/slave/slave.cpp e76daabad0d2d68aa42d1da809d4a23459eaaacb 
> 
> 
> Diff: https://reviews.apache.org/r/66144/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 66144: Enforced task launch order on the agent.

[Meng Zhu]

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

(Updated March 28, 2018, 12:05 p.m.)


Review request for mesos, Chun-Hung Hsiao and Greg Mann.


Bugs: MESOS-8617 and MESOS-8624
https://issues.apache.org/jira/browse/MESOS-8617
https://issues.apache.org/jira/browse/MESOS-8624


Repository: mesos


Description
---

Up until now, Mesos does not guarantee in-order
task launch on the agent. There are two asynchronous
steps (unschedule GC and task authorization) in the
agent task launch path. Depending on the CPU scheduling
order, a later task launch may finish these two steps earlier
than its predecessors and get to the launch executor stage
earlier, resulting in out-of-order task delivery.

This patch enforces the task delivery order by sequencing
task launch after the two asynchronous steps, specifically
right before entering `__run()`.


Diffs (updated)
-

  src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb 
  src/slave/slave.cpp e76daabad0d2d68aa42d1da809d4a23459eaaacb 


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

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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 66220: Added test for `GROW_VOLUME` and `SHRINK_VOLUME` operations.

[Zhitao Li]

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

(Updated March 28, 2018, 11:29 a.m.)


Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.


Changes
---

Rebase and verify content on volume remains.


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


Repository: mesos


Description
---

Added test for `GROW_VOLUME` and `SHRINK_VOLUME` operations.


Diffs (updated)
-

  src/tests/persistent_volume_tests.cpp 
4edf781711d9efdb994114aeb6289b6af750b87a 


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

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66052: Added new operator API to grow and shrink persistent volume.

[Zhitao Li]

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

(Updated March 28, 2018, 11:28 a.m.)


Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.


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


Repository: mesos


Description
---

The same API could be used in the future to grow or shrink CSI volumes,
but currently only persistent volumes are supported.


Diffs (updated)
-

  include/mesos/master/master.proto aa63904a33290a3beda162bbc9f44b56ab04a1e7 
  include/mesos/v1/master/master.proto ddb28f96b2a3a439bb9a829995a9a3015f65ba43 


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

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66050: Implemented grow and shrink of persistent volumes.

[Zhitao Li]

> On March 27, 2018, 3:24 p.m., Greg Mann wrote:
> > src/slave/slave.cpp
> > Line 4258 (original), 4258 (patched)
> > 
> >
> > Could you split the agent changes into a separate patch? This is a 
> > significant functional change for the agent; I think it would be good to 
> > have it explicitly represented in the commit log.

Yup I'm also mentioning this with Chun. Will do.


- Zhitao


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


On March 28, 2018, 11:26 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66050/
> ---
> 
> (Updated March 28, 2018, 11:26 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new offer operations are implemented as non-speculative operations,
> which means they cannot be chained with previous offer operations which
> depend on results of each other.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.cpp 141a444534b776a2c90e2a0daf9727cd21e39080 
>   src/common/resources_utils.cpp 9be01c1abd48264e308960f35cc7c2ee8a367518 
>   src/master/master.cpp 20692c36c021fdae91de0b156f26fc56cf7c4f45 
>   src/master/validation.hpp 7c129ceb929596acbb64d37025e055661277e6bf 
>   src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 
>   src/tests/mesos.hpp 46c271b5c5bedbdabd58b3cdbb82216d55c846bd 
>   src/tests/persistent_volume_tests.cpp 
> 4edf781711d9efdb994114aeb6289b6af750b87a 
>   src/tests/reservation_tests.cpp 5570df2e0b208512d0a0a3079a180e1acfe08f3d 
> 
> 
> Diff: https://reviews.apache.org/r/66050/diff/7/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66050: Implemented grow and shrink of persistent volumes.

[Zhitao Li]

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

(Updated March 28, 2018, 11:26 a.m.)


Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.


Changes
---

Review comments from Greg, and separate out agent change to different patch.


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


Repository: mesos


Description
---

The new offer operations are implemented as non-speculative operations,
which means they cannot be chained with previous offer operations which
depend on results of each other.


Diffs (updated)
-

  src/common/protobuf_utils.cpp 141a444534b776a2c90e2a0daf9727cd21e39080 
  src/common/resources_utils.cpp 9be01c1abd48264e308960f35cc7c2ee8a367518 
  src/master/master.cpp 20692c36c021fdae91de0b156f26fc56cf7c4f45 
  src/master/validation.hpp 7c129ceb929596acbb64d37025e055661277e6bf 
  src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 
  src/tests/mesos.hpp 46c271b5c5bedbdabd58b3cdbb82216d55c846bd 
  src/tests/persistent_volume_tests.cpp 
4edf781711d9efdb994114aeb6289b6af750b87a 
  src/tests/reservation_tests.cpp 5570df2e0b208512d0a0a3079a180e1acfe08f3d 


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

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66050: Implemented grow and shrink of persistent volumes.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [66050, 66049, 66343, 66342]

Failed command: python support/apply-reviews.py -n -r 66050

Error:
2018-03-28 18:26:21 URL:https://reviews.apache.org/r/66050/diff/raw/ 
[20637/20637] -> "66050.patch" [1]
error: patch failed: src/tests/persistent_volume_tests.cpp:204
error: src/tests/persistent_volume_tests.cpp: patch does not apply
error: patch failed: src/tests/reservation_tests.cpp:123
error: src/tests/reservation_tests.cpp: patch does not apply

Full log: https://builds.apache.org/job/Mesos-Reviewbot/22036/console

- Mesos Reviewbot


On March 26, 2018, 11:30 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66050/
> ---
> 
> (Updated March 26, 2018, 11:30 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new offer operations are implemented as non-speculative operations,
> which means they cannot be chained with previous offer operations which
> depend on results of each other.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.cpp 2acdc798200e82d14651b1edcc5f83e174477d53 
>   src/common/resources_utils.cpp 9be01c1abd48264e308960f35cc7c2ee8a367518 
>   src/master/master.cpp 20692c36c021fdae91de0b156f26fc56cf7c4f45 
>   src/master/validation.hpp 7c129ceb929596acbb64d37025e055661277e6bf 
>   src/master/validation.cpp 9254cd22e67d9aca53135701575fb0857ff6f7d6 
>   src/slave/slave.cpp e76daabad0d2d68aa42d1da809d4a23459eaaacb 
>   src/tests/mesos.hpp 01fd1c2bf21ef96dbab0769a1c1c92d7e3b60032 
>   src/tests/persistent_volume_tests.cpp 
> 924d8458e54e34a49c99593482b5908c5f7c7a48 
>   src/tests/reservation_tests.cpp 8d8e9c8390e65187269bd194bb322bbdff88e0bd 
> 
> 
> Diff: https://reviews.apache.org/r/66050/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66218: Ensured that agent does not delete volume upon grow or shrink.

[Zhitao Li]

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

(Updated March 28, 2018, 11:25 a.m.)


Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.


Changes
---

Move set difference out to own patch.


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


Repository: mesos


Description
---

Previously, `slave::syncCheckpointedResources` implementation will
delete a persistent volume using `Resources::contains` check, which
could cause a resized volume being deleted. The function was rewritten
to compare `set_difference` between old and new paths for all persistent
volumes and perform creation/deletion accordingly.


Diffs (updated)
-

  src/slave/slave.cpp e76daabad0d2d68aa42d1da809d4a23459eaaacb 


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

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


Testing
---


Thanks,

Zhitao Li

Review Request 66344: Supported non-speculative operations on agent default resources.

[Zhitao Li]

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

Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.


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


Repository: mesos


Description
---

Supported non-speculative operations on agent default resources.


Diffs
-

  src/slave/slave.cpp e76daabad0d2d68aa42d1da809d4a23459eaaacb 


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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66049: Added offer operation to grow and shrink persistent volumes.

[Zhitao Li]

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

(Updated March 28, 2018, 11:24 a.m.)


Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.


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


Repository: mesos


Description
---

Added offer operation to grow and shrink persistent volumes.


Diffs (updated)
-

  include/mesos/mesos.proto 676f0b090cad7ebf59eb32556f17ff8b5f247907 
  include/mesos/v1/mesos.proto 10d506517c9f098374ab0c8f4dcfda42e1896c95 


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

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


Testing
---


Thanks,

Zhitao Li

Review Request 66343: Added test for difference operator of hashset.

[Zhitao Li]

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

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


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


Repository: mesos


Description
---

Added test for difference operator of hashset.


Diffs
-

  3rdparty/stout/tests/hashset_tests.cpp 
d2b7bf248647c63c00c8c82b8176130c87c50eb4 


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


Testing
---


Thanks,

Zhitao Li

Review Request 66342: Added difference operator overload for hashset.

[Zhitao Li]

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

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


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


Repository: mesos


Description
---

Added difference operator overload for hashset.


Diffs
-

  3rdparty/stout/include/stout/hashset.hpp 
6af209c53185207b53396e7687e3bd7357e57bf1 


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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66327: WIP: Added the `LIST_RESOURCE_PROVIDER_CONFIGS` agent API call.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['66318', '66325', '66326', '66327']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66327

Relevant logs:

- 
[mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66327/logs/mesos-tests-stdout.log):

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

[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (31 
ms)
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (39 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (71 ms 
total)

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

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

[--] Global test environment tear-down
[==] 949 tests from 94 test cases ran. (440792 ms total)
[  PASSED  ] 948 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] CommandExecutorCheckTest.CommandCheckTimeout

 1 FAILED TEST
  YOU HAVE 214 DISABLED TESTS

```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66327/logs/mesos-tests-stderr.log):

```
I0328 18:14:06.345139  6924 exec.cpp:236] Executor registered on agent 
ae756040-1f28-4741-b582-c62b863eb4e9-S0
I0328 18:14:06.349126  5368 executor.cpp:176] Received SUBSCRIBED event
I0328 18:14:06.354125  5368 executor.cpp:180] Subscribed executor on 
winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I0328 18:14:06.354125  5368 executor.cpp:176] Received LAUNCH event
I0328 18:14:06.360126  5368 executor.cpp:648] Starting task 
65e2147b-4450-4bed-8115-42fb849da4a5
I0328 18:14:06.451171  5368 executor.cpp:483] Running 
'D:\DCOS\mesos\src\mesos-containerizer.exe launch '
I0328 18:14:06.478179  5368 executor.cpp:661] Forked command at 1568
I0328 18:14:06.509186  1176 exec.cpp:445] Executor asked to shutdown
I0328 18:14:06.510143  6392 executor.cpp:176] Received SHUTDOWN event
I0328 18:14:06.510143  6392 executor.cpp:758] Shutting down
I0328 18:14:06.510143  6392 executor.cpp:868] Sending SIGTERM to process tree 
at pid 1.507169  8216 hierarchical.cpp:405] Deactivated framework 
ae756040-1f28-4741-b582-c62b863eb4e9-
I0328 18:14:06.507169  3376 slave.cpp:3873] Shutting down framework 
ae756040-1f28-4741-b582-c62b863eb4e9-
I0328 18:14:06.507169  7824 master.cpp:10446] Updating the state of task 
65e2147b-4450-4bed-8115-42fb849da4a5 of framework 
ae756040-1f28-4741-b582-c62b863eb4e9- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0328 18:14:06.507169  3376 slave.cpp:6566] Shutting down executor 
'65e2147b-4450-4bed-8115-42fb849da4a5' of framework 
ae756040-1f28-4741-b582-c62b863eb4e9- at executor(1)@10.3.1.8:54008
I0328 18:14:06.508146  3376 slave.cpp:919] Agent terminating
W0328 18:14:06.508146  3376 slave.cpp:3869] Ignoring shutdown framework 
ae756040-1f28-4741-b582-c62b863eb4e9- because it is terminating
I0328 18:14:06.510143  7824 master.cpp:10545] Removing task 
65e2147b-4450-4bed-8115-42fb849da4a5 with resources cpus(allocated: *):4; 
mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
*):[31000-32000] of framework ae756040-1f28-4741-b582-c62b863eb4e9- on 
agent ae756040-1f28-4741-b582-c62b863eb4e9-S0 at slave(418)@10.3.1.8:53987 
(winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0328 18:14:06.512130  3376 containerizer.cpp:2338] Destroying container 
1fc35e64-161b-48ec-8ce1-f6f039c17993 in RUNNING state
I0328 18:14:06.512130  3376 containerizer.cpp:2952] Transitioning the state of 
container 1fc35e64-161b-48ec-8ce1-f6f039c17993 from RUNNING to DESTROYING
I0328 18:14:06.513144  3376 launcher.cpp:156] Asked to destroy container 
1fc35e64-161b-48ec-8ce1-f6f039c17993
I0328 18:14:06.514144  7864 hierarchical.cpp:344] Removed framework 
ae756040-1f28-4741-b582-c62b863eb4e9-
I0328 18:14:06.514144  7824 master.cpp:1295] Agent 
ae756040-1f28-4741-b582-c62b863eb4e9-S0

Re: Review Request 66314: Fix 3rdparty build commands for FreeBSD.

[David Forsythe]

> On March 28, 2018, 8:06 a.m., Benjamin Bannier wrote:
> > 3rdparty/CMakeLists.txt
> > Lines 58-63 (patched)
> > 
> >
> > Like discussed offline, I don't think there is a reason we need to bolt 
> > such logic on the used system, at least currently.
> > 
> > Let's instead use `find_program` to find a `make` and use it to set 
> > this variable.
> > 
> > Unless I miss something, we do not depend on GNU make here, so let's 
> > maybe reflect that in a more general name, e.g., just `MAKE` if it is 
> > available.

I took a look at find_program, but I don't think that will solve the problem.

We actually *do* depend on gmake here (even on darwin). On FreeBSD GHU make is 
a 3rd party application and is actually called and installed as `gmake`, 
whereas the system make is actually Pmake (which is incompatible).


- David


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


On March 27, 2018, 7:10 p.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66314/
> ---
> 
> (Updated March 27, 2018, 7:10 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-4176
> https://issues.apache.org/jira/browse/MESOS-4176
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix 3rdparty build commands for FreeBSD.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 2b63b58f7d6a88c9986b746283dcfa79b7bcb270 
>   cmake/CompilationConfigure.cmake 64cc56ee4208afe05df0f28af5890157e4c7d82c 
> 
> 
> Diff: https://reviews.apache.org/r/66314/diff/1/
> 
> 
> Testing
> ---
> 
> cmake --build on FreeBSD
> 
> 
> Thanks,
> 
> David Forsythe
> 
>

Re: Review Request 66051: Implemented operator API to grow and shrink persistent volume.

[Zhitao Li]

> On March 27, 2018, 6:58 p.m., Greg Mann wrote:
> > src/master/master.cpp
> > Lines 11881-11887 (original), 11885-11890 (patched)
> > 
> >
> > Currently, it looks like `Slave::usedResources` is the same as the 
> > "allocated resources" from that agent.
> > 
> > When a GROW or SHRINK operation is initiated by an operator, its 
> > consumed resource is in a unique state in the master while that operation 
> > is pending: the volume is not allocated, but it is "used" in some sense.
> > 
> > One place where we use `Slave::usedResources` is when validating 
> > DESTROY operations: 
> > https://github.com/apache/mesos/blob/21e4b45b388d0b272236b1c58313569f8a1d1fc8/src/master/master.cpp#L4841
> > 
> > This means if the master receives a DESTROY call for a persistent 
> > volume while there is a pending operator-initiated GROW_VOLUME call for 
> > that same volume (i.e. the operation was forwarded to the agent for 
> > processing but the master hasn't heard back yet), we would accept the 
> > DESTROY operation and forward it to the agent. Is this what we want?
> > 
> > This brings something else to mind: would it be possible for the 
> > allocator to send out an offer containing the consumed resource of a 
> > GROW_VOLUME or SHRINK_VOLUME operation, while the operation is pending on 
> > the agent? This seems bad, but I believe it's possible. I think this may be 
> > the first time we have a set of resources which we don't want the allocator 
> > to offer for a period of time, but which are not currently allocated to 
> > some framework/role. I wonder if we need some new tools in the allocator to 
> > handle this?
> 
> Chun-Hung Hsiao wrote:
> Yeah this sounds bad. Would it help if we remove the consumed resources 
> from the allocator and put it back after receiving the terminal status update?

I agree this is problematic.

It seems like we need to make sure several things:
1. `consumed` resources of a non-speculative operation can not offered to any 
framework while pending;
2. `consumed` resources should still be visible for various validation;

Therefore, I'm thinking about introducing a new field in `Master::Slave` struct 
which represents `pendingConsumedResources` (or a function in `Slave` which can 
extract this info from pending operations). We can then use it to validation of 
`DESTROY` call.

Another issue I found was in `Master::apply()`, which we directly call 
`allocator->updateAvailable(slave->id, {operation})`, but semantically we 
should be calling `allocator->updateAvailable(slave->id, consumed, empty)` so 
that allocator would not see the `converted` resources until operation 
confirmations comes back.


- Zhitao


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


On March 26, 2018, 11:37 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66051/
> ---
> 
> (Updated March 26, 2018, 11:37 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented operator API to grow and shrink persistent volume.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 34c9023906eca94965acc994f20e888c1f47b962 
>   src/master/master.hpp 0d9620dd0c232dc1df83477e838eeb7313bf8828 
>   src/master/master.cpp 20692c36c021fdae91de0b156f26fc56cf7c4f45 
>   src/master/validation.cpp 9254cd22e67d9aca53135701575fb0857ff6f7d6 
> 
> 
> Diff: https://reviews.apache.org/r/66051/diff/7/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66291: Added support to max_duration in default executor.

[Zhitao Li]

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




src/launcher/default_executor.cpp
Lines 1098-1102 (patched)


I think I also need to handle the case if a task run to success *before* 
reaching max duration. This is not a big issue for other built-in executors 
since they only run one task anyway, but we need to make sureJ:
- other tasks in the group are not affected;
- timer should be cancelled properly;
- timer triggered after this task completed should not bring executor down 
accidentally.


- Zhitao Li


On March 26, 2018, 2:48 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66291/
> ---
> 
> (Updated March 26, 2018, 2:48 p.m.)
> 
> 
> Review request for mesos, Jason Lai and James Peach.
> 
> 
> Bugs: MESOS-8725
> https://issues.apache.org/jira/browse/MESOS-8725
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added support to max_duration in default executor.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp 906836f3b8e0af79d7c61f90fd8a95f193b26e84 
> 
> 
> Diff: https://reviews.apache.org/r/66291/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66311: Implement recovery of resource provider manager.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['66308', '66309', '66310', '66311']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66311

Relevant logs:

- 
[mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66311/logs/mesos-tests-stdout.log):

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

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

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

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

[--] Global test environment tear-down
[==] 949 tests from 94 test cases ran. (423168 ms total)
[  PASSED  ] 948 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] CommandExecutorCheckTest.CommandCheckTimeout

 1 FAILED TEST
  YOU HAVE 214 DISABLED TESTS

```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66311/logs/mesos-tests-stderr.log):

```
I0328 15:46:36.892980  5608 exec.cpp:236] Executor registered on agent 
48f40b10-7e34-4fe9-8156-dc7b1068fc89-S0
I0328 15:46:36.896950  5360 executor.cpp:176] Received SUBSCRIBED event
I0328 15:46:36.901968  5360 executor.cpp:180] Subscribed executor on 
winbldsrv-02.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I0328 15:46:36.902966  5360 executor.cpp:176] Received LAUNCH event
I0328 15:46:36.907972  5360 executor.cpp:648] Starting task 
11371b21-8bcb-41fb-853e-415218194b75
I0328 15:46:36.996968  5360 executor.cpp:483] Running 
'D:\DCOS\mesos\src\mesos-containerizer.exe launch '
I0328 15:46:37.024973  5360 executor.cpp:661] Forked command at 7704
I0328 15:46:37.056937  1004 exec.cpp:445] Executor asked to shutdown
I0328 15:46:37.057945  5448 executor.cpp:176] Received SHUTDOWN event
I0328 15:46:37.057945  5448 executor.cpp:758] Shutting down
I0328 15:46:37.057945  5448 executor.cpp:868] Sending SIGTERM to process tree 
at pid 7e34-4fe9-8156-dc7b1068fc89-
I0328 15:46:37.054944  8400 master.cpp:10446] Updating the state of task 
11371b21-8bcb-41fb-853e-415218194b75 of framework 
48f40b10-7e34-4fe9-8156-dc7b1068fc89- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0328 15:46:37.054944  8264 slave.cpp:3871] Shutting down framework 
48f40b10-7e34-4fe9-8156-dc7b1068fc89-
I0328 15:46:37.055943  8264 slave.cpp:6579] Shutting down executor 
'11371b21-8bcb-41fb-853e-415218194b75' of framework 
48f40b10-7e34-4fe9-8156-dc7b1068fc89- at executor(1)@10.3.1.5:54190
I0328 15:46:37.055943  8264 slave.cpp:915] Agent terminating
W0328 15:46:37.056937  8264 slave.cpp:3867] Ignoring shutdown framework 
48f40b10-7e34-4fe9-8156-dc7b1068fc89- because it is terminating
I0328 15:46:37.057945  8400 master.cpp:10545] Removing task 
11371b21-8bcb-41fb-853e-415218194b75 with resources cpus(allocated: *):4; 
mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
*):[31000-32000] of framework 48f40b10-7e34-4fe9-8156-dc7b1068fc89- on 
agent 48f40b10-7e34-4fe9-8156-dc7b1068fc89-S0 at slave(417)@10.3.1.5:54169 
(winbldsrv-02.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0328 15:46:37.060938  4904 containerizer.cpp:2338] Destroying container 
6cb221ac-b026-4983-b310-d1ecc7936091 in RUNNING state
I0328 15:46:37.060938  4904 containerizer.cpp:2952] Transitioning the state of 
container 6cb221ac-b026-4983-b310-d1ecc7936091 from RUNNING to DESTROYING
I0328 15:46:37.060938  8400 master.cpp:1295] Agent 
48f40b10-7e34-4fe9-8156-dc7b1068fc89-S0 at slave(417)@10.3.1.5:54169 
(winbldsrv-02.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) disconnected
I0328 15:46:37.061939  8400 master.cpp:3283] Disconnecting agent 
48f40b10-7e34-4fe9-8156-dc7b1068fc89-S0 at slave(417)@10.3.1.5:54169 
(winbldsrv-02.zq4gs31qjdiunm1ryi1452nvnh.dx.i

Re: Review Request 63733: Don't print full usage for invocation errors.

[Till Toenshoff]

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


Ship it!




Thanks for this Benno - had annoyed me for so long already :)

- Till Toenshoff


On March 23, 2018, 2:39 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63733/
> ---
> 
> (Updated March 23, 2018, 2:39 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-8728
> https://issues.apache.org/jira/browse/MESOS-8728
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The current usage string for `mesos-master` comes in at 399 lines, and
> for `mesos-agent` at 685 lines. Printing such a wall of text will
> overflow almost any terminal window, making it necessary to scroll up
> to see the actual error when invoking mesos with an incorrect command
> line.
> 
> 
> Diffs
> -
> 
>   src/master/main.cpp 3d93fd4825f0f58fce664379e3337a61bb93aa0a 
>   src/slave/main.cpp 67437272ee538dd2eae44f5664630cf7c930c23f 
> 
> 
> Diff: https://reviews.apache.org/r/63733/diff/2/
> 
> 
> Testing
> ---
> 
> Started `mesos-agent` and `mesos-master` with an incorrect command-line.
> 
> ```
> bevers@poincare:~/mesos/worktrees/usage/build$ ./src/mesos-master --foo
> Failed to load unknown flag 'foo'
> 
> See `mesos-master --help` for a list of supported flags.
> bevers@poincare:~/mesos/worktrees/usage/build$ ./src/mesos-master
> I0323 17:33:45.526222 19899 main.cpp:246] Build: 2018-03-22 17:04:18 by bevers
> I0323 17:33:45.526304 19899 main.cpp:247] Version: 1.6.0
> I0323 17:33:45.536743 19899 main.cpp:346] Using 'HierarchicalDRF' allocator
> E0323 17:33:45.536763 19899 main.cpp:361] EXIT with status 1: --work_dir 
> needed for replicated log based registry
> ```
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 66330: Added 'OPENSSL_INCLUDE_DIR' to forwarded arguments.

[Benjamin Bannier]

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


Ship it!




- Benjamin Bannier


On March 28, 2018, 2:01 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66330/
> ---
> 
> (Updated March 28, 2018, 2:01 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To help third-party dependencies find and build with OpenSSL, certain
> options are forwarded to them. Dependencies using the 'FindOpenSSL'
> module need either 'OPENSSL_ROOT_DIR' or 'OPENSSL_INCLUDE_DIR' to figure
> out the location of OpenSSL. In the module 'OPENSSL_ROOT_DIR' acts as a
> hint for 'OPENSSL_INCLUDE_DIR'. I.e., 'OPENSSL_INCLUDE_DIR' will be set
> by the module. As a consequence, if 'OPENSSL_ROOT_DIR' is not configured
> in 'ccmake', users will only be prompted to provide
> 'OPENSSL_INCLUDE_DIR'. By forwarding 'OPENSSL_INCLUDE_DIR' we cover this
> case for third-party dependencies, thus allowing users to configure the
> project in 'ccmake' without having to explicitly set 'OPENSSL_ROOT_DIR'.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 2b63b58f7d6a88c9986b746283dcfa79b7bcb270 
> 
> 
> Diff: https://reviews.apache.org/r/66330/diff/2/
> 
> 
> Testing
> ---
> 
> On macOS 10.13.3 with OpenSSL installed via Homebrew:
> 
> $ mkdir build && cd build
> $ ccmake -GNinja ..
> 
> * Press "c" to configure.
> * Switch on "ENABLE_LIBEVENT" and "ENABLE_SSL".
> * Press "c" to configure. There will be an error explaining that 
> 'OPENSSL_ROOT_DIR' or 'OPENSSL_INCLUDE_DIR' is missing.
> * `ccmake` will ask for "OPENSSL_INCLUDE_DIR", set it to 
> `/usr/local/opt/openssl/include` (assuming `$(brew --prefix) == /usr/local`)
> * Press "c" to configure again. There shouldn't be an error now.
> * Press "g" to generate the configuration.
> 
> $ ninja
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 66308: Delayed construction of the agent's resource provider manager.

[Benjamin Bannier]

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

(Updated March 28, 2018, 4:28 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


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


Repository: mesos


Description
---

By delaying the construction of the agent's resource provider manager
we prepare for a following patch introducing a dependency on the
resource provider manager on the agent's ID.

Depending on whether the agent was able to recover an agent ID from
its log or still needs to obtain on in a first registration with the
master, we can only construct the resource provider manager after
different points in the initialization of the agent. To capture the
common code we introduce a helper function performing the necessary
steps.


Diffs (updated)
-

  src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb 
  src/slave/slave.cpp e76daabad0d2d68aa42d1da809d4a23459eaaacb 
  src/tests/resource_provider_manager_tests.cpp 
d947bd037190e6b7ea7b2277b5fbe47816878de4 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 66331: Fixed test to assert precondition.

[Jan Schlicht]

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


Ship it!




Ship It!

- Jan Schlicht


On March 28, 2018, 4:21 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66331/
> ---
> 
> (Updated March 28, 2018, 4:21 p.m.)
> 
> 
> Review request for mesos and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch changes an assert in
> `ResourceProviderManagerHttpApiTest.UpdateOperationStatus` to make
> sure a received message has the correct type. We later assume a
> certain type which would not only abort the test if an unexpected
> message type is received but also lead to hard runtime failures like
> segmentation faults.
> 
> 
> Diffs
> -
> 
>   src/tests/resource_provider_manager_tests.cpp 
> d947bd037190e6b7ea7b2277b5fbe47816878de4 
> 
> 
> Diff: https://reviews.apache.org/r/66331/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 66331: Fixed test to assert precondition.

[Benjamin Bannier]

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

Review request for mesos and Jan Schlicht.


Repository: mesos


Description
---

This patch changes an assert in
`ResourceProviderManagerHttpApiTest.UpdateOperationStatus` to make
sure a received message has the correct type. We later assume a
certain type which could and not aborting the test if an unexpected
message type is received would lead to hard runtime failures like
segmentation faults.


Diffs
-

  src/tests/resource_provider_manager_tests.cpp 
d947bd037190e6b7ea7b2277b5fbe47816878de4 


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


Testing
---


Thanks,

Benjamin Bannier

Re: Review Request 66309: Externalize creation of resource provider manager backing storage.

[Jan Schlicht]

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




src/resource_provider/registrar.hpp
Lines 28 (patched)


Include this below `` and above `https://reviews.apache.org/r/66309/#comment280753>

Delete `master's`.



src/resource_provider/registrar.hpp
Lines 68-69 (original), 70-71 (patched)


I don't like the semantics of this:
A `state::Storage` doesn't provide any information if it belongs to master 
or agent but we instantiate an `AgentRegistrar` anyways.

Hence, please rename `AgentRegistrar` to something more general that 
indicates that this isn't tied to an agent state.



src/resource_provider/registrar.hpp
Line 110 (original), 111 (patched)


Please rename, see above.


- Jan Schlicht


On March 27, 2018, 5:34 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66309/
> ---
> 
> (Updated March 27, 2018, 5:34 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-8735
> https://issues.apache.org/jira/browse/MESOS-8735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch changes the way the storage backing an agent's resource
> provider registrar is created: while before we created it implicitly
> when constructing the registrar, we now consume storage passed on
> construction.
> 
> Being able to explicitly inject the used storage simplifies testing.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/registrar.hpp 
> 39f45b0a2a7c35bfe72a9305f5248409e4a3ef45 
>   src/resource_provider/registrar.cpp 
> 9eb49f1327a0b598c5464a29a09ee286d7018f09 
>   src/tests/resource_provider_manager_tests.cpp 
> d947bd037190e6b7ea7b2277b5fbe47816878de4 
> 
> 
> Diff: https://reviews.apache.org/r/66309/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66330: Added 'OPENSSL_INCLUDE_DIR' to forwarded arguments.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['66330']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66330

Relevant logs:

- 
[mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66330/logs/mesos-tests-stdout.log):

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

[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (33 
ms)
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (39 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (74 ms 
total)

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

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

[--] Global test environment tear-down
[==] 949 tests from 94 test cases ran. (446888 ms total)
[  PASSED  ] 948 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] CommandExecutorCheckTest.CommandCheckTimeout

 1 FAILED TEST
  YOU HAVE 214 DISABLED TESTS

```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66330/logs/mesos-tests-stderr.log):

```
I0328 13:09:00.105005  7800 slave.cpp:3873] Shutting down framework 
455c7620-eeb9-43ae-acf0-a11bb2beb09a-
I0328 13:09:00.105005  9184 master.cpp:10446] Updating the state of task 
7d6e0af3-5187-4b61-b278-365dfd6ed40e of framework 
455c7620-eeb9-43ae-acf0-a11bI0328 13:08:59.923991  5256 exec.cpp:162] Version: 
1.6.0
I0328 13:08:59.950994  7400 exec.cpp:236] Executor registered on agent 
455c7620-eeb9-43ae-acf0-a11bb2beb09a-S0
I0328 13:08:59.954962  7660 executor.cpp:176] Received SUBSCRIBED event
I0328 13:08:59.959987  7660 executor.cpp:180] Subscribed executor on 
winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I0328 13:08:59.960988  7660 executor.cpp:176] Received LAUNCH event
I0328 13:08:59.965994  7660 executor.cpp:648] Starting task 
7d6e0af3-5187-4b61-b278-365dfd6ed40e
I0328 13:09:00.044960  7660 executor.cpp:483] Running 
'D:\DCOS\mesos\src\mesos-containerizer.exe launch '
I0328 13:09:00.075968  7660 executor.cpp:661] Forked command at 2340
I0328 13:09:00.106968 10068 exec.cpp:445] Executor asked to shutdown
I0328 13:09:00.107987  9452 executor.cpp:176] Received SHUTDOWN event
I0328 13:09:00.107987  9452 executor.cpp:758] Shutting down
I0328 13:09:00.107987  9452 executor.cpp:868] Sending SIGTERM to process tree 
at pid 2b2beb09a- (latest state: TASK_KILLED, status update state: 
TASK_KILLED)
I0328 13:09:00.105005  7800 slave.cpp:6566] Shutting down executor 
'7d6e0af3-5187-4b61-b278-365dfd6ed40e' of framework 
455c7620-eeb9-43ae-acf0-a11bb2beb09a- at executor(1)@10.3.1.8:55309
I0328 13:09:00.106007  7800 slave.cpp:919] Agent terminating
W0328 13:09:00.106007  7800 slave.cpp:3869] Ignoring shutdown framework 
455c7620-eeb9-43ae-acf0-a11bb2beb09a- because it is terminating
I0328 13:09:00.108968  9184 master.cpp:10545] Removing task 
7d6e0af3-5187-4b61-b278-365dfd6ed40e with resources cpus(allocated: *):4; 
mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
*):[31000-32000] of framework 455c7620-eeb9-43ae-acf0-a11bb2beb09a- on 
agent 455c7620-eeb9-43ae-acf0-a11bb2beb09a-S0 at slave(418)@10.3.1.8:55288 
(winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0328 13:09:00.110967  7852 containerizer.cpp:2338] Destroying container 
6713fbe2-c330-42f4-9d84-8cbb0cd132ab in RUNNING state
I0328 13:09:00.110967  7852 containerizer.cpp:2952] Transitioning the state of 
container 6713fbe2-c330-42f4-9d84-8cbb0cd132ab from RUNNING to DESTROYING
I0328 13:09:00.111968  9184 master.cpp:1295] Agent 
455c7620-eeb9-43ae-acf0-a11bb2beb09a-S0 at slave(418)@10.3.1.8:55288 
(winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) disconnected
I0328 13:09:00.111968  9184 master.cpp:3283] Disconnecting agent 
455c7620-eeb9-43ae-acf0-a11bb2beb09a-S0 at slave(418)@10.3.1.8:55288 
(winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.i

Re: Review Request 66330: Added 'OPENSSL_INCLUDE_DIR' to forwarded arguments.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [66330]

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 March 28, 2018, 2:01 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66330/
> ---
> 
> (Updated March 28, 2018, 2:01 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To help third-party dependencies find and build with OpenSSL, certain
> options are forwarded to them. Dependencies using the 'FindOpenSSL'
> module need either 'OPENSSL_ROOT_DIR' or 'OPENSSL_INCLUDE_DIR' to figure
> out the location of OpenSSL. In the module 'OPENSSL_ROOT_DIR' acts as a
> hint for 'OPENSSL_INCLUDE_DIR'. I.e., 'OPENSSL_INCLUDE_DIR' will be set
> by the module. As a consequence, if 'OPENSSL_ROOT_DIR' is not configured
> in 'ccmake', users will only be prompted to provide
> 'OPENSSL_INCLUDE_DIR'. By forwarding 'OPENSSL_INCLUDE_DIR' we cover this
> case for third-party dependencies, thus allowing users to configure the
> project in 'ccmake' without having to explicitly set 'OPENSSL_ROOT_DIR'.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 2b63b58f7d6a88c9986b746283dcfa79b7bcb270 
> 
> 
> Diff: https://reviews.apache.org/r/66330/diff/2/
> 
> 
> Testing
> ---
> 
> On macOS 10.13.3 with OpenSSL installed via Homebrew:
> 
> $ mkdir build && cd build
> $ ccmake -GNinja ..
> 
> * Press "c" to configure.
> * Switch on "ENABLE_LIBEVENT" and "ENABLE_SSL".
> * Press "c" to configure. There will be an error explaining that 
> 'OPENSSL_ROOT_DIR' or 'OPENSSL_INCLUDE_DIR' is missing.
> * `ccmake` will ask for "OPENSSL_INCLUDE_DIR", set it to 
> `/usr/local/opt/openssl/include` (assuming `$(brew --prefix) == /usr/local`)
> * Press "c" to configure again. There shouldn't be an error now.
> * Press "g" to generate the configuration.
> 
> $ ninja
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 66330: Added 'OPENSSL_INCLUDE_DIR' to forwarded arguments.

[Jan Schlicht]

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

(Updated March 28, 2018, 2:01 p.m.)


Review request for mesos, Andrew Schwartzmeyer and Benjamin Bannier.


Changes
---

Added a comment.


Repository: mesos


Description
---

To help third-party dependencies find and build with OpenSSL, certain
options are forwarded to them. Dependencies using the 'FindOpenSSL'
module need either 'OPENSSL_ROOT_DIR' or 'OPENSSL_INCLUDE_DIR' to figure
out the location of OpenSSL. In the module 'OPENSSL_ROOT_DIR' acts as a
hint for 'OPENSSL_INCLUDE_DIR'. I.e., 'OPENSSL_INCLUDE_DIR' will be set
by the module. As a consequence, if 'OPENSSL_ROOT_DIR' is not configured
in 'ccmake', users will only be prompted to provide
'OPENSSL_INCLUDE_DIR'. By forwarding 'OPENSSL_INCLUDE_DIR' we cover this
case for third-party dependencies, thus allowing users to configure the
project in 'ccmake' without having to explicitly set 'OPENSSL_ROOT_DIR'.


Diffs (updated)
-

  3rdparty/CMakeLists.txt 2b63b58f7d6a88c9986b746283dcfa79b7bcb270 


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

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


Testing
---

On macOS 10.13.3 with OpenSSL installed via Homebrew:

$ mkdir build && cd build
$ ccmake -GNinja ..

* Press "c" to configure.
* Switch on "ENABLE_LIBEVENT" and "ENABLE_SSL".
* Press "c" to configure. There will be an error explaining that 
'OPENSSL_ROOT_DIR' or 'OPENSSL_INCLUDE_DIR' is missing.
* `ccmake` will ask for "OPENSSL_INCLUDE_DIR", set it to 
`/usr/local/opt/openssl/include` (assuming `$(brew --prefix) == /usr/local`)
* Press "c" to configure again. There shouldn't be an error now.
* Press "g" to generate the configuration.

$ ninja


Thanks,

Jan Schlicht

Re: Review Request 66325: Implemented idempotency for agent resource provider config API calls.

[James DeFelice]

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




src/resource_provider/daemon.cpp
Line 186 (original), 187 (patched)


has RP ID already been brought into alignment here?



src/resource_provider/daemon.cpp
Lines 239 (patched)


ditto, has RP ID already been brought into alignment?


- James DeFelice


On March 28, 2018, 3:06 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66325/
> ---
> 
> (Updated March 28, 2018, 3:06 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, James DeFelice, and Jie Yu.
> 
> 
> Bugs: MESOS-8742
> https://issues.apache.org/jira/browse/MESOS-8742
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `ADD_RESOURCE_PROVIDER_CONFIG`, `UPDATE_RESOURCE_PROVIDER_CONFIG` and
> `REMOVE_RESOURCE_PROVIDER_CONFIG` now return 200 for identical
> repetivite calls.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/daemon.cpp d0a8186da80a1bffbb71f524e639bc7d75ef6243 
>   src/slave/http.cpp 65081c95c888a8236aafdfc627a7aa4e9a72b65d 
> 
> 
> Diff: https://reviews.apache.org/r/66325/diff/1/
> 
> 
> Testing
> ---
> 
> make
> 
> NOTE: Test `*/AgentResourceProviderConfigApiTest.RemoveNotFound` won't pass 
> since the semantics of `REMOVE_RESOURCE_PROVIDER_CONFIG` is changed.
> Tests are added in the next patch.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66330: Added 'OPENSSL_INCLUDE_DIR' to forwarded arguments.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['66330']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66330

Relevant logs:

- 
[mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66330/logs/mesos-tests-stdout.log):

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

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

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

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

[--] Global test environment tear-down
[==] 949 tests from 94 test cases ran. (432065 ms total)
[  PASSED  ] 948 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] CommandExecutorCheckTest.CommandCheckTimeout

 1 FAILED TEST
  YOU HAVE 214 DISABLED TESTS

```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66330/logs/mesos-tests-stderr.log):

```
I0328 11:26:06.442183  6340 master.cpp:10446] Updating the state of task 
8c06a3ee-f4e1-4a99-9fdd-cc47139d860a of framework 
b2e9d685-31a1-47a6-9ff6-2dd902f1eca1- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0328 11:26:06.442183  5784 slave.cpp:3873] Shutting down framework 
b2e9d685-31a1-47a6-9ff6-2dd902f1eca1-
I0328 11:26:06.442183  5784 slave.cpp:6566] Shutting down executor 
'8c06a3ee-f4e1-4a99-9fdd-cc47139d860a' of framework 
b2e9d685-31a1-47a6-9ff6-2dd902f1eca1- at executor(1)@10.3.1.5:53147
I0328 11:26:06.443179  5784 slave.cpp:919] Agent terminating
W0328 11:26:06.443179  5784 slave.cpp:3869] Ignoring shutdown framework 
b2e9d685-I0328 11:26:06.261209  5788 exec.cpp:162] Version: 1.6.0
I0328 11:26:06.290184  1524 exec.cpp:236] Executor registered on agent 
b2e9d685-31a1-47a6-9ff6-2dd902f1eca1-S0
I0328 11:26:06.295212  7020 executor.cpp:176] Received SUBSCRIBED event
I0328 11:26:06.299214  7020 executor.cpp:180] Subscribed executor on 
winbldsrv-02.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I0328 11:26:06.300212  7020 executor.cpp:176] Received LAUNCH event
I0328 11:26:06.304214  7020 executor.cpp:648] Starting task 
8c06a3ee-f4e1-4a99-9fdd-cc47139d860a
I0328 11:26:06.387215  7020 executor.cpp:483] Running 
'D:\DCOS\mesos\src\mesos-containerizer.exe launch '
I0328 11:26:06.415185  7020 executor.cpp:661] Forked command at 1264
I0328 11:26:06.444386  7332 exec.cpp:445] Executor asked to shutdown
I0328 11:26:06.445185  6840 executor.cpp:176] Received SHUTDOWN event
I0328 11:26:06.445185  6840 executor.cpp:758] Shutting down
I0328 11:26:06.445185  6840 executor.cpp:868] Sending SIGTERM to process tree 
at pid 131a1-47a6-9ff6-2dd902f1eca1- because it is terminating
I0328 11:26:06.445185  6340 master.cpp:10545] Removing task 
8c06a3ee-f4e1-4a99-9fdd-cc47139d860a with resources cpus(allocated: *):4; 
mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
*):[31000-32000] of framework b2e9d685-31a1-47a6-9ff6-2dd902f1eca1- on 
agent b2e9d685-31a1-47a6-9ff6-2dd902f1eca1-S0 at slave(418)@10.3.1.5:53126 
(winbldsrv-02.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0328 11:26:06.447180  7492 containerizer.cpp:2338] Destroying container 
530b8daa-5bc1-4b99-8b07-4ad3f1332471 in RUNNING state
I0328 11:26:06.447180  7492 containerizer.cpp:2952] Transitioning the state of 
container 530b8daa-5bc1-4b99-8b07-4ad3f1332471 from RUNNING to DESTROYING
I0328 11:26:06.448185  7492 launcher.cpp:156] Asked to destroy container 
530b8daa-5bc1-4b99-8b07-4ad3f1332471
I0328 11:26:06.448185  6340 master.cpp:1295] Agent 
b2e9d685-31a1-47a6-9ff6-2dd902f1eca1-S0 at slave(418)@10.3.1.5:53126 
(winbldsrv-02.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) disconnected
I0328 11:26:06.448185  6376 hierarchical.cpp:344] Removed framework 
b2

Review Request 66330: Added 'OPENSSL_INCLUDE_DIR' to forwarded arguments.

[Jan Schlicht]

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

Review request for mesos, Andrew Schwartzmeyer and Benjamin Bannier.


Repository: mesos


Description
---

To help third-party dependencies find and build with OpenSSL, certain
options are forwarded to them. Dependencies using the 'FindOpenSSL'
module need either 'OPENSSL_ROOT_DIR' or 'OPENSSL_INCLUDE_DIR' to figure
out the location of OpenSSL. In the module 'OPENSSL_ROOT_DIR' acts as a
hint for 'OPENSSL_INCLUDE_DIR'. I.e., 'OPENSSL_INCLUDE_DIR' will be set
by the module. As a consequence, if 'OPENSSL_ROOT_DIR' is not configured
in 'ccmake', users will only be prompted to provide
'OPENSSL_INCLUDE_DIR'. By forwarding 'OPENSSL_INCLUDE_DIR' we cover this
case for third-party dependencies, thus allowing users to configure the
project in 'ccmake' without having to explicitly set 'OPENSSL_ROOT_DIR'.


Diffs
-

  3rdparty/CMakeLists.txt 2b63b58f7d6a88c9986b746283dcfa79b7bcb270 


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


Testing
---

On macOS 10.13.3 with OpenSSL installed via Homebrew:

$ mkdir build && cd build
$ ccmake -GNinja ..

* Press "c" to configure.
* Switch on "ENABLE_LIBEVENT" and "ENABLE_SSL".
* Press "c" to configure. There will be an error explaining that 
'OPENSSL_ROOT_DIR' or 'OPENSSL_INCLUDE_DIR' is missing.
* `ccmake` will ask for "OPENSSL_INCLUDE_DIR", set it to 
`/usr/local/opt/openssl/include` (assuming `$(brew --prefix) == /usr/local`)
* Press "c" to configure again. There shouldn't be an error now.
* Press "g" to generate the configuration.

$ ninja


Thanks,

Jan Schlicht

Re: Review Request 66314: Fix 3rdparty build commands for FreeBSD.

[Benjamin Bannier]

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




3rdparty/CMakeLists.txt
Lines 58-63 (patched)


Like discussed offline, I don't think there is a reason we need to bolt 
such logic on the used system, at least currently.

Let's instead use `find_program` to find a `make` and use it to set this 
variable.

Unless I miss something, we do not depend on GNU make here, so let's maybe 
reflect that in a more general name, e.g., just `MAKE` if it is available.


- Benjamin Bannier


On March 27, 2018, 9:10 p.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66314/
> ---
> 
> (Updated March 27, 2018, 9:10 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-4176
> https://issues.apache.org/jira/browse/MESOS-4176
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix 3rdparty build commands for FreeBSD.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 2b63b58f7d6a88c9986b746283dcfa79b7bcb270 
>   cmake/CompilationConfigure.cmake 64cc56ee4208afe05df0f28af5890157e4c7d82c 
> 
> 
> Diff: https://reviews.apache.org/r/66314/diff/1/
> 
> 
> Testing
> ---
> 
> cmake --build on FreeBSD
> 
> 
> Thanks,
> 
> David Forsythe
> 
>