Review Request 66444: Windows: Made `signals.hpp` compile.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, and 
Michael Park.


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


Repository: mesos


Description
---

This file had never been included before, so it didn't compile. It
needed to include `unimplemented.hpp`, and because `Suppressor` isn't
implemented, the initializers had to be deleted.


Diffs
-

  3rdparty/stout/include/stout/os/windows/signals.hpp 
0ed24771625e58c1de8b1aa96b70f5aae1638bd4 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 66443: Fixed `Subprocess::ChildHook::CHDIR()` to use `os::chdir()`.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, and 
Michael Park.


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


Repository: mesos


Description
---

This needed to use the Stout API so that the correct Windows
implementation is used, as `::chdir` is part of the CRT.

Also included used but not included `stout/os/*` headers.


Diffs
-

  3rdparty/libprocess/src/subprocess.cpp 
898326360d6b4f0a50d6ef3f7c86141d0aa70438 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 66445: Windows: Cleaned up included CRT headers.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, and 
Michael Park.


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


Repository: mesos


Description
---

The set `errno` value in `os::kill()` is never checked (especially on
Windows), so `_set_errno()` and thus `errno.h` were removed.

The `fcntl.h` is used only to provide `O_CREAT` etc., and so belonged
in `open.hpp`, not `windows.hpp`.

The remaining headers, `direct.h`, `io.h`, `process.h`, and `stdlib.h`
were no longer used or needed as the respective CRT APIs were
replaced.


Diffs
-

  3rdparty/stout/include/stout/os/windows/kill.hpp 
9cec1117fac3cf6bd624fc7db524ef1ad10cd55d 
  3rdparty/stout/include/stout/os/windows/mkdtemp.hpp 
9181429383a991fe2b87701d2bfd0e858ac2537b 
  3rdparty/stout/include/stout/os/windows/open.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/windows/rmdir.hpp 
a2926dab2c8e219cf5938c4df27f83488198dd6b 
  3rdparty/stout/include/stout/os/windows/sendfile.hpp 
fff5872db6b3f69464e41e6d108a107e4eeabd12 
  3rdparty/stout/include/stout/windows.hpp 
1bfcdf4a5c097cc6d2293396ce39c8ad2c9ec993 
  3rdparty/stout/include/stout/windows/dynamiclibrary.hpp 
5b3cbf4f36ea9ac0411df52b4cfea8ef75fecbb5 
  3rdparty/stout/include/stout/windows/os.hpp 
739ee4da3f09d2a9597d4451e755e77903e9287d 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 66442: Windows: Fixed `os::abort()` to use `WriteFile()`.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, and 
Michael Park.


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


Repository: mesos


Description
---

Windows: Fixed `os::abort()` to use `WriteFile()`.


Diffs
-

  3rdparty/stout/include/stout/abort.hpp 
4fd233dfcd4359791dd176820f3a6040947bb291 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 66441: Fixed mismatched types in `process.cpp`.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, and 
Michael Park.


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


Repository: mesos


Description
---

Because of the ternary, the primitive integer `-1` must be explicitly
constructed into an `int_fd` to match the type held in the `sockets`
container.


Diffs
-

  3rdparty/libprocess/src/process.cpp 9eb37465cd86f408d69f5f98fb76c4f4b93b9acd 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 66440: Replaced `open()` with `os::open()` in `http_proxy.cpp`.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, and 
Michael Park.


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


Repository: mesos


Description
---

Replaced `open()` with `os::open()` in `http_proxy.cpp`.


Diffs
-

  3rdparty/libprocess/src/http_proxy.cpp 
25d63791e4788a488f96303aabeed0fa77ad7992 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 66439: Windows: Made `protobuf::write()` use CRT file descriptor explicitly.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, and 
Michael Park.


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


Repository: mesos


Description
---

This is another edge case where a third-party library (protobuf)
requires a CRT integer file descriptor. Thus we duplicate the `int_fd`
and then explicitly allocate via `crt()`, which requires that we also
manually close it via `_close()`.


Diffs
-

  3rdparty/stout/include/stout/protobuf.hpp 
2fa5072e3c62c487da0dccffdd38d2fa1a615dc0 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 66438: Windows: Made `libevent` use CRT file descriptor explicitly.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, and 
Michael Park.


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


Repository: mesos


Description
---

This is an edge case where a third-party library (libevent) requires a
CRT integer file descriptor. Thus we duplicate the `int_fd` and then
explicitly allocate via `crt()`, which requires that we also manually
close it via `_close()`.


Diffs
-

  3rdparty/libprocess/src/libevent_ssl_socket.cpp 
4de161dbf9198e9c74b1e80838b8a5d52006a562 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 66437: Windows: Removed `FD_CRT` from `WindowsFD` abstraction.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, and 
Michael Park.


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


Repository: mesos


Description
---

After all the CRT APIs were replaced with Windows APIs, we no longer
need to support the semantics of an `int` file descriptor in
general (in the sense of opening a CRT handle that's associated with
the actual kernel object for the given `HANDLE`). There are specific
use cases (usually third-party code) which still require a CRT
int-like file descriptor, which the `crt()` function explicitly
allocates (this allocation used to be done in the constructor).

Thus the entire `FD_CRT` type was removed from the `WindowsFD`
abstraction. It still acts like an `int` in the sense that it can be
constructed from one and compared to one. However, construction via
`int` only supports the standard file descriptors 0, 1, and 2 for
`stdin`, `stdout`, and `stderr`. Any other construction creates an
`int_fd` which holds an `INVALID_HANDLE` value. When being compared to
an `int`, the abstraction simply returns -1 if it is invalid (based on
the result of the `is_valid()` method) or 0 if it is valid. This is to
support the semantics of checking validity by something like
`if (fd < 0)` or `if (fd == -1)`.

With the deletion of the `FD_CRT` type from `WindowsFD`, all the Stout
APIs that switched on the type were simplified, with the last of the
CRT code deleted.

Note that a new enum type `INVALID` was added to support default
construction semantics, and only exists for that reason. An actual
"invalid" handle should be constructed via `int_fd fd = -1` (or
`int_fd fd = INVALID_HANDLE_VALUE` when being explicit on Windows).

Because `int_fd` is now castable to an `int`, and the `FD_CRT` type
was removed, the comparison operators became much simpler.


Diffs
-

  3rdparty/stout/include/stout/os/windows/close.hpp 
ff635e44235d63888a210cd68d49f6678a851e31 
  3rdparty/stout/include/stout/os/windows/dup.hpp 
265046cf7ffc14f7326711d295aa7dd4f0a8a1e3 
  3rdparty/stout/include/stout/os/windows/fcntl.hpp 
bf8c38acad60f9b0eb752053dcd53a9fda7b8bfa 
  3rdparty/stout/include/stout/os/windows/fd.hpp 
d7f8cdf1ad877eb55589bf5a9e75d295f91990a7 
  3rdparty/stout/include/stout/os/windows/read.hpp 
8047ad590fcc46d3ec46b551472d8c518ae49cc1 
  3rdparty/stout/include/stout/os/windows/write.hpp 
71006489918d9495d37d2fdfdca08b40b419481a 
  3rdparty/stout/tests/os/filesystem_tests.cpp 
c190baa2230298e428d4034b90dccffb59b4e710 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 66436: Removed use of `fstat()` from `http.cpp` and `http_proxy.cpp`.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, and 
Michael Park.


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


Repository: mesos


Description
---

The functions `os::stat::size()` and `os::stat::isdir()` are now
overloaded for an `int_fd` type, using `fstat()` on POSIX, and the
equivalent functions with a `HANDLE` on Windows. This allowed us to
remove the use of `::fstat()`, which was not abstracted, and not
supported on Windows without the use of a CRT integer file descriptor.


Diffs
-

  3rdparty/libprocess/src/http.cpp 63dd2c1f629ac316d0b31f8a854e482ae6eda634 
  3rdparty/libprocess/src/http_proxy.cpp 
25d63791e4788a488f96303aabeed0fa77ad7992 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 66435: Added overloads for `int_fd` to `os::stat::isdir()` and `size()`.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, and 
Michael Park.


Repository: mesos


Description
---

These should be refactored to share the common code, and the
additional overloads added to the other APIs too. However, it is not
currently necessary, and would go unused.


Diffs
-

  3rdparty/stout/include/stout/os/posix/stat.hpp 
58353742b39bac4fbfcb2ab7708f0f8719ea5b3b 
  3rdparty/stout/include/stout/os/windows/stat.hpp 
c04953ee42f45dd80b6362fbeeddf4a0a20e7412 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 66434: Windows: Refactored `subprocess_windows.cpp` to use `os::open()`.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, and 
Michael Park.


Repository: mesos


Description
---

Previously, `os::open()` used the CRT function `_wopen()`, and so this
file was written to use the `CreateFile()` API directly. Now that
`os::open()` uses the Windows API, all this duplicate code can be
deleted in favor of using the `os::open()` and
`internal::windows::set_inherit()`. The major benefit here is that the
logic now almost exactly matches the POSIX counterpart in
`subprocess_posix.cpp`, to the point that we may want to recombine
these files in the future.


Diffs
-

  3rdparty/libprocess/src/subprocess_windows.cpp 
1a91fbe7aeb44174ccfa2e7e299bc7dd52a11b8a 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 66433: Windows: Made `net::download()` use CRT file descriptor explicitly.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, and 
Michael Park.


Repository: mesos


Description
---

This is an edge case where a third-party library (libcurl) requires a
CRT integer file descriptor. Thus we explicitly allocate one via
`crt()`, which requires that we also manually close it via `_close()`,
not `os::close()`.


Diffs
-

  3rdparty/stout/include/stout/net.hpp d2992c05b221ea90dae1c06d27753932f7411925 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 66432: Windows: Fixed `os::write()` to use `WriteFile()`.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, and 
Michael Park.


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


Repository: mesos


Description
---

This can eventually support overlapped I/O.


Diffs
-

  3rdparty/stout/include/stout/os/windows/write.hpp 
71006489918d9495d37d2fdfdca08b40b419481a 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 66039: Check both disk and inode usage when slaves schedule gc.

[Gilbert Song]

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




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


I would leave this to @andschwa since I suspect we could have a windows 
implementation together.



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


See my comment on the previous patch. We could find out the max usage at 
the caller side.


- Gilbert Song


