Re: Review Request 63818: Added quick start instructions for CMake.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63818 was successfully built and tested.

Reviews applied: `['63818']`

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

- Mesos Reviewbot Windows


On Nov. 14, 2017, 3:12 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63818/
> ---
> 
> (Updated Nov. 14, 2017, 3:12 p.m.)
> 
> 
> Review request for mesos, Gaston Kleiman and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added quick start instructions for CMake.
> 
> 
> Diffs
> -
> 
>   docs/cmake.md b8f9e7caa84372404256767a0220ac1df9db8bc8 
> 
> 
> Diff: https://reviews.apache.org/r/63818/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 63577: Fixed a task status update race in default executor tests.

[Qian Zhang]

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

(Updated Nov. 15, 2017, 2:21 p.m.)


Review request for mesos, Alexander Rukletsov and Gaston Kleiman.


Changes
---

Addressed review comments.


Repository: mesos


Description
---

Previously in the test `DefaultExecutorTest.KillMultipleTasks` and
`DefaultExecutorTest.KillTaskGroupOnTaskFailure`, when launching a
task group which has multiple tasks, we expected the scheduler will
receive all the TASK_STARTING status updates before receiving any
TASK_RUNNING status updates. However this is not guaranteed, e.g.,
it is possible for the scheduler to receive TASK_RUNNING for the
first task before receiving TASK_STARTING for the second task.

So in this patch, we used `Sequence` to guarantee the order of
TASK_STARTING and TASK_RUNNING for each task but do not care about
the order between tasks.

The following 3 tests have their own solutions to handle this issue,
in this patch, I updated them to use the above solution.
  `DefaultExecutorTest.KillTask`
  `DefaultExecutorTest.CommitSuicideOnKillTask`
  `DefaultExecutorTest.ROOT_ContainerStatusForTask`


Diffs (updated)
-

  src/tests/default_executor_tests.cpp 65113985acc0a8fac00374b074ef568aaa22c7ac 
  src/tests/mesos.hpp e25defeb55608136e77363aa48cf820092a13a59 


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

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


Testing
---

This patch touched 5 tests:
1. KillTask
2. KillMultipleTasks
3. KillTaskGroupOnTaskFailure
4. CommitSuicideOnKillTask
5. ROOT_ContainerStatusForTask

I ran each of them repeatedly (20 times), and all of them succeeded.


Thanks,

Qian Zhang

Re: Review Request 63801: Windows: Fixed build of Google Test with Visual Studio Preview.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63801 was successfully built and tested.

Reviews applied: `['63801']`

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

- Mesos Reviewbot Windows