On March 13, 2018, 7:05 a.m., fei long wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66039/
> ---
> 
> (Updated March 13, 2018, 7:05 a.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Bugs: MESOS-8665
> https://issues.apache.org/jira/browse/MESOS-8665
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Check both disk and inode usage when slaves schedule gc.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 2f4ab157448eafc0f41372ee50255a76129e90db 
> 
> 
> Diff: https://reviews.apache.org/r/66039/diff/2/
> 
> 
> Testing
> ---
> 
> No new tests are added and all "make check" tests are passed.
> 
> 
> Thanks,
> 
> fei long
> 
>

Review Request 66429: Windows: Deleted dead code from `process::internal` namespace.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, and 
Michael Park.


Repository: mesos


Description
---

The deleted code was purely self-referential.


Diffs
-

  3rdparty/libprocess/src/subprocess_windows.cpp 
1a91fbe7aeb44174ccfa2e7e299bc7dd52a11b8a 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 66431: Windows: Fixed `os::read()` to use `ReadFile()`.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, and 
Michael Park.


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


Repository: mesos


Description
---

This can eventually support overlapped I/O.

The Windows API `ReadFile()` returns an error if the pipe is broken,
where `_read()` did not, but this is not an error for us as the data
is still read correctly. So we ignore it.


Diffs
-

  3rdparty/stout/include/stout/os/windows/ftruncate.hpp 
fc4a8b5040d56fa9766687e44ce17fbe47d9e8f0 
  3rdparty/stout/include/stout/os/windows/read.hpp 
8047ad590fcc46d3ec46b551472d8c518ae49cc1 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 66428: Windows: Fixed `os::lseek()` to use `SetFilePointerEx()`.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, and 
Michael Park.


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


Repository: mesos


Description
---

Note the TODO, we may want to synchronize this code later.


Diffs
-

  3rdparty/stout/include/stout/os/windows/lseek.hpp PRE-CREATION 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 66430: Windows: Fixed `os::dup()` to use `DuplicateHandle()`.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, and 
Michael Park.


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


Repository: mesos


Description
---

Note that for now we need to keep the original CRT code, as it can't
be removed until `FD_CRT` is removed too.


Diffs
-

  3rdparty/stout/include/stout/os/windows/dup.hpp 
265046cf7ffc14f7326711d295aa7dd4f0a8a1e3 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 66038: Add usageWithInode to check both disk and inode usage.

[Gilbert Song]

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




3rdparty/stout/include/stout/posix/fs.hpp
Lines 55 (patched)


as a library in stout, I would suggest to avoid adding `std::max`. Instead, 
we may want to just implement a function called `inodeUsage()`.

We could move the std::max to the caller size, which is more explicit.

Thoughts?


- Gilbert Song


On March 13, 2018, 7:08 a.m., fei long wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66038/
> ---
> 
> (Updated March 13, 2018, 7:08 a.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Bugs: MESOS-8665
> https://issues.apache.org/jira/browse/MESOS-8665
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add usageWithInode to check both disk and inode usage.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/posix/fs.hpp 
> 269a4f50f1df8d68be9e11030f885cf2c254c9d8 
> 
> 
> Diff: https://reviews.apache.org/r/66038/diff/2/
> 
> 
> Testing
> ---
> 
> No new tests are added and all "make check" tests are passed.
> 
> 
> Thanks,
> 
> fei long
> 
>

Review Request 66427: Split `stout/os/lseek.hpp` into Windows and POSIX files.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, and 
Michael Park.


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


Repository: mesos


Description
---

Split `stout/os/lseek.hpp` into Windows and POSIX files.


Diffs
-

  3rdparty/stout/include/Makefile.am 742bfc44d68d978dd2249ece500d6f64e4d7f02a 
  3rdparty/stout/include/stout/os/lseek.hpp 
77fe272afc89f41836c2540de42135dc364917ce 
  3rdparty/stout/include/stout/os/posix/lseek.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/windows/lseek.hpp PRE-CREATION 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 66425: Windows: Replaced `WindowsFD` with `int_fd` typedef.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, and 
Michael Park.


Repository: mesos


Description
---

The latter should be used everywhere but in the implementation for
consistency with the POSIX side of the code.

Also meant fixing the included header (and spacing).


Diffs
-

  3rdparty/stout/include/stout/internal/windows/inherit.hpp 
6da6f8eb1382d226c8b16a8e4cbb454205ef4045 
  3rdparty/stout/include/stout/os/windows/close.hpp 
ff635e44235d63888a210cd68d49f6678a851e31 
  3rdparty/stout/include/stout/os/windows/dup.hpp 
265046cf7ffc14f7326711d295aa7dd4f0a8a1e3 
  3rdparty/stout/include/stout/os/windows/fcntl.hpp 
bf8c38acad60f9b0eb752053dcd53a9fda7b8bfa 
  3rdparty/stout/include/stout/os/windows/fsync.hpp 
8405247280b51e74a172317816096ca77fdfd1e7 
  3rdparty/stout/include/stout/os/windows/ftruncate.hpp 
fc4a8b5040d56fa9766687e44ce17fbe47d9e8f0 
  3rdparty/stout/include/stout/os/windows/pipe.hpp 
365db9480f6258a03ef2e760a19abef8ab177e58 
  3rdparty/stout/include/stout/os/windows/read.hpp 
8047ad590fcc46d3ec46b551472d8c518ae49cc1 
  3rdparty/stout/include/stout/os/windows/sendfile.hpp 
fff5872db6b3f69464e41e6d108a107e4eeabd12 
  3rdparty/stout/include/stout/os/windows/shell.hpp 
aacd746922495f994891aa85d3e4fa95e2bd1c44 
  3rdparty/stout/include/stout/os/windows/socket.hpp 
259b05b8c85e399feaccec698d58b7d540cad368 
  3rdparty/stout/include/stout/os/windows/write.hpp 
71006489918d9495d37d2fdfdca08b40b419481a 
  3rdparty/stout/include/stout/windows/os.hpp 
739ee4da3f09d2a9597d4451e755e77903e9287d 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 66426: Windows: More constness in stout.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, and 
Michael Park.


Repository: mesos


Description
---

Also small fixes such as `reserve` over an allocation, and a bad name
`si` instead of `info`.


Diffs
-

  3rdparty/stout/include/stout/os/windows/getcwd.hpp 
f316d618226872b57d950b468352176a7a0cb45a 
  3rdparty/stout/include/stout/os/windows/getenv.hpp 
58012e03aca5dfa2f65fc183b21533dd0ed91d8d 
  3rdparty/stout/include/stout/os/windows/jobobject.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/windows/killtree.hpp 
ce8bdcd18305ffb758f22a6c2bbc7393675aebdf 
  3rdparty/stout/include/stout/os/windows/mkdir.hpp 
8d8d80bee77253086939c28333b0413bd8b8b8b6 
  3rdparty/stout/include/stout/os/windows/pagesize.hpp 
ddf23c1c5d15c1dd7de37e98673b70836a0e2c5c 
  3rdparty/stout/include/stout/os/windows/realpath.hpp 
c6bad5063c8d8255b29a3a6cb9ea51e13c42275c 
  3rdparty/stout/include/stout/os/windows/su.hpp 
1bfbb261edbd25b4552742fc0597e331012cad98 
  3rdparty/stout/include/stout/os/windows/temp.hpp 
9cf467fd9358cc4de702e0501263abcd28a0fa8c 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 66423: Split `stout/os/open.hpp` into Windows and POSIX files.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, and 
Michael Park.


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


Repository: mesos


Description
---

The logic remained the same, just with the Windows code removed from
the POSIX code, and vice versa.


Diffs
-

  3rdparty/stout/include/Makefile.am 742bfc44d68d978dd2249ece500d6f64e4d7f02a 
  3rdparty/stout/include/stout/os/open.hpp 
4dc5b087abf40ac9b81f0fd611aca192e5d33ce7 
  3rdparty/stout/include/stout/os/posix/open.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/windows/open.hpp PRE-CREATION 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 66420: Windows: Extracted job object code into `os/windows/jobobject.hpp`.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, and 
Michael Park.


Repository: mesos


Description
---

The functions written to deal with job objects on Windows had become
large enough to warrant being refactored into their own file. Also
was the perfect opportunity to fix formatting issues.

When including `jobobject.hpp` for `killtree.hpp`, other unnecessary
headers were removed.


Diffs
-

  3rdparty/stout/include/Makefile.am 742bfc44d68d978dd2249ece500d6f64e4d7f02a 
  3rdparty/stout/include/stout/os/windows/jobobject.hpp PRE-CREATION 
  3rdparty/stout/include/stout/os/windows/killtree.hpp 
ce8bdcd18305ffb758f22a6c2bbc7393675aebdf 
  3rdparty/stout/include/stout/windows/os.hpp 
739ee4da3f09d2a9597d4451e755e77903e9287d 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 66422: Windows: Included used `jobobject.hpp` stout header in Mesos.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, and 
Michael Park.


Repository: mesos


Description
---

The job object functions were refactored upstream from
`windows/os.hpp` to `os/windows/jobobject.hpp`.


Diffs
-

  src/launcher/executor.cpp 383655230fcdad30b652ea984cb9f5fc9c49dd38 
  src/slave/containerizer/mesos/isolators/windows/cpu.cpp 
69b0ff8ab1c391c8771954de64a17b6ad1c5c10c 
  src/slave/containerizer/mesos/isolators/windows/mem.cpp 
6cc65b942fd7d11d2ea35eab0225427ffdd9d1ee 
  src/slave/containerizer/mesos/launch.cpp 
8c739ccd39675d5417437609a31f5a21784535b7 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 66424: Windows: Replaced `_wopen()` with `CreateFileW()` in `os::open()`.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, and 
Michael Park.


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


Repository: mesos


Description
---

Instead of using the CRT implementation of `_wopen()` for the
`os::open()` API, we now use the Windows API `CreateFileW()`, mapping
each of the Linux `open()` flags to their semantic equivalents. This
will make implementing overlapped I/O possible, and is a step toward
removing the use of integer file descriptors on Windows.

Note that instead of redefining the C flags like `O_RDONLY`, we just
use them directly in our mapping logic, and set the used but
unsupported flags to zero.

This change uncovered several bugs such as incorrect access flags, and
used-but-not-included headers.

We currently ignore creation permissions as they will be handled in a
broader project to map permissions to Windows correctly.


Diffs
-

  3rdparty/stout/include/stout/net.hpp d2992c05b221ea90dae1c06d27753932f7411925 
  3rdparty/stout/include/stout/os/windows/fcntl.hpp 
bf8c38acad60f9b0eb752053dcd53a9fda7b8bfa 
  3rdparty/stout/include/stout/os/windows/mktemp.hpp 
5c775c45c415d9ddd6a80ab814fb55475e9f871e 
  3rdparty/stout/include/stout/os/windows/open.hpp PRE-CREATION 
  3rdparty/stout/include/stout/windows.hpp 
1bfcdf4a5c097cc6d2293396ce39c8ad2c9ec993 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Review Request 66421: Windows: Included used `jobobject.hpp` stout header in libprocess.

[Andrew Schwartzmeyer]

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

Review request for mesos, Akash Gupta, Eric Mumau, John Kordich, Joseph Wu, and 
Michael Park.


Repository: mesos


Description
---

The job object functions were refactored upstream from
`windows/os.hpp` to `os/windows/jobobject.hpp`.

Also removed `windows/os.hpp` because `os.hpp` includes it for us.


Diffs
-

  3rdparty/libprocess/include/process/windows/jobobject.hpp 
0de374d374804c2d4f06a352d80062d3b42ed9b2 


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


Testing
---


Thanks,

Andrew Schwartzmeyer

Re: Review Request 66392: Find sasl2 on non-Windows platforms before trying to link it.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['66384', '66385', '66387', '66392']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

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

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

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

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

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

 1 FAILED TEST
  YOU HAVE 214 DISABLED TESTS

```

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

```
I0404 05:42:43.383244 10852 master.cpp:10449] Updating the state of task 
10bb697e-9f99-4452-8a1c-fd342943ca7b of framework 
94d81d79-ea86-457c-a5c1-aa1f22d10581- (latest state: TASK_KILLED, status 
update state: TASKI0404 05:42:43.207343 13508 exec.cpp:162] Version: 1.6.0
I0404 05:42:43.232236 10728 exec.cpp:236] Executor registered on agent 
94d81d79-ea86-457c-a5c1-aa1f22d10581-S0
I0404 05:42:43.236229  2300 executor.cpp:176] Received SUBSCRIBED event
I0404 05:42:43.240231  2300 executor.cpp:180] Subscribed executor on 
winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I0404 05:42:43.240231  2300 executor.cpp:176] Received LAUNCH event
I0404 05:42:43.246279  2300 executor.cpp:648] Starting task 
10bb697e-9f99-4452-8a1c-fd342943ca7b
I0404 05:42:43.325275  2300 executor.cpp:483] Running 
'D:\DCOS\mesos\src\mesos-containerizer.exe launch '
I0404 05:42:43.352234  2300 executor.cpp:661] Forked command at 12580
I0404 05:42:43.385232 11656 exec.cpp:445] Executor asked to shutdown
I0404 05:42:43.386242 12400 executor.cpp:176] Received SHUTDOWN event
I0404 05:42:43.386242 12400 executor.cpp:758] Shutting down
I0404 05:42:43.386242 12400 executor.cpp:868] Sending SIGTERM to process tree 
at pid _KILLED)
I0404 05:42:43.383244 13868 slave.cpp:3877] Shutting down framework 
94d81d79-ea86-457c-a5c1-aa1f22d10581-
I0404 05:42:43.384235 13868 slave.cpp:6574] Shutting down executor 
'10bb697e-9f99-4452-8a1c-fd342943ca7b' of framework 
94d81d79-ea86-457c-a5c1-aa1f22d10581- at executor(1)@10.3.1.8:49868
I0404 05:42:43.384235  8864 slave.cpp:923] Agent terminating
W0404 05:42:43.385232  8864 slave.cpp:3873] Ignoring shutdown framework 
94d81d79-ea86-457c-a5c1-aa1f22d10581- because it is terminating
I0404 05:42:43.386242 10852 master.cpp:10548] Removing task 
10bb697e-9f99-4452-8a1c-fd342943ca7b with resources cpus(allocated: *):4; 
mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
*):[31000-32000] of framework 94d81d79-ea86-457c-a5c1-aa1f22d10581- on 
agent 94d81d79-ea86-457c-a5c1-aa1f22d10581-S0 at slave(418)@10.3.1.8:49847 
(winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0404 05:42:43.388272 10852 master.cpp:1295] Agent 
94d81d79-ea86-457c-a5c1-aa1f22d10581-S0 at slave(418)@10.3.1.8:49847 
(winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) disconnected
I0404 05:42:43.388272 14080 containerizer.cpp:2338] Destroying container 
3c10e52b-ee67-45f1-9ef4-44f52d6c34d9 in RUNNING state
I0404 05:42:43.389245 10852 master.cpp:3286] Disconnecting agent 
94d81d79-ea86-457c-a5c1-aa1f22d10581-S0 at slave(418)@10.3.1.8:49847 
(winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0404 05:42:43.389245 14080 containerizer.cpp:2952] Transitioning the state of 
container 

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

[Mesos Reviewbot]

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



Patch looks great!

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

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On April 4, 2018, 3:32 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66323/
> ---
> 
> (Updated April 4, 2018, 3:32 a.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-8617 and MESOS-8624
> https://issues.apache.org/jira/browse/MESOS-8617
> https://issues.apache.org/jira/browse/MESOS-8624
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These tests verify the agent behavior of launching
> several task groups using the same executor. When
> task groups are launching on the agent (before creating
> any executor), if the first received task group
> fails to launch, later task groups will get dropped.
> If a later received task group fails to launch, the first
> received task group should still launch successfully.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353 
> 
> 
> Diff: https://reviews.apache.org/r/66323/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 66418: Handled failed publish and unpublish CSI calls properly.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the dependent review: 66398.

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

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

Relevant logs:

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

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

- Mesos Reviewbot Windows


On April 4, 2018, 4:01 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66418/
> ---
> 
> (Updated April 4, 2018, 4:01 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8717
> https://issues.apache.org/jira/browse/MESOS-8717
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, when a `ControllerPublishVolume` or a `NodePublishVolume`
> fails, SLRP assumes the volume states remain unchanged, which are not
> suggested by the CSI spec. This patch handles such failures properly.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/provider.cpp 
> a07620d1c4bf618f669575b3e183bf598da905a6 
> 
> 
> Diff: https://reviews.apache.org/r/66418/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

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

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66323 was successfully built and tested.

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

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

- Mesos Reviewbot Windows


On April 4, 2018, 3:32 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66323/
> ---
> 
> (Updated April 4, 2018, 3:32 a.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-8617 and MESOS-8624
> https://issues.apache.org/jira/browse/MESOS-8617
> https://issues.apache.org/jira/browse/MESOS-8624
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These tests verify the agent behavior of launching
> several task groups using the same executor. When
> task groups are launching on the agent (before creating
> any executor), if the first received task group
> fails to launch, later task groups will get dropped.
> If a later received task group fails to launch, the first
> received task group should still launch successfully.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353 
> 
> 
> Diff: https://reviews.apache.org/r/66323/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 66384: Link libm in ZooKeeper build on FreeBSD.

[David Forsythe]

> On April 2, 2018, 5:06 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/zookeeper-3.4.8.patch
> > Line 154 (original), 154 (patched)
> > 
> >
> > Also, if this change should go upstream to ZooKeeper (looks to me like 
> > it should) then let's make sure it does. Their process is [pretty 
> > similar](https://cwiki.apache.org/confluence/display/ZOOKEEPER/HowToContribute).
> > 
> > If you don't want to, I can upstream it.
> 
> David Forsythe wrote:
> I can add it to my todo list.
> 
> Andrew Schwartzmeyer wrote:
> FWIW I can review/shepherd over on ZooKeeper too, though I can't commit 
> directly (yet). Michael is pretty good about it though.

I opened https://github.com/apache/zookeeper/pull/499 for 
https://issues.apache.org/jira/browse/ZOOKEEPER-3017


- David


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


On April 1, 2018, 2:32 a.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66384/
> ---
> 
> (Updated April 1, 2018, 2:32 a.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-4176
> https://issues.apache.org/jira/browse/MESOS-4176
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Link libm in ZooKeeper build on FreeBSD.
> 
> 
> Diffs
> -
> 
>   3rdparty/zookeeper-3.4.8.patch 2eaa056dd5668d5842b5b59a42f83ae307d4aef0 
> 
> 
> Diff: https://reviews.apache.org/r/66384/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Forsythe
> 
>

Review Request 66418: Handled failed publish and unpublish CSI calls properly.

[Chun-Hung Hsiao]

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

Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description
---

Previously, when a `ControllerPublishVolume` or a `NodePublishVolume`
fails, SLRP assumes the volume states remain unchanged, which are not
suggested by the CSI spec. This patch handles such failures properly.


Diffs
-

  src/resource_provider/storage/provider.cpp 
a07620d1c4bf618f669575b3e183bf598da905a6 


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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 66411: Made the test CSI plugin compatible to CSI v0.2.

[Chun-Hung Hsiao]

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

(Updated April 4, 2018, 4 a.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description
---

This patch contains necessary changes for the test CSI plugin to support
CSI v0.2. The `STAGE_UNSTAGE_VOLUME` node service capability is not
implemented in this patch yet.


Diffs (updated)
-

  src/examples/test_csi_plugin.cpp aed22623b6dadb791eeaac2ecde4c31236a5fc19 


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

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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 66408: Updated CSI helpers for v0.2.

[Chun-Hung Hsiao]

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

(Updated April 4, 2018, 3:59 a.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description (updated)
---

This patch adds new helper classes for CSI plugin and node capabilities,
removed helpers for the removed csi.Version proto message, and makes
the VolumeState proto message uses csi::v0::VolumeCapability.

NOTE: This is not future-proof if there is a breaking change in
VolumeCapability, we might need to change VolumeState to support
multiple CSI versions in the future.


Diffs (updated)
-

  src/csi/state.hpp dcbc7aba84e0e66dd0e6caf7a64fb95aae69de0a 
  src/csi/state.proto 0373c8ad217998a54b06d483f94052bcf4293677 
  src/csi/utils.hpp 58b071d4f1512e4cf10d077bd1e03e66626f6dff 
  src/csi/utils.cpp 9e0435762a16f2995bc5a04ec342af6ea231054a 


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

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


Testing
---

This patch cannot be compiled standalone. Tests done later in the chain.


Thanks,

Chun-Hung Hsiao

Re: Review Request 66392: Find sasl2 on non-Windows platforms before trying to link it.

[David Forsythe]

> On April 3, 2018, 4:36 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/CMakeLists.txt
> > Lines 267 (patched)
> > 
> >
> > This looks good; except I think in generaly it's going to find a shared 
> > version of `sasl2` not a static. I don't think it really does too much to 
> > imported targets, as they're not being built, but for accuracy's sake, 
> > `add_library(sasl2 SHARED IMPORTED GLOBAL)` (unless we really don't know, 
> > so `UNKNOWN`).

Yup. Fixed.


- David


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


On April 4, 2018, 3:46 a.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66392/
> ---
> 
> (Updated April 4, 2018, 3:46 a.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-4176
> https://issues.apache.org/jira/browse/MESOS-4176
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Find sasl2 on non-Windows platforms before trying to link it.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 2b63b58f7d6a88c9986b746283dcfa79b7bcb270 
> 
> 
> Diff: https://reviews.apache.org/r/66392/diff/3/
> 
> 
> Testing
> ---
> 
> make on FreeBSD
> 
> 
> Thanks,
> 
> David Forsythe
> 
>

Re: Review Request 66392: Find sasl2 on non-Windows platforms before trying to link it.

[David Forsythe]

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

(Updated April 4, 2018, 3:46 a.m.)


Review request for mesos and Andrew Schwartzmeyer.


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


Repository: mesos


Description
---

Find sasl2 on non-Windows platforms before trying to link it.


Diffs (updated)
-

  3rdparty/CMakeLists.txt 2b63b58f7d6a88c9986b746283dcfa79b7bcb270 


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

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


Testing (updated)
---

make on FreeBSD


Thanks,

David Forsythe

Re: Review Request 66412: Fixed flaky agent test.

[Meng Zhu]

> On April 3, 2018, 5:59 p.m., Greg Mann wrote:
> > src/tests/slave_tests.cpp
> > Lines 9093-9094 (patched)
> > 
> >
> > Why this change?

Removed. And changed await_ready to await.


- Meng


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


On April 3, 2018, 3:10 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66412/
> ---
> 
> (Updated April 3, 2018, 3:10 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In test `KillTaskGroupBetweenRunTaskParts`, added extra
> synchronization to ensure `unmocked__run` finishes before
> the test tear down.
> 
> Also updated all dispatch calls to capture by value to avoid
> potential races where the arguments might be destroyed
> while the dispatch call is still running.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353 
> 
> 
> Diff: https://reviews.apache.org/r/66412/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

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

[Meng Zhu]

> On April 3, 2018, 5:58 p.m., Greg Mann wrote:
> > src/tests/slave_tests.cpp
> > Lines 5592 (patched)
> > 
> >
> > Is this needed?

To avoid uninteresting mock call.


> On April 3, 2018, 5:58 p.m., Greg Mann wrote:
> > src/tests/slave_tests.cpp
> > Lines 5650 (patched)
> > 
> >
> > s/promiseTask2/promise/

I think naming the promise in a meaningful way improves readability.


- Meng


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


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

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

[Meng Zhu]

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

(Updated April 3, 2018, 8:38 p.m.)


Review request for mesos and Greg Mann.


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


Repository: mesos


Description
---

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


Diffs (updated)
-

  src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353 


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

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


Testing
---

make check


Thanks,

Meng Zhu

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

[Meng Zhu]

> On April 3, 2018, 5:17 p.m., Greg Mann wrote:
> > src/tests/slave_tests.cpp
> > Lines 5249 (patched)
> > 
> >
> > Is this necessary?

To avoid uninteresting mock calls.


> On April 3, 2018, 5:17 p.m., Greg Mann wrote:
> > src/tests/slave_tests.cpp
> > Lines 5251-5252 (patched)
> > 
> >
> > Is this needed?

To avoid uninteresting mock calls.


> On April 3, 2018, 5:17 p.m., Greg Mann wrote:
> > src/tests/slave_tests.cpp
> > Lines 5456-5457 (patched)
> > 
> >
> > Is this needed?

To avoid uninteresting mock call.


- Meng


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


On April 3, 2018, 8:32 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66323/
> ---
> 
> (Updated April 3, 2018, 8:32 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Bugs: MESOS-8617 and MESOS-8624
> https://issues.apache.org/jira/browse/MESOS-8617
> https://issues.apache.org/jira/browse/MESOS-8624
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These tests verify the agent behavior of launching
> several task groups using the same executor. When
> task groups are launching on the agent (before creating
> any executor), if the first received task group
> fails to launch, later task groups will get dropped.
> If a later received task group fails to launch, the first
> received task group should still launch successfully.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353 
> 
> 
> Diff: https://reviews.apache.org/r/66323/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

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

[David Forsythe]

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

(Updated April 4, 2018, 3:33 a.m.)


Review request for mesos, Andrew Schwartzmeyer and Benjamin Bannier.


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


Repository: mesos


Description
---

Fix 3rdparty build commands for FreeBSD.


Diffs (updated)
-

  3rdparty/CMakeLists.txt 2b63b58f7d6a88c9986b746283dcfa79b7bcb270 


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

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


Testing (updated)
---

make on FreeBSD


Thanks,

David Forsythe

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

[David Forsythe]

> On March 28, 2018, 8:06 a.m., Benjamin Bannier wrote:
> > 3rdparty/CMakeLists.txt
> > Lines 58-63 (patched)
> > 
> >
> > Like discussed offline, I don't think there is a reason we need to bolt 
> > such logic on the used system, at least currently.
> > 
> > Let's instead use `find_program` to find a `make` and use it to set 
> > this variable.
> > 
> > Unless I miss something, we do not depend on GNU make here, so let's 
> > maybe reflect that in a more general name, e.g., just `MAKE` if it is 
> > available.
> 
> David Forsythe wrote:
> I took a look at find_program, but I don't think that will solve the 
> problem.
> 
> We actually *do* depend on gmake here (even on darwin). On FreeBSD GHU 
> make is a 3rd party application and is actually called and installed as 
> `gmake`, whereas the system make is actually Pmake (which is incompatible).
> 
> David Forsythe wrote:
> I should clarify that there is incompability in a 3rdparty Makefile, not 
> that the two makes are completely incompatible.
> 
> Particularly there are three issues - LevelDB, libev, and glog.
> 
> LevelDB is always going to fail without `gmake`, because it does not use 
> autotools. I don't know how we can get around that.
> 
> libev and glog do use autotools. They have some strange behavior when 
> building, though --
> If I use `cmake --build` they both fail on a bad target (because of an 
> incompatible Makefile).
> If I use (bsd) `make`, they build fine.
> If I use `gmake` they both fail (bad target).
> 
> For both the first and third scenario, I can run the commands that are 
> failing outside of the build and they succeed.
> 
> Accordng to the cache, cmake is using `gmake` as `CMAKE_MAKE_PROGRAM` in 
> these cases, so it's not clear to me why anything would fail when I do the 
> whole build with `gmake`. It seems like the build can go make -> gmake, but 
> not gmake -> make.
> 
> I think the baseline should be having `cmake --build` work, and the only 
> way I have gotten everything to work as expected is by forcing `gmake` 
> everywhere. I'm learning Cmake as I go so I could be missing something 
> obvious to fix this.
> 
> David Forsythe wrote:
> I've been thinking about this a bit more, and I think that the best 
> approach might actually be to just leave the libev and glog calls as make and 
> leave the conditional gmake call. On FreeBSD we could just tell users to use 
> `make` instead of cmake build. It seems like a way less intrusive change.
> 
> Thoughts?
> 
> Benjamin Bannier wrote:
> I think it should be perfectly fine to use `gmake` if it is available, it 
> is usually a symlink to `make` anyway.
> 
> The implementation I was thinking of was to add below instead,
> 
> find_program(
> GNU_MAKE
> NAMES gmake make
> DOC "path to GNU make executable")
> 
> if (NOT GNU_MAKE)
>   message(FATAL_ERROR "Could not find a suitable GNU make executable")
> endif()
> 
> David Forsythe wrote:
> This would always succeed on FreeBSD (or any system with a make), even if 
> gmake wasn't installed. It is not a symlink:
> 
> ```
> 
> root@mesos0:/mesos-build # gmake --version
> GNU Make 4.2.1
> ...
> root@mesos0:/mesos-build # make --version
> usage: make [-BeikNnqrstWwX]
> ...
> 
> ```
> 
> With autotools we didn't run into this because (iirc) so much was 
> incompatible that I just used/assumed `gmake` for everything. With `cmake` 
> we're close enough that I think it might be worth it to try and support bsd 
> make.

I made everything use `gmake`, but I didn't add the check/message because the 
check always succeeds. Added a TODO as discussed offline.


- David


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


On April 4, 2018, 3:33 a.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66314/
> ---
> 
> (Updated April 4, 2018, 3:33 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Benjamin Bannier.
> 
> 
> Bugs: MESOS-4176
> https://issues.apache.org/jira/browse/MESOS-4176
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix 3rdparty build commands for FreeBSD.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 2b63b58f7d6a88c9986b746283dcfa79b7bcb270 
> 
> 
> Diff: https://reviews.apache.org/r/66314/diff/2/
> 
> 
> Testing
> ---
> 
> make on FreeBSD
> 
> 
> Thanks,
> 
> David Forsythe
> 
>

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

[Meng Zhu]

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

(Updated April 3, 2018, 8:32 p.m.)


Review request for mesos and Greg Mann.


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


Repository: mesos


Description (updated)
---

These tests verify the agent behavior of launching
several task groups using the same executor. When
task groups are launching on the agent (before creating
any executor), if the first received task group
fails to launch, later task groups will get dropped.
If a later received task group fails to launch, the first
received task group should still launch successfully.


Diffs (updated)
-

  src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353 


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

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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 66178: Added a test to verify that task launch order is enforced.

[Meng Zhu]

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

(Updated April 3, 2018, 8:24 p.m.)


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


Changes
---

Updated the test to use a matcher for a more robust test run.


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


Repository: mesos


Description
---

Agent should launch the task in their receiving order.
On the task launch path, there are currently two
asynchronous steps which may complete out of order:
unschedule GC and task authorization.

This test simulates the reordering of the completions
of task authorization step and verify that, despite the
reordering, tasks can still launch in their original order.


Diffs (updated)
-

  src/tests/mesos.hpp 46c271b5c5bedbdabd58b3cdbb82216d55c846bd 
  src/tests/mesos.cpp d82963195573dd9ed7d12a7708f64a236b28cdf1 
  src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353 


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

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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 66346: Added three matchers for agent tests.

[Meng Zhu]

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

(Updated April 3, 2018, 8:21 p.m.)


Review request for mesos and Greg Mann.


Changes
---

Added one more matcher `AuthorizationRequestHasTaskID`. Addressed comments.


Summary (updated)
-

Added three matchers for agent tests.


Repository: mesos


Description (updated)
---

Added a matcher to match the `TaskID` of `Option`.
Added a matcher to match an `Option` which
contains a task with the specified `TaskID`.
Added a matcher to match the `TaskID` of
`authorization::Request.Object.TaskInfo`.


Diffs (updated)
-

  src/tests/mesos.hpp 46c271b5c5bedbdabd58b3cdbb82216d55c846bd 


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

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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 66412: Fixed flaky agent test.

[Mesos Reviewbot]

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



Patch looks great!

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

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On April 3, 2018, 10:10 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66412/
> ---
> 
> (Updated April 3, 2018, 10:10 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In test `KillTaskGroupBetweenRunTaskParts`, added extra
> synchronization to ensure `unmocked__run` finishes before
> the test tear down.
> 
> Also updated all dispatch calls to capture by value to avoid
> potential races where the arguments might be destroyed
> while the dispatch call is still running.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353 
> 
> 
> Diff: https://reviews.apache.org/r/66412/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 66412: Fixed flaky agent test.

[Greg Mann]

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




src/tests/slave_tests.cpp
Lines 9093-9094 (patched)


Why this change?


- Greg Mann


On April 3, 2018, 10:10 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66412/
> ---
> 
> (Updated April 3, 2018, 10:10 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In test `KillTaskGroupBetweenRunTaskParts`, added extra
> synchronization to ensure `unmocked__run` finishes before
> the test tear down.
> 
> Also updated all dispatch calls to capture by value to avoid
> potential races where the arguments might be destroyed
> while the dispatch call is still running.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353 
> 
> 
> Diff: https://reviews.apache.org/r/66412/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

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

[Greg Mann]

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




src/tests/slave_tests.cpp
Lines 5563 (patched)


Move this down above its first use.



src/tests/slave_tests.cpp
Lines 5584 (patched)


I think we usually split these onto separate lines in the tests (i.e., 
`.WillOnce` on a new line). Could you do that here and below?



src/tests/slave_tests.cpp
Lines 5592 (patched)


Is this needed?



src/tests/slave_tests.cpp
Lines 5641 (patched)


s/taskgroup2/task group 2/

or

s/taskgroup2/`taskGroup2`/

here and below.



src/tests/slave_tests.cpp
Lines 5650 (patched)


s/promiseTask2/promise/



src/tests/slave_tests.cpp
Lines 5651 (patched)


Is the underscore necessary in this variable name?



src/tests/slave_tests.cpp
Lines 5673 (patched)


s/Taskgroup1/Task group 1/

or

s/Taskgroup1/`taskGroup1`/

here and below.



src/tests/slave_tests.cpp
Lines 5702 (patched)


As discussed in chat, let's capture by value here.


- Greg Mann


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

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

[Greg Mann]

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




src/tests/slave_tests.cpp
Lines 5208 (patched)


This test does actually launch task groups, so maybe:

s/task(group)s/task groups/

here and below.



src/tests/slave_tests.cpp
Lines 5210 (patched)


s/first task (in the agent receiving order)/first received task/

here and below.



src/tests/slave_tests.cpp
Lines 5212 (patched)


s/TasksLaunch/LaunchTasks/



src/tests/slave_tests.cpp
Lines 5220 (patched)


Could you move this down above the site of its first use?



src/tests/slave_tests.cpp
Lines 5249 (patched)


Is this necessary?



src/tests/slave_tests.cpp
Lines 5251-5252 (patched)


Is this needed?



src/tests/slave_tests.cpp
Lines 5272 (patched)


Looks like this could be just `executorInfo`? Here and below.



src/tests/slave_tests.cpp
Lines 5275 (patched)


s/two/three/

Also, I would add a newline after this comment, since it applies to 
multiple blocks of code below.



src/tests/slave_tests.cpp
Lines 5317 (patched)


s/both/all/



src/tests/slave_tests.cpp
Lines 5320-5344 (patched)


As discussed in chat, here we're making the assumption that the first 
invocation of `_run()` corresponds to `taskGroup1`, which is not necessarily 
true. Let's make use of your matchers to address this, here and below.



src/tests/slave_tests.cpp
Lines 5348-5349 (patched)


Indent two more spaces.



src/tests/slave_tests.cpp
Lines 5351 (patched)


Let's split this into 3 separate AWAIT_READY calls.



src/tests/slave_tests.cpp
Lines 5364 (patched)


I would either do

s/taskgroup1/`taskGroup1`/

or

s/taskgroup1/task group 1/

here and below.



src/tests/slave_tests.cpp
Lines 5365 (patched)


As discussed in chat, let's capture by value here and elsewhere.



src/tests/slave_tests.cpp
Lines 5447 (patched)


Let's put the `.WillOnce` on a new line for consistency.



src/tests/slave_tests.cpp
Lines 5456-5457 (patched)


Is this needed?



src/tests/slave_tests.cpp
Lines 5506 (patched)


Indented too far.



src/tests/slave_tests.cpp
Lines 5537 (patched)


Indent two more spaces.



src/tests/slave_tests.cpp
Lines 5539 (patched)


Let's split this into 2 AWAIT_READY calls.


- Greg Mann


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

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

[Greg Mann]

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


Fix it, then Ship it!





src/tests/mesos.hpp
Lines 3604 (patched)


s/taskID/task ID/



src/tests/mesos.hpp
Lines 3605 (patched)


s/taskID/taskId/

here and below.

Also, I like your use of "Has" in the name below; let's do
s/OptionTaskEqTaskID/OptionTaskHasTaskID/



src/tests/mesos.hpp
Lines 3612 (patched)


s/taskID/task ID/



src/tests/mesos.hpp
Lines 3619 (patched)


I don't think you need the underscore here? Just `taskInfo` should be OK.


- Greg Mann


On March 28, 2018, 10:45 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66346/
> ---
> 
> (Updated March 28, 2018, 10:45 p.m.)
> 
> 
> Review request for mesos and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a matcher to match the taskID of `Option`.
> Added a matcher to match an `Option` which
> contains a task with the specified taskID.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp 46c271b5c5bedbdabd58b3cdbb82216d55c846bd 
> 
> 
> Diff: https://reviews.apache.org/r/66346/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 66412: Fixed flaky agent test.

[Mesos Reviewbot Windows]

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



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

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

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

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

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

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

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

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

 1 FAILED TEST
  YOU HAVE 214 DISABLED TESTS

```

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

```
I0403 23:04:33.120618  6728 master.cpp:10449] Updating the state of task 
8383afcf-80b8-4b1d-8f92-357dd43a05e5 of framework 
20e0e59c-20f3-4d45-8f75-d85e140a7e00- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0403 23:04:33.120618  5904 slave.cpp:3973] Shutting down framework 
20e0e59c-20f3-4d45-8f75-d85e140a7e00-
I0403 23:04:33.120618  5904 slave.cpp:6670] Shutting down executor 
'8383afcf-80b8-4b1d-8f92-357dd43a05e5' of framework 
20e0e59c-20f3-4d45-8f75-d85e140a7e00- at executor(1)@10.3.1.8:61262
I0403 23:04:33.121605  5700 slave.cI0403 23:04:32.941620  9416 exec.cpp:162] 
Version: 1.6.0
I0403 23:04:32.970605  9808 exec.cpp:236] Executor registered on agent 
20e0e59c-20f3-4d45-8f75-d85e140a7e00-S0
I0403 23:04:32.974613  7368 executor.cpp:176] Received SUBSCRIBED event
I0403 23:04:32.978612  7368 executor.cpp:180] Subscribed executor on 
winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I0403 23:04:32.978612  7368 executor.cpp:176] Received LAUNCH event
I0403 23:04:32.983613  7368 executor.cpp:648] Starting task 
8383afcf-80b8-4b1d-8f92-357dd43a05e5
I0403 23:04:33.061606  7368 executor.cpp:483] Running 
'D:\DCOS\mesos\src\mesos-containerizer.exe launch '
I0403 23:04:33.087615  7368 executor.cpp:661] Forked command at 10384
I0403 23:04:33.122601  6160 exec.cpp:445] Executor asked to shutdown
I0403 23:04:33.122601  7020 executor.cpp:176] Received SHUTDOWN event
I0403 23:04:33.122601  7020 executor.cpp:758] Shutting down
I0403 23:04:33.122601  7020 executor.cpp:868] Sending SIGTERM to process tree 
at pid pp:923] Agent terminating
W0403 23:04:33.122601  5700 slave.cpp:3969] Ignoring shutdown framework 
20e0e59c-20f3-4d45-8f75-d85e140a7e00- because it is terminating
I0403 23:04:33.122601  6728 master.cpp:10548] Removing task 
8383afcf-80b8-4b1d-8f92-357dd43a05e5 with resources cpus(allocated: *):4; 
mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
*):[31000-32000] of framework 20e0e59c-20f3-4d45-8f75-d85e140a7e00- on 
agent 20e0e59c-20f3-4d45-8f75-d85e140a7e00-S0 at slave(423)@10.3.1.8:61241 
(winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0403 23:04:33.125607  6728 master.cpp:1295] Agent 
20e0e59c-20f3-4d45-8f75-d85e140a7e00-S0 at slave(423)@10.3.1.8:61241 
(winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) disconnected
I0403 23:04:33.125607  6728 master.cpp:3286] Disconnecting agent 
20e0e59c-20f3-4d45-8f75-d85e140a7e00-S0 at slave(423)@10.3.1.8:61241 
(winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0403 23:04:33.125607  6728 master.cpp:3305] Deactivating agent 
20e0e59c-20f3-4d45-8f75-d85e140a7e00-S0 at slave(423)@10.3.1.8:61241 