On Nov. 14, 2017, 6:47 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63801/
> ---
> 
> (Updated Nov. 14, 2017, 6:47 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, John Kordich, and James Peach.
> 
> 
> Bugs: MESOS-8220
> https://issues.apache.org/jira/browse/MESOS-8220
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The preview version of Visual Studio (15.5) adds a new deprecation
> warning that Google Test emits. Google Test is also built with
> `-Werror`, and thus fails to build unless this warning is silenced.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 7d38a9cba84b86e1e428033ade1bd0ab5d9eaae7 
> 
> 
> Diff: https://reviews.apache.org/r/63801/diff/3/
> 
> 
> Testing
> ---
> 
> Been building with this patch on Windows with the Preview version of Visual 
> Studio since last week.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 63804: Added plumbing for operation reconciliation between master and agent.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63804 was successfully built and tested.

Reviews applied: `['63767', '63768', '63804']`

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

- Mesos Reviewbot Windows


On Nov. 14, 2017, 10:56 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63804/
> ---
> 
> (Updated Nov. 14, 2017, 10:56 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, Jie Yu, and Jan 
> Schlicht.
> 
> 
> Bugs: MESOS-8199
> https://issues.apache.org/jira/browse/MESOS-8199
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds new internal protobuf messages to be used by the master
> and agent when reconciling pending/unacknowledged offer operations.
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_provider/resource_provider.proto 
> 1e44b952691fa57b546e979bc5876df3d82d746f 
>   src/messages/messages.proto 33732e28d087a17080eb9aa7d9a91c367172be7f 
>   src/resource_provider/storage/provider.cpp 
> 46224997430ac0c568904d80014166a6f059907f 
>   src/slave/slave.hpp c0acaa639a2bacaa6955ae6c5ab41e75dc1d11f7 
>   src/slave/slave.cpp d8bacebc74790e955490a158c37ac0d9e75fd6b5 
> 
> 
> Diff: https://reviews.apache.org/r/63804/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 63554: Added a publish function in resource provider manager.

[Chun-Hung Hsiao]

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

(Updated Nov. 15, 2017, 2:46 a.m.)


Review request for mesos, Jie Yu, Joseph Wu, and Jan Schlicht.


Changes
---

Rebased.


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


Repository: mesos


Description (updated)
---

The publish function takes a SlaveID and a Resources structure that
includes all resources allocated by all tasks currently running or ready
to run on the agent. Also added validation tests.


Diffs (updated)
-

  src/resource_provider/manager.hpp e7a9a6cc6b15372d8f3f11e17fa2297792b00ff9 
  src/resource_provider/manager.cpp 6dfc42900a1e4249f37cec585f7fe50f5aa94e43 
  src/resource_provider/manager_process.hpp PRE-CREATION 
  src/tests/resource_provider/mock_manager.hpp PRE-CREATION 
  src/tests/resource_provider/mock_manager.cpp PRE-CREATION 
  src/tests/resource_provider_manager_tests.cpp 
ecfe2b4c0952838d6312df603f8eb2f458725175 
  src/tests/resource_provider_validation_tests.cpp 
bf789a05771b7c25f2fc2a8a5b35d38519e4793b 


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

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


Testing
---

make


Thanks,

Chun-Hung Hsiao

Re: Review Request 63389: Added a mock resource provider manager.

[Chun-Hung Hsiao]

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

(Updated Nov. 15, 2017, 2:46 a.m.)


Review request for mesos, Jie Yu, Joseph Wu, and Jan Schlicht.


Changes
---

Rebased.


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


Repository: mesos


Description
---

This patch adds a `ResourceProviderManager*` parameter to the slave
constructor, so we can pass in a mock resource provider manager. The
mock manager can be used to test against either the agent or the
resource provider.

The declaration of `ResourceProviderManagerProcess` is moved to a
separated `manager_process.hpp` header file so we can mock the internal
process.


Diffs (updated)
-

  src/Makefile.am 955f01a665019d7750980ff16f126ad63b524594 
  src/local/local.cpp 99c1b087d75a1759ed61a4e6178e15919bee1e77 
  src/resource_provider/manager.hpp e7a9a6cc6b15372d8f3f11e17fa2297792b00ff9 
  src/resource_provider/manager.cpp 6dfc42900a1e4249f37cec585f7fe50f5aa94e43 
  src/resource_provider/manager_process.hpp PRE-CREATION 
  src/slave/main.cpp 94431b036524ef4db16a594c80f45b64a45728f7 
  src/slave/slave.hpp c0acaa639a2bacaa6955ae6c5ab41e75dc1d11f7 
  src/slave/slave.cpp d8bacebc74790e955490a158c37ac0d9e75fd6b5 
  src/tests/cluster.hpp 65634121b7fe076a7cd9a1c5aa6101a96b1c009d 
  src/tests/cluster.cpp d657da6d84e5ea28e0dad180dce069de569c5d38 
  src/tests/mesos.hpp f25173232df4a9b460194e5e041230e307b80e36 
  src/tests/mesos.cpp 9185b5bf2175be5b0f6b6a03a04e9e5445bf22fd 
  src/tests/mock_slave.hpp 57189cee20511d145ae6b47a4dc2c66a14638dad 
  src/tests/mock_slave.cpp db24f9e5b71f558d2f811f0da8a9cc9c7c2dd341 
  src/tests/resource_provider/mock_manager.hpp PRE-CREATION 
  src/tests/resource_provider/mock_manager.cpp PRE-CREATION 


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

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


Testing
---

make


Thanks,

Chun-Hung Hsiao

Re: Review Request 63388: Handling offer operations in storage local resource provider.

[Chun-Hung Hsiao]

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

(Updated Nov. 15, 2017, 2:45 a.m.)


Review request for mesos, James DeFelice, Jie Yu, Joseph Wu, and Jan Schlicht.


Changes
---

Removed the use of `Promise` and used `getService()`.


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


Repository: mesos


Description (updated)
---

For legacy operations, we just call Resources::apply(). New operations
CREATE_VOLUME, DESTROY_VOLUME, CREATE_BLOCK, DESTROY_BLOCK are
implemented through CSI. Specially, DESTROY_* requires unpublishing
the resources first.


Diffs (updated)
-

  src/resource_provider/storage/provider.cpp 
46224997430ac0c568904d80014166a6f059907f 


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

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


Testing
---

make


Thanks,

Chun-Hung Hsiao

Re: Review Request 63387: Added publish/unpublish in storage local resource provider.

[Chun-Hung Hsiao]

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

(Updated Nov. 15, 2017, 2:44 a.m.)


Review request for mesos, James DeFelice, Jie Yu, Joseph Wu, and Jan Schlicht.


Changes
---

Used `getService()`.


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


Repository: mesos


Description (updated)
---

Storage local resource provider can now handle PUBLISH events.
Although we don't do UNPUBLISH for now, the unpublish function is
still required for deleting volumes.


Diffs (updated)
-

  src/resource_provider/storage/provider.cpp 
46224997430ac0c568904d80014166a6f059907f 


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

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


Testing
---

make


Thanks,

Chun-Hung Hsiao

Re: Review Request 63022: Imported resources from CSI plugins in storage local resource provider.

[Chun-Hung Hsiao]

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

(Updated Nov. 15, 2017, 2:42 a.m.)


Review request for mesos, James DeFelice, Jie Yu, Joseph Wu, and Jan Schlicht.


Changes
---

Used `getService()` to get CSI clients and prepare the plugins during 
initialization.


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


Repository: mesos


Description (updated)
---

The following lists the steps to import resources from a CSI plugin:
1. Launch the node plugin
  1.1 GetSupportedVersions
  1.2 GetPluginInfo
  1.3 ProbeNode
  1.4 GetNodeCapabilities
2. Launch the controller plugin
  2.1 GetSuportedVersions
  2.2 GetPluginInfo
  2.3 GetControllerCapabilities
3. GetCapacity
4. ListVolumes
5. Report to the resource provider through UPDATE_STATE


Diffs (updated)
-

  src/resource_provider/storage/provider.cpp 
46224997430ac0c568904d80014166a6f059907f 


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

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


Testing
---


Thanks,

Chun-Hung Hsiao

Re: Review Request 63021: Added `getService()` function to launch CSI plugins.

[Chun-Hung Hsiao]

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

(Updated Nov. 15, 2017, 2:40 a.m.)


Review request for mesos, James DeFelice, Jie Yu, Joseph Wu, and Jan Schlicht.


Changes
---

Used the new `ContainerDaemon` and split some unrelated changes into r63823.


Summary (updated)
-

Added `getService()` function to launch CSI plugins.


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


Repository: mesos


Description (updated)
---

The `getService()` method first checks if there is already a container
daemon for the specified plugin, and creates a new one if not. The
post-start hook for the container daemon will call `connect()` to
wait for the endpoint socket file to appear and connect to it, then
set up the corresponding promise of CSI client. The post-stop hook will
remove the socket file and create a new promise for the next call to the
post-start hook to set it up.


Diffs (updated)
-

  src/resource_provider/storage/provider.cpp 
46224997430ac0c568904d80014166a6f059907f 


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

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


Testing
---


Thanks,

Chun-Hung Hsiao

Review Request 63823: Initialized and subscribed storage local resource provider.

[Chun-Hung Hsiao]

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

Review request for mesos, Jie Yu, Joseph Wu, and Jan Schlicht.


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


Repository: mesos


Description
---

This patch validates `ResourceProviderInfo` for storage local resource
providers upon creation. During initialization, the storage local
resource provider first tries to recover its ID of the last session
through reading the actual path linked by
`/meta/resource_providers///latest`,
then subscribe to the agent's resource provider manager.

This patch is splitted from https://reviews.apache.org/r/63021/.


Diffs
-

  src/resource_provider/storage/provider.cpp 
46224997430ac0c568904d80014166a6f059907f 


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


Testing
---


Thanks,

Chun-Hung Hsiao

Re: Review Request 63368: Added MemoryProfiler class to Libprocess.

[Benjamin Mahler]

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



Some suggestions from going over it together:

* Maybe add an optional duration to the profiling start request? I personally 
would want to enable it on a window basis so that I don't have to remember to 
follow up and stop to prevent an OOM.
* Could we look into the ability to get a profile that has the symbols 
resolved? (i.e. "--raw"?) That way, we could analyze the profile on a different 
machine or on a mac laptop.
* In the markdown documentation, in addition to the changes we made, it would 
be good to explain what the different profiles are (image vs text vs raw).

- Benjamin Mahler


On Oct. 27, 2017, 7:04 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> ---
> 
> (Updated Oct. 27, 2017, 7:04 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This class exposes profiling functionality of jemalloc memory allocator
> when it is detected to be the memory allocator of the current process.
> 
> In particular, it gives developers an easy method to collect and
> access heap profiles which report which pieces of code were
> responsible for allocating memory.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/Makefile.am 03a0ca87f31744c716c99e05aa07242fed480675 
>   3rdparty/libprocess/include/process/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 
> dc3375ce62556322eb2bc60ade61f313ade123b8 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 
> 71ae7129ffbd0e22eda2863b17bbcf588298c37b 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 63818: Added quick start instructions for CMake.

[Gaston Kleiman]

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


Ship it!




This is super helpful, thanks!


docs/cmake.md
Lines 43 (patched)


I had to look up how to pass more parameters to make in order to use more 
threads:

`cmake --build . -- -j 4`

It might be useful to mention that here =).


- Gaston Kleiman


On Nov. 14, 2017, 3:12 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63818/
> ---
> 
> (Updated Nov. 14, 2017, 3:12 p.m.)
> 
> 
> Review request for mesos, Gaston Kleiman and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added quick start instructions for CMake.
> 
> 
> Diffs
> -
> 
>   docs/cmake.md b8f9e7caa84372404256767a0220ac1df9db8bc8 
> 
> 
> Diff: https://reviews.apache.org/r/63818/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 63799: Improved log messages of offer operations.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63799 was successfully built and tested.

Reviews applied: `['63751', '63797', '63798', '63799']`

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

- Mesos Reviewbot Windows


On Nov. 14, 2017, 2:12 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63799/
> ---
> 
> (Updated Nov. 14, 2017, 2:12 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved log messages of offer operations.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/message.hpp 05879cdc01dca88440ce89beb487168c033fab37 
>   src/slave/slave.cpp d8bacebc74790e955490a158c37ac0d9e75fd6b5 
> 
> 
> Diff: https://reviews.apache.org/r/63799/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 63817: Windows: Enabled more agent tests.

[Andrew Schwartzmeyer]

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

(Updated Nov. 14, 2017, 3:56 p.m.)


Review request for mesos, Akash Gupta, Jie Yu, John Kordich, Joseph Wu, and 
Michael Park.


Changes
---

Add test results.


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


Repository: mesos


Description
---

These tests "just worked" once the isolators and command was fixed to be
Windows-compatible. Particularly, `/bin/echo --author` was replaced with
an equivalent PowerShell command, and `posix/cpu,posix/mem` was replaced
with `windows/cpu`.


Diffs
-

  src/tests/slave_tests.cpp 61dd940e4452a1dbc6c0361bfc917bdc2dd16be8 


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


Testing (updated)
---

This is the preliminary work I was doing to start fixing the agent recovery 
path on Windows. However, since we don't do _any_ checkpointing currently, that 
bug is much more involved, and I wanted to get these patches up sooner rather 
than later as they enable a dozen more agent tests.

This is dependent on the isolator chain because of the cleanup work done in 
`stout/windows/os.hpp` etc. that this code also touched.

Test results in non-administrative prompt:

# mesos-tests

```
[--] Global test environment tear-down
[==] 755 tests from 78 test cases ran. (156570 ms total)
[  PASSED  ] 755 tests.

  YOU HAVE 182 DISABLED TESTS
```

In comparison to `master`:

```
[--] Global test environment tear-down
[==] 747 tests from 76 test cases ran. (337529 ms total)
[  PASSED  ] 747 tests.

  YOU HAVE 187 DISABLED TESTS
```

# libprocess-tests

```
[--] Global test environment tear-down
[==] 158 tests from 29 test cases ran. (992 ms total)
[  PASSED  ] 158 tests.
```

# stout-tests

```
[--] Global test environment tear-down
[==] 259 tests from 43 test cases ran. (5185 ms total)
[  PASSED  ] 259 tests.
```


Thanks,

Andrew Schwartzmeyer

Re: Review Request 63677: Changed `os:Stack` to allocate with `mmap`.

[James Peach]

> On Nov. 14, 2017, 3:23 p.m., Andrei Budnik wrote:
> > 3rdparty/stout/include/stout/os/linux.hpp
> > Line 66 (original), 66 (patched)
> > 
> >
> > `ErrnoError` is not async-signal-safe as it constructs empty 
> > `std::string` in c'tor.
> > 
> > I would suggest to change the return type of `allocate()` to `int` and 
> > get rid of `create` static function.
> 
> James Peach wrote:
> Yup, this isn't intended to be async-safe. If you need that, you are 
> supposed to call `allocate` manually. I'm planning make a change to propagate 
> this `Error` in MESOS-8155.
> 
> Why do you suggest `int` as the return type? To match the syscall 
> convention, or to return an `errno` directly?
> 
> Andrei Budnik wrote:
> Ah, got it. Yeah, I meant matching syscall convention. If you decide to 
> leave `create`, it would be nice to add a comment like:
> ```c++
> // NOTE: this function is not async-signal safe.
> ```

Done.

FWIW I experimented with returning `errno` but it was unexpectedly awkward and 
the lack of an `errno_t` type make it confusing. I chose `bool` to match the 
syscall interface and remove the ambiguity of `int`.


- James


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


On Nov. 14, 2017, 7:11 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63677/
> ---
> 
> (Updated Nov. 14, 2017, 7:11 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8159
> https://issues.apache.org/jira/browse/MESOS-8159
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If `os:Stack` is allocated with `mmap`, this automatically
> guarantees the correct alignment (since `mmap` is page-aligned
> by definition).
> 
> Although `mmap` is not required to be async signal safe
> (i.e. it might not be safe to call from a signal handler), in
> practice it should be safe to call between `fork` and `exec`
> since it is a raw system call that does not require any user
> space locks to be held.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/linux.hpp 
> 68b4090aa5e6f23609c487128d91301755ae0e46 
> 
> 
> Diff: https://reviews.apache.org/r/63677/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Review Request 63816: Windows: Fixed MESOS-6816 to enable `ExecutorEnvironmentVariables`.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Jie Yu, John Kordich, Joseph Wu, and 
Michael Park.


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


Repository: mesos


Description
---

By removing the explicit system environment override code, and instead
setting the system environment as the default in the `CreateProcess`
wrapper, the `SlaveTest.ExecutorEnvironmentVariables` can be enabled.
Note that this also required fixing `os::host_default_path()` and using
it, because the test used `PATH = /bin`, which breaks on Windows.


Diffs
-

  src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67 
  src/slave/containerizer/mesos/launch.cpp 
b1584ff292ada5463917792908a13e00859fd1ae 
  src/tests/slave_tests.cpp 61dd940e4452a1dbc6c0361bfc917bdc2dd16be8 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 63812: Windows: Fixed `os::realpath` to behave like POSIX version.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Jie Yu, John Kordich, Joseph Wu, and 
Michael Park.


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


Repository: mesos


Description
---

The existing implementation (1) did not resolve symlinks and (2) did not
check for existence of the path. This resulted in unexpected behaviors
by users of the function.


Diffs
-

  3rdparty/stout/include/stout/os/windows/realpath.hpp 
81a33bd2708c0871c4f23c12fdd29fc8d35682e3 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 63817: Windows: Enabled more agent tests.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Jie Yu, John Kordich, Joseph Wu, and 
Michael Park.


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


Repository: mesos


Description
---

These tests "just worked" once the isolators and command was fixed to be
Windows-compatible. Particularly, `/bin/echo --author` was replaced with
an equivalent PowerShell command, and `posix/cpu,posix/mem` was replaced
with `windows/cpu`.


Diffs
-

  src/tests/slave_tests.cpp 61dd940e4452a1dbc6c0361bfc917bdc2dd16be8 


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


Testing
---

Lost the test results screen, re-running.


Thanks,

Andrew Schwartzmeyer

Review Request 63809: Windows: Fixed symlink code to not need admin privileges.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Jie Yu, John Kordich, Joseph Wu, and 
Michael Park.


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


Repository: mesos


Description
---

This new feature of Windows 10 allows us to make symlinks without being
an administrator. In the future, we may want to do a version check, as
this is likely to fail on versions of Windows which don't support this
flag. Resolves MESOS-7370.


Diffs
-

  3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp 
e3d04480cf793482d958ea9571d96a29b6bf09b1 


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


Testing
---

Ran `mesos-tests` not in an admin prompt!


Thanks,

Andrew Schwartzmeyer

Review Request 63815: Windows: Fixed environment priorities in `shell.hpp`.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Jie Yu, John Kordich, Joseph Wu, and 
Michael Park.


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


Repository: mesos


Description
---

Legacy code had caused the system environment to explicitly override the
supplied environment, but this is incorrect. The system environment
should be the default, with any supplied environment variables
overriding those.


Diffs
-

  3rdparty/stout/include/stout/os/windows/shell.hpp 
5c711837a3adbc89ad8793666e68eff7102566b3 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 63814: Windows: Fixed `os::host_default_path()`.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Jie Yu, John Kordich, Joseph Wu, and 
Michael Park.


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


Repository: mesos


Description
---

This adds the necessary and expected default paths to the default `PATH`
value represented by this function. Especially needed was the
`WindowsPowerShell` folder so that tasks using PowerShell can run.


Diffs
-

  3rdparty/stout/include/stout/windows/os.hpp 
09ddec6d69472cd13b453fe1a77fdbe343fc23c8 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 63813: Windows: Fixed name of default executor.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Jie Yu, John Kordich, Joseph Wu, and 
Michael Park.


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


Repository: mesos


Description
---

This path is checked with `os::realpath`, which now correctly errors if
the path does not exist. Therefore the name must be correct.


Diffs
-

  src/slave/constants.hpp 401590605350ef1749b45b32c659afdaa43f43ad 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 63810: Windows: Added internal `fullpath` API to normalize paths.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Jie Yu, John Kordich, Joseph Wu, and 
Michael Park.


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


Repository: mesos


Description
---

This explicitly does not check for existence, nor does it follow
symlinks. It simply normalizes a given path to an absolute path. This
removes the `reparsepoint.hpp` and `symlink.hpp` dependency os
`os::realpath`.


Diffs
-

  3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp 
e3d04480cf793482d958ea9571d96a29b6bf09b1 
  3rdparty/stout/include/stout/internal/windows/symlink.hpp 
b9cf1229ec0b6c70fe9b9d358e867e91e475dfaa 
  3rdparty/stout/include/stout/windows/fs.hpp 
d7b883ca1f3f8972e1bf9fbd211cc564c7c30f14 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 63811: Windows: Added `get_handle_follow` which follows symlinks.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Jie Yu, John Kordich, Joseph Wu, and 
Michael Park.


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


Repository: mesos


Description
---

This is the opposite of `get_handle_no_follow`. They both use the
inappropriately named Windows API `CreateFile` to retrieve a `HANDLE` to
an existing file or directory. This existing `get_handle_no_follow`
explicitly gets a handle to the symlink itself, and the new
`get_handle_follow` explicitly resolves symlinks and gets a handle to
the target.

This may fail if the target doesn't exist.


Diffs
-

  3rdparty/stout/include/stout/internal/windows/reparsepoint.hpp 
e3d04480cf793482d958ea9571d96a29b6bf09b1 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 63818: Added quick start instructions for CMake.

[Andrew Schwartzmeyer]

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

Review request for mesos, Gaston Kleiman and Greg Mann.


Repository: mesos


Description
---

Added quick start instructions for CMake.


Diffs
-

  docs/cmake.md b8f9e7caa84372404256767a0220ac1df9db8bc8 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 63804: Added plumbing for operation reconciliation between master and agent.

[Greg Mann]

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

(Updated Nov. 14, 2017, 10:56 p.m.)


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


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


Repository: mesos


Description (updated)
---

This patch adds new internal protobuf messages to be used by the master
and agent when reconciling pending/unacknowledged offer operations.


Diffs (updated)
-

  include/mesos/resource_provider/resource_provider.proto 
1e44b952691fa57b546e979bc5876df3d82d746f 
  src/messages/messages.proto 33732e28d087a17080eb9aa7d9a91c367172be7f 
  src/resource_provider/storage/provider.cpp 
46224997430ac0c568904d80014166a6f059907f 
  src/slave/slave.hpp c0acaa639a2bacaa6955ae6c5ab41e75dc1d11f7 
  src/slave/slave.cpp d8bacebc74790e955490a158c37ac0d9e75fd6b5 


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

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


Testing
---

make check


Thanks,

Greg Mann

Re: Review Request 63804: Added plumbing for operation reconciliation between master and agent.

[Greg Mann]

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

(Updated Nov. 14, 2017, 10:38 p.m.)


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


Summary (updated)
-

Added plumbing for operation reconciliation between master and agent.


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


Repository: mesos


Description (updated)
---

This patch adds new internal protobuf messages to be used by the master
and agent when reconciling pending/unacknowledged offer operations. It
also adds a stub handler to the agent.


Diffs
-

  include/mesos/resource_provider/resource_provider.proto 
1e44b952691fa57b546e979bc5876df3d82d746f 
  src/messages/messages.proto 33732e28d087a17080eb9aa7d9a91c367172be7f 
  src/resource_provider/storage/provider.cpp 
46224997430ac0c568904d80014166a6f059907f 
  src/slave/slave.hpp c0acaa639a2bacaa6955ae6c5ab41e75dc1d11f7 
  src/slave/slave.cpp d8bacebc74790e955490a158c37ac0d9e75fd6b5 


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


Testing
---

make check


Thanks,

Greg Mann

Re: Review Request 63804: Added protos for operation reconciliation between master and agent.

[Greg Mann]

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

(Updated Nov. 14, 2017, 10:37 p.m.)


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


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


Repository: mesos


Description
---

This patch adds new internal protobuf messages to be used by the master
and agent when reconciling pending/unacknowledged offer operations.


Diffs (updated)
-

  include/mesos/resource_provider/resource_provider.proto 
1e44b952691fa57b546e979bc5876df3d82d746f 
  src/messages/messages.proto 33732e28d087a17080eb9aa7d9a91c367172be7f 
  src/resource_provider/storage/provider.cpp 
46224997430ac0c568904d80014166a6f059907f 
  src/slave/slave.hpp c0acaa639a2bacaa6955ae6c5ab41e75dc1d11f7 
  src/slave/slave.cpp d8bacebc74790e955490a158c37ac0d9e75fd6b5 


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

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


Testing
---

make check


Thanks,

Greg Mann

Re: Review Request 63768: Added 'ReconcileOfferOperations' response to scheduler API.

[Gaston Kleiman]

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


Fix it, then Ship it!





src/scheduler/scheduler.cpp
Lines 123-124 (original), 123 (patched)


For consistency I think that it'd also be nice to remove the `using 
process::http::Request` statement.

Also I noticed that we use `::Request` in this file... so removing the 
`using` statement and replacing `::Request` with the more explicit 
`process::http::Request` feels to me like an improvement.


- Gaston Kleiman


On Nov. 14, 2017, 1:44 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63768/
> ---
> 
> (Updated Nov. 14, 2017, 1:44 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, Jie Yu, and Jan 
> Schlicht.
> 
> 
> Bugs: MESOS-8198
> https://issues.apache.org/jira/browse/MESOS-8198
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a first-class `Response` message to the scheduler API
> along with its first member, the `ReconcileOfferOperations` response
> message.
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler/scheduler.proto 
> f5dda10a076ecae53a56875d0ed4d201a1c92f12 
>   include/mesos/v1/scheduler/scheduler.proto 
> 4fa6606b0db701aebe208b962b7768da68b180ab 
>   src/scheduler/scheduler.cpp 286a7603352a1e639a0502e496d4409be11bf9d7 
> 
> 
> Diff: https://reviews.apache.org/r/63768/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 63796: Made `CheckerProcess` support IPv6 for HTTP/TCP check.

[Mesos Reviewbot Windows]

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



FAIL: Some Mesos libprocess-tests failed.

Reviews applied: `['63794', '63795', '63796']`

Failed command: 
`C:\DCOS\mesos\3rdparty\libprocess\src\tests\Debug\libprocess-tests.exe`

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

Relevant logs:

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

```
[   OK ] Scheme/HTTPTest.WWWAuthenticateHeader/0 (683 ms)
[ RUN  ] Scheme/HTTPTest.WWWAuthenticateHeader/1
[   OK ] Scheme/HTTPTest.WWWAuthenticateHeader/1 (77 ms)
[ RUN  ] Scheme/HTTPTest.Accepts/0
[   OK ] Scheme/HTTPTest.Accepts/0 (448 ms)
[ RUN  ] Scheme/HTTPTest.Accepts/1
[   OK ] Scheme/HTTPTest.Accepts/1 (102 ms)
[--] 34 tests from Scheme/HTTPTest (14148 ms total)

[--] 4 tests from SSLVerifyIPAdd/SSLTest
[ RUN  ] SSLVerifyIPAdd/SSLTest.BasicSameProcess/0
[   OK ] SSLVerifyIPAdd/SSLTest.BasicSameProcess/0 (418 ms)
[ RUN  ] SSLVerifyIPAdd/SSLTest.BasicSameProcess/1
[   OK ] SSLVerifyIPAdd/SSLTest.BasicSameProcess/1 (1134 ms)
[ RUN  ] SSLVerifyIPAdd/SSLTest.RequireCertificate/0
[   OK ] SSLVerifyIPAdd/SSLTest.RequireCertificate/0 (1193 ms)
[ RUN  ] SSLVerifyIPAdd/SSLTest.RequireCertificate/1
[   OK ] SSLVerifyIPAdd/SSLTest.RequireCertificate/1 (950 ms)
[--] 4 tests from SSLVerifyIPAdd/SSLTest (3791 ms total)

[--] Global test environment tear-down
[==] 201 tests from 33 test cases ran. (45647 ms total)
[  PASSED  ] 200 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] SSLTest.ProtocolMismatch

 1 FAILED TEST
  YOU HAVE 19 DISABLED TESTS

```

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

```
I1114 22:04:18.386386  1016 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1114 22:04:18.386386  1016 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peWARNING: Logging before 
InitGoogleLogging() is written to STDERR
I1114 22:04:18.689396  1428 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1114 22:04:18.690397  1428 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I1114 22:04:18.690397  1428 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1114 22:04:18.696396  1428 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\LAwIJC\cert.pem
WARNING: Logging before InitGoogleLogging() is written to STDERR
I1114 22:04:19.719311  2776 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1114 22:04:19.720314  2776 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I1114 22:04:19.720314  2776 openssl.cpp:526] Will use IP address verification 
in subject alternative name certificate extension.
I1114 22:04:19.720314  2776 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1114 22:04:19.721314  2776 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\7aFJSF\cert.pem
er certificate verification
I1114 22:04:18.402449  1016 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1114 22:04:18.403388  1016 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\LAwIJC\cert.pem
I1114 22:04:19.413309  1016 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1114 22:04:19.413309  1016 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I1114 22:04:19.413309  1016 openssl.cpp:526] Will use IP address verification 
in subject alternative name certificate extension.
I1114 22:04:19.413309  1016 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1114 22:04:19.414309  1016 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\7aFJSF\cert.pem
I1114 22:04:19.901316  2760 process.cpp:1052] Failed to accept socket: future 
discarded
```

- Mesos Reviewbot Windows


On Nov. 14, 2017, 2:31 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https

Re: Review Request 63804: Added protos for operation reconciliation between master and agent.

[Greg Mann]

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

(Updated Nov. 14, 2017, 9:56 p.m.)


Review request for mesos and Greg Mann.


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


Repository: mesos


Description
---

This patch adds new internal protobuf messages to be used by the master
and agent when reconciling pending/unacknowledged offer operations.


Diffs (updated)
-

  include/mesos/resource_provider/resource_provider.proto 
1e44b952691fa57b546e979bc5876df3d82d746f 
  src/messages/messages.proto 33732e28d087a17080eb9aa7d9a91c367172be7f 
  src/resource_provider/storage/provider.cpp 
46224997430ac0c568904d80014166a6f059907f 


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

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


Testing
---

make check


Thanks,

Greg Mann

Re: Review Request 63804: Added protos for operation reconciliation between master and agent.

[Greg Mann]

> On Nov. 14, 2017, 7:18 p.m., Gaston Kleiman wrote:
> > include/mesos/resource_provider/resource_provider.proto
> > Lines 72-75 (patched)
> > 
> >
> > Isn't the idea to use this message only when the master wants to check 
> > if an operation was received by the RP or dropped on its way?
> > 
> > In such case the RP should only generate an offer operation status 
> > update if the operation is unknown; the state of this update would be 
> > `OFFER_OPERATION_DROPPED`.

Yep you're right, thanks :)


- Greg


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


On Nov. 14, 2017, 6:41 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63804/
> ---
> 
> (Updated Nov. 14, 2017, 6:41 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-8199
> https://issues.apache.org/jira/browse/MESOS-8199
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds new internal protobuf messages to be used by the master
> and agent when reconciling pending/unacknowledged offer operations.
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_provider/resource_provider.proto 
> 1e44b952691fa57b546e979bc5876df3d82d746f 
>   src/messages/messages.proto 33732e28d087a17080eb9aa7d9a91c367172be7f 
>   src/resource_provider/storage/provider.cpp 
> 46224997430ac0c568904d80014166a6f059907f 
> 
> 
> Diff: https://reviews.apache.org/r/63804/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 63768: Added 'ReconcileOfferOperations' response to scheduler API.

[Greg Mann]

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

(Updated Nov. 14, 2017, 9:44 p.m.)


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


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


Repository: mesos


Description
---

This patch adds a first-class `Response` message to the scheduler API
along with its first member, the `ReconcileOfferOperations` response
message.


Diffs (updated)
-

  include/mesos/scheduler/scheduler.proto 
f5dda10a076ecae53a56875d0ed4d201a1c92f12 
  include/mesos/v1/scheduler/scheduler.proto 
4fa6606b0db701aebe208b962b7768da68b180ab 
  src/scheduler/scheduler.cpp 286a7603352a1e639a0502e496d4409be11bf9d7 


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

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


Testing
---

make check


Thanks,

Greg Mann

Re: Review Request 63680: Added a container daemon to monitor a long-running standalone container.

[Chun-Hung Hsiao]

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

(Updated Nov. 14, 2017, 9:41 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Joseph Wu.


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


Repository: mesos


Description (updated)
---

The `ContanierDaemon` class is responsible to monitor if a long-running
service running in a standalone container, and restart the service
container after it exits. It takes the following hook functions:

* `postStartHook`: called after the container is launched every time.
* `postStopHook`: called after the container exits every time.

`ContainerDaemon` does not manage the lifecycle of the contanier it
monitors, so the container persists across `ContainerDaemon`s. However,
it provides a `terminate()` method to completely shutdown the service
container.


Diffs (updated)
-

  src/Makefile.am 955f01a665019d7750980ff16f126ad63b524594 
  src/slave/container_daemon.hpp PRE-CREATION 
  src/slave/container_daemon.cpp PRE-CREATION 


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

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


Testing
---


Thanks,

Chun-Hung Hsiao

Re: Review Request 63652: Added d_type check in containerizer backend validation.

[Meng Zhu]

> On Nov. 14, 2017, 9:32 a.m., James Peach wrote:
> > This is looking pretty good.
> > 
> > You should be able to write a test for this using the 
> > [ROOT_XFS_TestBase](https://github.com/apache/mesos/blob/master/src/tests/containerizer/xfs_quota_tests.cpp)
> >  fixture. Probably a reasonable approach would be to make a XFS filesystem 
> > with and without `d_type` support and verify that `Provisioner::create` 
> > succeeds or fails.

Thank you for the quick and informative review, James.

I was planning to add a unit test. But the problem is that it would require XFS 
installed. We will have to add a new configuration flag which seems to 
overkill. Thus I only did manual tests following a similar procedure. 

What do you think?


> On Nov. 14, 2017, 9:32 a.m., James Peach wrote:
> > src/slave/containerizer/mesos/provisioner/provisioner.cpp
> > Lines 154 (patched)
> > 
> >
> > The caller is responsible for logging the error. Let's be consistent 
> > and remove this `LOG`.

I am worrying that, in the case of default backend selection, if the 
underlying, say XFS, does not support `d_type`, the provisioner will just 
silently eat the error message (near line 275) and fall back to a less ideal 
backend. The user would have no clue of what happened. Putting the warning 
message here is a simple thing that we can do to give some hint to the user.

What do you think?


- Meng


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


On Nov. 14, 2017, 1:30 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63652/
> ---
> 
> (Updated Nov. 14, 2017, 1:30 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and James Peach.
> 
> 
> Bugs: MESOS-8121
> https://issues.apache.org/jira/browse/MESOS-8121
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In unified containerizer, the d_type cannot be 1
> if we are using the overlay fs backend.
> Raise error if user specifies overlayfs as backend.
> Fallback to other backends in the default case and
> raise a warning.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp cbc8bf79083ce2bc34fa698808eaf92764a577a9 
>   src/linux/fs.cpp 40c31a3ad2ccbeb59868c5e2d7b1809c613da8de 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 450a3b32d69d2882973a6ed4e94e169a0256056b 
> 
> 
> Diff: https://reviews.apache.org/r/63652/diff/4/
> 
> 
> Testing
> ---
> 
> Manually tested slave creation with default and overlayfs backends on XFS 
> based work_dir with different ftype mount options.
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 63652: Added d_type check in containerizer backend validation.

[Meng Zhu]

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

(Updated Nov. 14, 2017, 1:30 p.m.)


Review request for mesos, Gilbert Song and James Peach.


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


Repository: mesos


Description
---

In unified containerizer, the d_type cannot be 1
if we are using the overlay fs backend.
Raise error if user specifies overlayfs as backend.
Fallback to other backends in the default case and
raise a warning.


Diffs (updated)
-

  src/linux/fs.hpp cbc8bf79083ce2bc34fa698808eaf92764a577a9 
  src/linux/fs.cpp 40c31a3ad2ccbeb59868c5e2d7b1809c613da8de 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
450a3b32d69d2882973a6ed4e94e169a0256056b 


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

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


Testing
---

Manually tested slave creation with default and overlayfs backends on XFS based 
work_dir with different ftype mount options.


Thanks,

Meng Zhu

Re: Review Request 63753: Upgraded vendored protobuf to 3.5.0.

[Dmitry Zhuk]

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

(Updated Nov. 14, 2017, 8:57 p.m.)


Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

Version 3.4 adds and version 3.5 improves move support in generated
C++ code.
`protobuf-3.5.0.tar.gz` was built using the script:
```
curl -L -O https://github.com/google/protobuf/archive/v3.5.0.tar.gz
tar -xzf v3.5.0.tar.gz
cd protobuf-3.5.0
./autogen.sh
cd ..
tar -czf protobuf-3.5.0.tar.gz protobuf-3.5.0
```


Diffs (updated)
-

  3rdparty/cmake/Versions.cmake da2d837e327535c793f20929e8c6e9ddaf5ca54a 
  3rdparty/protobuf-3.3.0.tar.gz 98fbec86484032753c4022b4519ec40243e4e951 
  3rdparty/protobuf-3.5.0.tar.gz PRE-CREATION 
  3rdparty/versions.am 2fc2f2c3932ff86c348d954bd4d78aa5c75bb052 
  LICENSE 1327411d038dfbda5a8324290469707ae02a4aa3 
  configure.ac 067005e1be3a09e8320be120c8784f195e30ac5d 
  src/CMakeLists.txt b0e223121f24ac0dc604dc2c639158b817eec535 
  src/java/mesos.pom.in 56428832820efccd72d302d1b2cf1b3a4554960b 
  src/python/interface/setup.py.in abf160c9741f61a37f5a0fe3f5fc64bbddc02142 
  src/python/native_common/ext_modules.py.in 
39e0faad414f2c826dacfe1302106f2d5e7c8967 
  src/python/protocol/setup.py.in 0fa0c1724bd39ab573e14a9137e4e3fe517c9c93 
  support/mesos-tidy/entrypoint.sh dbea146a882510bda4e5868d8c2ced2e97a36d83 


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

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


Testing
---

make check

cmake --build .


Thanks,

Dmitry Zhuk

Re: Review Request 63674: Updated a comment about resubscribing completed frameworks.

[Gaston Kleiman]

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




src/master/master.cpp
Line 2671 (original), 2671 (patched)


s/opeartor/operator/


- Gaston Kleiman


On Nov. 13, 2017, 12:45 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63674/
> ---
> 
> (Updated Nov. 13, 2017, 12:45 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Greg Mann, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 49dbaa979d692061a7b479b1db4511e8357b8baf 
> 
> 
> Diff: https://reviews.apache.org/r/63674/diff/2/
> 
> 
> Testing
> ---
> 
> Tested in repetition on Mac OS 10.11.6
> 
> `GTEST_FILTER="*SchedulerTest.MasterFailover*" ./bin/mesos-tests.sh --verbose 
> --gtest_repeat=500 --gtest_break_on_failure`
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 63768: Added 'ReconcileOfferOperations' response to scheduler API.

[Greg Mann]

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

(Updated Nov. 14, 2017, 8:48 p.m.)


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


Summary (updated)
-

Added 'ReconcileOfferOperations' response to scheduler API.


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


Repository: mesos


Description (updated)
---

This patch adds a first-class `Response` message to the scheduler API
along with its first member, the `ReconcileOfferOperations` response
message.


Diffs
-

  include/mesos/scheduler/scheduler.proto 
f5dda10a076ecae53a56875d0ed4d201a1c92f12 
  include/mesos/v1/scheduler/scheduler.proto 
4fa6606b0db701aebe208b962b7768da68b180ab 
  src/scheduler/scheduler.cpp 286a7603352a1e639a0502e496d4409be11bf9d7 


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


Testing
---

make check


Thanks,

Greg Mann

Re: Review Request 63677: Changed `os:Stack` to allocate with `mmap`.

[Andrei Budnik]

> On Nov. 14, 2017, 3:23 p.m., Andrei Budnik wrote:
> > 3rdparty/stout/include/stout/os/linux.hpp
> > Line 66 (original), 66 (patched)
> > 
> >
> > `ErrnoError` is not async-signal-safe as it constructs empty 
> > `std::string` in c'tor.
> > 
> > I would suggest to change the return type of `allocate()` to `int` and 
> > get rid of `create` static function.
> 
> James Peach wrote:
> Yup, this isn't intended to be async-safe. If you need that, you are 
> supposed to call `allocate` manually. I'm planning make a change to propagate 
> this `Error` in MESOS-8155.
> 
> Why do you suggest `int` as the return type? To match the syscall 
> convention, or to return an `errno` directly?

Ah, got it. Yeah, I meant matching syscall convention. If you decide to leave 
`create`, it would be nice to add a comment like:
```c++
// NOTE: this function is not async-signal safe.
```


- Andrei


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


On Nov. 14, 2017, 7:11 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63677/
> ---
> 
> (Updated Nov. 14, 2017, 7:11 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8159
> https://issues.apache.org/jira/browse/MESOS-8159
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If `os:Stack` is allocated with `mmap`, this automatically
> guarantees the correct alignment (since `mmap` is page-aligned
> by definition).
> 
> Although `mmap` is not required to be async signal safe
> (i.e. it might not be safe to call from a signal handler), in
> practice it should be safe to call between `fork` and `exec`
> since it is a raw system call that does not require any user
> space locks to be held.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/linux.hpp 
> 68b4090aa5e6f23609c487128d91301755ae0e46 
> 
> 
> Diff: https://reviews.apache.org/r/63677/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 63767: Added a missing protobuf field to the unversioned definitions.

[Gaston Kleiman]

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


Ship it!




Ship It!

- Gaston Kleiman


On Nov. 13, 2017, 3:17 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63767/
> ---
> 
> (Updated Nov. 13, 2017, 3:17 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, Jie Yu, and Jan 
> Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new `OfferOperationUpdate` event type was added to the unversioned
> scheduler API protobuf definitions, but the corresponding field was not
> added to the `Event` message.
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler/scheduler.proto 
> f5dda10a076ecae53a56875d0ed4d201a1c92f12 
> 
> 
> Diff: https://reviews.apache.org/r/63767/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 63577: Fixed a task status update race in default executor tests.

[Gaston Kleiman]

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


Fix it, then Ship it!





src/tests/default_executor_tests.cpp
Line 400 (original), 458 (patched)


Nit: at this point the `killedUpdate` futures could be ready, so doing the 
following would be nice:

```
ASSERT_TRUE(killedUpdate1.isPending());
ASSERT_TRUE(killedUpdate2.isPending());
ASSERT_TRUE(killedUpdate3.isPending());
```



src/tests/default_executor_tests.cpp
Line 419 (original), 465 (patched)


Nit: the following assert would be nice here:

```
// Verify that only the tasks in the first group were killed.
ASSERT_TRUE(killedUpdate3.isPending());
```



src/tests/default_executor_tests.cpp
Line 1176 (original), 1282 (patched)


I'd add here:

```
ASSERT_TRUE(killedUpdate2.isPending());
```


- Gaston Kleiman


On Nov. 10, 2017, 5:02 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63577/
> ---
> 
> (Updated Nov. 10, 2017, 5:02 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Gaston Kleiman.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously in the test `DefaultExecutorTest.KillMultipleTasks` and
> `DefaultExecutorTest.KillTaskGroupOnTaskFailure`, when launching a
> task group which has multiple tasks, we expected the scheduler will
> receive all the TASK_STARTING status updates before receiving any
> TASK_RUNNING status updates. However this is not guaranteed, e.g.,
> it is possible for the scheduler to receive TASK_RUNNING for the
> first task before receiving TASK_STARTING for the second task.
> 
> So in this patch, we used `Sequence` to guarantee the order of
> TASK_STARTING and TASK_RUNNING for each task but do not care about
> the order between tasks.
> 
> The following 3 tests have their own solutions to handle this issue,
> in this patch, I updated them to use the above solution.
>   `DefaultExecutorTest.KillTask`
>   `DefaultExecutorTest.CommitSuicideOnKillTask`
>   `DefaultExecutorTest.ROOT_ContainerStatusForTask`
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> 65113985acc0a8fac00374b074ef568aaa22c7ac 
>   src/tests/mesos.hpp e25defeb55608136e77363aa48cf820092a13a59 
> 
> 
> Diff: https://reviews.apache.org/r/63577/diff/2/
> 
> 
> Testing
> ---
> 
> This patch touched 5 tests:
> 1. KillTask
> 2. KillMultipleTasks
> 3. KillTaskGroupOnTaskFailure
> 4. CommitSuicideOnKillTask
> 5. ROOT_ContainerStatusForTask
> 
> I ran each of them repeatedly (20 times), and all of them succeeded.
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 63804: Added protos for operation reconciliation between master and agent.

[Gaston Kleiman]

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




include/mesos/resource_provider/resource_provider.proto
Lines 72-75 (patched)


Isn't the idea to use this message only when the master wants to check if 
an operation was received by the RP or dropped on its way?

In such case the RP should only generate an offer operation status update 
if the operation is unknown; the state of this update would be 
`OFFER_OPERATION_DROPPED`.


- Gaston Kleiman


On Nov. 14, 2017, 10:41 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63804/
> ---
> 
> (Updated Nov. 14, 2017, 10:41 a.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-8199
> https://issues.apache.org/jira/browse/MESOS-8199
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds new internal protobuf messages to be used by the master
> and agent when reconciling pending/unacknowledged offer operations.
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_provider/resource_provider.proto 
> 1e44b952691fa57b546e979bc5876df3d82d746f 
>   src/messages/messages.proto 33732e28d087a17080eb9aa7d9a91c367172be7f 
>   src/resource_provider/storage/provider.cpp 
> 46224997430ac0c568904d80014166a6f059907f 
> 
> 
> Diff: https://reviews.apache.org/r/63804/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 63768: Added 'ReconcileOfferOperations' reponse to scheduler API.

[Gaston Kleiman]

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



A few minor comments.

Also please `s/reponse/response/` in the patch summary =).


include/mesos/scheduler/scheduler.proto
Lines 211 (patched)


s/master operator API/scheduler API/



include/mesos/v1/scheduler/scheduler.proto
Lines 209 (patched)


Ditto.


- Gaston Kleiman


On Nov. 13, 2017, 3:25 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63768/
> ---
> 
> (Updated Nov. 13, 2017, 3:25 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, Jie Yu, and Jan 
> Schlicht.
> 
> 
> Bugs: MESOS-8198
> https://issues.apache.org/jira/browse/MESOS-8198
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added 'ReconcileOfferOperations' reponse to scheduler API.
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler/scheduler.proto 
> f5dda10a076ecae53a56875d0ed4d201a1c92f12 
>   include/mesos/v1/scheduler/scheduler.proto 
> 4fa6606b0db701aebe208b962b7768da68b180ab 
>   src/scheduler/scheduler.cpp 286a7603352a1e639a0502e496d4409be11bf9d7 
> 
> 
> Diff: https://reviews.apache.org/r/63768/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 63678: Improved the signal safety of `ns::clone`.

[James Peach]

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

(Updated Nov. 14, 2017, 7:11 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description
---

`ns::clone` was implicitly using malloc to allocate a stack
after fork but before exec, where we should be avoiding
`malloc`.

Updated the second clone to use `os::signal_safe::clone`. We
still explicitly allocate a stack after the fork, but since
that is allocated with `mmap` is it safe in this particular
case.


Diffs (updated)
-

  src/linux/ns.cpp 33751b518e95f4f67e6864728e1fdf621f50dd52 


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

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


Testing
---

sudo make check (Fedora 26)

NOTE: I tried to avoid changing `ns::clone` too much in this patch because 
[MESOS-8155](https://issues.apache.org/jira/browse/MESOS-8155) and 
[MESOS-8156](https://issues.apache.org/jira/browse/MESOS-8156) are also going 
to also hit this code.


Thanks,

James Peach

Re: Review Request 63677: Changed `os:Stack` to allocate with `mmap`.

[James Peach]

> On Nov. 14, 2017, 3:23 p.m., Andrei Budnik wrote:
> > 3rdparty/stout/include/stout/os/linux.hpp
> > Line 66 (original), 66 (patched)
> > 
> >
> > `ErrnoError` is not async-signal-safe as it constructs empty 
> > `std::string` in c'tor.
> > 
> > I would suggest to change the return type of `allocate()` to `int` and 
> > get rid of `create` static function.

Yup, this isn't intended to be async-safe. If you need that, you are supposed 
to call `allocate` manually. I'm planning make a change to propagate this 
`Error` in MESOS-8155.

Why do you suggest `int` as the return type? To match the syscall convention, 
or to return an `errno` directly?


> On Nov. 14, 2017, 3:23 p.m., Andrei Budnik wrote:
> > 3rdparty/stout/include/stout/os/linux.hpp
> > Line 109 (original), 111 (patched)
> > 
> >
> > Why it's needed to store MAP_FAILED ((void *) -1) instead of `nullptr`?

I fixed this my removing the temporary in the `mmap`. The intention was that we 
use `MAP_FAILED` so there's a single consistent "unset" value for the address.


- James


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


On Nov. 8, 2017, 5:47 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63677/
> ---
> 
> (Updated Nov. 8, 2017, 5:47 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8159
> https://issues.apache.org/jira/browse/MESOS-8159
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If `os:Stack` is allocated with `mmap`, this automatically
> guarantees the correct alignment (since `mmap` is page-aligned
> by definition).
> 
> Although `mmap` is not required to be async signal safe
> (i.e. it might not be safe to call from a signal handler), in
> practice it should be safe to call between `fork` and `exec`
> since it is a raw system call that does not require any user
> space locks to be held.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/linux.hpp 
> 68b4090aa5e6f23609c487128d91301755ae0e46 
> 
> 
> Diff: https://reviews.apache.org/r/63677/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 63677: Changed `os:Stack` to allocate with `mmap`.

[James Peach]

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

(Updated Nov. 14, 2017, 7:11 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description
---

If `os:Stack` is allocated with `mmap`, this automatically
guarantees the correct alignment (since `mmap` is page-aligned
by definition).

Although `mmap` is not required to be async signal safe
(i.e. it might not be safe to call from a signal handler), in
practice it should be safe to call between `fork` and `exec`
since it is a raw system call that does not require any user
space locks to be held.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/linux.hpp 
68b4090aa5e6f23609c487128d91301755ae0e46 


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

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


Testing
---

sudo make check (Fedora 26)


Thanks,

James Peach

Re: Review Request 63515: Updated some isolators' usage of ExecutorInfo.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Nov. 14, 2017, 1:41 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63515/
> ---
> 
> (Updated Nov. 14, 2017, 1:41 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-7305
> https://issues.apache.org/jira/browse/MESOS-7305
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These four isolators refered to the ExecutorInfo field of the given
> ContainerConfig to determine the total resources to isolate.  This
> changes these isolators to refer instead to the Resources field of the
> given ContainerConfig because the resource amounts should be identical
> (enforced by the caller to Containerizer::launch, such as the agent)
> and in order to support standalone containers, which specify resources
> but no ExecutorInfo.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 549a4551736d1b88a79f275a2d6e19c586c49f26 
>   src/slave/containerizer/mesos/isolators/gpu/isolator.cpp 
> 25636b50314cb4ea53d1931c97c87c65ecb658a8 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 55672b14b44005673214aa49aa5ea7be8e7bffb8 
>   src/slave/containerizer/mesos/isolators/xfs/disk.cpp 
> 2113f8665e4e84bdf12868f605b480522816a9ba 
> 
> 
> Diff: https://reviews.apache.org/r/63515/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> sudo src/mesos-tests
> 
> On... 
> OSX and Ubuntu 16
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 63781: Updated Docker path for Containerizer::launch interface change.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Nov. 14, 2017, 1:38 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63781/
> ---
> 
> (Updated Nov. 14, 2017, 1:38 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-7305
> https://issues.apache.org/jira/browse/MESOS-7305
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The change in the return type for Containerizer::launch has less of an
> impact on the Docker containerizer, as it (currently) will not return
> the "new" enum 'ALREADY_LAUNCHED'.
> 
> Note that in changing Containerizer::launch, a private helper of the
> Docker containerizer `reapExecutor`s return value was changed.
> The return value was originally `Future` because `reapExecutor`
> is the final continuation in the launch path so it needed to match
> the return value of Containerizer::launch.  However, `reapExecutor`
> never returns `false` (only `true` or a Failure).
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270 
>   src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67 
> 
> 
> Diff: https://reviews.apache.org/r/63781/diff/1/
> 
> 
> Testing
> ---
> 
> Requires the next review in the chain (test changes).
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 63056: Parameterized test for nested container launch.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Nov. 14, 2017, 1:34 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63056/
> ---
> 
> (Updated Nov. 14, 2017, 1:34 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-7305
> https://issues.apache.org/jira/browse/MESOS-7305
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This introduces a new test class that is parameterized based
> on the type of API used to launch (1) the parent container,
> (2) the nested container, and (3) the content type (like all
> other API tests).  The test is also parameterized based
> on the enabled set of isolators.
> 
> Because this change overlaps with existing nested container
> API tests, those test(s) will be removed.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 955f01a665019d7750980ff16f126ad63b524594 
>   src/tests/CMakeLists.txt 81c85d9bb1b50c6897526d5c207d09042883d771 
>   src/tests/agent_container_api_tests.cpp PRE-CREATION 
>   src/tests/api_tests.cpp 4e2cbe1b12b6d6e44e1d774397a78011beff0f95 
> 
> 
> Diff: https://reviews.apache.org/r/63056/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> On... 
> OSX and Ubuntu 16
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 62145: Implemented Standalone Container API.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/http.cpp
Line 2421 (original), 2453 (patched)


`switch_user` is posix only flag. You need to wrap with `ifndef 
__WINDOWS__`?



src/slave/http.cpp
Line 2422 (original), 2454-2456 (patched)


For nested container, we by default use executor's user? Can you add that 
logic back?


- Jie Yu


On Nov. 14, 2017, 1:32 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62145/
> ---
> 
> (Updated Nov. 14, 2017, 1:32 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-7305
> https://issues.apache.org/jira/browse/MESOS-7305
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Standalone and Nested Container APIs are very similar.
> This commit combines the two API implementations by adding a
> translation function (i.e. `launchNestedContainer` and 
> `launchContainer`) which unpacks the V1 protobuf into fields
> which can be passed into a common function (i.e. `_launchContainer`).
> 
> The common functions authorize based on the type of container being
> launched and it is possible to use both Standalone and Nested
> Container APIs interchangably for nested containers.
> 
> This approach is somewhat messy for for the `WAIT_(NESTED_)CONTAINER`
> calls, as these methods require different return protobufs based on
> the original call.
> 
> 
> Diffs
> -
> 
>   src/slave/http.hpp 44a95dec4c9b8bb65d712c5538bbd7afffe2cf7b 
>   src/slave/http.cpp 22cdac97e1a8a28ca149043ffa1d0646073d7fdb 
> 
> 
> Diff: https://reviews.apache.org/r/62145/diff/5/
> 
> 
> Testing
> ---
> 
> See later in chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 63675: Added a non-allocating variant of `os::clone`.

[James Peach]

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

(Updated Nov. 14, 2017, 7:06 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description
---

Added a non-allocating variant of `os::clone` in the
`os::signal_safe` namespace. This encapsulates the lambda
trampoline, but requires the caller to bring their own stack
since we can't know when it is safe to implicitly allocate
a stack.

Now that we have `os::signal_safe::clone`, there is no need for
`os::clone` to take an optional `os::Stack` argument. We can
remove that and simplify the cleanup logic.


Diffs (updated)
-

  3rdparty/stout/include/stout/os/linux.hpp 
68b4090aa5e6f23609c487128d91301755ae0e46 


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

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


Testing
---

sudo make check (Fedora 26)


Thanks,

James Peach

Re: Review Request 62143: Added validation for Standalone Container APIs.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Nov. 14, 2017, 1:27 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62143/
> ---
> 
> (Updated Nov. 14, 2017, 1:27 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-7305
> https://issues.apache.org/jira/browse/MESOS-7305
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Standalone Container APIs act much like the nested container APIs,
> except that ContainerIDs do not necessarily need to have a parent.
> 
> Additionally, the 'resources' field in the `LAUNCH_CONTAINER` API
> has some restrictions due to how these resources are not reported
> to the master.
> 
> 
> Diffs
> -
> 
>   src/slave/validation.cpp a575d88dffd2714447d7780a7433506a6e8c085f 
> 
> 
> Diff: https://reviews.apache.org/r/62143/diff/4/
> 
> 
> Testing
> ---
> 
> See later in chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 60891: Added ACLs and AuthZ for standalone containers.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Nov. 14, 2017, 1:24 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60891/
> ---
> 
> (Updated Nov. 14, 2017, 1:24 a.m.)
> 
> 
> Review request for mesos, Alexander Rojas, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-7305
> https://issues.apache.org/jira/browse/MESOS-7305
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This defines some coarse-grained AuthZ for launching and managing
> standalone containers.  Each HTTP principal can be given the right
> to Launch, Wait upon, Kill, or Remove standalone containers under
> a given (posix) user.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/acls.proto 
> 587b71489730f9a1252c73c0239e3d9892b3ae8e 
>   include/mesos/authorizer/authorizer.proto 
> 87a805794f430fc8b2e47de6d624b95deef162b4 
>   src/authorizer/local/authorizer.cpp 
> 2fe7b879e649b13322cfcb300c21ef1ed0fea410 
> 
> 
> Diff: https://reviews.apache.org/r/60891/diff/5/
> 
> 
> Testing
> ---
> 
> See later in chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 63753: Upgraded vendored protobuf to 3.5.0.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the current review.

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

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

Relevant logs:

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

```
error: missing binary patch data for '3rdparty/protobuf-3.5.0.tar.gz'
error: binary patch does not apply to '3rdparty/protobuf-3.5.0.tar.gz'
error: 3rdparty/protobuf-3.5.0.tar.gz: patch does not apply
```

- Mesos Reviewbot Windows


On Nov. 14, 2017, 5 a.m., Dmitry Zhuk wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63753/
> ---
> 
> (Updated Nov. 14, 2017, 5 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8139
> https://issues.apache.org/jira/browse/MESOS-8139
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Version 3.4 adds and version 3.5 improves move support in generated
> C++ code.
> `protobuf-3.5.0.tar.gz` was built using the script:
> ```
> curl -L -O https://github.com/google/protobuf/archive/v3.5.0.tar.gz
> tar -xzf v3.5.0.tar.gz
> cd protobuf-3.5.0
> ./autogen.sh
> cd ..
> tar -czf protobuf-3.5.0.tar.gz protobuf-3.5.0
> ```
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake da2d837e327535c793f20929e8c6e9ddaf5ca54a 
>   3rdparty/protobuf-3.3.0.tar.gz 98fbec86484032753c4022b4519ec40243e4e951 
>   3rdparty/protobuf-3.5.0.tar.gz PRE-CREATION 
>   3rdparty/versions.am 2fc2f2c3932ff86c348d954bd4d78aa5c75bb052 
>   LICENSE 1327411d038dfbda5a8324290469707ae02a4aa3 
>   configure.ac 067005e1be3a09e8320be120c8784f195e30ac5d 
>   src/CMakeLists.txt b0e223121f24ac0dc604dc2c639158b817eec535 
>   src/java/mesos.pom.in 56428832820efccd72d302d1b2cf1b3a4554960b 
>   src/python/interface/setup.py.in abf160c9741f61a37f5a0fe3f5fc64bbddc02142 
>   src/python/native_common/ext_modules.py.in 
> 39e0faad414f2c826dacfe1302106f2d5e7c8967 
>   src/python/protocol/setup.py.in 0fa0c1724bd39ab573e14a9137e4e3fe517c9c93 
>   support/mesos-tidy/entrypoint.sh dbea146a882510bda4e5868d8c2ced2e97a36d83 
> 
> 
> Diff: https://reviews.apache.org/r/63753/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> cmake --build .
> 
> 
> Thanks,
> 
> Dmitry Zhuk
> 
>

Re: Review Request 63801: Windows: Fixed build of Google Test with Visual Studio Preview.

[Andrew Schwartzmeyer]

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

(Updated Nov. 14, 2017, 10:47 a.m.)


Review request for mesos, Akash Gupta, John Kordich, and James Peach.


Changes
---

Fixed ordering bug. Don't rush things kids.


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


Repository: mesos


Description
---

The preview version of Visual Studio (15.5) adds a new deprecation
warning that Google Test emits. Google Test is also built with
`-Werror`, and thus fails to build unless this warning is silenced.


Diffs (updated)
-

  3rdparty/CMakeLists.txt 7d38a9cba84b86e1e428033ade1bd0ab5d9eaae7 


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

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


Testing
---

Been building with this patch on Windows with the Preview version of Visual 
Studio since last week.


Thanks,

Andrew Schwartzmeyer

Re: Review Request 63555: Publish resource provider resources before container launch or update.

[Chun-Hung Hsiao]

> On Nov. 9, 2017, 12:52 p.m., Jan Schlicht wrote:
> > src/slave/slave.cpp
> > Lines 6816-6822 (patched)
> > 
> >
> > So this will try to publish all RP resources of all executor of all 
> > frameworks? Or am I missing something here? I'd expect that only the RP 
> > resources of the task/executor that's about to get started should be 
> > published. Hence `resourceProviderManager->publish(info.id(), 
> > executor->allocatedResources())` should be enough.
> 
> Chun-Hung Hsiao wrote:
> For resources with unique identifiers (such as storage volumes in SLRP), 
> it is enough to publish resources about to use in the executor. However, for 
> resources without any identifier, such as the default resources of an agent 
> (CPUs, memory, disk), since we don't do unpublish, there's no way for 
> resource providers to know what the allocations are. Image that the resource 
> provider keeps receiving "publish 2 CPUs", should it keeps publishing 2 new 
> CPUs every time? This is the motivation to have a "ensure-all" semantics for 
> `PUBLISH`. Given the semantics, we need to publish all RP resources for all 
> executors.
> 
> An alternative is that make the resource provider manager to be aware of 
> executors, then it can keep track of the resources used by each executor, 
> then compute what the total resourcse should be. But then 1) this is for 
> agent only so I think this is not appropriate if we want to have the same 
> manager code in both agent and master; 2) the agent needs to notify the 
> manager that an executor is finished.
> 
> Jan Schlicht wrote:
> Thanks for the explanation! I don't understand though, why a resource 
> provider should be asked to "publish 2 CPUs". In my understanding "publish" 
> is only meant for resource provider resources, thus agent resources should be 
> part of these operations. Or are there use cases where a resource provider 
> might need to know the agent resources of a task?

Sorry for not being clear. I'm taking default resources (CPUs) as an example 
since we have some vision to manage the default resources with a default 
resource provider in the future. My point is that if we want to support RP that 
provider resources with only types and quantities but no IDs, then unless we do 
some refactoring to support `UNPUBLISH`, we need to tell the total amount of 
resources used every time we want to ask the RP to publish.


- Chun-Hung


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


On Nov. 4, 2017, 1:55 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63555/
> ---
> 
> (Updated Nov. 4, 2017, 1:55 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Joseph Wu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-7550
> https://issues.apache.org/jira/browse/MESOS-7550
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `Slave::publishAllocatedResources()` will compute the total allocated
> resources for all currently running executor containers, and takes an
> `extra` argument for resources that will be used by the executor that
> is about to launch, then sums them up and asks the resource provider
> manager to publish the resources.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp df1b0205124555dcb6a0efa5c237f5e77fa2bdf7 
>   src/slave/slave.cpp 337083dbe60bba2d3773b785bdceeaf0b8fcd070 
> 
> 
> Diff: https://reviews.apache.org/r/63555/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Review Request 63804: Added protos for operation reconciliation between master and agent.

[Greg Mann]

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

Review request for mesos and Greg Mann.


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


Repository: mesos


Description
---

This patch adds new internal protobuf messages to be used by the master
and agent when reconciling pending/unacknowledged offer operations.


Diffs
-

  include/mesos/resource_provider/resource_provider.proto 
1e44b952691fa57b546e979bc5876df3d82d746f 
  src/messages/messages.proto 33732e28d087a17080eb9aa7d9a91c367172be7f 
  src/resource_provider/storage/provider.cpp 
46224997430ac0c568904d80014166a6f059907f 


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


Testing
---

make check


Thanks,

Greg Mann

Re: Review Request 63798: Added resource provider support for all offer operations.

[Jie Yu]

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




src/slave/slave.cpp
Lines 3756-3765 (patched)


Hum, looks like this is common for both agent default resources or resource 
provider resources. Can we do that in `addOfferOperation` (like we did in the 
master)?

Then, that'll break the following code for agent default resources :( Maybe 
it's better to just not re-use the same `checkpointResources` handler?

Please do more thinking here.


- Jie Yu


On Nov. 14, 2017, 2:11 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63798/
> ---
> 
> (Updated Nov. 14, 2017, 2:11 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8218
> https://issues.apache.org/jira/browse/MESOS-8218
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `RESERVE`/`UNRESERVE`/`CREATE`/`DESTROY` operations for
> reservations and persistent volumes have been enabled for resource
> provider resources. Similar to agent resources, they are speculatively
> applied to allow offer pipelining.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp d8bacebc74790e955490a158c37ac0d9e75fd6b5 
>   src/tests/resource_provider_manager_tests.cpp 
> ecfe2b4c0952838d6312df603f8eb2f458725175 
> 
> 
> Diff: https://reviews.apache.org/r/63798/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 63797: Changed resource checkpointing to no longer failover.

[Jie Yu]

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




src/slave/slave.cpp
Lines 3568-3574 (original), 3573-3578 (patched)


Hum, what if the checkpoint of the `TargetPath` succeeded but the commit 
failed? Should we delete the `TargetPath` so that it'll not be retried after 
agent failover? What if the removal also fails?

This is indeed quite tricky. The reason that the apple folks did the 
prepare+commit thing is to make sure master and agent are in sync in case of a 
failed old operation. That's exactly the problem we're trying to solve here. 
That makes me wondering if we still need this prepare+commit style 
checkpointing or not for the `ApplyOfferOperationMessage` path (this is 
guaranteed to be a new master).

Also, we might need to checkpoint offer operations along with total 
resources atomically for agent default resources, that means we have to use a 
different checkpoint file for that.

Based on that, my suggestion is that we don't touch the original 
`checkpointResources` method. Instead, let's introduce a new one that don't do 
this prepare+commit style checkpointing.

Also, is this strictly required for our MVP. if not, I'd suggest we deal 
with that later.


- Jie Yu


On Nov. 14, 2017, 2:11 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63797/
> ---
> 
> (Updated Nov. 14, 2017, 2:11 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8211
> https://issues.apache.org/jira/browse/MESOS-8211
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With offer operation handling an agent can send feedback to the master
> when checkpointing fails.
> Old masters will still send a 'CheckpointResourcesMessage', a wrapper
> has been added that fails over the agent when checkpointing fails.
> As before this will result in an agent re-registration and
> reconciliation of resources.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp c0acaa639a2bacaa6955ae6c5ab41e75dc1d11f7 
>   src/slave/slave.cpp d8bacebc74790e955490a158c37ac0d9e75fd6b5 
> 
> 
> Diff: https://reviews.apache.org/r/63797/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 63801: Windows: Fixed build of Google Test with Visual Studio Preview.

[Andrew Schwartzmeyer]

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

(Updated Nov. 14, 2017, 10:14 a.m.)


Review request for mesos, Akash Gupta, John Kordich, and James Peach.


Changes
---

Moved into Windows section.


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


Repository: mesos


Description
---

The preview version of Visual Studio (15.5) adds a new deprecation
warning that Google Test emits. Google Test is also built with
`-Werror`, and thus fails to build unless this warning is silenced.


Diffs (updated)
-

  3rdparty/CMakeLists.txt 7d38a9cba84b86e1e428033ade1bd0ab5d9eaae7 


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

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


Testing
---

Been building with this patch on Windows with the Preview version of Visual 
Studio since last week.


Thanks,

Andrew Schwartzmeyer

Review Request 63801: Windows: Fixed build of Google Test with Visual Studio Preview.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, John Kordich, and James Peach.


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


Repository: mesos


Description
---

The preview version of Visual Studio (15.5) adds a new deprecation
warning that Google Test emits. Google Test is also built with
`-Werror`, and thus fails to build unless this warning is silenced.


Diffs
-

  3rdparty/CMakeLists.txt 7d38a9cba84b86e1e428033ade1bd0ab5d9eaae7 


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


Testing
---

Been building with this patch on Windows with the Preview version of Visual 
Studio since last week.


Thanks,

Andrew Schwartzmeyer

Re: Review Request 63277: Windows: Ported CPU and memory isolator tests.

[Andrew Schwartzmeyer]

> On Nov. 14, 2017, 9:07 a.m., Akash Gupta wrote:
> > src/tests/containerizer/cpu_isolator_tests.cpp
> > Line 251 (original), 270 (patched)
> > 
> >
> > Personally, I think it makes sense to increase this time on Windows. 
> > Powershell takes some time to start up, so one second isn't enough.

Yeah, we could probably adjust the timings to get a non-flaky test.


- Andrew


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


On Nov. 6, 2017, 3:19 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63277/
> ---
> 
> (Updated Nov. 6, 2017, 3:19 p.m.)
> 
> 
> Review request for mesos, Aaron Wood, Akash Gupta, Jeff Coffler, Jie Yu, John 
> Kordich, Joseph Wu, Li Li, and Michael Park.
> 
> 
> Bugs: MESOS-6690
> https://issues.apache.org/jira/browse/MESOS-6690
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These test the limitation and usage reporting of the new Windows `Cpu`
> and `Mem` isolators.
> 
> 
> Diffs
> -
> 
>   src/tests/CMakeLists.txt 81c85d9bb1b50c6897526d5c207d09042883d771 
>   src/tests/containerizer/cpu_isolator_tests.cpp 
> 846b2e255547a02f5eb0590747edca62bc560ac3 
>   src/tests/containerizer/memory_isolator_tests.cpp 
> b8ea5d35b3a0a4820d9ec4c6d7d916dc6101b570 
>   src/tests/mesos.cpp 9185b5bf2175be5b0f6b6a03a04e9e5445bf22fd 
> 
> 
> Diff: https://reviews.apache.org/r/63277/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 63751: Triggered 'ApplyOfferOperationMessage' for agent local resources.

[Jie Yu]

> On Nov. 14, 2017, 5:40 p.m., Jie Yu wrote:
> > src/slave/slave.cpp
> > Line 3720 (original), 3731 (patched)
> > 
> >
> > Let's add a CHECK here to test the operation is old operation.
> > 
> > ```
> > CHECK(protobuf::isSpeculativeOperation(message.operation_info()));
> > ```

maybe just do
```
CHECK(message.operation_info().type() == Offer::Operation::RESERVE ||
  message.operation_info().type() == Offer::Operation::UNRESERVE ||
  message.operation_info().type() == Offer::Operation::CREATE ||
  message.operation_info().type() == Offer::Operation::DESTROY);