Review Request 66412: Fixed flaky agent test.

[Meng Zhu]

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

Review request for mesos and Greg Mann.


Repository: mesos


Description
---

In test `KillTaskGroupBetweenRunTaskParts`, added extra
synchronization to ensure `unmocked__run` finishes before
the test tear down.

Also updated all dispatch calls to capture by value to avoid
potential races where the arguments might be destroyed
while the dispatch call is still running.


Diffs
-

  src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353 


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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 66145: Added a test to verify that task launch order is enforced.

[Meng Zhu]

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

(Updated April 3, 2018, 3:08 p.m.)


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


Changes
---

Relaxed GC module expectation calls to avoid rare test failure.


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


Repository: mesos


Description
---

Agent should launch the task in their receiving order.
On the task launch path, there are currently two
asynchronous steps which may complete out of order:
unschedule GC and task authorization.

This test simulates the reordering of the completions
of unschedule GC step and verify that, despite the
reordering, tasks can still launch in their original order.


Diffs (updated)
-

  src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353 


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

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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 66411: Made the test CSI plugin compatible to CSI v0.2.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [66411, 66410, 66409, 66408, 66407, 66398]

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

Error:
2018-04-03 20:40:02 URL:https://reviews.apache.org/r/66398/diff/raw/ 
[1889/1889] -> "66398.patch" [1]
error: missing binary patch data for '3rdparty/csi-0.2.0.tar.gz'
error: binary patch does not apply to '3rdparty/csi-0.2.0.tar.gz'
error: 3rdparty/csi-0.2.0.tar.gz: patch does not apply

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

- Mesos Reviewbot