- Jie


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


On Nov. 13, 2017, 2:21 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63751/
> ---
> 
> (Updated Nov. 13, 2017, 2:21 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8211
> https://issues.apache.org/jira/browse/MESOS-8211
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Agents the have the 'RESOURCE_PROVIDER' capability set, should get sent
> 'ApplyOfferOperationMessage' instead of 'CheckpointResourcesMessage'.
> The agent will then figure out how to apply the operation. For agent
> local resources the agent will checkpoint resources.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp 0ca4c6d689bf7d2c477174b039432ed7b6d0b650 
>   src/common/protobuf_utils.cpp 5739a63f8d87923c034375b88c4f0b3b19f4b521 
>   src/master/master.cpp 49dbaa979d692061a7b479b1db4511e8357b8baf 
>   src/slave/slave.cpp 7cb6661b55fb5437a1ffc447f974076aadd1eced 
>   src/tests/reservation_tests.cpp 470f7341686e69d0a71fb234a26b277c45c29780 
> 
> 
> Diff: https://reviews.apache.org/r/63751/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 63751: Triggered 'ApplyOfferOperationMessage' for agent local resources.

[Jie Yu]

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




src/slave/slave.cpp
Line 3720 (original), 3731 (patched)


Let's add a CHECK here to test the operation is old operation.

```
CHECK(protobuf::isSpeculativeOperation(message.operation_info()));
```



src/slave/slave.cpp
Line 3726 (original), 3737 (patched)


insert a line above



src/slave/slave.cpp
Lines 3744-3745 (patched)


Use `protobuf::createOfferOperationStatus`?



src/slave/slave.cpp
Lines 3747-3754 (patched)


Can you introduce a `protobuf::createOfferOperationStatusUpdate` helper?



src/tests/reservation_tests.cpp
Lines 2300-2303 (patched)


I feel that a better way to test this is to parameterize the existing 
`ReservationTest` to handle agents w/ or w/o `RESOURCE_PROVIDER` capabilities.

You might need to adjust some tests that explicitly test 
`CheckpointResourcesMessage`. I feel that `SendingCheckpointResourcesMessage` 
is not necessary, we can probably just remove that test.

We probably should do the same for `PersistentVolumeTest`

For those test that wait on `FUTURE_PROTOBUF(CheckpointResourcesMessage, 
...)`, we can probably change those to use `FUTURE_DISPATCH`


- Jie Yu


On Nov. 13, 2017, 2:21 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63751/
> ---
> 
> (Updated Nov. 13, 2017, 2:21 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8211
> https://issues.apache.org/jira/browse/MESOS-8211
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Agents the have the 'RESOURCE_PROVIDER' capability set, should get sent
> 'ApplyOfferOperationMessage' instead of 'CheckpointResourcesMessage'.
> The agent will then figure out how to apply the operation. For agent
> local resources the agent will checkpoint resources.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp 0ca4c6d689bf7d2c477174b039432ed7b6d0b650 
>   src/common/protobuf_utils.cpp 5739a63f8d87923c034375b88c4f0b3b19f4b521 
>   src/master/master.cpp 49dbaa979d692061a7b479b1db4511e8357b8baf 
>   src/slave/slave.cpp 7cb6661b55fb5437a1ffc447f974076aadd1eced 
>   src/tests/reservation_tests.cpp 470f7341686e69d0a71fb234a26b277c45c29780 
> 
> 
> Diff: https://reviews.apache.org/r/63751/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 63652: Added d_type check in containerizer backend validation.

[James Peach]

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



This is looking pretty good.

You should be able to write a test for this using the 
[ROOT_XFS_TestBase](https://github.com/apache/mesos/blob/master/src/tests/containerizer/xfs_quota_tests.cpp)
 fixture. Probably a reasonable approach would be to make a XFS filesystem with 
and without `d_type` support and verify that `Provisioner::create` succeeds or 
fails.


src/linux/fs.hpp
Lines 180 (patched)


Let's spell this out for the casual reader:
```
... supports the `d_type` field in `struct dirent`.
``



src/linux/fs.hpp
Lines 184 (patched)


Let's rename this to `dtypeSupported()` to be consistent with other APIs.



src/linux/fs.cpp
Lines 110 (patched)


You don't need the `Try` since there is an implicit conversion. We can also 
separate the variable declarations from clearing `errno` to improve readability:

```
  bool result = true;
  struct dirent* entry;
  
  errno = 0;
  while ( ...) {
...
  }
```



src/linux/fs.cpp
Lines 113 (patched)


Please add braces:
```
if (entry->d_type == DT_UNKNOWN) {
  result = false;
  break;
}

```



src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 126 (patched)


This comment should be a full sentence. Let's capture your experience 
testing this. Consider:
```
// Check whether d_type is supported. We need to create a directory entry 
to probe this since `.` and `..` will be marked `DT_DIR` even if the filesystem 
doesn't otherwise have d_type support.
```



src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 127 (patched)


Can we give this a more descriptive name, e.g. `.probe` or `.dtype`?



src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 146 (patched)


Let's be consistent about formatting the `Error` line wrapping. Everywhere 
else here is doing:
```
return Error(
"foo" +
"bar");
^ 4-space indent
```



src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 153 (patched)


`due to missing d_type support`?

Also, this will read `is not supported on this 'XFS' due to ...` which 
sounds awkward.

How about:
```
return Error(
"Backend '" + stringify(OVERLAY_BACKEND) +
"' is not supported due to missing d_type support " +
"on the underlying filesystem");
```



src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 154 (patched)


The caller is responsible for logging the error. Let's be consistent and 
remove this `LOG`.


- James Peach


On Nov. 13, 2017, 10:03 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63652/
> ---
> 
> (Updated Nov. 13, 2017, 10:03 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and James Peach.
> 
> 
> Bugs: MESOS-8121
> https://issues.apache.org/jira/browse/MESOS-8121
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In unified containerizer, the d_type cannot be 1
> if we are using the overlay fs backend.
> Raise error if user specifies overlayfs as backend.
> Fallback to other backends in the default case and
> raise a warning.
> 
> 
> Diffs
> -
> 
>   src/linux/fs.hpp cbc8bf79083ce2bc34fa698808eaf92764a577a9 
>   src/linux/fs.cpp 40c31a3ad2ccbeb59868c5e2d7b1809c613da8de 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 450a3b32d69d2882973a6ed4e94e169a0256056b 
> 
> 
> Diff: https://reviews.apache.org/r/63652/diff/3/
> 
> 
> Testing
> ---
> 
> Manually tested slave creation with default and overlayfs backends on XFS 
> based work_dir with different ftype mount options.
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 63276: Windows: Added `Cpu` and `Mem` isolators.

[Akash Gupta]

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


Ship it!




Ship It!

- Akash Gupta


On Nov. 14, 2017, 1:57 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63276/
> ---
> 
> (Updated Nov. 14, 2017, 1:57 a.m.)
> 
> 
> Review request for mesos, Aaron Wood, Akash Gupta, Jeff Coffler, Jie Yu, John 
> Kordich, Joseph Wu, Li Li, and Michael Park.
> 
> 
> Bugs: MESOS-5462 and MESOS-6690
> https://issues.apache.org/jira/browse/MESOS-5462
> https://issues.apache.org/jira/browse/MESOS-6690
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead of deriving from the POSIX isolators, we now have two real
> Windows isolators that can be used together or separately. The `Cpu`
> isolator enables a hard-cap CPU limit, and the `Mem` isolator enables a
> hard-cap memory limit on the job object for the container.
> 
> These classes are separate derivations of `MesosIsolatorProcess`,
> because introducing a `WindowsIsolatorProcess` base class would be
> abstraction for the sole purpose of code deduplication.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt b0e223121f24ac0dc604dc2c639158b817eec535 
>   src/Makefile.am 955f01a665019d7750980ff16f126ad63b524594 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 100e3bbda543d87808da9ff6bea42da5099ea8c5 
>   src/slave/containerizer/mesos/isolators/windows.hpp 
> b0621a5fc411b8812722f7fcf6580ed64dac5382 
>   src/slave/containerizer/mesos/isolators/windows/cpu.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/windows/cpu.cpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/windows/mem.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/windows/mem.cpp PRE-CREATION 
>   src/slave/flags.cpp 0eeecdc6023443a99c8e8fe29c2f7bf791c0a36e 
> 
> 
> Diff: https://reviews.apache.org/r/63276/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 63277: Windows: Ported CPU and memory isolator tests.

[Akash Gupta]

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




src/tests/containerizer/cpu_isolator_tests.cpp
Line 251 (original), 270 (patched)


Personally, I think it makes sense to increase this time on Windows. 
Powershell takes some time to start up, so one second isn't enough.


- Akash Gupta


On Nov. 6, 2017, 11:19 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63277/
> ---
> 
> (Updated Nov. 6, 2017, 11:19 p.m.)
> 
> 
> Review request for mesos, Aaron Wood, Akash Gupta, Jeff Coffler, Jie Yu, John 
> Kordich, Joseph Wu, Li Li, and Michael Park.
> 
> 
> Bugs: MESOS-6690
> https://issues.apache.org/jira/browse/MESOS-6690
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These test the limitation and usage reporting of the new Windows `Cpu`
> and `Mem` isolators.
> 
> 
> Diffs
> -
> 
>   src/tests/CMakeLists.txt 81c85d9bb1b50c6897526d5c207d09042883d771 
>   src/tests/containerizer/cpu_isolator_tests.cpp 
> 846b2e255547a02f5eb0590747edca62bc560ac3 
>   src/tests/containerizer/memory_isolator_tests.cpp 
> b8ea5d35b3a0a4820d9ec4c6d7d916dc6101b570 
>   src/tests/mesos.cpp 9185b5bf2175be5b0f6b6a03a04e9e5445bf22fd 
> 
> 
> Diff: https://reviews.apache.org/r/63277/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 63791: Handled the resource conversion for new operations in master.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Nov. 14, 2017, 9:33 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63791/
> ---
> 
> (Updated Nov. 14, 2017, 9:33 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When a terminal status update for an offer operaiton is received, we
> need to apply the conversion for the offer operation to the master's
> view about the total resources of the corresponding agent.
> 
> Fix for 776bfab969c4df6d1ecea67f8a5aedf1ec3bed83
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 59a533940736f5cfd5ec31e0ed924f0b2ab13f9c 
> 
> 
> Diff: https://reviews.apache.org/r/63791/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> Source review request: https://reviews.apache.org/r/63737/ which was reverted 
> in 1ac932a29cfbd612f66f3efb20e597227b979d56
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 63271: Windows: Added `os::set_job_mem_limit` to stout.

[Aaron Wood via Review Board]

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


Ship it!




Ship It!

- Aaron Wood


On Nov. 14, 2017, 12:20 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63271/
> ---
> 
> (Updated Nov. 14, 2017, 12:20 a.m.)
> 
> 
> Review request for mesos, Aaron Wood, Akash Gupta, Jeff Coffler, Jie Yu, John 
> Kordich, Joseph Wu, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is used to set a hard cap on the memory usage for a job object.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/windows/os.hpp 
> 09ddec6d69472cd13b453fe1a77fdbe343fc23c8 
> 
> 
> Diff: https://reviews.apache.org/r/63271/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 63555: Publish resource provider resources before container launch or update.

[Jan Schlicht]

> On Nov. 9, 2017, 1:52 p.m., Jan Schlicht wrote:
> > src/slave/slave.hpp
> > Lines 538 (patched)
> > 
> >
> > What's the motivation for this `extra` parameter? It isn't used 
> > anywhere, probably we should remove it.
> 
> Chun-Hung Hsiao wrote:
> It's for publishing resources for launching new executors, and used in 
> line 2907 of `slave.cpp`.

Ah, didn't see that, thanks!


> On Nov. 9, 2017, 1:52 p.m., Jan Schlicht wrote:
> > src/slave/slave.cpp
> > Lines 6816-6822 (patched)
> > 
> >
> > So this will try to publish all RP resources of all executor of all 
> > frameworks? Or am I missing something here? I'd expect that only the RP 
> > resources of the task/executor that's about to get started should be 
> > published. Hence `resourceProviderManager->publish(info.id(), 
> > executor->allocatedResources())` should be enough.
> 
> Chun-Hung Hsiao wrote:
> For resources with unique identifiers (such as storage volumes in SLRP), 
> it is enough to publish resources about to use in the executor. However, for 
> resources without any identifier, such as the default resources of an agent 
> (CPUs, memory, disk), since we don't do unpublish, there's no way for 
> resource providers to know what the allocations are. Image that the resource 
> provider keeps receiving "publish 2 CPUs", should it keeps publishing 2 new 
> CPUs every time? This is the motivation to have a "ensure-all" semantics for 
> `PUBLISH`. Given the semantics, we need to publish all RP resources for all 
> executors.
> 
> An alternative is that make the resource provider manager to be aware of 
> executors, then it can keep track of the resources used by each executor, 
> then compute what the total resourcse should be. But then 1) this is for 
> agent only so I think this is not appropriate if we want to have the same 
> manager code in both agent and master; 2) the agent needs to notify the 
> manager that an executor is finished.

Thanks for the explanation! I don't understand though, why a resource provider 
should be asked to "publish 2 CPUs". In my understanding "publish" is only 
meant for resource provider resources, thus agent resources should be part of 
these operations. Or are there use cases where a resource provider might need 
to know the agent resources of a task?


- Jan


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


On Nov. 4, 2017, 2:55 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63555/
> ---
> 
> (Updated Nov. 4, 2017, 2:55 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, Joseph Wu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-7550
> https://issues.apache.org/jira/browse/MESOS-7550
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `Slave::publishAllocatedResources()` will compute the total allocated
> resources for all currently running executor containers, and takes an
> `extra` argument for resources that will be used by the executor that
> is about to launch, then sums them up and asks the resource provider
> manager to publish the resources.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp df1b0205124555dcb6a0efa5c237f5e77fa2bdf7 
>   src/slave/slave.cpp 337083dbe60bba2d3773b785bdceeaf0b8fcd070 
> 
> 
> Diff: https://reviews.apache.org/r/63555/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 63677: Changed `os:Stack` to allocate with `mmap`.

[Andrei Budnik]

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




3rdparty/stout/include/stout/os/linux.hpp
Line 66 (original), 66 (patched)


`ErrnoError` is not async-signal-safe as it constructs empty `std::string` 
in c'tor.

I would suggest to change the return type of `allocate()` to `int` and get 
rid of `create` static function.



3rdparty/stout/include/stout/os/linux.hpp
Line 83 (original), 83 (patched)


`void* ptr`



3rdparty/stout/include/stout/os/linux.hpp
Line 94 (original), 96 (patched)


We check return value of `munmap` in `stout/os/posix/fork.hpp`. Should we 
check it here?



3rdparty/stout/include/stout/os/linux.hpp
Line 105 (original), 107 (patched)


Can it be defined closer to the beginning of the class definition?



3rdparty/stout/include/stout/os/linux.hpp
Line 109 (original), 111 (patched)


Why it's needed to store MAP_FAILED ((void *) -1) instead of `nullptr`?


- Andrei Budnik


On Nov. 8, 2017, 5:47 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63677/
> ---
> 
> (Updated Nov. 8, 2017, 5:47 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8159
> https://issues.apache.org/jira/browse/MESOS-8159
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If `os:Stack` is allocated with `mmap`, this automatically
> guarantees the correct alignment (since `mmap` is page-aligned
> by definition).
> 
> Although `mmap` is not required to be async signal safe
> (i.e. it might not be safe to call from a signal handler), in
> practice it should be safe to call between `fork` and `exec`
> since it is a raw system call that does not require any user
> space locks to be held.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/linux.hpp 
> 68b4090aa5e6f23609c487128d91301755ae0e46 
> 
> 
> Diff: https://reviews.apache.org/r/63677/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 26)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 63678: Improved the signal safety of `ns::clone`.

[Andrei Budnik]

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




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


Should we use C++ type casting here?


- Andrei Budnik


On Nov. 8, 2017, 5:51 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63678/
> ---
> 
> (Updated Nov. 8, 2017, 5:51 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8159
> https://issues.apache.org/jira/browse/MESOS-8159
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> `ns::clone` was implicitly using malloc to allocate a stack
> after fork but before exec, where we should be avoiding
> `malloc`.
> 
> Updated the second clone to use `os::signal_safe::clone`. We
> still explicitly allocate a stack after the fork, but since
> that is allocated with `mmap` is it safe in this particular
> case.
> 
> 
> Diffs
> -
> 
>   src/linux/ns.cpp 33751b518e95f4f67e6864728e1fdf621f50dd52 
> 
> 
> Diff: https://reviews.apache.org/r/63678/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check (Fedora 26)
> 
> NOTE: I tried to avoid changing `ns::clone` too much in this patch because 
> [MESOS-8155](https://issues.apache.org/jira/browse/MESOS-8155) and 
> [MESOS-8156](https://issues.apache.org/jira/browse/MESOS-8156) are also going 
> to also hit this code.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 63732: Reconciled offer operations between agent and master.

[Jan Schlicht]

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




src/master/master.cpp
Lines 7093 (patched)


This needs to handle the case where the master might have offer operations 
that the agent doesn't report here, e.g. after an agent failover. We should 
send operation feedback to frameworks in that case, indicating an error with 
the operation. Otherwise frameworks will receive no feedback for these 
operations.


- Jan Schlicht


On Nov. 13, 2017, 7:33 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63732/
> ---
> 
> (Updated Nov. 13, 2017, 7:33 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-8207
> https://issues.apache.org/jira/browse/MESOS-8207
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp e1381a9a4f0183548a2b8ea8431bf52b0739629f 
> 
> 
> Diff: https://reviews.apache.org/r/63732/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`, still need to implement dedicated tests.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 63515: Updated some isolators' usage of ExecutorInfo.

[Mesos Reviewbot Windows]

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



FAIL: Mesos tests failed to build.

Reviews applied: `['60888', '60889', '60890', '60891', '62142', '62143', 
'62145', '63054', '63056', '63057', '63058', '63063', '63781', '63514', 
'63515']`

Failed command: `cmake.exe --build . --target mesos-tests --config Debug`

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

Relevant logs:

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

```
  c:\dcos\mesos\mesos\src\slave\containerizer\mesos\io\switchboard.cpp(628): 
warning C4715: 'mesos::internal::slave::IOSwitchboard::_prepare': not all 
control paths return a value [C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\authentication\cram_md5\authenticatee.cpp(75): 
warning C4267: '=': conversion from 'size_t' to 'unsigned long', possible loss 
of data [C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\authentication\cram_md5\authenticatee.cpp(271): 
warning C4267: 'argument': conversion from 'size_t' to 'unsigned int', possible 
loss of data [C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\authentication\cram_md5\authenticatee.cpp(333): 
warning C4267: '=': conversion from 'size_t' to 'unsigned int', possible loss 
of data [C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\authentication\cram_md5\authenticator.cpp(216): 
warning C4267: 'argument': conversion from 'size_t' to 'unsigned int', possible 
loss of data [C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\authentication\cram_md5\authenticator.cpp(242): 
warning C4267: 'argument': conversion from 'size_t' to 'unsigned int', possible 
loss of data [C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\authentication\cram_md5\authenticator.cpp(274): 
warning C4267: '=': conversion from 'size_t' to 'unsigned int', possible loss 
of data [C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\master/master.hpp(2105): warning C4244: 'return': 
conversion from 'unsigned __int64' to 'double', possible loss of data 
(compiling source file C:\DCOS\mesos\mesos\src\common\protobuf_utils.cpp) 
[C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\files\files.cpp(703): warning C4267: 'argument': 
conversion from 'size_t' to 'off_t', possible loss of data 
[C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\master/master.hpp(2105): warning C4244: 'return': 
conversion from 'unsigned __int64' to 'double', possible loss of data 
(compiling source file C:\DCOS\mesos\mesos\src\master\master.cpp) 
[C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\master\master.cpp(6167): warning C4244: 'argument': 
conversion from 'const ::size_t' to 'double', possible loss of data 
[C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\master\master.cpp(6254): warning C4244: 'argument': 
conversion from 'const ::size_t' to 'double', possible loss of data 
[C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\master\master.cpp(6837): warning C4244: 'argument': 
conversion from 'const ::size_t' to 'double', possible loss of data 
[C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\master\master.cpp(8365): warning C4244: 'argument': 
conversion from 'const ::size_t' to 'double', possible loss of data 
[C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\master\master.cpp(10190): warning C4244: 'return': 
conversion from '::size_t' to 'double', possible loss of data 
[C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\master/master.hpp(2105): warning C4244: 'return': 
conversion from 'unsigned __int64' to 'double', possible loss of data 
(compiling source file C:\DCOS\mesos\mesos\src\master\quota_handler.cpp) 
[C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\master/master.hpp(2105): warning C4244: 'return': 
conversion from 'unsigned __int64' to 'double', possible loss of data 
(compiling source file C:\DCOS\mesos\mesos\src\master\weights_handler.cpp) 
[C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\master\allocator\sorter\drf\sorter.cpp(589): warning 
C4267: 'return': conversion from 'size_t' to 'int', possible loss of data 
[C:\DCOS\mesos\src\mesos.vcxproj]


"C:\DCOS\mesos\src\tests\mesos-tests.vcxproj" (default target) (1) ->
"C:\DCOS\mesos\src\mesos.vcxproj" (default target) (12) ->
(ClCompile target) -> 
  C:\DCOS\mesos\mesos\src\slave\http.cpp(2453): error C2039: 'switch_user': is 
not a member of 'mesos::internal::slave::Flags' 
[C:\DCOS\mesos\src\mesos.vcxproj]

135 Warning(s)
1 Error(s)

Time Elapsed 00:18:03.22
```

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

```
  

Re: Review Request 63730: Passed operations from resource provider to agent.

[Jan Schlicht]

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


Fix it, then Ship it!




Please rebase, the formatting fix in `message.hpp` is already fixed in `master`.


src/resource_provider/manager.cpp
Line 477 (original), 477 (patched)


Remove the space after `resourceProvider`.


- Jan Schlicht


On Nov. 13, 2017, 7:33 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63730/
> ---
> 
> (Updated Nov. 13, 2017, 7:33 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-8207
> https://issues.apache.org/jira/browse/MESOS-8207
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/manager.cpp bcc833b6c96344762a693dd6d4e0a9129aa735f8 
>   src/resource_provider/message.hpp a1a84c1fd374b740c56ed2175ee5a2dbc8f96dbc 
> 
> 
> Diff: https://reviews.apache.org/r/63730/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`, still need to implement dedicated tests.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 63799: Improved log messages of offer operations.

[Jan Schlicht]

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

Review request for mesos, Benjamin Bannier and Jie Yu.


Repository: mesos


Description
---

Improved log messages of offer operations.


Diffs
-

  src/resource_provider/message.hpp 05879cdc01dca88440ce89beb487168c033fab37 
  src/slave/slave.cpp d8bacebc74790e955490a158c37ac0d9e75fd6b5 


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


Testing
---

make check


Thanks,

Jan Schlicht

Review Request 63798: Added resource provider support for all offer operations.

[Jan Schlicht]

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

Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description
---

The `RESERVE`/`UNRESERVE`/`CREATE`/`DESTROY` operations for
reservations and persistent volumes have been enabled for resource
provider resources. Similar to agent resources, they are speculatively
applied to allow offer pipelining.


Diffs
-

  src/slave/slave.cpp d8bacebc74790e955490a158c37ac0d9e75fd6b5 
  src/tests/resource_provider_manager_tests.cpp 
ecfe2b4c0952838d6312df603f8eb2f458725175 


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


Testing
---

make check


Thanks,

Jan Schlicht

Review Request 63797: Changed resource checkpointing to no longer failover.

[Jan Schlicht]

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

Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description
---

With offer operation handling an agent can send feedback to the master
when checkpointing fails.
Old masters will still send a 'CheckpointResourcesMessage', a wrapper
has been added that fails over the agent when checkpointing fails.
As before this will result in an agent re-registration and
reconciliation of resources.


Diffs
-

  src/slave/slave.hpp c0acaa639a2bacaa6955ae6c5ab41e75dc1d11f7 
  src/slave/slave.cpp d8bacebc74790e955490a158c37ac0d9e75fd6b5 


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


Testing
---

make check


Thanks,

Jan Schlicht

Review Request 63796: Made `CheckerProcess` support IPv6 for HTTP/TCP check.

[Qian Zhang]

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

Review request for mesos, Alexander Rukletsov and Avinash sridharan.


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


Repository: mesos


Description
---

Made `CheckerProcess` support IPv6 for HTTP/TCP check.


Diffs
-

  src/checks/checker_process.cpp 7985e8d4dc6115f81a4daef0ce4eee67f4233c9f 


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


Testing
---

sudo make check


Thanks,

Qian Zhang

Re: Review Request 63768: Added 'ReconcileOfferOperations' reponse to scheduler API.

[Mesos Reviewbot Windows]

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



FAIL: Some Mesos libprocess-tests failed.

Reviews applied: `['63767', '63768']`

Failed command: 
`C:\DCOS\mesos\3rdparty\libprocess\src\tests\Debug\libprocess-tests.exe`

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

Relevant logs:

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

```
[   OK ] Scheme/HTTPTest.WWWAuthenticateHeader/0 (715 ms)
[ RUN  ] Scheme/HTTPTest.WWWAuthenticateHeader/1
[   OK ] Scheme/HTTPTest.WWWAuthenticateHeader/1 (71 ms)
[ RUN  ] Scheme/HTTPTest.Accepts/0
[   OK ] Scheme/HTTPTest.Accepts/0 (803 ms)
[ RUN  ] Scheme/HTTPTest.Accepts/1
[   OK ] Scheme/HTTPTest.Accepts/1 (87 ms)
[--] 34 tests from Scheme/HTTPTest (13383 ms total)

[--] 4 tests from SSLVerifyIPAdd/SSLTest
[ RUN  ] SSLVerifyIPAdd/SSLTest.BasicSameProcess/0
[   OK ] SSLVerifyIPAdd/SSLTest.BasicSameProcess/0 (873 ms)
[ RUN  ] SSLVerifyIPAdd/SSLTest.BasicSameProcess/1
[   OK ] SSLVerifyIPAdd/SSLTest.BasicSameProcess/1 (1125 ms)
[ RUN  ] SSLVerifyIPAdd/SSLTest.RequireCertificate/0
[   OK ] SSLVerifyIPAdd/SSLTest.RequireCertificate/0 (932 ms)
[ RUN  ] SSLVerifyIPAdd/SSLTest.RequireCertificate/1
[   OK ] SSLVerifyIPAdd/SSLTest.RequireCertificate/1 (599 ms)
[--] 4 tests from SSLVerifyIPAdd/SSLTest (3660 ms total)

[--] Global test environment tear-down
[==] 201 tests from 33 test cases ran. (46029 ms total)
[  PASSED  ] 200 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] SSLTest.ProtocolMismatch

 1 FAILED TEST
  YOU HAVE 19 DISABLED TESTS

```

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

```
I1114 13WARNING: Logging before InitGoogleLogging() is written to STDERR
I1114 13:31:08.973026  5732 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1114 13:31:08.974030  5732 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I1114 13:31:08.974030  5732 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1114 13:31:08.974030  5732 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\mpxpLK\cert.pem
WARNING: Logging before InitGoogleLogging() is written to STDERR
I1114 13:31:09.638362  3916 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1114 13:31:09.643370  3916 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I1114 13:31:09.643370  3916 openssl.cpp:526] Will use IP address verification 
in subject alternative name certificate extension.
I1114 13:31:09.644359  3916 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1114 13:31:09.644359  3916 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\F7Cpnw\cert.pem
:31:08.679018  1596 openssl.cpp:509] CA directory path unspecified! NOTE: Set 
CA directory path with LIBPROCESS_SSL_CA_DIR=
I1114 13:31:08.687037  1596 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I1114 13:31:08.687037  1596 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1114 13:31:08.688019  1596 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\mpxpLK\cert.pem
I1114 13:31:09.337347  1596 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1114 13:31:09.337347  1596 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I1114 13:31:09.342013  1596 openssl.cpp:526] Will use IP address verification 
in subject alternative name certificate extension.
I1114 13:31:09.342013  1596 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1114 13:31:09.342347  1596 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\F7Cpnw\cert.pem
I1114 13:31:09.791648  1688 process.cpp:1052] Failed to accept socket: future 
discarded
```

- Mesos Reviewbot Windows


On Nov. 14, 2017, 12:25 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.a

Review Request 63795: Made `mesos-tcp-connect` support IPv6.

[Qian Zhang]

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

Review request for mesos, Alexander Rukletsov and Avinash sridharan.


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


Repository: mesos


Description
---

Made `mesos-tcp-connect` support IPv6.


Diffs
-

  src/checks/tcp_connect.cpp 2514f4aebcc634b18cd2b3c36529222fe456e903 


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


Testing
---


Thanks,

Qian Zhang

Review Request 63794: Added a new member field `ipv6` to the `CheckerProcess` class.

[Qian Zhang]

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

Review request for mesos, Alexander Rukletsov and Avinash sridharan.


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


Repository: mesos


Description
---

Added a new member field `ipv6` to the `CheckerProcess` class.


Diffs
-

  src/checks/checker_process.hpp 5d0c36dd3e1edf68527fc109d8c0708f09b7e12f 
  src/checks/checker_process.cpp 7985e8d4dc6115f81a4daef0ce4eee67f4233c9f 
  src/checks/health_checker.cpp d4bda6ed1747ae4c970619bbfa336321aeea52ea 


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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 63753: Upgraded vendored protobuf to 3.5.0.

[Dmitry Zhuk]

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

(Updated Nov. 14, 2017, 1 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Switched to 3.5.0.


Summary (updated)
-

Upgraded vendored protobuf to 3.5.0.


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


Repository: mesos


Description (updated)
---

Version 3.4 adds and version 3.5 improves move support in generated
C++ code.
`protobuf-3.5.0.tar.gz` was built using the script:
```
curl -L -O https://github.com/google/protobuf/archive/v3.5.0.tar.gz
tar -xzf v3.5.0.tar.gz
cd protobuf-3.5.0
./autogen.sh
cd ..
tar -czf protobuf-3.5.0.tar.gz protobuf-3.5.0
```


Diffs (updated)
-

  3rdparty/cmake/Versions.cmake da2d837e327535c793f20929e8c6e9ddaf5ca54a 
  3rdparty/protobuf-3.3.0.tar.gz 98fbec86484032753c4022b4519ec40243e4e951 
  3rdparty/protobuf-3.5.0.tar.gz PRE-CREATION 
  3rdparty/versions.am 2fc2f2c3932ff86c348d954bd4d78aa5c75bb052 
  LICENSE 1327411d038dfbda5a8324290469707ae02a4aa3 
  configure.ac 067005e1be3a09e8320be120c8784f195e30ac5d 
  src/CMakeLists.txt b0e223121f24ac0dc604dc2c639158b817eec535 
  src/java/mesos.pom.in 56428832820efccd72d302d1b2cf1b3a4554960b 
  src/python/interface/setup.py.in abf160c9741f61a37f5a0fe3f5fc64bbddc02142 
  src/python/native_common/ext_modules.py.in 
39e0faad414f2c826dacfe1302106f2d5e7c8967 
  src/python/protocol/setup.py.in 0fa0c1724bd39ab573e14a9137e4e3fe517c9c93 
  support/mesos-tidy/entrypoint.sh dbea146a882510bda4e5868d8c2ced2e97a36d83 


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

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


Testing
---

make check

cmake --build .


Thanks,

Dmitry Zhuk

Re: Review Request 63766: Clarified log message when selecting backend.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63766 was successfully built and tested.

Reviews applied: `['63766']`

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

- Mesos Reviewbot Windows


On Nov. 13, 2017, 10:10 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63766/
> ---
> 
> (Updated Nov. 13, 2017, 10:10 p.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Clarified log message when selecting backend.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 450a3b32d69d2882973a6ed4e94e169a0256056b 
> 
> 
> Diff: https://reviews.apache.org/r/63766/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 63652: Added d_type check in containerizer backend validation.

[Mesos Reviewbot Windows]

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



FAIL: Some Mesos libprocess-tests failed.

Reviews applied: `['63652']`

Failed command: 
`C:\DCOS\mesos\3rdparty\libprocess\src\tests\Debug\libprocess-tests.exe`

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

Relevant logs:

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

```
[   OK ] Scheme/HTTPTest.WWWAuthenticateHeader/0 (696 ms)
[ RUN  ] Scheme/HTTPTest.WWWAuthenticateHeader/1
[   OK ] Scheme/HTTPTest.WWWAuthenticateHeader/1 (76 ms)
[ RUN  ] Scheme/HTTPTest.Accepts/0
[   OK ] Scheme/HTTPTest.Accepts/0 (642 ms)
[ RUN  ] Scheme/HTTPTest.Accepts/1
[   OK ] Scheme/HTTPTest.Accepts/1 (99 ms)
[--] 34 tests from Scheme/HTTPTest (16104 ms total)

[--] 4 tests from SSLVerifyIPAdd/SSLTest
[ RUN  ] SSLVerifyIPAdd/SSLTest.BasicSameProcess/0
[   OK ] SSLVerifyIPAdd/SSLTest.BasicSameProcess/0 (526 ms)
[ RUN  ] SSLVerifyIPAdd/SSLTest.BasicSameProcess/1
[   OK ] SSLVerifyIPAdd/SSLTest.BasicSameProcess/1 (1264 ms)
[ RUN  ] SSLVerifyIPAdd/SSLTest.RequireCertificate/0
[   OK ] SSLVerifyIPAdd/SSLTest.RequireCertificate/0 (1234 ms)
[ RUN  ] SSLVerifyIPAdd/SSLTest.RequireCertificate/1
[   OK ] SSLVerifyIPAdd/SSLTest.RequireCertificate/1 (842 ms)
[--] 4 tests from SSLVerifyIPAdd/SSLTest (3976 ms total)

[--] Global test environment tear-down
[==] 201 tests from 33 test cases ran. (52460 ms total)
[  PASSED  ] 200 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] SSLTest.SSLSocket

 1 FAILED TEST
  YOU HAVE 19 DISABLED TESTS

```

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

```
I1114 10:11:04.472410   572 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1114 10:11:04.472410   572 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peWARNING: Logging before 
InitGoogleLogging() is written to STDERR
I1114 10:11:04.785419  4708 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1114 10:11:04.786419  4708 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I1114 10:11:04.786419  4708 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1114 10:11:04.787420  4708 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\QZ0fwN\cert.pem
WARNING: Logging before InitGoogleLogging() is written to STDERR
I1114 10:11:05.688441  5936 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1114 10:11:05.693531  5936 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I1114 10:11:05.693531  5936 openssl.cpp:526] Will use IP address verification 
in subject alternative name certificate extension.
I1114 10:11:05.693531  5936 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1114 10:11:05.69  5936 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\wtFUht\cert.pem
er certificate verification
I1114 10:11:04.485419   572 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1114 10:11:04.486403   572 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\QZ0fwN\cert.pem
I1114 10:11:05.388438   572 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1114 10:11:05.388438   572 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I1114 10:11:05.388438   572 openssl.cpp:526] Will use IP address verification 
in subject alternative name certificate extension.
I1114 10:11:05.388438   572 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1114 10:11:05.388438   572 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\wtFUht\cert.pem
I1114 10:11:05.850450  5920 process.cpp:1052] Failed to accept socket: future 
discarded
```

- Mesos Reviewbot Windows


On Nov. 13, 2017, 10:03 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/636

Re: Review Request 63767: Added a missing protobuf field to the unversioned definitions.

[Jan Schlicht]

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


Ship it!




Ship It!

- Jan Schlicht


On Nov. 14, 2017, 12:17 a.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63767/
> ---
> 
> (Updated Nov. 14, 2017, 12:17 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Gaston Kleiman, Jie Yu, and Jan 
> Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The new `OfferOperationUpdate` event type was added to the unversioned
> scheduler API protobuf definitions, but the corresponding field was not
> added to the `Event` message.
> 
> 
> Diffs
> -
> 
>   include/mesos/scheduler/scheduler.proto 
> f5dda10a076ecae53a56875d0ed4d201a1c92f12 
> 
> 
> Diff: https://reviews.apache.org/r/63767/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 63791: Handled the resource conversion for new operations in master.

[Jan Schlicht]

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

(Updated Nov. 14, 2017, 10:33 a.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Repository: mesos


Description
---

When a terminal status update for an offer operaiton is received, we
need to apply the conversion for the offer operation to the master's
view about the total resources of the corresponding agent.

Fix for 776bfab969c4df6d1ecea67f8a5aedf1ec3bed83


Diffs
-

  src/master/master.cpp 59a533940736f5cfd5ec31e0ed924f0b2ab13f9c 


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


Testing (updated)
---

make check

Source review request: https://reviews.apache.org/r/63737/ which was reverted 
in 1ac932a29cfbd612f66f3efb20e597227b979d56


Thanks,

Jan Schlicht

Review Request 63791: Handled the resource conversion for new operations in master.

[Jan Schlicht]

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

Review request for mesos, Benjamin Bannier and Jie Yu.


Repository: mesos


Description
---

When a terminal status update for an offer operaiton is received, we
need to apply the conversion for the offer operation to the master's
view about the total resources of the corresponding agent.

Fix for 776bfab969c4df6d1ecea67f8a5aedf1ec3bed83


Diffs
-

  src/master/master.cpp 59a533940736f5cfd5ec31e0ed924f0b2ab13f9c 


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


Testing
---

make check


Thanks,

Jan Schlicht