On April 3, 2018, 9:22 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66411/
> ---
> 
> (Updated April 3, 2018, 9:22 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8717
> https://issues.apache.org/jira/browse/MESOS-8717
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch contains necessary changes for the test CSI plugin to support
> CSI v0.2. The `STAGE_UNSTAGE_VOLUME` node service capability is not
> implemented in this patch yet.
> 
> 
> Diffs
> -
> 
>   src/examples/test_csi_plugin.cpp aed22623b6dadb791eeaac2ecde4c31236a5fc19 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 2872f1aec1a7b94fc302a533f5ae9e1be9658087 
> 
> 
> Diff: https://reviews.apache.org/r/66411/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 63372: Added documentation for memory profiling.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [63372, 63371, 63370, 63368, 63366, 65462, 65461, 63367]

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

Error:
2018-04-03 19:50:34 URL:https://reviews.apache.org/r/63366/diff/raw/ 
[12984/12984] -> "63366.patch" [1]
error: missing binary patch data for '3rdparty/jemalloc-5.0.1.tar.bz2'
error: binary patch does not apply to '3rdparty/jemalloc-5.0.1.tar.bz2'
error: 3rdparty/jemalloc-5.0.1.tar.bz2: patch does not apply

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

- Mesos Reviewbot


On Jan. 24, 2018, 4:40 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63372/
> ---
> 
> (Updated Jan. 24, 2018, 4:40 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for memory profiling.
> 
> 
> Diffs
> -
> 
>   docs/memory-profiling.md PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63372/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 66411: Made the test CSI plugin compatible to CSI v0.2.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the dependent review: 66398.

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

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

Relevant logs:

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

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

- Mesos Reviewbot Windows


On April 3, 2018, 7:22 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66411/
> ---
> 
> (Updated April 3, 2018, 7:22 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8717
> https://issues.apache.org/jira/browse/MESOS-8717
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch contains necessary changes for the test CSI plugin to support
> CSI v0.2. The `STAGE_UNSTAGE_VOLUME` node service capability is not
> implemented in this patch yet.
> 
> 
> Diffs
> -
> 
>   src/examples/test_csi_plugin.cpp aed22623b6dadb791eeaac2ecde4c31236a5fc19 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 2872f1aec1a7b94fc302a533f5ae9e1be9658087 
> 
> 
> Diff: https://reviews.apache.org/r/66411/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 66410: Adapted storage local resource provider to use CSI v0.2.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the dependent review: 66398.

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

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

Relevant logs:

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

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

- Mesos Reviewbot Windows


On April 3, 2018, 7:20 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66410/
> ---
> 
> (Updated April 3, 2018, 7:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8717
> https://issues.apache.org/jira/browse/MESOS-8717
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch contains necessary changes for the storage local resource
> provider to use CSI v0.2. Support for the `STAGE_UNSTAGE_VOLUME` CSI
> node service capability is not implemented in this patch yet.
> 
> 
> Diffs
> -
> 
>   src/csi/state.proto 0373c8ad217998a54b06d483f94052bcf4293677 
>   src/resource_provider/storage/provider.cpp 
> a07620d1c4bf618f669575b3e183bf598da905a6 
> 
> 
> Diff: https://reviews.apache.org/r/66410/diff/1/
> 
> 
> Testing
> ---
> 
> This patch cannot be compiled standalone. Tests done later in the chain.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Review Request 66411: Made the test CSI plugin compatible to CSI v0.2.

[Chun-Hung Hsiao]

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

Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description
---

This patch contains necessary changes for the test CSI plugin to support
CSI v0.2. The `STAGE_UNSTAGE_VOLUME` node service capability is not
implemented in this patch yet.


Diffs
-

  src/examples/test_csi_plugin.cpp aed22623b6dadb791eeaac2ecde4c31236a5fc19 
  src/tests/storage_local_resource_provider_tests.cpp 
2872f1aec1a7b94fc302a533f5ae9e1be9658087 


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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Review Request 66410: Adapted storage local resource provider to use CSI v0.2.

[Chun-Hung Hsiao]

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

Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description
---

This patch contains necessary changes for the storage local resource
provider to use CSI v0.2. Support for the `STAGE_UNSTAGE_VOLUME` CSI
node service capability is not implemented in this patch yet.


Diffs
-

  src/csi/state.proto 0373c8ad217998a54b06d483f94052bcf4293677 
  src/resource_provider/storage/provider.cpp 
a07620d1c4bf618f669575b3e183bf598da905a6 


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


Testing
---

This patch cannot be compiled standalone. Tests done later in the chain.


Thanks,

Chun-Hung Hsiao

Review Request 66409: Used `csi::v0::VolumeCapability` in disk profile adaptors.

[Chun-Hung Hsiao]

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

Review request for mesos, Benjamin Bannier, Jie Yu, and Joseph Wu.


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


Repository: mesos


Description
---

To adapt the change in CSI package names (from `csi` to `csi.v0`), we
now use `csi::v0::VolumeCapability` in the disk profile adaptor module
interface.

NOTE: This is not future-proof if there is a breaking change in
`VolumeCapability`, we might need to change the module interface to
support multiple CSI versions in the future.


Diffs
-

  include/mesos/resource_provider/storage/disk_profile_adaptor.hpp 
3a2d86a630fa949df2a5741f12f80fc434407cdc 
  src/resource_provider/storage/disk_profile.proto 
b9448d259b053a62fe2ffe5ae29c9f9be00e 
  src/resource_provider/storage/disk_profile_utils.hpp 
9a0a31b48274dfa4d74427aed2a72a4ff245ef7f 
  src/resource_provider/storage/disk_profile_utils.cpp 
d07c11fdcf7108e1d680f8e2df743426a65331ac 
  src/tests/disk_profile_adaptor_tests.cpp 
948f6efc916d6fad0703944ad9acb77d85e4c25e 


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


Testing
---

This patch cannot be compiled standalone. Tests done later in the chain.


Thanks,

Chun-Hung Hsiao

Review Request 66408: Updated CSI helpers for v0.2.

[Chun-Hung Hsiao]

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

Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description
---

This patch adds new helper classes for CSI plugin and node capabilities,
and removed helpers for the removed `csi.Version` proto message.


Diffs
-

  src/csi/utils.hpp 58b071d4f1512e4cf10d077bd1e03e66626f6dff 
  src/csi/utils.cpp 9e0435762a16f2995bc5a04ec342af6ea231054a 


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


Testing
---

This patch cannot be compiled standalone. Tests done later in the chain.


Thanks,

Chun-Hung Hsiao

Re: Review Request 66404: Removed check for nested cgroup support in `cgroups::prepare()`.

[Gilbert Song]

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


Ship it!




LGTM, before landing this patch, could we verify the nested cgroup is alwasy 
supportted after kernel 2.6.23?

- Gilbert Song


On April 3, 2018, 9:19 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66404/
> ---
> 
> (Updated April 3, 2018, 9:19 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-8489
> https://issues.apache.org/jira/browse/MESOS-8489
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, we were checking for nested cgroup support while trying to
> create a linux launcher. This was necessary to make sure that the
> kernel supports creating nested cgroups. As we don't support old
> kernel versions which don't support nested cgroups, this check can be
> safely removed.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp 682b288da8d56a623ef32cf6f0beecd9fc327622 
> 
> 
> Diff: https://reviews.apache.org/r/66404/diff/1/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Review Request 66407: Updated CSI client to support v0.2.

[Chun-Hung Hsiao]

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

Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description
---

To adapt the change in CSI package names (from `csi` to `csi.v0`), we
introduce a new `csi::v0` namespace in Mesos for v0 helpers. The CSI
client class is now defined in this namespace, and its public methods
are updated to reflect the changes in CSI v0.2.


Diffs
-

  include/csi/spec.hpp da0caaedab76a203a12db8518a15444958b11975 
  src/csi/client.hpp 1d55994423df3e539205d5353677b8258fd30abf 
  src/csi/client.cpp 5df788f9b2f2331428375097660f8e4bd371e70d 
  src/tests/csi_client_tests.cpp 566a85eba5632d182bb100f793e73321523c94b4 
  src/tests/mock_csi_plugin.hpp b19870dc46709517a581c0fa4f1ab3cdec27ac82 
  src/tests/mock_csi_plugin.cpp 1dc87617b4ab904d9c50aaa3b3a07dd101a4a952 


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


Testing
---

This patch cannot be compiled standalone.


Thanks,

Chun-Hung Hsiao

Re: Review Request 66178: Added a test to verify that task launch order is enforced.

[Meng Zhu]

> On April 3, 2018, 10:51 a.m., Greg Mann wrote:
> > src/tests/slave_tests.cpp
> > Lines 5114 (patched)
> > 
> >
> > Is this necessary?

To avoid uninteresing mock call.


> On April 3, 2018, 10:51 a.m., Greg Mann wrote:
> > src/tests/slave_tests.cpp
> > Lines 5181-5182 (patched)
> > 
> >
> > Should we assert here that task 2 has not launched yet?

Good point.


- Meng


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


On March 21, 2018, 2:58 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66178/
> ---
> 
> (Updated March 21, 2018, 2:58 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-8617 and MESOS-8624
> https://issues.apache.org/jira/browse/MESOS-8617
> https://issues.apache.org/jira/browse/MESOS-8624
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Agent should launch the task in their receiving order.
> On the task launch path, there are currently two
> asynchronous steps which may complete out of order:
> unschedule GC and task authorization.
> 
> This test simulates the reordering of the completions
> of task authorization step and verify that, despite the
> reordering, tasks can still launch in their original order.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp 46c271b5c5bedbdabd58b3cdbb82216d55c846bd 
>   src/tests/mesos.cpp d82963195573dd9ed7d12a7708f64a236b28cdf1 
>   src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353 
> 
> 
> Diff: https://reviews.apache.org/r/66178/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 66145: Added a test to verify that task launch order is enforced.

[Meng Zhu]

> On April 3, 2018, 10:40 a.m., Greg Mann wrote:
> > src/tests/slave_tests.cpp
> > Lines 4969 (patched)
> > 
> >
> > Is this necessary?

To avoid uninteresting mock call.


> On April 3, 2018, 10:40 a.m., Greg Mann wrote:
> > src/tests/slave_tests.cpp
> > Lines 5049-5051 (patched)
> > 
> >
> > Should we confirm here that the second task has not launched yet?
> > 
> > i.e.
> > ```
> > ASSERT_TRUE(taskStarting2.isPending());
> > ```
> > 
> > 
> > Also, enclose `taskGroup2` in backticks, here and below.

Good point.


- Meng


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


On March 21, 2018, 2:57 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66145/
> ---
> 
> (Updated March 21, 2018, 2:57 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-8617 and MESOS-8624
> https://issues.apache.org/jira/browse/MESOS-8617
> https://issues.apache.org/jira/browse/MESOS-8624
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Agent should launch the task in their receiving order.
> On the task launch path, there are currently two
> asynchronous steps which may complete out of order:
> unschedule GC and task authorization.
> 
> This test simulates the reordering of the completions
> of unschedule GC step and verify that, despite the
> reordering, tasks can still launch in their original order.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353 
> 
> 
> Diff: https://reviews.apache.org/r/66145/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 66404: Removed check for nested cgroup support in `cgroups::prepare()`.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [66404]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On April 3, 2018, 4:19 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66404/
> ---
> 
> (Updated April 3, 2018, 4:19 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-8489
> https://issues.apache.org/jira/browse/MESOS-8489
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, we were checking for nested cgroup support while trying to
> create a linux launcher. This was necessary to make sure that the
> kernel supports creating nested cgroups. As we don't support old
> kernel versions which don't support nested cgroups, this check can be
> safely removed.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp 682b288da8d56a623ef32cf6f0beecd9fc327622 
> 
> 
> Diff: https://reviews.apache.org/r/66404/diff/1/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 66398: Bumped the CSI spec bundle to 0.2.0.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the current review.

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

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

Relevant logs:

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

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

- Mesos Reviewbot Windows


On April 3, 2018, 6:45 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66398/
> ---
> 
> (Updated April 3, 2018, 6:45 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8650
> https://issues.apache.org/jira/browse/MESOS-8650
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The bundle is generated as follows:
> 
> 1. Download the tarball from the following link:
>https://github.com/container-storage-interface/spec/archive/
>v0.2.0.tar.gz
> 2. Run the following commands:
>tar zxf v0.2.0.tar.gz
>mv spec-0.2.0 csi-0.2.0
>tar zcf csi-0.2.0.tar.gz csi-0.2.0
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake 93f0322c1ac926bcfdcd4c1cfd9ba9f22bcf7099 
>   3rdparty/csi-0.1.0.tar.gz 37b1b4ca71aec3e036796b404baba887ac047957 
>   3rdparty/csi-0.2.0.tar.gz PRE-CREATION 
>   3rdparty/versions.am 1bc87bb1ec2e74ebb2072f63163e3c1e8b4aad00 
> 
> 
> Diff: https://reviews.apache.org/r/66398/diff/1/
> 
> 
> Testing
> ---
> 
> This patch cannot be compiled standalone.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Review Request 66398: Bumped the CSI spec bundle to 0.2.0.

[Chun-Hung Hsiao]

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

Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description
---

The bundle is generated as follows:

1. Download the tarball from the following link:
   https://github.com/container-storage-interface/spec/archive/
   v0.2.0.tar.gz
2. Run the following commands:
   tar zxf v0.2.0.tar.gz
   mv spec-0.2.0 csi-0.2.0
   tar zcf csi-0.2.0.tar.gz csi-0.2.0


Diffs
-

  3rdparty/cmake/Versions.cmake 93f0322c1ac926bcfdcd4c1cfd9ba9f22bcf7099 
  3rdparty/csi-0.1.0.tar.gz 37b1b4ca71aec3e036796b404baba887ac047957 
  3rdparty/csi-0.2.0.tar.gz PRE-CREATION 
  3rdparty/versions.am 1bc87bb1ec2e74ebb2072f63163e3c1e8b4aad00 


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


Testing
---

This patch cannot be compiled standalone.


Thanks,

Chun-Hung Hsiao

Re: Review Request 66178: Added a test to verify that task launch order is enforced.

[Greg Mann]

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




src/tests/slave_tests.cpp
Lines 5070 (patched)


s/runTask(Group)Message/RunTask(Group)Message/



src/tests/slave_tests.cpp
Lines 5073 (patched)


I would recommend the name `LaunchTasksReorderTaskAuthorization`.



src/tests/slave_tests.cpp
Lines 5081 (patched)


Ditto here - move this down a bit.



src/tests/slave_tests.cpp
Lines 5083-5088 (patched)


Ditto on consistent newlines here.



src/tests/slave_tests.cpp
Lines 5114 (patched)


Is this necessary?



src/tests/slave_tests.cpp
Lines 5176-5177 (patched)


Need a period at the end of this comment.



src/tests/slave_tests.cpp
Lines 5180 (patched)


Nit: enclose `taskGroup2` in backticks, here and elsewhere.



src/tests/slave_tests.cpp
Lines 5181-5182 (patched)


Should we assert here that task 2 has not launched yet?



src/tests/slave_tests.cpp
Lines 5186 (patched)


s/subseuqently/subsequently/


- Greg Mann


On March 21, 2018, 9:58 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66178/
> ---
> 
> (Updated March 21, 2018, 9:58 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-8617 and MESOS-8624
> https://issues.apache.org/jira/browse/MESOS-8617
> https://issues.apache.org/jira/browse/MESOS-8624
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Agent should launch the task in their receiving order.
> On the task launch path, there are currently two
> asynchronous steps which may complete out of order:
> unschedule GC and task authorization.
> 
> This test simulates the reordering of the completions
> of task authorization step and verify that, despite the
> reordering, tasks can still launch in their original order.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp 2c3d0c9350cd2223ff20c0797d1849d38c19 
>   src/tests/mesos.cpp d82963195573dd9ed7d12a7708f64a236b28cdf1 
>   src/tests/slave_tests.cpp f76500ebdb67f131a57a3b5aaae8c952d019e354 
> 
> 
> Diff: https://reviews.apache.org/r/66178/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

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

[David Forsythe]

> On March 28, 2018, 8:06 a.m., Benjamin Bannier wrote:
> > 3rdparty/CMakeLists.txt
> > Lines 58-63 (patched)
> > 
> >
> > Like discussed offline, I don't think there is a reason we need to bolt 
> > such logic on the used system, at least currently.
> > 
> > Let's instead use `find_program` to find a `make` and use it to set 
> > this variable.
> > 
> > Unless I miss something, we do not depend on GNU make here, so let's 
> > maybe reflect that in a more general name, e.g., just `MAKE` if it is 
> > available.
> 
> David Forsythe wrote:
> I took a look at find_program, but I don't think that will solve the 
> problem.
> 
> We actually *do* depend on gmake here (even on darwin). On FreeBSD GHU 
> make is a 3rd party application and is actually called and installed as 
> `gmake`, whereas the system make is actually Pmake (which is incompatible).
> 
> David Forsythe wrote:
> I should clarify that there is incompability in a 3rdparty Makefile, not 
> that the two makes are completely incompatible.
> 
> Particularly there are three issues - LevelDB, libev, and glog.
> 
> LevelDB is always going to fail without `gmake`, because it does not use 
> autotools. I don't know how we can get around that.
> 
> libev and glog do use autotools. They have some strange behavior when 
> building, though --
> If I use `cmake --build` they both fail on a bad target (because of an 
> incompatible Makefile).
> If I use (bsd) `make`, they build fine.
> If I use `gmake` they both fail (bad target).
> 
> For both the first and third scenario, I can run the commands that are 
> failing outside of the build and they succeed.
> 
> Accordng to the cache, cmake is using `gmake` as `CMAKE_MAKE_PROGRAM` in 
> these cases, so it's not clear to me why anything would fail when I do the 
> whole build with `gmake`. It seems like the build can go make -> gmake, but 
> not gmake -> make.
> 
> I think the baseline should be having `cmake --build` work, and the only 
> way I have gotten everything to work as expected is by forcing `gmake` 
> everywhere. I'm learning Cmake as I go so I could be missing something 
> obvious to fix this.
> 
> David Forsythe wrote:
> I've been thinking about this a bit more, and I think that the best 
> approach might actually be to just leave the libev and glog calls as make and 
> leave the conditional gmake call. On FreeBSD we could just tell users to use 
> `make` instead of cmake build. It seems like a way less intrusive change.
> 
> Thoughts?
> 
> Benjamin Bannier wrote:
> I think it should be perfectly fine to use `gmake` if it is available, it 
> is usually a symlink to `make` anyway.
> 
> The implementation I was thinking of was to add below instead,
> 
> find_program(
> GNU_MAKE
> NAMES gmake make
> DOC "path to GNU make executable")
> 
> if (NOT GNU_MAKE)
>   message(FATAL_ERROR "Could not find a suitable GNU make executable")
> endif()

This would always succeed on FreeBSD (or any system with a make), even if gmake 
wasn't installed. It is not a symlink:

```

root@mesos0:/mesos-build # gmake --version
GNU Make 4.2.1
...
root@mesos0:/mesos-build # make --version
usage: make [-BeikNnqrstWwX]
...

```

With autotools we didn't run into this because (iirc) so much was incompatible 
that I just used/assumed `gmake` for everything. With `cmake` we're close 
enough that I think it might be worth it to try and support bsd make.


- David


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


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

Re: Review Request 66145: Added a test to verify that task launch order is enforced.

[Greg Mann]

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




src/tests/slave_tests.cpp
Lines 4927 (patched)


s/runTask(Group)Message/RunTask(Group)Message/


- Greg Mann


On March 21, 2018, 9:57 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66145/
> ---
> 
> (Updated March 21, 2018, 9:57 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-8617 and MESOS-8624
> https://issues.apache.org/jira/browse/MESOS-8617
> https://issues.apache.org/jira/browse/MESOS-8624
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Agent should launch the task in their receiving order.
> On the task launch path, there are currently two
> asynchronous steps which may complete out of order:
> unschedule GC and task authorization.
> 
> This test simulates the reordering of the completions
> of unschedule GC step and verify that, despite the
> reordering, tasks can still launch in their original order.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 028cd32c7043eba4e6f2045956471bd0bf42371c 
> 
> 
> Diff: https://reviews.apache.org/r/66145/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 66145: Added a test to verify that task launch order is enforced.

[Greg Mann]

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




src/tests/slave_tests.cpp
Lines 4930 (patched)


I think the following name is a little easier to read:

s/TasksLaunchReorderUnscheduleGC/LaunchTasksReorderUnscheduleGC/



src/tests/slave_tests.cpp
Lines 4938 (patched)


Let's move this declaration/assignment down so it's directly above where it 
first gets used in the `EXPECT_CALL`.



src/tests/slave_tests.cpp
Lines 4940-4943 (patched)


Be consistent here for the creation of the agent dependencies: let's either 
have a newline between each line, or no newlines at all.



src/tests/slave_tests.cpp
Lines 4969 (patched)


Is this necessary?



src/tests/slave_tests.cpp
Lines 5045-5046 (patched)


Usually when only one word of a comment runs onto the next line, i'll split 
a bit earlier to improve readability. Also, need a period at the end of the 
comment:

```
  // Reorder the task group launches by resuming
  // the processing of `taskGroup2` first.
```



src/tests/slave_tests.cpp
Lines 5049-5051 (patched)


Should we confirm here that the second task has not launched yet?

i.e.
```
ASSERT_TRUE(taskStarting2.isPending());
```

Also, enclose `taskGroup2` in backticks, here and below.



src/tests/slave_tests.cpp
Lines 5055 (patched)


s/subseuqently/subsequently/


- Greg Mann


On March 21, 2018, 9:57 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66145/
> ---
> 
> (Updated March 21, 2018, 9:57 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-8617 and MESOS-8624
> https://issues.apache.org/jira/browse/MESOS-8617
> https://issues.apache.org/jira/browse/MESOS-8624
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Agent should launch the task in their receiving order.
> On the task launch path, there are currently two
> asynchronous steps which may complete out of order:
> unschedule GC and task authorization.
> 
> This test simulates the reordering of the completions
> of unschedule GC step and verify that, despite the
> reordering, tasks can still launch in their original order.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 028cd32c7043eba4e6f2045956471bd0bf42371c 
> 
> 
> Diff: https://reviews.apache.org/r/66145/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

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

[Meng Zhu]

> On April 2, 2018, 4:57 p.m., Greg Mann wrote:
> > src/slave/slave.cpp
> > Line 2233 (original), 2274 (patched)
> > 
> >
> > We should defer this callback.
> 
> Meng Zhu wrote:
> That would change the old behavior i.e. `sendExitedExecutorMessage` is 
> sent synchronously along with other error handling code. 
> https://github.com/apache/mesos/blob/594ee20c2453dad836313769aef9f8655cd75cd5/src/slave/slave.cpp#L2226-L2231
> 
> I found making the error handling asynchronous unnecessarily difficult to 
> reason. e.g. making it asynchronous means that there is a brief moment that 
> the first task has failed but the sequence is still alive--contradicting our 
> comments. Tieing the sequence lifecycle and `exitedExecutorMessage` to task 
> launch success atomically makes the code much easier to reason.
> 
> I am not sure if it would make a difference now, but we should stick to 
> the old behavior unless there is compelling reason not to.

OK, need to defer, as the error handler might be called in a different context 
other than the agent actor.


- Meng


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


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

Re: Review Request 63372: Added documentation for memory profiling.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the dependent review: 63366.

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

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

Relevant logs:

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

```
error: missing binary patch data for '3rdparty/jemalloc-5.0.1.tar.bz2'
error: binary patch does not apply to '3rdparty/jemalloc-5.0.1.tar.bz2'
error: 3rdparty/jemalloc-5.0.1.tar.bz2: patch does not apply
```

- Mesos Reviewbot Windows


On Jan. 24, 2018, 8:40 a.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63372/
> ---
> 
> (Updated Jan. 24, 2018, 8:40 a.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added documentation for memory profiling.
> 
> 
> Diffs
> -
> 
>   docs/memory-profiling.md PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63372/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 66143: Refactored agent task launch for better composition [2/2].

[Meng Zhu]

> On April 2, 2018, 4:21 p.m., Greg Mann wrote:
> > src/slave/slave.cpp
> > Lines 2396-2398 (patched)
> > 
> >
> > Looks like we don't need this local variable?

You mean passing the whole error message directly to the function? I feel that 
hurts readability.


- Meng


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


On April 3, 2018, 10:38 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66143/
> ---
> 
> (Updated April 3, 2018, 10:38 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-8617 and MESOS-8624
> https://issues.apache.org/jira/browse/MESOS-8617
> https://issues.apache.org/jira/browse/MESOS-8624
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This helps to encapsulate a task launch into a single
> future which will come in handy when enforcing the task
> launch order.
> 
> Affected tests are also updated.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb 
>   src/slave/slave.cpp a4bd4ccd3fc59c3c0e462d9b480f5424b3e52d7a 
>   src/tests/mock_slave.hpp 8ec2b65a75f774d970ef6507df8b0c02fda5f2fd 
>   src/tests/mock_slave.cpp 8ec55b6aa84dd8b1c3743cbcdc246311585d0f63 
>   src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353 
> 
> 
> Diff: https://reviews.apache.org/r/66143/diff/8/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 66404: Removed check for nested cgroup support in `cgroups::prepare()`.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 66404 was successfully built and tested.

Reviews applied: `['66404']`

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

- Mesos Reviewbot Windows


On April 3, 2018, 4:19 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66404/
> ---
> 
> (Updated April 3, 2018, 4:19 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-8489
> https://issues.apache.org/jira/browse/MESOS-8489
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, we were checking for nested cgroup support while trying to
> create a linux launcher. This was necessary to make sure that the
> kernel supports creating nested cgroups. As we don't support old
> kernel versions which don't support nested cgroups, this check can be
> safely removed.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.cpp 682b288da8d56a623ef32cf6f0beecd9fc327622 
> 
> 
> Diff: https://reviews.apache.org/r/66404/diff/1/
> 
> 
> Testing
> ---
> 
> internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 66143: Refactored agent task launch for better composition [2/2].

[Meng Zhu]

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

(Updated April 3, 2018, 10:38 a.m.)


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


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


Repository: mesos


Description
---

This helps to encapsulate a task launch into a single
future which will come in handy when enforcing the task
launch order.

Affected tests are also updated.


Diffs (updated)
-

  src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb 
  src/slave/slave.cpp a4bd4ccd3fc59c3c0e462d9b480f5424b3e52d7a 
  src/tests/mock_slave.hpp 8ec2b65a75f774d970ef6507df8b0c02fda5f2fd 
  src/tests/mock_slave.cpp 8ec55b6aa84dd8b1c3743cbcdc246311585d0f63 
  src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353 


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

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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 66126: Refactored agent task launch for better composition [1/2].

[Meng Zhu]

> On April 2, 2018, 3:38 p.m., Greg Mann wrote:
> > src/slave/slave.cpp
> > Line 2166 (original), 2221-2227 (patched)
> > 
> >
> > Ah whoops, we should defer this continuation to `self()`. I think it's 
> > probably safe at the moment, but best to always defer/dispatch except for 
> > trivial continuations which do not access actor state. Even if the current 
> > implementation is safe, it makes the code less fragile if we defer.
> 
> Meng Zhu wrote:
> See my reply in https://reviews.apache.org/r/66144/

OK, need to defer, as the error handler might be called in a different context 
other than the agent actor.


- Meng


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


On April 2, 2018, 5:33 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66126/
> ---
> 
> (Updated April 2, 2018, 5:33 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-8617 and MESOS-8624
> https://issues.apache.org/jira/browse/MESOS-8617
> https://issues.apache.org/jira/browse/MESOS-8624
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This helps to encapsulate a task launch into a single
> future which will come in handy when enforcing the task
> launch order.
> 
> This patch also consolidated the error handling code
> in the task launch path.
> 
> Affected tests are also updated.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb 
>   src/slave/slave.cpp a4bd4ccd3fc59c3c0e462d9b480f5424b3e52d7a 
>   src/tests/mock_slave.hpp 8ec2b65a75f774d970ef6507df8b0c02fda5f2fd 
>   src/tests/mock_slave.cpp 8ec55b6aa84dd8b1c3743cbcdc246311585d0f63 
>   src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353 
> 
> 
> Diff: https://reviews.apache.org/r/66126/diff/6/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 66392: Find sasl2 on non-Windows platforms before trying to link it.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [66384, 66385, 66387, 66392]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 
MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On April 3, 2018, 2:11 p.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66392/
> ---
> 
> (Updated April 3, 2018, 2:11 p.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-4176
> https://issues.apache.org/jira/browse/MESOS-4176
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Find sasl2 on non-Windows platforms before trying to link it.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 2b63b58f7d6a88c9986b746283dcfa79b7bcb270 
> 
> 
> Diff: https://reviews.apache.org/r/66392/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Forsythe
> 
>

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

[Andrew Schwartzmeyer]

> On April 2, 2018, 11:42 a.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/CMakeLists.txt
> > Lines 359-360 (original), 365-366 (patched)
> > 
> >
> > If I understood the above discussion correctly, I think the following 
> > would work well:
> > 
> > Use `${CMAKE_MAKE_PROGRAM}` and `${CMAKE_MAKE_PROGRAM} install` for the 
> > BUILD/INSTALL commands, instead of adding `${GNU_MAKE}`.
> > 
> > Then in the MesosConfigure.cmake, we can have a platform check `if 
> > (FREEBSD) ... make sure CMAKE_MAKE_PROGRAM is gmake` or something to that 
> > effect.
> 
> David Forsythe wrote:
> If we do that, we would be forcing gmake for the entire build, right? 
> It's not clear to me that is the best approach, since the only problem is 
> leveldb (and since newer versions of leveldb seem to use cmake?). I might 
> even be more incline to try patching LevelDBs Makefile rather than force that.
> 
> Benjamin Bannier wrote:
> @andschwa: `CMAKE_MAKE_PROGRAM` would point to whatever generator the 
> user of the cmake build selected which does not necessarily implement `make` 
> functionality (e.g., `ninja`, msvc, etc).

Oooh okay never mind me then. Its name is very misleading... I should I have 
checked docs before I spoke.


- Andrew


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


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

Re: Review Request 66384: Link libm in ZooKeeper build on FreeBSD.

[Andrew Schwartzmeyer]

> On April 2, 2018, 10:06 a.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/zookeeper-3.4.8.patch
> > Line 154 (original), 154 (patched)
> > 
> >
> > Also, if this change should go upstream to ZooKeeper (looks to me like 
> > it should) then let's make sure it does. Their process is [pretty 
> > similar](https://cwiki.apache.org/confluence/display/ZOOKEEPER/HowToContribute).
> > 
> > If you don't want to, I can upstream it.
> 
> David Forsythe wrote:
> I can add it to my todo list.

FWIW I can review/shepherd over on ZooKeeper too, though I can't commit 
directly (yet). Michael is pretty good about it though.


- Andrew


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


On March 31, 2018, 7:32 p.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66384/
> ---
> 
> (Updated March 31, 2018, 7:32 p.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-4176
> https://issues.apache.org/jira/browse/MESOS-4176
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Link libm in ZooKeeper build on FreeBSD.
> 
> 
> Diffs
> -
> 
>   3rdparty/zookeeper-3.4.8.patch 2eaa056dd5668d5842b5b59a42f83ae307d4aef0 
> 
> 
> Diff: https://reviews.apache.org/r/66384/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Forsythe
> 
>

Re: Review Request 66385: Link execinfo in Glog build on FreeBSD.

[Andrew Schwartzmeyer]

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


Ship it!




Ship It!

- Andrew Schwartzmeyer


On April 2, 2018, 9:58 p.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66385/
> ---
> 
> (Updated April 2, 2018, 9:58 p.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-4176 and MESOS-6849
> https://issues.apache.org/jira/browse/MESOS-4176
> https://issues.apache.org/jira/browse/MESOS-6849
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Link execinfo in Glog build on FreeBSD.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 2b63b58f7d6a88c9986b746283dcfa79b7bcb270 
>   cmake/CompilationConfigure.cmake 64cc56ee4208afe05df0f28af5890157e4c7d82c 
> 
> 
> Diff: https://reviews.apache.org/r/66385/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Forsythe
> 
>

Re: Review Request 66392: Find sasl2 on non-Windows platforms before trying to link it.

[Andrew Schwartzmeyer]

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




3rdparty/CMakeLists.txt
Lines 267 (patched)


This looks good; except I think in generaly it's going to find a shared 
version of `sasl2` not a static. I don't think it really does too much to 
imported targets, as they're not being built, but for accuracy's sake, 
`add_library(sasl2 SHARED IMPORTED GLOBAL)` (unless we really don't know, so 
`UNKNOWN`).


- Andrew Schwartzmeyer


On April 3, 2018, 7:11 a.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66392/
> ---
> 
> (Updated April 3, 2018, 7:11 a.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-4176
> https://issues.apache.org/jira/browse/MESOS-4176
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Find sasl2 on non-Windows platforms before trying to link it.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 2b63b58f7d6a88c9986b746283dcfa79b7bcb270 
> 
> 
> Diff: https://reviews.apache.org/r/66392/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Forsythe
> 
>

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

[Benno Evers]

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

(Updated April 3, 2018, 4:29 p.m.)


Review request for mesos, Alexander Rukletsov and Benjamin Mahler.


Changes
---

Rebased onto latest master; worked around clang bug with this-capturing lambdas.


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 (updated)
-

  3rdparty/libprocess/Makefile.am c895d3ac7b9cc5ffd6c8b57ff24def866eb0213d 
  3rdparty/libprocess/include/process/memory_profiler.hpp PRE-CREATION 
  3rdparty/libprocess/include/process/process.hpp 
8661706cb058efb26f5bfbcc84972f9930d3670f 
  3rdparty/libprocess/src/CMakeLists.txt 
0ce7dac5deea94623530820d173ce3ffe5b42ea4 
  3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
  3rdparty/libprocess/src/process.cpp 9eb37465cd86f408d69f5f98fb76c4f4b93b9acd 


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

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


Testing
---


Thanks,

Benno Evers

Re: Review Request 63366: Add jemalloc release tarball and build rules.

[Benno Evers]

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

(Updated April 3, 2018, 4:28 p.m.)


Review request for mesos, Alexander Rukletsov and Benjamin Mahler.


Changes
---

Rebased onto latest master.


Summary (updated)
-

Add jemalloc release tarball and build rules.


Repository: mesos


Description (updated)
---

Add jemalloc release tarball and build rules.


Diffs (updated)
-

  3rdparty/CMakeLists.txt 2b63b58f7d6a88c9986b746283dcfa79b7bcb270 
  3rdparty/Makefile.am 10b29c9e2d10073a817273c7038226d0ed4fd6ad 
  3rdparty/cmake/Versions.cmake 93f0322c1ac926bcfdcd4c1cfd9ba9f22bcf7099 
  3rdparty/jemalloc-5.0.1.tar.bz2 PRE-CREATION 
  3rdparty/versions.am 1bc87bb1ec2e74ebb2072f63163e3c1e8b4aad00 
  cmake/CompilationConfigure.cmake 64cc56ee4208afe05df0f28af5890157e4c7d82c 
  configure.ac f0f901f2e565352c2804cae7b2ac255da81ce45d 
  src/Makefile.am 9f4b6d369a23af337e2384e52e3e41f4017df38a 
  src/master/CMakeLists.txt b8953bd576586ac8548a1b51cf0a87bc8e9da4fc 
  src/slave/CMakeLists.txt 943e8f523f867e3dd4030f78eca7830500578faf 


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

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


Testing
---


Thanks,

Benno Evers

Re: Review Request 63367: Added overloads for strings::startsWith and strings::endsWith().

[Benno Evers]

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

(Updated April 3, 2018, 4:27 p.m.)


Review request for mesos, Alexander Rukletsov and Benjamin Mahler.


Changes
---

Rebased onto latest master.


Repository: mesos


Description
---

This saves an unnecessary memory allocation when
testing if a string starts or ends with a string literal,
which accounts for almost all usages of these functions
in Mesos and in libprocess.


Diffs (updated)
-

  3rdparty/stout/include/stout/strings.hpp 
067a7923c02342bccd9be1136a981fd6b0e0e9b4 
  3rdparty/stout/tests/strings_tests.cpp 
395540aad88c76a66a43a54edfe9ca1a2d46d3b4 


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

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


Testing
---


Thanks,

Benno Evers

Review Request 66404: Removed check for nested cgroup support in `cgroups::prepare()`.

[Andrei Budnik]

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

Review request for mesos, Alexander Rukletsov, Gilbert Song, and Jie Yu.


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


Repository: mesos


Description
---

Previously, we were checking for nested cgroup support while trying to
create a linux launcher. This was necessary to make sure that the
kernel supports creating nested cgroups. As we don't support old
kernel versions which don't support nested cgroups, this check can be
safely removed.


Diffs
-

  src/linux/cgroups.cpp 682b288da8d56a623ef32cf6f0beecd9fc327622 


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


Testing
---

internal CI


Thanks,

Andrei Budnik

Re: Review Request 66396: Bugfix for MESOS-8308.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['66396']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
[ RUN  ] RmdirTest.SYMLINK_RemoveDirectoryWithSymbolicLinkTargetDirectory
[   OK ] RmdirTest.SYMLINK_RemoveDirectoryWithSymbolicLinkTargetDirectory 
(5 ms)
[ RUN  ] RmdirTest.SYMLINK_RemoveDirectoryWithSymbolicLinkTargetFile
[   OK ] RmdirTest.SYMLINK_RemoveDirectoryWithSymbolicLinkTargetFile (4 ms)
[ RUN  ] RmdirTest.RemoveDirectoryButPreserveRoot
[   OK ] RmdirTest.RemoveDirectoryButPreserveRoot (6 ms)
[--] 11 tests from RmdirTest (52 ms total)

[--] 1 test from SocketTests
[ RUN  ] SocketTests.InitSocket
[   OK ] SocketTests.InitSocket (1 ms)
[--] 1 test from SocketTests (2 ms total)

[--] 2 tests from StrerrorTest
[ RUN  ] StrerrorTest.ValidErrno
[   OK ] StrerrorTest.ValidErrno (0 ms)
[ RUN  ] StrerrorTest.InvalidErrno
[   OK ] StrerrorTest.InvalidErrno (0 ms)
[--] 2 tests from StrerrorTest (1 ms total)

[--] Global test environment tear-down
[==] 275 tests from 46 test cases ran. (3993 ms total)
[  PASSED  ] 274 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] OsTest.System

 1 FAILED TEST
  YOU HAVE 7 DISABLED TESTS

```

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

```
ERROR: Input redirection is not supported, exiting the process immediately.
'invalid.command' is not recognized as an internal or external command,
operable program or batch file.
Subcommand 'subcommand' is not available
Usage: command  [OPTIONS]

Available subcommands:
help
subcommand2

Multiple subcommands have name 'subcommand'
```

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

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

[--] Global test environment tear-down
[==] 949 tests from 94 test cases ran. (501995 ms total)
[  PASSED  ] 932 tests.
[  FAILED  ] 17 tests, listed below:
[  FAILED  ] CommandExecutorCheckTest.CommandCheckStatusChange
[  FAILED  ] CommandExecutorCheckTest.CommandCheckTimeout
[  FAILED  ] CommandExecutorCheckTest.CommandCheckAndHealthCheckNoShadowing
[  FAILED  ] DefaultExecutorCheckTest.CommandCheckAndHealthCheckNoShadowing
[  FAILED  ] HealthCheckTest.HealthStatusChange
[  FAILED  ] HealthCheckTest.ConsecutiveFailures
[  FAILED  ] HealthCheckTest.GracePeriod
[  FAILED  ] HealthCheckTest.HealthyToUnhealthyTransitionWithinGracePeriod
[  FAILED  ] PartitionTest.AgentWithTerminalTaskPartitioned
[  FAILED  ] SlaveRecoveryTest/0.ReconcileKillTask, where TypeParam = class 
mesos::internal::slave::MesosContainerizer
[  FAILED  ] SlaveRecoveryTest/0.MultipleFrameworks, where TypeParam = class 
mesos::internal::slave::MesosContainerizer
[  FAILED  ] SlaveRecoveryTest/0.MultipleSlaves, where TypeParam = class 
mesos::internal::slave::MesosContainerizer
[  FAILED  ] SlaveRecoveryTest/0.AgentReconfigurationWithRunningTask, where 
TypeParam = class mesos::internal::slave::MesosContainerizer
[  FAILED  ] MesosContainerizer/DefaultExecutorTest.KillTask/0, where 
GetParam() = "mesos"
[  FAILED  ] MesosContainerizer/DefaultExecutorTest.CommitSuicideOnKillTask/0, 
where GetParam() = "mesos"
[  FAILED  ] 
ROOT_DOCKER_DockerAndMesosContainerizers/DefaultExecutorTest.KillTask/0, where 
GetParam() = "docker,mesos"
[  FAILED  ] 
ROOT_DOCKER_DockerAndMesosContainerizers/DefaultExecutorTest.CommitSuicideOnKillTask/0,
 where GetParam() = "docker,mesos"

17 FAILED TESTS
  YOU HAVE 214 DISABLED TESTS

```

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

```
I0403 15:38:15.936280  7328 master.cpp:10449] Updating the state of task 
487fcdd6-ec0e-4dc8-9370-31dd048db53a of framework 
030d21ba-086c-455c-89cf-5783535329b6- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0403 15:38:15.936280 10708 slave.cpp:3877] Shutting down framework 
030d21ba-086c-455c-89cf-5783535329b6-
I0403 15:38:15.937281 10708 slave.cpp:6574] Shutting down executor 
'487fcdd6-ec0e-4dc8-9370-31dd048db53a' of framework 
030d21ba-086c-455c-89cf-5783535329b6- at executor(1)@10.3.1.5:52116
I0403 

Re: Review Request 66392: Find sasl2 on non-Windows platforms before trying to link it.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['66384', '66385', '66387', '66392']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

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

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

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

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

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

 1 FAILED TEST
  YOU HAVE 214 DISABLED TESTS

```

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

```
I0403 15:13:36.149655 12064 master.cpp:10449] Updating the state of task 
ce033352-3304-4c10-91be-ec51323bc780 of framework 
c90b45e2-87e6-4830-9850-929c00660c52- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0403 15:13:36.149655 11792 slave.cpp:3877] Shutting down framework 
c90b45e2-87e6-4830-9850-929c00660c52-0I0403 15:13:35.974665 10404 exec.cpp:162] 
Version: 1.6.0
I0403 15:13:36.001674  2792 exec.cpp:236] Executor registered on agent 
c90b45e2-87e6-4830-9850-929c00660c52-S0
I0403 15:13:36.005637 10704 executor.cpp:176] Received SUBSCRIBED event
I0403 15:13:36.009672 10704 executor.cpp:180] Subscribed executor on 
winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I0403 15:13:36.010671 10704 executor.cpp:176] Received LAUNCH event
I0403 15:13:36.014672 10704 executor.cpp:648] Starting task 
ce033352-3304-4c10-91be-ec51323bc780
I0403 15:13:36.092672 10704 executor.cpp:483] Running 
'D:\DCOS\mesos\src\mesos-containerizer.exe launch '
I0403 15:13:36.122673 10704 executor.cpp:661] Forked command at 13268
I0403 15:13:36.152819 12036 exec.cpp:445] Executor asked to shutdown
I0403 15:13:36.152819 10912 executor.cpp:176] Received SHUTDOWN event
I0403 15:13:36.153641 10912 executor.cpp:758] Shutting down
I0403 15:13:36.153641 10912 executor.cpp:868] Sending SIGTERM to process tree 
at pid 000
I0403 15:13:36.150672 11792 slave.cpp:6574] Shutting down executor 
'ce033352-3304-4c10-91be-ec51323bc780' of framework 
c90b45e2-87e6-4830-9850-929c00660c52- at executor(1)@10.3.1.8:58407
I0403 15:13:36.151674 11792 slave.cpp:923] Agent terminating
W0403 15:13:36.151674 11792 slave.cpp:3873] Ignoring shutdown framework 
c90b45e2-87e6-4830-9850-929c00660c52- because it is terminating
I0403 15:13:36.152819 12064 master.cpp:10548] Removing task 
ce033352-3304-4c10-91be-ec51323bc780 with resources cpus(allocated: *):4; 
mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
*):[31000-32000] of framework c90b45e2-87e6-4830-9850-929c00660c52- on 
agent c90b45e2-87e6-4830-9850-929c00660c52-S0 at slave(418)@10.3.1.8:58386 
(winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0403 15:13:36.155654 12716 containerizer.cpp:2338] Destroying container 
390359d2-214f-4827-b98f-1945b7c32b5a in RUNNING state
I0403 15:13:36.155654 12716 containerizer.cpp:2952] Transitioning the state of 
container 390359d2-214f-4827-b98f-1945b7c32b5a from RUNNING to DESTROYING
I0403 15:13:36.156657 12064 master.cpp:1295] Agent 
c90b45e2-87e6-4830-9850-929c00660c52-S0 at slave(418)@10.3.1.8:58386 
(winbldsrv-01.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) disconnected
I0403 15:13:36.156657  7872 hierarchical.cpp:344] Removed framework 
c90b45e2-87e6-4830-9850-929c00660c52-
I0403 15:13:36.156657 12064 

Re: Review Request 66392: Find sasl2 on non-Windows platforms before trying to link it.

[David Forsythe]

> On April 2, 2018, 6:44 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/CMakeLists.txt
> > Line 1051 (original), 1051 (patched)
> > 
> >
> > Don't think you mean to have this here.

Ah sorry, will remove.


> On April 2, 2018, 6:44 p.m., Andrew Schwartzmeyer wrote:
> > src/CMakeLists.txt
> > Lines 513-514 (original), 512-516 (patched)
> > 
> >
> > Instead of this, we should have a `NOT WIN32` section in 
> > 3rdparty/CMakeLists.txt that does `find_library(sasl2)` and then creates an 
> > imported `sasl2` target, that way `sasl2` will be a target on both Windows 
> > and non-Windows platforms, and this code can go unchanged.

Fixed. Let me know if there is a better way to do it.


- David


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


On April 3, 2018, 2:11 p.m., David Forsythe wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66392/
> ---
> 
> (Updated April 3, 2018, 2:11 p.m.)
> 
> 
> Review request for mesos and Andrew Schwartzmeyer.
> 
> 
> Bugs: MESOS-4176
> https://issues.apache.org/jira/browse/MESOS-4176
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Find sasl2 on non-Windows platforms before trying to link it.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt 2b63b58f7d6a88c9986b746283dcfa79b7bcb270 
> 
> 
> Diff: https://reviews.apache.org/r/66392/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> David Forsythe
> 
>

Re: Review Request 66392: Find sasl2 on non-Windows platforms before trying to link it.

[David Forsythe]

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

(Updated April 3, 2018, 2:11 p.m.)


Review request for mesos and Andrew Schwartzmeyer.


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


Repository: mesos


Description
---

Find sasl2 on non-Windows platforms before trying to link it.


Diffs (updated)
-

  3rdparty/CMakeLists.txt 2b63b58f7d6a88c9986b746283dcfa79b7bcb270 


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

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


Testing
---


Thanks,

David Forsythe

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

[Benjamin Bannier]

> On April 2, 2018, 8:42 p.m., Andrew Schwartzmeyer wrote:
> > 3rdparty/CMakeLists.txt
> > Lines 359-360 (original), 365-366 (patched)
> > 
> >
> > If I understood the above discussion correctly, I think the following 
> > would work well:
> > 
> > Use `${CMAKE_MAKE_PROGRAM}` and `${CMAKE_MAKE_PROGRAM} install` for the 
> > BUILD/INSTALL commands, instead of adding `${GNU_MAKE}`.
> > 
> > Then in the MesosConfigure.cmake, we can have a platform check `if 
> > (FREEBSD) ... make sure CMAKE_MAKE_PROGRAM is gmake` or something to that 
> > effect.
> 
> David Forsythe wrote:
> If we do that, we would be forcing gmake for the entire build, right? 
> It's not clear to me that is the best approach, since the only problem is 
> leveldb (and since newer versions of leveldb seem to use cmake?). I might 
> even be more incline to try patching LevelDBs Makefile rather than force that.

@andschwa: `CMAKE_MAKE_PROGRAM` would point to whatever generator the user of 
the cmake build selected which does not necessarily implement `make` 
functionality (e.g., `ninja`, msvc, etc).


- Benjamin


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


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

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

[Benjamin Bannier]

> On March 28, 2018, 10:06 a.m., Benjamin Bannier wrote:
> > 3rdparty/CMakeLists.txt
> > Lines 58-63 (patched)
> > 
> >
> > Like discussed offline, I don't think there is a reason we need to bolt 
> > such logic on the used system, at least currently.
> > 
> > Let's instead use `find_program` to find a `make` and use it to set 
> > this variable.
> > 
> > Unless I miss something, we do not depend on GNU make here, so let's 
> > maybe reflect that in a more general name, e.g., just `MAKE` if it is 
> > available.
> 
> David Forsythe wrote:
> I took a look at find_program, but I don't think that will solve the 
> problem.
> 
> We actually *do* depend on gmake here (even on darwin). On FreeBSD GHU 
> make is a 3rd party application and is actually called and installed as 
> `gmake`, whereas the system make is actually Pmake (which is incompatible).
> 
> David Forsythe wrote:
> I should clarify that there is incompability in a 3rdparty Makefile, not 
> that the two makes are completely incompatible.
> 
> Particularly there are three issues - LevelDB, libev, and glog.
> 
> LevelDB is always going to fail without `gmake`, because it does not use 
> autotools. I don't know how we can get around that.
> 
> libev and glog do use autotools. They have some strange behavior when 
> building, though --
> If I use `cmake --build` they both fail on a bad target (because of an 
> incompatible Makefile).
> If I use (bsd) `make`, they build fine.
> If I use `gmake` they both fail (bad target).
> 
> For both the first and third scenario, I can run the commands that are 
> failing outside of the build and they succeed.
> 
> Accordng to the cache, cmake is using `gmake` as `CMAKE_MAKE_PROGRAM` in 
> these cases, so it's not clear to me why anything would fail when I do the 
> whole build with `gmake`. It seems like the build can go make -> gmake, but 
> not gmake -> make.
> 
> I think the baseline should be having `cmake --build` work, and the only 
> way I have gotten everything to work as expected is by forcing `gmake` 
> everywhere. I'm learning Cmake as I go so I could be missing something 
> obvious to fix this.
> 
> David Forsythe wrote:
> I've been thinking about this a bit more, and I think that the best 
> approach might actually be to just leave the libev and glog calls as make and 
> leave the conditional gmake call. On FreeBSD we could just tell users to use 
> `make` instead of cmake build. It seems like a way less intrusive change.
> 
> Thoughts?

I think it should be perfectly fine to use `gmake` if it is available, it is 
usually a symlink to `make` anyway.

The implementation I was thinking of was to add below instead,

find_program(
GNU_MAKE
NAMES gmake make
DOC "path to GNU make executable")

if (NOT GNU_MAKE)
  message(FATAL_ERROR "Could not find a suitable GNU make executable")
endif()


- Benjamin


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


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

Re: Review Request 66165: Re-fixed many master allocator tests.

[Benjamin Bannier]

> On March 21, 2018, 11:52 a.m., Benjamin Bannier wrote:
> > src/tests/master_allocator_tests.cpp
> > Line 759 (original), 748 (patched)
> > 
> >
> > This test seems to get flaky for me with this patch, could you please 
> > confirm it works under load (e.g., using `stress` or some actual workload)? 
> > I haven't verified all touched tests, please do.
> > 
> > [ RUN  ] MasterAllocatorTest/0.SlaveLost
> > ../src/tests/master_allocator_tests.cpp:838: Failure
> > Mock function called more times than expected - taking default 
> > action specified at:
> > ../src/tests/allocator.hpp:273:
> > Function call: addSlave(@0x7f2414006ab8 
> > 6d430237-e4d5-4852-8459-2020f598449f-S2, @0x7f2414006ad8 hostname: 
> > "gru1.hw.ca1.mesosphere.com"
> > resources {
> >   name: "cpus"
> >   type: SCALAR
> >   scalar {
> > value: 3
> >   }
> > }
> > resources {
> >   name: "mem"
> >   type: SCALAR
> >   scalar {
> > value: 256
> >   }
> > }
> > resources {
> >   name: "disk"
> >   type: SCALAR
> >   scalar {
> > value: 1024
> >   }
> > }
> > resources {
> >   name: "ports"
> >   type: RANGES
> >   ranges {
> > range {
> >   begin: 31000
> >   end: 32000
> > }
> >   }
> > }
> > id {
> >   value: "6d430237-e4d5-4852-8459-2020f598449f-S2"
> > }
> > checkpoint: true
> > port: 39521
> > , @0x7f2423e76c28 { 32-byte object <78-A9 BC-2B 24-7F 00-00 00-00 
> > 00-00 00-00 00-00 01-00 00-00 00-00 00-00 01-00 00-00 24-7F 00-00>, 32-byte 
> > object <78-A9 BC-2B 24-7F 00-00 00-00 00-00 00-00 00-00 01-00 00-00 00-00 
> > 00-00 02-00 00-00 24-7F 00-00>, 32-byte object <78-A9 BC-2B 24-7F 00-00 
> > 00-00 00-00 00-00 00-00 01-00 00-00 00-00 00-00 03-00 00-00 00-00 00-00> }, 
> > @0x7f2423e76f20 48-byte object <01-00 00-00 24-7F 00-00 00-00 00-00 00-00 
> > 00-00 BF-83 8E-4D FE-7F 00-00 C0-89 E7-23 24-7F 00-00 00-87 E7-23 24-7F 
> > 00-00 8C-52 15-29 24-7F 00-00>, @0x7f2414006e98 { cpus:3, mem
> > :256, disk:1024, ports:[31000-32000] }, @0x7f2414006e30 {})
> >  Expected: to be called once
> >Actual: called twice - over-saturated and active
> > *** Aborted at 1521624413 (unix time) try "date -d @1521624413" if 
> > you are using GNU date ***
> > PC: @  0x2cb968b testing::UnitTest::AddTestPartResult()
> > *** SIGSEGV (@0x0) received by PID 14803 (TID 0x7f2423e78700) from 
> > PID 0; stack trace: ***
> > @ 0x7f242cba25d0 (unknown)
> > @  0x2cb968b testing::UnitTest::AddTestPartResult()
> > @  0x2cb9219 
> > testing::internal::AssertHelper::operator=()
> > @  0x2cfc809 
> > testing::internal::GoogleTestFailureReporter::ReportFailure()
> > @   0xe36438 testing::internal::Expect()
> > @  0x2cf6ef4 
> > testing::internal::UntypedFunctionMockerBase::UntypedInvokeWith()
> > @  0x135367a 
> > _ZN7testing8internal18FunctionMockerBaseIFvRKN5mesos7SlaveIDERKNS2_9SlaveInfoERKSt6vectorINS2_20SlaveInfo_CapabilityESaISA_EERK6OptionINS2_14UnavailabilityEERKNS2_9ResourcesERK7hashmapINS2_11FrameworkIDESK_St4hashISO_ESt8equal_toISO_10InvokeWithERKSt5tupleIJS5_S8_SE_SJ_SM_SV_EE
> > @  0x135362b 
> > testing::internal::FunctionMocker<>::Invoke()
> > @  0x12ebc75 
> > mesos::internal::tests::TestAllocator<>::addSlave()
> > @ 0x7f2433f04cad mesos::internal::master::Master::addSlave()
> > @ 0x7f2433f030e6 
> > mesos::internal::master::Master::__registerSlave()
> > @ 0x7f243402d3b3 
> > _ZZN7process8dispatchIN5mesos8internal6master6MasterERKNS_4UPIDEONS2_20RegisterSlaveMessageERKNS_6FutureIbEES7_S8_SD_EEvRKNS_3PIDIT_EEMSF_FvT0_T1_T2_EOT3_OT4_OT5_ENKUlOS5_S9_OSB_PNS_11ProcessBaseEE_clESU_S9_SV_SX_
> > @ 0x7f243402cfa1 
> > _ZN5cpp176invokeIZN7process8dispatchIN5mesos8internal6master6MasterERKNS1_4UPIDEONS4_20RegisterSlaveMessageERKNS1_6FutureIbEES9_SA_SF_EEvRKNS1_3PIDIT_EEMSH_FvT0_T1_T2_EOT3_OT4_OT5_EUlOS7_SB_OSD_PNS1_11ProcessBaseEE_JS7_SA_SD_SZ_EEEDTclclsr3stdE7forwardISH_Efp_Espclsr3stdE7forwardIT0_Efp0_EEEOSH_DpOS11_
> > @ 0x7f243402cf0d 
> >