Re: Review Request 63642: Added a test for ExecutorID validation in ReregisterSlaveMessage.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [63642]

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 Nov. 7, 2017, 6:47 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63642/
> ---
> 
> (Updated Nov. 7, 2017, 6:47 p.m.)
> 
> 
> Review request for mesos, James DeFelice and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-8169
> https://issues.apache.org/jira/browse/MESOS-8169
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test to ensure that the ReregisterSlaveMessage validation
> correctly allows duplicate ExecutorIDs as long as they are scoped
> to different frameworks.
> 
> 
> Diffs
> -
> 
>   src/tests/master_validation_tests.cpp 
> be8c8920fdd6eb72fc77247844a731070db5fa1c 
> 
> 
> Diff: https://reviews.apache.org/r/63642/diff/4/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26) with PR #248 applied.
> 
> 
> Thanks,
> 
> James Peach
> 
>

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

[Meng Zhu]

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

Review request for mesos and Gilbert Song.


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


Repository: mesos


Description
---

In unified containerizer, the d_type cannot be 1
if we are using the overlay fs backend.
In particular, XFS can be mounted with ftype = 0
which renders d_type == 0. Raise error if user
specifies overlayfs as backend. Fallback to other
backends in the default case and raise a warning.


Diffs
-

  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
450a3b32d69d2882973a6ed4e94e169a0256056b 
  src/slave/containerizer/mesos/provisioner/utils.hpp 
5b6c162fe4ade16131b2207d707e76228b0ec51a 
  src/slave/containerizer/mesos/provisioner/utils.cpp 
7fd7315dda99f49f967a665afe27c8db7835c04c 


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


Testing
---

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


Thanks,

Meng Zhu

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

[Mesos Reviewbot Windows]

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



FAIL: Mesos tests failed to build.

Reviews applied: `['63652']`

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

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

Relevant logs:

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

```
  C:\DCOS\mesos\mesos\src\authentication\cram_md5\authenticatee.cpp(271): 
warning C4267: 'argument': conversion from 'size_t' to 'unsigned int', possible 
loss of data [C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\authentication\cram_md5\authenticatee.cpp(333): 
warning C4267: '=': conversion from 'size_t' to 'unsigned int', possible loss 
of data [C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\authentication\cram_md5\authenticator.cpp(216): 
warning C4267: 'argument': conversion from 'size_t' to 'unsigned int', possible 
loss of data [C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\authentication\cram_md5\authenticator.cpp(242): 
warning C4267: 'argument': conversion from 'size_t' to 'unsigned int', possible 
loss of data [C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\authentication\cram_md5\authenticator.cpp(274): 
warning C4267: '=': conversion from 'size_t' to 'unsigned int', possible loss 
of data [C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\master/master.hpp(2103): warning C4244: 'return': 
conversion from 'unsigned __int64' to 'double', possible loss of data 
(compiling source file C:\DCOS\mesos\mesos\src\common\protobuf_utils.cpp) 
[C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\files\files.cpp(703): warning C4267: 'argument': 
conversion from 'size_t' to 'off_t', possible loss of data 
[C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\master/master.hpp(2103): warning C4244: 'return': 
conversion from 'unsigned __int64' to 'double', possible loss of data 
(compiling source file C:\DCOS\mesos\mesos\src\master\master.cpp) 
[C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\master\master.cpp(6185): warning C4244: 'argument': 
conversion from 'const ::size_t' to 'double', possible loss of data 
[C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\master\master.cpp(6272): warning C4244: 'argument': 
conversion from 'const ::size_t' to 'double', possible loss of data 
[C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\master\master.cpp(6855): warning C4244: 'argument': 
conversion from 'const ::size_t' to 'double', possible loss of data 
[C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\master\master.cpp(8376): warning C4244: 'argument': 
conversion from 'const ::size_t' to 'double', possible loss of data 
[C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\master\master.cpp(10147): warning C4244: 'return': 
conversion from '::size_t' to 'double', possible loss of data 
[C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\master/master.hpp(2103): warning C4244: 'return': 
conversion from 'unsigned __int64' to 'double', possible loss of data 
(compiling source file C:\DCOS\mesos\mesos\src\master\quota_handler.cpp) 
[C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\master/master.hpp(2103): warning C4244: 'return': 
conversion from 'unsigned __int64' to 'double', possible loss of data 
(compiling source file C:\DCOS\mesos\mesos\src\master\weights_handler.cpp) 
[C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\master\allocator\sorter\drf\sorter.cpp(589): warning 
C4267: 'return': conversion from 'size_t' to 'int', possible loss of data 
[C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\slave/containerizer/mesos/isolators/posix.hpp(55): 
warning C4244: 'argument': conversion from 'google::protobuf::uint64' to 'const 
pid_t', possible loss of data [C:\DCOS\mesos\src\mesos.vcxproj]
  C:\DCOS\mesos\mesos\src\slave\containerizer\mesos\containerizer.cpp(755): 
warning C4244: 'argument': conversion from 'google::protobuf::uint64' to 
'pid_t', possible loss of data [C:\DCOS\mesos\src\mesos.vcxproj]


"C:\DCOS\mesos\src\tests\mesos-tests.vcxproj" (default target) (1) ->
"C:\DCOS\mesos\src\mesos.vcxproj" (default target) (12) ->
(ClCompile target) -> 
  C:\DCOS\mesos\mesos\src\slave\containerizer\mesos\provisioner\utils.cpp(29): 
fatal error C1083: Cannot open include file: 'linux/fs.h': No such file or 
directory [C:\DCOS\mesos\src\mesos.vcxproj]

137 Warning(s)
1 Error(s)

Time Elapsed 00:23:06.67
```

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

```
  Creating directory "C:\DCOS\mesos\CMakeFiles\CMakeTmp\Debug\".

Re: Review Request 63642: Added a test for ExecutorID validation in ReregisterSlaveMessage.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63642 was successfully built and tested.

Reviews applied: `['63642']`

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

- Mesos Reviewbot Windows


On Nov. 7, 2017, 6:47 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63642/
> ---
> 
> (Updated Nov. 7, 2017, 6:47 p.m.)
> 
> 
> Review request for mesos, James DeFelice and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-8169
> https://issues.apache.org/jira/browse/MESOS-8169
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test to ensure that the ReregisterSlaveMessage validation
> correctly allows duplicate ExecutorIDs as long as they are scoped
> to different frameworks.
> 
> 
> Diffs
> -
> 
>   src/tests/master_validation_tests.cpp 
> be8c8920fdd6eb72fc77247844a731070db5fa1c 
> 
> 
> Diff: https://reviews.apache.org/r/63642/diff/4/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26) with PR #248 applied.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI handling on Windows.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63253 was successfully built and tested.

Reviews applied: `['60620', '60621', '60622', '60623', '60624', '60626', 
'60628', '63253']`

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

- Mesos Reviewbot Windows


On Nov. 8, 2017, 6:07 a.m., Jeff Coffler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63253/
> ---
> 
> (Updated Nov. 8, 2017, 6:07 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Gaston Kleiman, John Kordich, 
> Joseph Wu, Li Li, and Michael Park.
> 
> 
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
> for proper operation on Windows. Changed to use new URI methods
> introduced as part of MESOS-6705, and used powershell on Windows
> to replace "test" command on Linux.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> 5078bd4d70698f5cbd14c971fcecfd58f8467a04 
> 
> 
> Diff: https://reviews.apache.org/r/63253/diff/5/
> 
> 
> Testing
> ---
> 
> Ran mesos-tests on both Windows and Linux with no errors.
> 
> Specifically ran mesos-tests with 
> `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the 
> modified tests ran properly.
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>

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

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63655 was successfully built and tested.

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

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

- Mesos Reviewbot Windows


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

Re: Review Request 60621: Added new stout capability: os::copyfile(source, dest).

[Jeff Coffler]

> On Nov. 3, 2017, 6:29 p.m., Benjamin Mahler wrote:
> > 3rdparty/stout/include/stout/os/windows/copyfile.hpp
> > Lines 56-57 (patched)
> > 
> >
> > The general pattern is to just include the reason for an error, and to 
> > not include any information the caller already has, because otherwise the 
> > callers will double log:
> > 
> > ```
> > Try copy = copyfile(source, destination);
> > 
> > if (copy.isError()) {
> >   return ("Failed to copy '" + source + "'"
> >   " to '" + destination + "': " + copy.error();
> > }
> > ```
> > 
> > The current code would log:
> > 
> > "Failed to copy 's' to 'd': Failed to copy 's' to 'd': No disk space 
> > left"
> > 
> > Can you guys do a sweep for this issue in the windows related code that 
> > has been added?
> 
> Andrew Schwartzmeyer wrote:
> I'm not sure I follow. Are you saying the _caller_ should always write 
> the actual error message, versus the _callee_ preparing it here?
> 
> I guess in your example, I don't get why the user of `os::copyfile` would 
> add `"Failed to copy..."` instead of just doing:
> 
> ```
> if (copy.isError()) {
> return Error(copy.error());
> }
> ```
> 
> And then the error message is only written once, in `os::copyfile`.
> 
> Benjamin Mahler wrote:
> What is the "actual" error message? :)
> 
> An error message consists of several parts, much like an exception: the 
> "reason" for the error, and multiple "stacks" of context. If you're referring 
> to the "reason" when you said "actual", either approach (the one we use, or 
> the one used in this patch) includes the reason in their returned error 
> message. The distinction lies in where the "stacks" of context get included.
> 
> The decision we took some time ago was to have the "owner" of the context 
> be responsible for including it. So if I call `os::copyfile` I know which 
> function I'm calling and which source and destination I'm passing into it. 
> This matches posix-style programming which I why I think we chose this 
> approach:
> 
> ```
> int main()
> {
>   int fd = open("/file");
>   
>   if (fd == -1) {
> // Caller logs the thing it was doing, and gets the reason for the 
> error:
> LOG(ERROR) << "Failed to initialize: Failed to open '/file': " << 
> strerror(errno);
>   }
> }
> 
> vs
> 
> int main()
> {
>   Try fd = open("/file");
>   
>   if (fd.isError()) {
> // Caller logs the thing it was doing, and gets the reason for the 
> error:
> LOG(ERROR) << "Failed to initialize: Failed to open '/file': " << 
> fd.error();
>   }
> }
> ```
> 
> Now of course, if you use the alternative approach to have the leaf 
> include all the information it has, then you have to compose differently:
> 
> ```
> int main()
> {
>   Try fd = os::open("/file");
>   
>   if (fd.isError()) {
> // Caller knows that no additional context needs to be added because 
> callee has all of it.
> LOG(ERROR) << "Failed to initialize: " << fd.error();
>   }
> }
> ```
> 
> The approach we chose was to treat the error as just the "reason" (like 
> strerror), so if you want to add context to it you can.
> 
> Both approaches work, but we have to pick one and apply it consistently 
> as best we can. In retrospect, I actually think the other approach would have 
> been a better choice because it fits more easily into Future::then style 
> composition. But we would have to discuss the change of approach and do a 
> sweep, because most of the code follows the first pattern shown.
> 
> Andrew Schwartzmeyer wrote:
> Ah, thank you for the detailed explanation. Unfortunately, the existing 
> error handling code was using the leaf approach, and we kept it consistent, 
> but consistently wrong. We'll have to do a sweep to fix it.
> 
> This explanation was really good, do we have something like it 
> documented? If not, let's get it added to the style guide.

This is a general issue all over the place. Rather than fix that here, I think 
this should be covered in a sweep of the code where we fix everything in one 
shot. So I'll drop this for now, but we will plan on doing a sweep to fix up a 
bunch of things like this.


- Jeff


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


On Nov. 6, 2017, 6:08 p.m., Jeff Coffler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60621/
> 

Re: Review Request 63642: Added a test for ExecutorID validation in ReregisterSlaveMessage.

[Jiang Yan Xu]

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




src/tests/master_validation_tests.cpp
Lines 4215 (patched)


2 space indentation.



src/tests/master_validation_tests.cpp
Lines 4225-4226 (patched)


Nit: I feel the grouping looks clearer to have:

```
frameworks.push_back(DEFAULT_FRAMEWORK_INFO);
frameworks[0].set_name("framework1");
frameworks[0].mutable_id()->set_value("framework1");

frameworks.push_back(DEFAULT_FRAMEWORK_INFO);
frameworks[1].set_name("framework2");
frameworks[1].mutable_id()->set_value("framework2");
```

BTW you could use `back()` as well.

Same for the executors. 

Up to you.



src/tests/master_validation_tests.cpp
Lines 4241-4242 (patched)


These are `ASSERT_*`s? Same below.



src/tests/master_validation_tests.cpp
Lines 4243 (patched)


These are `EXPECT_*`? Same below.


- Jiang Yan Xu


On Nov. 7, 2017, 10:47 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63642/
> ---
> 
> (Updated Nov. 7, 2017, 10:47 a.m.)
> 
> 
> Review request for mesos, James DeFelice and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-8169
> https://issues.apache.org/jira/browse/MESOS-8169
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test to ensure that the ReregisterSlaveMessage validation
> correctly allows duplicate ExecutorIDs as long as they are scoped
> to different frameworks.
> 
> 
> Diffs
> -
> 
>   src/tests/master_validation_tests.cpp 
> be8c8920fdd6eb72fc77247844a731070db5fa1c 
> 
> 
> Diff: https://reviews.apache.org/r/63642/diff/3/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26) with PR #248 applied.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI handling on Windows.

[Jeff Coffler]

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

(Updated Nov. 8, 2017, 12:37 a.m.)


Review request for mesos, Andrew Schwartzmeyer, Gaston Kleiman, John Kordich, 
Joseph Wu, Li Li, and Michael Park.


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


Repository: mesos


Description
---

Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
for proper operation on Windows. Changed to use new URI methods
introduced as part of MESOS-6705, and used powershell on Windows
to replace "test" command on Linux.


Diffs (updated)
-

  src/tests/default_executor_tests.cpp 5078bd4d70698f5cbd14c971fcecfd58f8467a04 


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

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


Testing
---

Ran mesos-tests on both Windows and Linux with no errors.

Specifically ran mesos-tests with 
`--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the 
modified tests ran properly.


Thanks,

Jeff Coffler

Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI handling on Windows.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [60620, 60621, 60622, 60623, 60624, 60626, 60628, 63253]

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 Nov. 8, 2017, 12:37 a.m., Jeff Coffler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63253/
> ---
> 
> (Updated Nov. 8, 2017, 12:37 a.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Gaston Kleiman, John Kordich, 
> Joseph Wu, Li Li, and Michael Park.
> 
> 
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
> for proper operation on Windows. Changed to use new URI methods
> introduced as part of MESOS-6705, and used powershell on Windows
> to replace "test" command on Linux.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> 5078bd4d70698f5cbd14c971fcecfd58f8467a04 
> 
> 
> Diff: https://reviews.apache.org/r/63253/diff/5/
> 
> 
> Testing
> ---
> 
> Ran mesos-tests on both Windows and Linux with no errors.
> 
> Specifically ran mesos-tests with 
> `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the 
> modified tests ran properly.
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>

Re: Review Request 60890: Defined API for launching standalone containers.

[Jie Yu]

> On Nov. 2, 2017, 3:56 p.m., Joseph Wu wrote:
> > include/mesos/agent/agent.proto
> > Lines 253-255 (patched)
> > 
> >
> > Leaving this up for discussion:
> > 
> > The existing call (`RemoveNestedContainer`) either returns 200 or 500, 
> > so for backwards compatibility, `RemoveContainer` does the same.
> > 
> > On the other hand, returning a 404 may be somewhat difficult as 
> > `Containerizer::remove` returns a `Future`.  Other methods like 
> > `::launch`, `::wait`, or `::kill` all a magic value if the container 
> > doesn't exist.

Can you add a TODO. I think we should add a magic value for `remove` interface 
as well!


- Jie


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


On Nov. 2, 2017, 3:50 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60890/
> ---
> 
> (Updated Nov. 2, 2017, 3:50 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-7305
> https://issues.apache.org/jira/browse/MESOS-7305
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Launching a standalone container is very similar to launching a
> nested container, except that the caller must specify some Resources.
> As such, this patch deprecates some nested container APIs
> and replaces them with more generically named APIs.
> 
> This applies to the Launch, Wait, Kill, and Remove
> (nested) container APIs.
> 
> 
> Diffs
> -
> 
>   include/mesos/agent/agent.proto 4df3dcef4bd1d42cb8b4955e290bd38038355e46 
>   include/mesos/v1/agent/agent.proto e99d23d55a0951f2ed728360e103d83ea5a1ad7f 
> 
> 
> Diff: https://reviews.apache.org/r/60890/diff/5/
> 
> 
> Testing
> ---
> 
> make
> 
> See later patches in chain.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

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

[Jie Yu]

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




src/slave/validation.cpp
Lines 381 (patched)


You may want to normalize the resources to `POST_RESERVATION_REFINEMENT` 
format and use `isUnreserved` check below?



src/slave/validation.cpp
Lines 396-400 (patched)


This can be `isUnreserved` if the resources are normalized to post 
refinement format.



src/slave/validation.cpp
Lines 407-408 (patched)


You can use `Resources::isPersistentVolume`


- Jie Yu


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

Re: Review Request 63175: Do not generate UnavailableResources for inactive frameworks.

[Benjamin Mahler]

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




src/master/allocator/mesos/hierarchical.hpp
Lines 319 (patched)


I think this should be `connected`?

Right now there's some confusion around `active` and `connected`. `active` 
in the past meant that the framework was receiving offers, but we never added 
the ability for frameworks to change this state. When we added suppression, 
that was equivalent to deactivation (note that activation has become a per-role 
thing).

Mostly `connected` == `active`. But I think the point of this boolean is to 
track whether we can talk to the framework? Thoughts?


- Benjamin Mahler


On Nov. 8, 2017, 1:02 a.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63175/
> ---
> 
> (Updated Nov. 8, 2017, 1:02 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, James Peach, and Joseph Wu.
> 
> 
> Bugs: MESOS-8085
> https://issues.apache.org/jira/browse/MESOS-8085
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Do not generate UnavailableResources for inactive frameworks.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> c2346054b2c98516f15ab8ce2dc798224ff4def4 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5b6efe5faa3c3b10f1f714f582a155b368f8ccaf 
> 
> 
> Diff: https://reviews.apache.org/r/63175/diff/1/
> 
> 
> Testing
> ---
> 
> make check.
> 
> I didn' write a new test as the externally observable behavior doesn't change.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

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

[James Peach]

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

Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description
---

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


Diffs
-

  src/linux/ns.cpp 33751b518e95f4f67e6864728e1fdf621f50dd52 


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


Testing
---

make check (Fedora 26, macOS)


Thanks,

James Peach

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

[James Peach]

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

Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description
---

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


Diffs
-

  3rdparty/stout/include/stout/os/posix/socket.hpp 
bab0b808f53abd1314a7d13fc0cba75e5717f96f 
  3rdparty/stout/tests/os/sendfile_tests.cpp 
05966ae067ae3972598da3370eb16fdce5736c21 


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


Testing
---

make check (Fedora 26, macOS)


Thanks,

James Peach

Re: Review Request 63642: Added a test for ExecutorID validation in ReregisterSlaveMessage.

[Jiang Yan Xu]

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


Fix it, then Ship it!





src/tests/master_validation_tests.cpp
Lines 4240-4241 (patched)


I was suggesting `ASSERT_*` as strictly speaking the test code prepared 
these, not the code to be tested so it should always succeed otherwise we 
should abort the test.

Here and below.


- Jiang Yan Xu


On Nov. 7, 2017, 10:47 a.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63642/
> ---
> 
> (Updated Nov. 7, 2017, 10:47 a.m.)
> 
> 
> Review request for mesos, James DeFelice and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-8169
> https://issues.apache.org/jira/browse/MESOS-8169
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test to ensure that the ReregisterSlaveMessage validation
> correctly allows duplicate ExecutorIDs as long as they are scoped
> to different frameworks.
> 
> 
> Diffs
> -
> 
>   src/tests/master_validation_tests.cpp 
> be8c8920fdd6eb72fc77247844a731070db5fa1c 
> 
> 
> Diff: https://reviews.apache.org/r/63642/diff/4/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26) with PR #248 applied.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 63642: Added a test for ExecutorID validation in ReregisterSlaveMessage.

[James Peach]

> On Nov. 8, 2017, 12:42 a.m., Jiang Yan Xu wrote:
> > src/tests/master_validation_tests.cpp
> > Lines 4240-4241 (patched)
> > 
> >
> > I was suggesting `ASSERT_*` as strictly speaking the test code prepared 
> > these, not the code to be tested so it should always succeed otherwise we 
> > should abort the test.
> > 
> > Here and below.

Usually I only use `ASSERT` when the subsequent code would crash if it was 
executed which is not the case here.


- James


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


On Nov. 7, 2017, 6:47 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63642/
> ---
> 
> (Updated Nov. 7, 2017, 6:47 p.m.)
> 
> 
> Review request for mesos, James DeFelice and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-8169
> https://issues.apache.org/jira/browse/MESOS-8169
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test to ensure that the ReregisterSlaveMessage validation
> correctly allows duplicate ExecutorIDs as long as they are scoped
> to different frameworks.
> 
> 
> Diffs
> -
> 
>   src/tests/master_validation_tests.cpp 
> be8c8920fdd6eb72fc77247844a731070db5fa1c 
> 
> 
> Diff: https://reviews.apache.org/r/63642/diff/4/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26) with PR #248 applied.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 63642: Added a test for ExecutorID validation in ReregisterSlaveMessage.

[James DeFelice]

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


Ship it!




Ship It!

- James DeFelice


On Nov. 7, 2017, 6:47 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63642/
> ---
> 
> (Updated Nov. 7, 2017, 6:47 p.m.)
> 
> 
> Review request for mesos, James DeFelice and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-8169
> https://issues.apache.org/jira/browse/MESOS-8169
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test to ensure that the ReregisterSlaveMessage validation
> correctly allows duplicate ExecutorIDs as long as they are scoped
> to different frameworks.
> 
> 
> Diffs
> -
> 
>   src/tests/master_validation_tests.cpp 
> be8c8920fdd6eb72fc77247844a731070db5fa1c 
> 
> 
> Diff: https://reviews.apache.org/r/63642/diff/4/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26) with PR #248 applied.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 61183: Triggered 'UpdateSlaveMessage' when 'ResourceProviderManager' updates.

[Benjamin Bannier]

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

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


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Addressed comment by Jie.


Repository: mesos


Description
---

The agent's resource provider manager sends a
'ResourceProviderMessage' when its managed resources change. This
commit adds handling in the agent so that an 'UpdateSlaveMessage' is
sent to the master to update the total resource available on the
agent.

In order to provide push-like handling of the resource provider
manager's message queue, we chain recursive calls to the handler for
continuous processing. Initially, processing is kicked off from
'Slave::initialize'. In this simple implementation we e.g., provide no
direct way to stop processing of messages, yet, but it can be achieved
by e.g., replacing the manager with a new instance (this would also
require updating routes).

Since the agent can only send an 'UpdateSlaveMessage' when it is
registered with a master, a simple back-off of 5 s is implemented which
will defer processing of a ready message should the agent not yet have
registered.

To facilitate logging we add a stringification function for
'ResourceProviderMessage's.

This patch also adjusts a number of tests to no expect two
'UpdateSlaveMessage's.


Diffs (updated)
-

  src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
  src/slave/slave.hpp df1b0205124555dcb6a0efa5c237f5e77fa2bdf7 
  src/slave/slave.cpp c10823985154bac19f8952b94311a03b2b9b4ea1 
  src/tests/slave_tests.cpp f9c2e6b41bdbc54ee0d8d06a2a41c92b7a1156cc 


Diff: https://reviews.apache.org/r/61183/diff/23/

Changes: https://reviews.apache.org/r/61183/diff/22-23/


Testing
---

Tested on a number of platforms on internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 63540: Added flag protobuf message for agent capabilities.

[Benjamin Bannier]

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

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


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Fixed typo.


Repository: mesos


Description (updated)
---

See summary.


Diffs (updated)
-

  src/messages/flags.hpp b267677541077342f01f9014e6dbf0ab29e437ca 
  src/messages/flags.proto 077353cbea8f8c45426ab343f09e9fee3e5462a6 


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

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


Testing
---

`make check`, tested as part of https://reviews.apache.org/r/63496/.


Thanks,

Benjamin Bannier

Re: Review Request 63519: Allowed toggling of agent capabilities via command line flags.

[Benjamin Bannier]

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

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


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Addressed comment by Jie.


Repository: mesos


Description
---

This patch introduces a new agent command line flags
'--agent_features' which can be used to whitelist capabilities to
enable. This flag is intended to enable introducing feature flags in
the future so experimental features can be added while still reducing
the risk of breaking existing functionality. Currently only the
'RESOURCE_PROVIDER' capability can be toggled.


Diffs (updated)
-

  docs/configuration/agent.md 116e7c6b10d6eafdf6a163c821d2204db29f6d4c 
  src/slave/flags.hpp d02edbfd68266c9f2d5c78fdbd5c2ba5f497adba 
  src/slave/flags.cpp 63aaac218fdc067742d39f1fc8497723784d9595 
  src/slave/slave.hpp df1b0205124555dcb6a0efa5c237f5e77fa2bdf7 
  src/slave/slave.cpp c10823985154bac19f8952b94311a03b2b9b4ea1 


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

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


Testing
---

`make check`, tested as part of https://reviews.apache.org/r/63496/


Thanks,

Benjamin Bannier

Re: Review Request 63492: Synchronized agent resource versions via 'UpdateSlaveMessage'.

[Benjamin Bannier]

> On Nov. 7, 2017, 7:08 p.m., Jie Yu wrote:
> > src/slave/slave.cpp
> > Lines 6648-6651 (patched)
> > 
> >
> > I am a little hesitate about this. Resource estimator can change quite 
> > frequently, and any change will invalidate some operations for the regular 
> > agent default resources.
> > 
> > Currently, we don't allow any operation to operate on oversubscribed 
> > resources yet. So I am learning towards not changing the clock here with a 
> > big TODO saying that we might want to do that in the future if we allow 
> > operation for oversubscribed resources.
> > 
> > Alternatively, we can keep separate clock for oversubscribed resources. 
> > We don't have to do it for now.

Good point. I removed the update for now and added a `TODO`.


> On Nov. 7, 2017, 7:08 p.m., Jie Yu wrote:
> > src/slave/slave.cpp
> > Lines 6719-6722 (patched)
> > 
> >
> > can this simply be:
> > 
> > `resourceVersions[resourcProviderId] = resourceVersionUuid;`

No, this is not possible as `operator[]` would need to be able to 
default-construct an entry; `UUID` cannot be default-constructed.


- Benjamin


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


On Nov. 7, 2017, 11 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63492/
> ---
> 
> (Updated Nov. 7, 2017, 11 p.m.)
> 
> 
> Review request for mesos, Gaston Kleiman, Greg Mann, Jie Yu, and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit introduces agent resource versions to the master and
> agents. Agents are responsible for maintaining their resource
> versions. The resource versions are synchronized with the master via
> 'UpdateSlaveMessage'.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp 95f57da820db676874302f1c3138a76c22a88593 
>   src/common/protobuf_utils.cpp 7a4b87becbe73f0fa97ebceee6b71ede8dce90dd 
>   src/master/master.hpp 0c1253aa1a88d113d42ce362d6992ad84217728c 
>   src/master/master.cpp ee212c1a21b432d3aa8e90d748d12cb0a754d57c 
>   src/slave/slave.hpp df1b0205124555dcb6a0efa5c237f5e77fa2bdf7 
>   src/slave/slave.cpp c10823985154bac19f8952b94311a03b2b9b4ea1 
> 
> 
> Diff: https://reviews.apache.org/r/63492/diff/6/
> 
> 
> Testing
> ---
> 
> `make check`, additional testing as part of 
> https://reviews.apache.org/r/63496/.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 63175: Do not generate UnavailableResources for inactive frameworks.

[James Peach]

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


Fix it, then Ship it!





src/master/allocator/mesos/hierarchical.cpp
Line 363 (original), 366 (patched)


Update this to

```
CHECK(frameworks.contains(frameworkId))
  << frameworkId;
```

like you do in other places?



src/master/allocator/mesos/hierarchical.cpp
Line 390 (original), 395 (patched)


Update this to

```
CHECK(frameworks.contains(frameworkId))
  << frameworkId;
```

like you do in other places?



src/master/allocator/mesos/hierarchical.cpp
Lines 399 (patched)


Move this to line 411 where the rest of the framework state is reset.



src/master/allocator/mesos/hierarchical.cpp
Lines 2009 (patched)


I think this needs an explanatory comment.


- James Peach


On Nov. 7, 2017, 9:34 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63175/
> ---
> 
> (Updated Nov. 7, 2017, 9:34 p.m.)
> 
> 
> Review request for mesos, James Peach and Joseph Wu.
> 
> 
> Bugs: MESOS-8085
> https://issues.apache.org/jira/browse/MESOS-8085
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Do not generate UnavailableResources for inactive frameworks.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> c2346054b2c98516f15ab8ce2dc798224ff4def4 
>   src/master/allocator/mesos/hierarchical.cpp 
> 5b6efe5faa3c3b10f1f714f582a155b368f8ccaf 
> 
> 
> Diff: https://reviews.apache.org/r/63175/diff/1/
> 
> 
> Testing
> ---
> 
> make check.
> 
> I didn' write a new test as the externally observable behavior doesn't change.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 60888: Added recovery logic for standalone containers.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Nov. 2, 2017, 3:43 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60888/
> ---
> 
> (Updated Nov. 2, 2017, 3:43 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-7305
> https://issues.apache.org/jira/browse/MESOS-7305
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Although there is no way to launch standalone containers yet,
> this commit outlines the expected layout of container metadata
> which should be populated when launching standalone containers.
> 
> The layout is fairly simple, as standalone containers have no
> framework, executor, or tasks to worry about.  The sandbox directory
> will live under a new top-level directory `containers` and there is
> no metadata to checkpoint at the moment.
> 
> The containerizer will checkpoint a marker file (in the runtime
> directory) so that it knows to recover all standalone containers.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 100e3bbda543d87808da9ff6bea42da5099ea8c5 
>   src/slave/containerizer/mesos/paths.hpp 
> a03f15e01e8eb8a1326baad8d7db96a79d785482 
>   src/slave/containerizer/mesos/paths.cpp 
> 0c61c20c345a327ec469b382558aaeed0280e754 
>   src/slave/paths.hpp f000508d414daf9f943561f89c7105503a8a98b3 
>   src/slave/paths.cpp fd546525b900cb6524fb9196d19616ef18de0f30 
> 
> 
> Diff: https://reviews.apache.org/r/60888/diff/4/
> 
> 
> Testing
> ---
> 
> See next patch and later ones too.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 63649: Introduced `.clang-tidy` configuration file.

[Benjamin Bannier]

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


Fix it, then Ship it!





support/mesos-tidy/entrypoint.sh
Lines 26 (patched)


Let's add a comment here that we require this so `.clang-tidy` is present 
at the project root.


- Benjamin Bannier


On Nov. 7, 2017, 11:02 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63649/
> ---
> 
> (Updated Nov. 7, 2017, 11:02 p.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   bootstrap d0dd491e99b1e60dfe5c1ada3388e78673112b96 
>   support/clang-tidy PRE-CREATION 
>   support/gitignore 90b6697d19a5e0a68805b23b587b362731a1df25 
>   support/mesos-tidy.sh c33dfe04266ccf83ee95a3da00468611ff71aa5e 
>   support/mesos-tidy/entrypoint.sh bdaa4539cb736a15c92d55b66036df753438e283 
> 
> 
> Diff: https://reviews.apache.org/r/63649/diff/1/
> 
> 
> Testing
> ---
> 
> `support/mesos-tidy.sh`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 63649: Introduced `.clang-tidy` configuration file.

[Michael Park]

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

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


Review request for mesos and Benjamin Bannier.


Changes
---

Addressed Benjamin's comment.


Repository: mesos


Description (updated)
---

Introduced `.clang-tidy` configuration file.


Diffs (updated)
-

  bootstrap d0dd491e99b1e60dfe5c1ada3388e78673112b96 
  support/clang-tidy PRE-CREATION 
  support/gitignore 90b6697d19a5e0a68805b23b587b362731a1df25 
  support/mesos-tidy.sh c33dfe04266ccf83ee95a3da00468611ff71aa5e 
  support/mesos-tidy/entrypoint.sh bdaa4539cb736a15c92d55b66036df753438e283 


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

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


Testing
---

`support/mesos-tidy.sh`


Thanks,

Michael Park

Re: Review Request 63492: Synchronized agent resource versions via 'UpdateSlaveMessage'.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Nov. 7, 2017, 10 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63492/
> ---
> 
> (Updated Nov. 7, 2017, 10 p.m.)
> 
> 
> Review request for mesos, Gaston Kleiman, Greg Mann, Jie Yu, and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit introduces agent resource versions to the master and
> agents. Agents are responsible for maintaining their resource
> versions. The resource versions are synchronized with the master via
> 'UpdateSlaveMessage'.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.hpp 95f57da820db676874302f1c3138a76c22a88593 
>   src/common/protobuf_utils.cpp 7a4b87becbe73f0fa97ebceee6b71ede8dce90dd 
>   src/master/master.hpp 0c1253aa1a88d113d42ce362d6992ad84217728c 
>   src/master/master.cpp ee212c1a21b432d3aa8e90d748d12cb0a754d57c 
>   src/slave/slave.hpp df1b0205124555dcb6a0efa5c237f5e77fa2bdf7 
>   src/slave/slave.cpp c10823985154bac19f8952b94311a03b2b9b4ea1 
> 
> 
> Diff: https://reviews.apache.org/r/63492/diff/6/
> 
> 
> Testing
> ---
> 
> `make check`, additional testing as part of 
> https://reviews.apache.org/r/63496/.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 63519: Allowed toggling of agent capabilities via command line flags.

[Benjamin Bannier]

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

(Updated Nov. 8, 2017, 12:35 a.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Updated docs.


Repository: mesos


Description
---

This patch introduces a new agent command line flags
'--agent_features' which can be used to whitelist capabilities to
enable. This flag is intended to enable introducing feature flags in
the future so experimental features can be added while still reducing
the risk of breaking existing functionality. Currently only the
'RESOURCE_PROVIDER' capability can be toggled.


Diffs (updated)
-

  docs/configuration/agent.md 116e7c6b10d6eafdf6a163c821d2204db29f6d4c 
  src/slave/flags.hpp d02edbfd68266c9f2d5c78fdbd5c2ba5f497adba 
  src/slave/flags.cpp 63aaac218fdc067742d39f1fc8497723784d9595 
  src/slave/slave.hpp df1b0205124555dcb6a0efa5c237f5e77fa2bdf7 
  src/slave/slave.cpp c10823985154bac19f8952b94311a03b2b9b4ea1 


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

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


Testing
---

`make check`, tested as part of https://reviews.apache.org/r/63496/


Thanks,

Benjamin Bannier

Re: Review Request 60888: Added recovery logic for standalone containers.

[Jie Yu]

> On Oct. 17, 2017, 5:39 p.m., Jie Yu wrote:
> > src/slave/paths.hpp
> > Lines 53-54 (patched)
> > 
> >
> > Putting this path here is a bit wierd because this directory is kind of 
> > containerizer specific.
> > 
> > I'd suggest we create a `src/slave/containerizer/paths.hpp` to put 
> > those path helper functions.
> > 
> > `src/slave/paths.hpp` should probably only have paths that are related 
> > to slave, but not containers. In the future, the sandbox for executors can 
> > be a symlink to the actual container sandbox.
> 
> Joseph Wu wrote:
> Leaving this open as I didn't change the directory structure...
> 
> Sandbox directories (for normal and standalone containers) are currently 
> specified by the agent, so it makes sense to have this directory in the 
> agent's helper.  The containerizer only specifies sandboxes for nested 
> containers by appending onto an existing sandbox (which itself is directly or 
> indirectly (for multi-level nested containers) specified by the agent).

ok, fair enough. I think we can revisit this once we want to support just 
running containerizer alone without the agent.


- Jie


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


On Nov. 2, 2017, 3:43 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60888/
> ---
> 
> (Updated Nov. 2, 2017, 3:43 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-7305
> https://issues.apache.org/jira/browse/MESOS-7305
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Although there is no way to launch standalone containers yet,
> this commit outlines the expected layout of container metadata
> which should be populated when launching standalone containers.
> 
> The layout is fairly simple, as standalone containers have no
> framework, executor, or tasks to worry about.  The sandbox directory
> will live under a new top-level directory `containers` and there is
> no metadata to checkpoint at the moment.
> 
> The containerizer will checkpoint a marker file (in the runtime
> directory) so that it knows to recover all standalone containers.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 100e3bbda543d87808da9ff6bea42da5099ea8c5 
>   src/slave/containerizer/mesos/paths.hpp 
> a03f15e01e8eb8a1326baad8d7db96a79d785482 
>   src/slave/containerizer/mesos/paths.cpp 
> 0c61c20c345a327ec469b382558aaeed0280e754 
>   src/slave/paths.hpp f000508d414daf9f943561f89c7105503a8a98b3 
>   src/slave/paths.cpp fd546525b900cb6524fb9196d19616ef18de0f30 
> 
> 
> Diff: https://reviews.apache.org/r/60888/diff/4/
> 
> 
> Testing
> ---
> 
> See next patch and later ones too.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 63519: Allowed toggling of agent capabilities via command line flags.

[Jie Yu]

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




docs/configuration/agent.md
Lines 101-103 (patched)


Please update these.



src/slave/flags.cpp
Lines 653-655 (patched)


Ditto.


- Jie Yu


On Nov. 7, 2017, 10 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63519/
> ---
> 
> (Updated Nov. 7, 2017, 10 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch introduces a new agent command line flags
> '--agent_features' which can be used to whitelist capabilities to
> enable. This flag is intended to enable introducing feature flags in
> the future so experimental features can be added while still reducing
> the risk of breaking existing functionality. Currently only the
> 'RESOURCE_PROVIDER' capability can be toggled.
> 
> 
> Diffs
> -
> 
>   docs/configuration/agent.md 116e7c6b10d6eafdf6a163c821d2204db29f6d4c 
>   src/slave/flags.hpp d02edbfd68266c9f2d5c78fdbd5c2ba5f497adba 
>   src/slave/flags.cpp 63aaac218fdc067742d39f1fc8497723784d9595 
>   src/slave/slave.hpp df1b0205124555dcb6a0efa5c237f5e77fa2bdf7 
>   src/slave/slave.cpp c10823985154bac19f8952b94311a03b2b9b4ea1 
> 
> 
> Diff: https://reviews.apache.org/r/63519/diff/6/
> 
> 
> Testing
> ---
> 
> `make check`, tested as part of https://reviews.apache.org/r/63496/
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 63649: Introduced `.clang-tidy` configuration file.

[Michael Park]

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

Review request for mesos and Benjamin Bannier.


Repository: mesos


Description
---

See summary.


Diffs
-

  bootstrap d0dd491e99b1e60dfe5c1ada3388e78673112b96 
  support/clang-tidy PRE-CREATION 
  support/gitignore 90b6697d19a5e0a68805b23b587b362731a1df25 
  support/mesos-tidy.sh c33dfe04266ccf83ee95a3da00468611ff71aa5e 
  support/mesos-tidy/entrypoint.sh bdaa4539cb736a15c92d55b66036df753438e283 


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


Testing
---

`support/mesos-tidy.sh`


Thanks,

Michael Park

Re: Review Request 63496: Added tests for agent resource version transmission.

[Benjamin Bannier]

> On Nov. 7, 2017, 6:40 p.m., Greg Mann wrote:
> > src/tests/slave_tests.cpp
> > Lines 8679-8681 (patched)
> > 
> >
> > Would be good if this test or another could verify that a new resource 
> > version is sent for an RP after the RP makes an UPDATE_STATE call.
> 
> Jie Yu wrote:
> +1
> 
> maybe a TODO or a ticket for now. Let's note down all the tests we'll 
> need to write.

We'll only trigger a version change if an operation failed. 
https://reviews.apache.org/r/63625/ implements a test for a successful 
operation; I created https://issues.apache.org/jira/browse/MESOS-8181 to track 
adding tests for this.


- Benjamin


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


On Nov. 7, 2017, 11:01 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63496/
> ---
> 
> (Updated Nov. 7, 2017, 11:01 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch introduces separate tests for clock values communicated
> from resource providers and from agents to masters.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp f9c2e6b41bdbc54ee0d8d06a2a41c92b7a1156cc 
> 
> 
> Diff: https://reviews.apache.org/r/63496/diff/7/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 63496: Added tests for agent resource version transmission.

[Benjamin Bannier]

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

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


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Remove not applicable test; fixed comments pointed out by Jie.


Repository: mesos


Description
---

This patch introduces separate tests for clock values communicated
from resource providers and from agents to masters.


Diffs (updated)
-

  src/tests/slave_tests.cpp f9c2e6b41bdbc54ee0d8d06a2a41c92b7a1156cc 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 63492: Synchronized agent resource versions via 'UpdateSlaveMessage'.

[Benjamin Bannier]

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

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


Review request for mesos, Gaston Kleiman, Greg Mann, Jie Yu, and Jan Schlicht.


Changes
---

Introduced protobuf helper functions; comment fixes as suggested by Jie & Greg.


Summary (updated)
-

Synchronized agent resource versions via 'UpdateSlaveMessage'.


Repository: mesos


Description (updated)
---

This commit introduces agent resource versions to the master and
agents. Agents are responsible for maintaining their resource
versions. The resource versions are synchronized with the master via
'UpdateSlaveMessage'.


Diffs (updated)
-

  src/common/protobuf_utils.hpp 95f57da820db676874302f1c3138a76c22a88593 
  src/common/protobuf_utils.cpp 7a4b87becbe73f0fa97ebceee6b71ede8dce90dd 
  src/master/master.hpp 0c1253aa1a88d113d42ce362d6992ad84217728c 
  src/master/master.cpp ee212c1a21b432d3aa8e90d748d12cb0a754d57c 
  src/slave/slave.hpp df1b0205124555dcb6a0efa5c237f5e77fa2bdf7 
  src/slave/slave.cpp c10823985154bac19f8952b94311a03b2b9b4ea1 


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

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


Testing
---

`make check`, additional testing as part of https://reviews.apache.org/r/63496/.


Thanks,

Benjamin Bannier

Re: Review Request 63493: Transmitted agent resource versions in (re)registration.

[Benjamin Bannier]

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

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


Review request for mesos, Gaston Kleiman, Greg Mann, Jie Yu, and Jan Schlicht.


Changes
---

Rebased.


Repository: mesos


Description
---

This commit introduces resource version fields into agent registration
and reregistration message. The agent is changed to set these fields
when needed; the master in turn stores the versions for use in to be
added speculated offer operations.


Diffs (updated)
-

  src/messages/messages.proto 0cc6b40dff5d68ca7ab1a25990646f87c8a321cd 
  src/slave/slave.cpp c10823985154bac19f8952b94311a03b2b9b4ea1 


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

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


Testing
---

`make check`, additional testing as part of https://reviews.apache.org/r/63496/.


Thanks,

Benjamin Bannier

Re: Review Request 63491: Added resource version to resource provider UpdateTotalResources call.

[Benjamin Bannier]

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

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


Review request for mesos, Chun-Hung Hsiao, Jie Yu, and Jan Schlicht.


Changes
---

Fixed serialization.


Repository: mesos


Description
---

This patch surfaces this information to resource provider manager
users like the agent. In a later patch we will modify the agent to
forward this information to the master.


Diffs (updated)
-

  src/resource_provider/manager.cpp a878507d71a09a415d8a4573cf2b33c26c985451 
  src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 


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

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


Testing
---

`make check`, additional testing as part of https://reviews.apache.org/r/63496/.


Thanks,

Benjamin Bannier

Re: Review Request 63434: Added IPv6 capabilities for TCP and HTTP healthchecks.

[Avinash sridharan]

> On Nov. 7, 2017, 2:22 p.m., Qian Zhang wrote:
> > I think you need to do the similar changes to the `CheckInfo` message.

`CheckInfo` is specific to the `default-executor` and hence UCR. We don't 
support IPv6 for CNI and hence UCR at this point, so hence not adding this to 
`CheckInfo` at this point.


- Avinash


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


On Nov. 3, 2017, 12:38 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63434/
> ---
> 
> (Updated Nov. 3, 2017, 12:38 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Qian Zhang, and Vinod Kone.
> 
> 
> Bugs: MESOS-8050
> https://issues.apache.org/jira/browse/MESOS-8050
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added IPv6 capabilities for TCP and HTTP healthchecks.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 12652fe2804efdd2228c8e8c95b88e1f4b762f7b 
>   include/mesos/v1/mesos.proto e0797aea6ffe200aab5b0bde5450dbd7c332618b 
> 
> 
> Diff: https://reviews.apache.org/r/63434/diff/2/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 63434: Added IPv6 capabilities for TCP and HTTP healthchecks.

[Avinash sridharan]

> On Nov. 2, 2017, 6:48 p.m., Alexander Rukletsov wrote:
> > include/mesos/mesos.proto
> > Lines 512-516 (patched)
> > 
> >
> > When I see this enum, I have several questions, for example, "how this 
> > fits into overall IPv6 story?", "shall this enum be global?", "what about 
> > `CheckInfo` message?". Since we are going to deprecate `HealchCheck` at 
> > some point, I'm fine with adding an ad-hoc solution here, but I would like 
> > to have a note here about what our long-term plan is (and that provides 
> > some sorts of answers to the questions above).
> 
> Vinod Kone wrote:
> Yea, one option is to directly use "NetworkInfo::Protocol" instead of 
> re-declaring it here.

I like that option, since it will be tied to `NetworkInfo` which will 
encapsulate our support for IPv4 and IPv6.


- Avinash


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


On Nov. 3, 2017, 12:38 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63434/
> ---
> 
> (Updated Nov. 3, 2017, 12:38 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Qian Zhang, and Vinod Kone.
> 
> 
> Bugs: MESOS-8050
> https://issues.apache.org/jira/browse/MESOS-8050
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added IPv6 capabilities for TCP and HTTP healthchecks.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 12652fe2804efdd2228c8e8c95b88e1f4b762f7b 
>   include/mesos/v1/mesos.proto e0797aea6ffe200aab5b0bde5450dbd7c332618b 
> 
> 
> Diff: https://reviews.apache.org/r/63434/diff/2/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Re: Review Request 63641: Used move for events consumption in master.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [63628, 63629, 63630, 63631, 63632, 63633, 63634, 63635, 
63636, 63637, 63638, 63639, 63641]

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 Nov. 7, 2017, 5:42 p.m., Dmitry Zhuk wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63641/
> ---
> 
> (Updated Nov. 7, 2017, 5:42 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Michael Park.
> 
> 
> Bugs: MESOS-6972
> https://issues.apache.org/jira/browse/MESOS-6972
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used move for events consumption in master.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 0c1253aa1a88d113d42ce362d6992ad84217728c 
>   src/master/master.cpp ee212c1a21b432d3aa8e90d748d12cb0a754d57c 
> 
> 
> Diff: https://reviews.apache.org/r/63641/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Dmitry Zhuk
> 
>

Re: Review Request 63622: Provided handling for offer operation updates.

[Jie Yu]

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




src/resource_provider/message.hpp
Lines 48-52 (patched)


Given this is really just `OfferOperationStatusUpdate` and we'll need to 
construct that in the agent anyway, i'd prefer to use just that:
```
struct UpdateOfferOperationStatus
{
  OfferOperationStatusUpdate update;
};
```

I was debating if we need to include `ResourceProviderID` here or not. One 
idea is to add an optional resource_provider_id in `OfferOperationStatusUpdate` 
in the future for ERP. We don't have to do it now.



src/slave/slave.hpp
Lines 670 (patched)


This is related to if we want to do bookkeeping for Offer operations in the 
agent.

Given that in `UpdateState` call, RP will inform the RP manager about all 
known `OfferOperation`s, i think it makes sense to bookkeep `OfferOperation`s 
in the agent too (instead of just Offer::Operation).

We probably need to mimic what we do in the master for bookkeeping.

Think about a agent failover, this information will be lost and agent needs 
to reconstruct that information. Otherwise, when it receives a status udpate 
about an offer operation, it'll drop it according to the current logic below!



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


Ditto. THis will probably be `addOfferOperation(...)`



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


You cannot remove this if the latest status is not terminal.



src/slave/slave.cpp
Lines 6823-6825 (patched)


We still want to forward the status udpate for those operations, right? 
Just we don't need to adjust the `totalResources` of the agent.

I think a better flow here is:
```
vector conversions;

// Calculate conversions based on the status update.
// This only applies to new operations and latest
// status is terminal. For other cases, conversions
// will be empty.

totalResources.apply(conversions);

// Forward status update.
```



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


we should look at `latest_status`.



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


Let's do the subtraction first (and a contains check as Benjamin suggested).



src/slave/slave.cpp
Lines 6861-6865 (patched)


Let's log a warning here (dropping status update). Status update manager in 
the RP will retry the status update eventually.


- Jie Yu


On Nov. 7, 2017, 2:20 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63622/
> ---
> 
> (Updated Nov. 7, 2017, 2:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When a resource provider has finished an offer operation, it will send
> a status update to the resource provider manager and subsequently to an
> agent. The agent then updates its internal bookkeeping and forwards the
> status update to the master.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/manager.cpp a878507d71a09a415d8a4573cf2b33c26c985451 
>   src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
>   src/slave/slave.hpp df1b0205124555dcb6a0efa5c237f5e77fa2bdf7 
>   src/slave/slave.cpp c10823985154bac19f8952b94311a03b2b9b4ea1 
>   src/tests/resource_provider_manager_tests.cpp 
> 4008b1c751d6227b99adef756e95174d7d8a62f2 
> 
> 
> Diff: https://reviews.apache.org/r/63622/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 63642: Added a test for ExecutorID validation in ReregisterSlaveMessage.

[Mesos Reviewbot Windows]

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



FAIL: Some Mesos libprocess-tests failed.

Reviews applied: `['63642']`

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

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

Relevant logs:

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

```
[   OK ] Scheme/HTTPTest.WWWAuthenticateHeader/0 (642 ms)
[ RUN  ] Scheme/HTTPTest.WWWAuthenticateHeader/1
[   OK ] Scheme/HTTPTest.WWWAuthenticateHeader/1 (74 ms)
[ RUN  ] Scheme/HTTPTest.Accepts/0
[   OK ] Scheme/HTTPTest.Accepts/0 (890 ms)
[ RUN  ] Scheme/HTTPTest.Accepts/1
[   OK ] Scheme/HTTPTest.Accepts/1 (96 ms)
[--] 34 tests from Scheme/HTTPTest (13598 ms total)

[--] 4 tests from SSLVerifyIPAdd/SSLTest
[ RUN  ] SSLVerifyIPAdd/SSLTest.BasicSameProcess/0
[   OK ] SSLVerifyIPAdd/SSLTest.BasicSameProcess/0 (612 ms)
[ RUN  ] SSLVerifyIPAdd/SSLTest.BasicSameProcess/1
[   OK ] SSLVerifyIPAdd/SSLTest.BasicSameProcess/1 (502 ms)
[ RUN  ] SSLVerifyIPAdd/SSLTest.RequireCertificate/0
[   OK ] SSLVerifyIPAdd/SSLTest.RequireCertificate/0 (1127 ms)
[ RUN  ] SSLVerifyIPAdd/SSLTest.RequireCertificate/1
[   OK ] SSLVerifyIPAdd/SSLTest.RequireCertificate/1 (1510 ms)
[--] 4 tests from SSLVerifyIPAdd/SSLTest (3891 ms total)

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

 1 FAILED TEST
  YOU HAVE 19 DISABLED TESTS

```

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

```
I1107 19:58:38.642331  1992 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\aPYfLx\cert.pem
WARNING: Logging before InitGoogleLogging() is written to STDERR
I1107 19:58:40.196025  3484 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1107 19:58:40.201027  3484 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I1107 19:58:40.201027  3484 openssl.cpp:526] Will use IP address verification 
in subject alternative name certificate extension.
I1107 19:58:40.201027  3484 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1107 19:58:40.202312  3484 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\UQiHlS\cert.pem
ble peer certificate verification
I1107 19:58:37.618350  4900 openssl.cpp:526] Will use IP address verification 
in subject alternative name certificate extension.
I1107 19:58:37.618350  4900 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1107 19:58:37.619318  4900 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\Ms4ejE\cert.pem
I1107 19:58:37.619318  4900 openssl.cpp:566] Using CA dir: 
C:\Users\mesos\AppData\Local\Temp\Ms4ejE
I1107 19:58:38.339325  4900 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1107 19:58:38.339325  4900 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I1107 19:58:38.340327  4900 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1107 19:58:38.340327  4900 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\aPYfLx\cert.pem
I1107 19:58:39.894395  4900 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1107 19:58:39.894395  4900 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I1107 19:58:39.894395  4900 openssl.cpp:526] Will use IP address verification 
in subject alternative name certificate extension.
I1107 19:58:39.895020  4900 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1107 19:58:39.895020  4900 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\UQiHlS\cert.pem
I1107 19:58:40.387504  4144 process.cpp:1067] Failed to accept socket: future 
discarded
```

- Mesos Reviewbot Windows


On Nov. 7, 2017, 6:47 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> 

Re: Review Request 62997: Added checkpoint and recover capability for layers in provisioner.

[Gilbert Song]

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




src/slave/containerizer/mesos/provisioner/paths.hpp
Line 55 (original), 59-60 (patched)


We should use `xxxPath()` consistently in this file. Could you add a TODO 
for refactoring?



src/slave/containerizer/mesos/provisioner/provisioner.hpp
Lines 170 (patched)


Should we add comments for removing `Option` after a deprecation cycle?



src/slave/containerizer/mesos/provisioner/provisioner.cpp
Lines 152-187 (patched)


do we really need the static functions? I think we can just do the 
checkpointing and read in the code since I dont see posibilities we will 
checkpoint twice in the near future.



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


let's use state::checkpoint() instead.


- Gilbert Song


On Oct. 27, 2017, 11:03 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62997/
> ---
> 
> (Updated Oct. 27, 2017, 11:03 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-8079
> https://issues.apache.org/jira/browse/MESOS-8079
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added checkpoint and recover capability for layers in provisioner.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> 689acfcbbb07f071b6195472118a7a7520a44abd 
>   src/slave/containerizer/mesos/provisioner/paths.hpp 
> 466f5edab40732b0d8da4252a71fde9c2956e8e9 
>   src/slave/containerizer/mesos/provisioner/paths.cpp 
> 268dbeb4b18374ef53bc73254bf20ce6830e384f 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 450a3b32d69d2882973a6ed4e94e169a0256056b 
> 
> 
> Diff: https://reviews.apache.org/r/62997/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 63641: Used move for events consumption in master.

[Mesos Reviewbot Windows]

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



FAIL: mesos-java failed to build.

Reviews applied: `['63628', '63629', '63630', '63631', '63632', '63633', 
'63634', '63635', '63636', '63637', '63638', '63639', '63641']`

Failed command: `cmake.exe --build . --target mesos-java`

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

Relevant logs:

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

```
  C:\DCOS\mesos\mesos\3rdparty\libprocess\include\process/future.hpp(412): 
error C2440: '': cannot convert from 
'process::network::internal::PollSocketImpl::accept::'
 to 'lambda::CallableOnce> (const T &)>' 
[C:\DCOS\mesos\3rdparty\libprocess\src\process.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\libprocess\include\process/future.hpp(412): 
error C2440: '': cannot convert from 
'std::_Binder' to 'lambda::CallableOnce' [C:\DCOS\mesos\3rdparty\libprocess\src\process.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\libprocess\include\process/future.hpp(412): 
error C2440: '': cannot convert from 
'std::_Binder (__cdecl *)(const 
std::shared_ptr &,const char 
*,::size_t),const std::shared_ptr 
&,const char *&,size_t &>' to 'lambda::CallableOnce 
(const T &)>' [C:\DCOS\mesos\3rdparty\libprocess\src\process.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\libprocess\include\process/future.hpp(412): 
error C2440: '': cannot convert from 
'std::_Binder (__cdecl *)(const 
std::shared_ptr 
&,int_fd,off_t,::size_t),const 
std::shared_ptr &,int_fd &,off_t 
&,size_t &>' to 'lambda::CallableOnce (const T &)>' 
[C:\DCOS\mesos\3rdparty\libprocess\src\process.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\libprocess\include\process/future.hpp(412): 
error C2440: '': cannot convert from 
'std::_Binder (__cdecl *)(const 
std::shared_ptr &,const char 
*,::size_t),std::shared_ptr,const 
char *&,size_t &>' to 'lambda::CallableOnce (const T 
&)>' [C:\DCOS\mesos\3rdparty\libprocess\src\process.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\libprocess\include\process/future.hpp(412): 
error C2440: '': cannot convert from 
'std::_Binder (__cdecl *)(const 
std::shared_ptr 
&,int_fd,off_t,::size_t),std::shared_ptr,int_fd
 &,off_t &,size_t &>' to 'lambda::CallableOnce (const 
T &)>' [C:\DCOS\mesos\3rdparty\libprocess\src\process.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\libprocess\include\process/dispatch.hpp(340): 
error C2665: 'lambda::CallableOnce::CallableOnce': none of the 3 overloads could convert all the argument 
types (compiling source file 
C:\DCOS\mesos\mesos\3rdparty\libprocess\src\reap.cpp) 
[C:\DCOS\mesos\3rdparty\libprocess\src\process.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\libprocess\include\process/future.hpp(412): 
error C2440: '': cannot convert from 
'std::_Binder &,const Option 
&,process::Owned,::size_t,boost::shared_array,::size_t),const
 std::shared_ptr<_Ty> &,const Option &,process::Owned 
&,size_t &,boost::shared_array &,const std::_Ph<1> &>' to 
'lambda::CallableOnce' 
[C:\DCOS\mesos\3rdparty\libprocess\src\process.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\libprocess\include\process/future.hpp(412): 
error C2440: '': cannot convert from 
'std::_Binder &,const Option 
&,process::Owned,::size_t,boost::shared_array,::size_t),std::shared_ptr<_Ty>,const
 Option &,process::Owned &,size_t 
&,boost::shared_array &,const std::_Ph<1> &>' to 
'lambda::CallableOnce' 
[C:\DCOS\mesos\3rdparty\libprocess\src\process.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\libprocess\include\process/future.hpp(412): 
error C2440: '': cannot convert from 
'std::_Binder &,process::Owned,::size_t,::size_t),const 
std::shared_ptr<_Ty> &,process::Owned &,size_t &,const std::_Ph<1> 
&>' to 'lambda::CallableOnce' 
[C:\DCOS\mesos\3rdparty\libprocess\src\process.vcxproj]
  C:\DCOS\mesos\mesos\3rdparty\libprocess\include\process/future.hpp(412): 
error C2440: '': cannot convert from 
'std::_Binder 

Re: Review Request 63625: Added a test for resource conversion using a resource provider.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the dependent review: 63622.

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

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

Relevant logs:

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

```
error: patch failed: src/resource_provider/message.hpp:34
error: src/resource_provider/message.hpp: patch does not apply
```

- Mesos Reviewbot Windows


On Nov. 7, 2017, 3:36 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63625/
> ---
> 
> (Updated Nov. 7, 2017, 3:36 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test for resource conversion using a resource provider.
> 
> 
> Diffs
> -
> 
>   src/tests/resource_provider_manager_tests.cpp 
> 4008b1c751d6227b99adef756e95174d7d8a62f2 
> 
> 
> Diff: https://reviews.apache.org/r/63625/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 63615: Passed scheduler as a shared pointer into the callback.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63615 was successfully built and tested.

Reviews applied: `['63611', '63612', '63613', '63614', '63615']`

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

- Mesos Reviewbot Windows


On Nov. 7, 2017, 12:58 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63615/
> ---
> 
> (Updated Nov. 7, 2017, 12:58 p.m.)
> 
> 
> Review request for mesos, Michael Park and Vinod Kone.
> 
> 
> Bugs: MESOS-8096
> https://issues.apache.org/jira/browse/MESOS-8096
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To ensure the lifetime of the scheduler is longer than the lifetime
> of the scheduler library driver, pass scheduler as a shared_ptr.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp e25defeb55608136e77363aa48cf820092a13a59 
> 
> 
> Diff: https://reviews.apache.org/r/63615/diff/1/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.11.6 with Apple LLVM version 8.0.0 (clang-800.0.42.1)
> make check on various linux distributions
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Review Request 63642: Added a test for ExecutorID validation in ReregisterSlaveMessage.

[James Peach]

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

Review request for mesos, James DeFelice and Jiang Yan Xu.


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


Repository: mesos


Description
---

Added a test to ensure that the ReregisterSlaveMessage validation
correctly allows duplicate ExecutorIDs as long as they are scoped
to different frameworks.


Diffs
-

  src/python/cli_new/README.md 847141db9a1f9eb5c78b2d50367b599a5c72ce61 
  src/python/cli_new/lib/cli/util.py 307b22293a9c7199ad7088dfd0db6dff83a08ac8 
  src/python/cli_new/pip-requirements.txt 
ac040eec3d4d2eaa5dd7b431c2f590d8ae323258 
  src/tests/api_tests.cpp d3e812a2a6302035634d14995939aafb216970df 
  src/tests/containerizer/cgroups_isolator_tests.cpp 
aad3f53c87af5e87f1b85b746d16d483ca632f83 
  src/tests/containerizer/cni_isolator_tests.cpp 
a6b660de63f767ff0ddc34791540de1427206e92 
  src/tests/containerizer/runtime_isolator_tests.cpp 
089605d9b0708cb35c171bad2c141c70336c07bd 
  src/tests/master_validation_tests.cpp 
be8c8920fdd6eb72fc77247844a731070db5fa1c 


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


Testing
---

make check (Fedora 26) with PR #248 applied.


Thanks,

James Peach

Re: Review Request 63620: Updated offer operation handling to set resource versions.

[Jie Yu]

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




src/master/master.cpp
Line 9905 (original), 9969 (patched)


I think `checkpointedResources` should be limited to agent default 
resources. So we might want to tweak `needCheckpointing` filter.

Also, I think we probably should consider using `_apply` for new operations 
too.

With the above in mind, here, we probably need to adjust the code to only 
call `slave->apply(operation)` when the opreation is old operaitons because for 
new operations, slave's total doesn't change until terminal status update.



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


I'd prefer we use CHECK here and move the validation to caller (and drop 
there). And I am pretty sure this is validated in master operation validation.

Utility functions like this should be kept without too many branches.



src/master/master.cpp
Lines 9980-9985 (patched)


YOu can use the following:
```
Option resourceVersion = resourceProviderId.isSome()
  ? slave->resourceVersions.get(resourceProviderId.get())
  : slave->resourceVersions.get(None());
```



src/master/master.cpp
Lines 9987-9995 (patched)


I would just use a CHECK_SOME here.


- Jie Yu


On Nov. 7, 2017, 2:18 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63620/
> ---
> 
> (Updated Nov. 7, 2017, 2:18 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Resource version UUIDs are used to establish a relationship between
> operation and the resource they are operating on. For each agent the
> master keeps track of its resource version and the resource version of
> the agents' resource providers. As resource versions are required in a
> 'ApplyOfferOperationMessage', the master has to find the resource
> version that's matching the resources an operation should apply to and
> set it accordingly.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp ee212c1a21b432d3aa8e90d748d12cb0a754d57c 
> 
> 
> Diff: https://reviews.apache.org/r/63620/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 63496: Added tests for agent resource version transmission.

[Jie Yu]

> On Nov. 7, 2017, 5:40 p.m., Greg Mann wrote:
> > src/tests/slave_tests.cpp
> > Lines 8679-8681 (patched)
> > 
> >
> > Would be good if this test or another could verify that a new resource 
> > version is sent for an RP after the RP makes an UPDATE_STATE call.

+1

maybe a TODO or a ticket for now. Let's note down all the tests we'll need to 
write.


- Jie


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


On Nov. 7, 2017, 12:56 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63496/
> ---
> 
> (Updated Nov. 7, 2017, 12:56 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch introduces separate tests for clock values communicated
> from resource providers and from agents to masters.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 9c2db7adf45af4209ecc475d938ce4b77c5a3ab4 
> 
> 
> Diff: https://reviews.apache.org/r/63496/diff/6/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 63496: Added tests for agent resource version transmission.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Nov. 7, 2017, 12:56 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63496/
> ---
> 
> (Updated Nov. 7, 2017, 12:56 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch introduces separate tests for clock values communicated
> from resource providers and from agents to masters.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 9c2db7adf45af4209ecc475d938ce4b77c5a3ab4 
> 
> 
> Diff: https://reviews.apache.org/r/63496/diff/6/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 63496: Added tests for agent resource version transmission.

[Jie Yu]

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




src/tests/slave_tests.cpp
Lines 8679-8681 (patched)


Let's consistently use resource version uuid (instead of 'clock') in the 
code as well as in the comment.



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


Ditto on clock



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


Ditto


- Jie Yu


On Nov. 7, 2017, 12:56 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63496/
> ---
> 
> (Updated Nov. 7, 2017, 12:56 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch introduces separate tests for clock values communicated
> from resource providers and from agents to masters.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 9c2db7adf45af4209ecc475d938ce4b77c5a3ab4 
> 
> 
> Diff: https://reviews.apache.org/r/63496/diff/6/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 63492: Synchronized agent clock with master via 'UpdateSlaveMessage'.

[Jie Yu]

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




src/master/master.cpp
Lines 6931-6932 (patched)


I'd move this to protobuf_utils `protobuf::parseResourceVersions`



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


let's use version uuid consistently, and avoid the word `clock`



src/slave/slave.cpp
Lines 1188-1207 (patched)


I'd move this helper to `src/common/protobuf_utils.hpp|cpp` and call it 
`createResourceVersions`



src/slave/slave.cpp
Line 1376 (original), 1404 (patched)


Not yours, but can you kill one line here?



src/slave/slave.cpp
Lines 6648-6651 (patched)


I am a little hesitate about this. Resource estimator can change quite 
frequently, and any change will invalidate some operations for the regular 
agent default resources.

Currently, we don't allow any operation to operate on oversubscribed 
resources yet. So I am learning towards not changing the clock here with a big 
TODO saying that we might want to do that in the future if we allow operation 
for oversubscribed resources.

Alternatively, we can keep separate clock for oversubscribed resources. We 
don't have to do it for now.



src/slave/slave.cpp
Lines 6719-6722 (patched)


can this simply be:

`resourceVersions[resourcProviderId] = resourceVersionUuid;`


- Jie Yu


On Nov. 7, 2017, 12:25 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63492/
> ---
> 
> (Updated Nov. 7, 2017, 12:25 p.m.)
> 
> 
> Review request for mesos, Gaston Kleiman, Greg Mann, Jie Yu, and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit introduces agent clocks to the master and agents. Agents
> are responsible for maintaining their clock. The clocks are
> synchronized with the master via 'UpdateSlaveMessage'.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp afcc2e46882e4c610e9047ab3db7b6f100d47e17 
>   src/master/master.cpp 49bc50ead75adba82a7d8de23a87b06ccd269c48 
>   src/slave/slave.hpp df1b0205124555dcb6a0efa5c237f5e77fa2bdf7 
>   src/slave/slave.cpp 79ee163b41ade93cae1054985379d61faf6a081a 
> 
> 
> Diff: https://reviews.apache.org/r/63492/diff/5/
> 
> 
> Testing
> ---
> 
> `make check`, additional testing as part of 
> https://reviews.apache.org/r/63496/.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 63606: Avoid an extra copy during ProtobufProcess::send.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63606 was successfully built and tested.

Reviews applied: `['63604', '63605', '63606']`

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

- Mesos Reviewbot Windows


On Nov. 7, 2017, 4:23 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63606/
> ---
> 
> (Updated Nov. 7, 2017, 4:23 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/protobuf.hpp 
> fe152f273332470ac50f9715291897bb04cf95b9 
> 
> 
> Diff: https://reviews.apache.org/r/63606/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Review Request 63641: Used move for events consumption in master.

[Dmitry Zhuk]

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

Review request for mesos, Benjamin Mahler and Michael Park.


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


Repository: mesos


Description
---

Used move for events consumption in master.


Diffs
-

  src/master/master.hpp 0c1253aa1a88d113d42ce362d6992ad84217728c 
  src/master/master.cpp ee212c1a21b432d3aa8e90d748d12cb0a754d57c 


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


Testing
---

make check


Thanks,

Dmitry Zhuk

Re: Review Request 63496: Added tests for agent resource version transmission.

[Greg Mann]

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




src/tests/slave_tests.cpp
Lines 8679-8681 (patched)


Would be good if this test or another could verify that a new resource 
version is sent for an RP after the RP makes an UPDATE_STATE call.



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


Could you update this comment?


- Greg Mann


On Nov. 7, 2017, 12:56 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63496/
> ---
> 
> (Updated Nov. 7, 2017, 12:56 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch introduces separate tests for clock values communicated
> from resource providers and from agents to masters.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 9c2db7adf45af4209ecc475d938ce4b77c5a3ab4 
> 
> 
> Diff: https://reviews.apache.org/r/63496/diff/6/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 63625: Added a test for resource conversion using a resource provider.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [63625, 63622, 63621, 63620, 63496, 63495, 63494, 63493, 
63492, 63491, 61183, 63519, 63540]

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

Error:
2017-11-07 17:22:07 URL:https://reviews.apache.org/r/63622/diff/raw/ 
[11597/11597] -> "63622.patch" [1]
error: patch failed: src/resource_provider/message.hpp:34
error: src/resource_provider/message.hpp: patch does not apply

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

- Mesos Reviewbot


On Nov. 7, 2017, 3:36 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63625/
> ---
> 
> (Updated Nov. 7, 2017, 3:36 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test for resource conversion using a resource provider.
> 
> 
> Diffs
> -
> 
>   src/tests/resource_provider_manager_tests.cpp 
> 4008b1c751d6227b99adef756e95174d7d8a62f2 
> 
> 
> Diff: https://reviews.apache.org/r/63625/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Review Request 63639: Enabled rvalue reference parameters in protobuf handlers.

[Dmitry Zhuk]

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

Review request for mesos, Benjamin Mahler and Michael Park.


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


Repository: mesos


Description
---

Using rvalue reference parameter in protobuf handler opts-out of arena
usage, and allows moving message further in dispatch chain.


Diffs
-

  3rdparty/libprocess/include/process/protobuf.hpp 
fe152f273332470ac50f9715291897bb04cf95b9 


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


Testing
---

make check


Thanks,

Dmitry Zhuk

Review Request 63638: Added callable once support in Future.

[Dmitry Zhuk]

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

Review request for mesos, Benjamin Mahler and Michael Park.


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


Repository: mesos


Description
---

`Future` guarantees that callbacks are called at most once, so it can
use `lambda::CallableOnce` to expicitly declare this, and allow
corresponding optimizations with moves.


Diffs
-

  3rdparty/libprocess/include/process/future.hpp 
4cf44ba1a7052d5f84beb8fd6914b8e9e0437c0a 
  3rdparty/libprocess/src/tests/future_tests.cpp 
76a32bd69499e52ea1623ab982d65e1c7a0cbd32 


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


Testing
---

make check


Thanks,

Dmitry Zhuk

Review Request 63637: Added callable once support in defer.

[Dmitry Zhuk]

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

Review request for mesos, Benjamin Mahler and Michael Park.


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


Repository: mesos


Description
---

This allows `defer` result to be converted to `CallableOnce`, ensuring
that bound arguments are moved, when call is made, and avoiding making
copies of bound arguments.


Diffs
-

  3rdparty/libprocess/include/process/deferred.hpp 
af65eadee6f29a5bcd29b5c8ccf7444f39f8899a 
  3rdparty/libprocess/src/tests/process_tests.cpp 
952c92c033e2363cff0c2c68610d3820b97d177e 


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


Testing
---

make check


Thanks,

Dmitry Zhuk

Review Request 63631: Separated event visiting and consumption.

[Dmitry Zhuk]

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

Review request for mesos, Benjamin Mahler and Michael Park.


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


Repository: mesos


Description
---

This introduces `EventConsumer` interface, which add support of events
with movable only data. This allows consumers to move data out of
event, rather than copying it. This is required to implement movable
only objects support in `defer` to guarantee that deferred functor is
invoked only once, allowing deferred parameters to be moved into call.


Diffs
-

  3rdparty/libprocess/include/process/event.hpp 
2e246205020c3c5b8c2eb5187a8eb3643d1e6d4d 
  3rdparty/libprocess/include/process/process.hpp 
dc3375ce62556322eb2bc60ade61f313ade123b8 
  3rdparty/libprocess/include/process/protobuf.hpp 
fe152f273332470ac50f9715291897bb04cf95b9 
  3rdparty/libprocess/include/process/run.hpp 
1540a78e52a90d4c1d4165c46be353caaad21bce 
  3rdparty/libprocess/src/process.cpp 71ae7129ffbd0e22eda2863b17bbcf588298c37b 
  3rdparty/libprocess/src/tests/benchmarks.cpp 
e6c77d565d5acf72b475a085e9504679253b4b97 
  3rdparty/libprocess/src/tests/process_tests.cpp 
952c92c033e2363cff0c2c68610d3820b97d177e 


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


Testing
---


Thanks,

Dmitry Zhuk

Review Request 63629: Added partial function application implementation.

[Dmitry Zhuk]

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

Review request for mesos, Benjamin Mahler and Michael Park.


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


Repository: mesos


Description
---

`lambda::partial` performs partial function application,
similar to `std::bind`. However, unlike `std::bind`, it supports
functors accepting rvalue reference arguments. In this case `operator()`
must be called on `std::move`d partial.


Diffs
-

  3rdparty/stout/include/stout/lambda.hpp 
a61d97b69e7eebd057c94148d39c6e1fc3066017 
  3rdparty/stout/tests/lambda_tests.cpp 
ad8c2efddb6b64184670d0cfb33188ef843351ab 


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


Testing
---

make check


Thanks,

Dmitry Zhuk

Review Request 63633: Added ability to protect nested bind expressions from composition.

[Dmitry Zhuk]

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

Review request for mesos, Benjamin Mahler and Michael Park.


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


Repository: mesos


Description
---

`std::bind` performs function composition when it encounters nested bind
expression. However in some cases this can have undesirable effects, and
`lambda::protect` can be used to avoid such behavior.


Diffs
-

  3rdparty/stout/include/stout/lambda.hpp 
a61d97b69e7eebd057c94148d39c6e1fc3066017 


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


Testing
---


Thanks,

Dmitry Zhuk

Review Request 63636: Added placeholder implementation.

[Dmitry Zhuk]

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

Review request for mesos, Benjamin Mahler and Michael Park.


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


Repository: mesos


Description
---

Added placeholder implementation.


Diffs
-

  3rdparty/stout/include/stout/lambda.hpp 
a61d97b69e7eebd057c94148d39c6e1fc3066017 


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


Testing
---


Thanks,

Dmitry Zhuk

Review Request 63635: Prepared defer for use in callable once contexts.

[Dmitry Zhuk]

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

Review request for mesos, Benjamin Mahler and Michael Park.


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


Repository: mesos


Description
---

This changes `defer` to use `lambda::partial` instead of `std::bind`,
which allows it be used in callbale once contexts.


Diffs
-

  3rdparty/libprocess/include/process/defer.hpp 
9457080a638d7bf444504b82629a7bc6b2de4116 
  3rdparty/libprocess/include/process/deferred.hpp 
af65eadee6f29a5bcd29b5c8ccf7444f39f8899a 


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


Testing
---

make check


Thanks,

Dmitry Zhuk

Review Request 63630: Added support for callable once functors.

[Dmitry Zhuk]

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

Review request for mesos, Benjamin Mahler and Michael Park.


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


Repository: mesos


Description
---

`CallableOnce` class is similar to `std::function`, but allows it to be
called only once. Together with `partial` this provides foundation for
copy-less `defer`, `dispatch` and `Future`.


Diffs
-

  3rdparty/stout/include/stout/lambda.hpp 
a61d97b69e7eebd057c94148d39c6e1fc3066017 


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


Testing
---


Thanks,

Dmitry Zhuk

Review Request 63634: Changed dispatch to use callable once functors.

[Dmitry Zhuk]

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

Review request for mesos, Benjamin Mahler and Michael Park.


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


Repository: mesos


Description
---

`dispatch` guarantees that functor will be called at most once, and
therefore it allows optimizations, such as moves of deferred objects.


Diffs
-

  3rdparty/libprocess/include/process/dispatch.hpp 
096cae037aec29059e1f13ac467bffe94ecd14ba 
  3rdparty/libprocess/include/process/event.hpp 
2e246205020c3c5b8c2eb5187a8eb3643d1e6d4d 
  3rdparty/libprocess/src/process.cpp 71ae7129ffbd0e22eda2863b17bbcf588298c37b 
  3rdparty/libprocess/src/tests/process_tests.cpp 
952c92c033e2363cff0c2c68610d3820b97d177e 


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


Testing
---

make check


Thanks,

Dmitry Zhuk

Review Request 63632: Migrated to event consumer interface.

[Dmitry Zhuk]

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

Review request for mesos, Benjamin Mahler and Michael Park.


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


Repository: mesos


Description
---

Migrated to event consumer interface.


Diffs
-

  src/common/recordio.hpp 378363492e04c141671fbed0467bf558b61e5dae 
  src/master/master.hpp 0c1253aa1a88d113d42ce362d6992ad84217728c 
  src/master/master.cpp ee212c1a21b432d3aa8e90d748d12cb0a754d57c 
  src/tests/fetcher_cache_tests.cpp e2ecb102e2e7b4c4b3b2b736fdc453feeb15816e 


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


Testing
---

make check


Thanks,

Dmitry Zhuk

Review Request 63628: Implemented index_sequence and related functionality.

[Dmitry Zhuk]

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

Review request for mesos, Benjamin Mahler and Michael Park.


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


Repository: mesos


Description
---

This adds implementation of C++14 STL's `index_squence`
and `make_index_sequence` templates.


Diffs
-

  3rdparty/stout/include/stout/utils.hpp 
82cab830817842c23056235973ec4419ee9ec8b1 


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


Testing
---


Thanks,

Dmitry Zhuk

Re: Review Request 61183: Triggered 'UpdateSlaveMessage' when 'ResourceProviderManager' updates.

[Greg Mann]

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


Ship it!




Ship It!

- Greg Mann


On Nov. 7, 2017, 12:25 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61183/
> ---
> 
> (Updated Nov. 7, 2017, 12:25 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The agent's resource provider manager sends a
> 'ResourceProviderMessage' when its managed resources change. This
> commit adds handling in the agent so that an 'UpdateSlaveMessage' is
> sent to the master to update the total resource available on the
> agent.
> 
> In order to provide push-like handling of the resource provider
> manager's message queue, we chain recursive calls to the handler for
> continuous processing. Initially, processing is kicked off from
> 'Slave::initialize'. In this simple implementation we e.g., provide no
> direct way to stop processing of messages, yet, but it can be achieved
> by e.g., replacing the manager with a new instance (this would also
> require updating routes).
> 
> Since the agent can only send an 'UpdateSlaveMessage' when it is
> registered with a master, a simple back-off of 5 s is implemented which
> will defer processing of a ready message should the agent not yet have
> registered.
> 
> To facilitate logging we add a stringification function for
> 'ResourceProviderMessage's.
> 
> This patch also adjusts a number of tests to no expect two
> 'UpdateSlaveMessage's.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
>   src/slave/slave.hpp df1b0205124555dcb6a0efa5c237f5e77fa2bdf7 
>   src/slave/slave.cpp 79ee163b41ade93cae1054985379d61faf6a081a 
>   src/tests/slave_tests.cpp 9c2db7adf45af4209ecc475d938ce4b77c5a3ab4 
> 
> 
> Diff: https://reviews.apache.org/r/61183/diff/22/
> 
> 
> Testing
> ---
> 
> Tested on a number of platforms on internal CI.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 63625: Added a test for resource conversion using a resource provider.

[Benjamin Bannier]

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




src/tests/resource_provider_manager_tests.cpp
Lines 730 (patched)


Let's instead `ASSERT` on the value from `subscribed->provider_id()`.



src/tests/resource_provider_manager_tests.cpp
Lines 738-739 (patched)


Maybe use a helper here?

disk.mutable_disk()->mutable_source()->CopyFrom(createDiskSourceRaw());



src/tests/resource_provider_manager_tests.cpp
Lines 749 (patched)


This does not guarantee that the allocator will offer the resources from 
the resource provider in the next offer cycle since it might still be behind 
the master's view.

We could instead e.g., reject decline all offers not containing the 
resource provider resources below; maybe you have another idea.



src/tests/resource_provider_manager_tests.cpp
Lines 754 (patched)


Might not need this for a single use below.



src/tests/resource_provider_manager_tests.cpp
Lines 817 (patched)


We should add helpers for the new operations to `src/tests/mesos.hpp`, and 
use them here, e.g.,

mesos::v1::Offer::Operation = v1::CREATE_BLOCK(disk);



src/tests/resource_provider_manager_tests.cpp
Lines 828 (patched)


Let's `ASSERT(operation->info().has_create_block())`.



src/tests/resource_provider_manager_tests.cpp
Lines 835 (patched)


Can we run the whole test with paused clock? It seems that the allocator 
could still be assembling an offer before we pause the clock so we do not get 
the expected offer below.



src/tests/resource_provider_manager_tests.cpp
Lines 858 (patched)


`const` ref.



src/tests/resource_provider_manager_tests.cpp
Lines 867 (patched)


`ASSERT_SOME(block)`.



src/tests/resource_provider_manager_tests.cpp
Lines 869 (patched)


Break the line after `,`.


- Benjamin Bannier


On Nov. 7, 2017, 4:36 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63625/
> ---
> 
> (Updated Nov. 7, 2017, 4:36 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test for resource conversion using a resource provider.
> 
> 
> Diffs
> -
> 
>   src/tests/resource_provider_manager_tests.cpp 
> 4008b1c751d6227b99adef756e95174d7d8a62f2 
> 
> 
> Diff: https://reviews.apache.org/r/63625/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 63496: Added tests for agent resource version transmission.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63496 was successfully built and tested.

Reviews applied: `['63540', '63519', '61183', '63491', '63492', '63493', 
'63494', '63495', '63496']`

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

- Mesos Reviewbot Windows


On Nov. 7, 2017, 12:56 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63496/
> ---
> 
> (Updated Nov. 7, 2017, 12:56 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch introduces separate tests for clock values communicated
> from resource providers and from agents to masters.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 9c2db7adf45af4209ecc475d938ce4b77c5a3ab4 
> 
> 
> Diff: https://reviews.apache.org/r/63496/diff/6/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 63581: Created virtual environment for linters in /support.

[Kevin Klues]

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


Ship it!





support/mesos-style.py
Line 383 (original), 331 (patched)


This should be abstracted out so it's not hardcoded here. I talked to 
Armand offline about this though, and this change is coming in a subsequent 
commit.


- Kevin Klues


On Nov. 6, 2017, 4:50 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63581/
> ---
> 
> (Updated Nov. 6, 2017, 4:50 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7924
> https://issues.apache.org/jira/browse/MESOS-7924
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change affects the Python linter but not the C++ linter as we
> use a customized version of cpplint that we cannot get using pip.
> 
> 
> Diffs
> -
> 
>   src/python/pylint.config  
>   support/build-virtualenv PRE-CREATION 
>   support/mesos-style.py b2fdac9f8f76bdf8d8ede9ad8a056e4bb8c2754c 
>   support/pip-requirements.txt PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63581/diff/4/
> 
> 
> Testing
> ---
> 
> Used the linters after applying this patch.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 63615: Passed scheduler as a shared pointer into the callback.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [63611, 63612, 63613, 63614, 63615]

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 Nov. 7, 2017, 12:58 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63615/
> ---
> 
> (Updated Nov. 7, 2017, 12:58 p.m.)
> 
> 
> Review request for mesos, Michael Park and Vinod Kone.
> 
> 
> Bugs: MESOS-8096
> https://issues.apache.org/jira/browse/MESOS-8096
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> To ensure the lifetime of the scheduler is longer than the lifetime
> of the scheduler library driver, pass scheduler as a shared_ptr.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp e25defeb55608136e77363aa48cf820092a13a59 
> 
> 
> Diff: https://reviews.apache.org/r/63615/diff/1/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.11.6 with Apple LLVM version 8.0.0 (clang-800.0.42.1)
> make check on various linux distributions
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 63581: Created virtual environment for linters in /support.

[Kevin Klues]

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




support/mesos-style.py
Line 372 (original), 370 (patched)


This comment is innacurrate now. We can probably just remove it.


- Kevin Klues


On Nov. 6, 2017, 4:50 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63581/
> ---
> 
> (Updated Nov. 6, 2017, 4:50 p.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Bugs: MESOS-7924
> https://issues.apache.org/jira/browse/MESOS-7924
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change affects the Python linter but not the C++ linter as we
> use a customized version of cpplint that we cannot get using pip.
> 
> 
> Diffs
> -
> 
>   src/python/pylint.config  
>   support/build-virtualenv PRE-CREATION 
>   support/mesos-style.py b2fdac9f8f76bdf8d8ede9ad8a056e4bb8c2754c 
>   support/pip-requirements.txt PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63581/diff/2/
> 
> 
> Testing
> ---
> 
> Used the linters after applying this patch.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 62938: Added ZooKeeper leader resolution to CLI.

[Kevin Klues]

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


Ship it!




Ship It!

- Kevin Klues


On Nov. 7, 2017, 3:50 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62938/
> ---
> 
> (Updated Nov. 7, 2017, 3:50 p.m.)
> 
> 
> Review request for mesos, Eric Chung and Kevin Klues.
> 
> 
> Bugs: MESOS-8012
> https://issues.apache.org/jira/browse/MESOS-8012
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change allows users to use the CLI with a Mesos running in high
> availability mode. The `zookeeper` field was already here before this
> commit, with an `addresses` array and a `path` field. This change
> only adds the backend to actually make it usable.
> 
> Interacting with ZooKeeper requires a new dependency, kazoo, that has
> been added to the list of requirements for the CLI.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/README.md a4b270d9b54cdd83bd62530fb44ddfaffb8a014b 
>   src/python/cli_new/lib/cli/util.py dd109c09368e650b7d7dabd663c11bc2e3e5180a 
>   src/python/cli_new/pip-requirements.txt 
> 7aeac344c47ccd2588fded44d7314db7abd85653 
> 
> 
> Diff: https://reviews.apache.org/r/62938/diff/5/
> 
> 
> Testing
> ---
> 
> Tested with a Mesos cluster running with 3 masters and 1 agent, run some CLI 
> commands successfully.
> 
> To use Mesos in HAM locally I have done (on macOS):
> ```
> zkServer start
> bash mesos-master.sh --port='5061' --zk='zk://127.0.0.1:2181/mesos' 
> --work_dir='/tmp/master1' --quorum=1
> bash mesos-master.sh --port='5062' --zk='zk://127.0.0.1:2181/mesos' 
> --work_dir='/tmp/master2' --quorum=1
> bash mesos-master.sh --port='5063' --zk='zk://127.0.0.1:2181/mesos' 
> --work_dir='/tmp/master3' --quorum=1
> bash mesos-agent.sh --master='zk://127.0.0.1:2181/mesos' 
> --work_dir='/tmp/agent'
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 62938: Added ZooKeeper leader resolution to CLI.

[Armand Grillet]

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

(Updated Nov. 7, 2017, 3:50 p.m.)


Review request for mesos, Eric Chung and Kevin Klues.


Changes
---

Fixed issues.


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


Repository: mesos


Description (updated)
---

This change allows users to use the CLI with a Mesos running in high
availability mode. The `zookeeper` field was already here before this
commit, with an `addresses` array and a `path` field. This change
only adds the backend to actually make it usable.

Interacting with ZooKeeper requires a new dependency, kazoo, that has
been added to the list of requirements for the CLI.


Diffs (updated)
-

  src/python/cli_new/README.md a4b270d9b54cdd83bd62530fb44ddfaffb8a014b 
  src/python/cli_new/lib/cli/util.py dd109c09368e650b7d7dabd663c11bc2e3e5180a 
  src/python/cli_new/pip-requirements.txt 
7aeac344c47ccd2588fded44d7314db7abd85653 


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

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


Testing
---

Tested with a Mesos cluster running with 3 masters and 1 agent, run some CLI 
commands successfully.


Thanks,

Armand Grillet

Re: Review Request 63622: Provided handling for offer operation updates.

[Benjamin Bannier]

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




src/resource_provider/manager.cpp
Lines 430 (patched)


`Call.UpdateOfferOperationStatus.latest_status` is `optional`; we should 
check if it is set before using it.



src/resource_provider/message.hpp
Lines 51 (patched)


Since we seem to mirror `Call.UpdateOfferOperationStatus` pretty closely 
here, let's use an `Option latestStatus`.



src/resource_provider/message.hpp
Lines 80 (patched)


We should implement this.



src/slave/slave.cpp
Lines 6823-6824 (patched)


Let's remove the reference to `used` from the comment.



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


Let's do a contains check before the substraction.



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


Let's not use `default` when switching over enum values; please enumerate 
all possible values instead.



src/slave/slave.cpp
Lines 6861-6865 (patched)


It seems this would mean that we drop status updates when we are in these 
states which doesn't sound great.

We do not start handling resource provider messages before we entered a 
running state, but could enter another state later. Given that we don't have a 
status update manager for offer operation status messaes yet, I believe it 
might make sense to fail over the agent should we enter any of these states and 
add a `TODO` to revisit this code once we can reliably deliver status updates.



src/tests/resource_provider_manager_tests.cpp
Lines 319 (patched)


I believe we could we could use a `MockResourceProvider` here to simplify 
the test a bit.



src/tests/resource_provider_manager_tests.cpp
Lines 376 (patched)


Not sure we need this, but if you want to keep it let's make it an `ASSERT` 
on the value from the `event` instead of going through the assignment.


- Benjamin Bannier


On Nov. 7, 2017, 3:20 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63622/
> ---
> 
> (Updated Nov. 7, 2017, 3:20 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When a resource provider has finished an offer operation, it will send
> a status update to the resource provider manager and subsequently to an
> agent. The agent then updates its internal bookkeeping and forwards the
> status update to the master.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/manager.cpp a878507d71a09a415d8a4573cf2b33c26c985451 
>   src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
>   src/slave/slave.hpp df1b0205124555dcb6a0efa5c237f5e77fa2bdf7 
>   src/slave/slave.cpp c10823985154bac19f8952b94311a03b2b9b4ea1 
>   src/tests/resource_provider_manager_tests.cpp 
> 4008b1c751d6227b99adef756e95174d7d8a62f2 
> 
> 
> Diff: https://reviews.apache.org/r/63622/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 63253: Fixed DefaultExecutorTest for proper URI handling on Windows.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 63253 was successfully built and tested.

Reviews applied: `['60620', '60621', '60622', '60623', '60624', '60626', 
'60628', '63253']`

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

- Mesos Reviewbot Windows


On Nov. 6, 2017, 6:10 p.m., Jeff Coffler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63253/
> ---
> 
> (Updated Nov. 6, 2017, 6:10 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Gaston Kleiman, John Kordich, 
> Joseph Wu, Li Li, and Michael Park.
> 
> 
> Bugs: MESOS-6705
> https://issues.apache.org/jira/browse/MESOS-6705
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed changes from Gaston (git commit 772c8f5 on 10/02/2017)
> for proper operation on Windows. Changed to use new URI methods
> introduced as part of MESOS-6705, and used powershell on Windows
> to replace "test" command on Linux.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> 5078bd4d70698f5cbd14c971fcecfd58f8467a04 
> 
> 
> Diff: https://reviews.apache.org/r/63253/diff/4/
> 
> 
> Testing
> ---
> 
> Ran mesos-tests on both Windows and Linux with no errors.
> 
> Specifically ran mesos-tests with 
> `--gtest_filter=MesosContainerizer/DefaultExecutorTest.*` to verify that the 
> modified tests ran properly.
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>

Re: Review Request 62938: Added ZooKeeper leader resolution to CLI.

[Kevin Klues]

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




src/python/cli_new/lib/cli/util.py
Lines 258 (patched)


We don't need this if statement. At a minimum we will get a "{}" back from 
the zk.get() call (which will make this if statement always return true.



src/python/cli_new/lib/cli/util.py
Lines 266 (patched)


Let's pull the zk.stop() up above the if statememnt here and wrap it in a 
try/except block.

Then we can do the if/return followed immediately by the raise on failure.



src/python/cli_new/lib/cli/util.py
Lines 270 (patched)


Can this throw an exception?


- Kevin Klues


On Oct. 26, 2017, 4:52 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62938/
> ---
> 
> (Updated Oct. 26, 2017, 4:52 p.m.)
> 
> 
> Review request for mesos, Eric Chung and Kevin Klues.
> 
> 
> Bugs: MESOS-8012
> https://issues.apache.org/jira/browse/MESOS-8012
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change allows users to use the CLI with a Mesos running in high
> availability mode. The `zookeeper` field was already here before this
> commit, with an `addresses` array and a `path` field. This change
> only adds the backend to actually make it usable.
> 
> Interacting with ZooKeeper requires a new dependency, kazoo, that has
> been added to the list of requirements for the CLI.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/README.md a4b270d9b54cdd83bd62530fb44ddfaffb8a014b 
>   src/python/cli_new/lib/cli/util.py dd109c09368e650b7d7dabd663c11bc2e3e5180a 
>   src/python/cli_new/pip-requirements.txt 
> 7aeac344c47ccd2588fded44d7314db7abd85653 
> 
> 
> Diff: https://reviews.apache.org/r/62938/diff/4/
> 
> 
> Testing
> ---
> 
> Tested with a Mesos cluster running with 3 masters and 1 agent, run some CLI 
> commands successfully.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Review Request 63625: Added a test for resource conversion using a resource provider.

[Jan Schlicht]

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

Review request for mesos, Benjamin Bannier and Jie Yu.


Repository: mesos


Description
---

Added a test for resource conversion using a resource provider.


Diffs
-

  src/tests/resource_provider_manager_tests.cpp 
4008b1c751d6227b99adef756e95174d7d8a62f2 


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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 63606: Avoid an extra copy during ProtobufProcess::send.

[Benjamin Hindman]

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


Ship it!




Ship It!

- Benjamin Hindman


On Nov. 7, 2017, 4:23 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63606/
> ---
> 
> (Updated Nov. 7, 2017, 4:23 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/protobuf.hpp 
> fe152f273332470ac50f9715291897bb04cf95b9 
> 
> 
> Diff: https://reviews.apache.org/r/63606/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 63605: Support moving in data during ProcessBase::send.

[Benjamin Hindman]

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


Ship it!




Ship It!

- Benjamin Hindman


On Nov. 7, 2017, 3:40 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63605/
> ---
> 
> (Updated Nov. 7, 2017, 3:40 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This allows callers that can move in data to avoid an extra copy.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/event.hpp 
> 2e246205020c3c5b8c2eb5187a8eb3643d1e6d4d 
>   3rdparty/libprocess/include/process/process.hpp 
> dc3375ce62556322eb2bc60ade61f313ade123b8 
>   3rdparty/libprocess/src/process.cpp 
> 71ae7129ffbd0e22eda2863b17bbcf588298c37b 
>   3rdparty/libprocess/src/tests/benchmarks.cpp 
> e6c77d565d5acf72b475a085e9504679253b4b97 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> 952c92c033e2363cff0c2c68610d3820b97d177e 
> 
> 
> Diff: https://reviews.apache.org/r/63605/diff/1/
> 
> 
> Testing
> ---
> 
> Ran benchmarks before and after per benh's request, no difference:
> https://docs.google.com/spreadsheets/d/1I6iUziBwouQSn2veoj-xVpq7B9n6zdO9zPimi1IlJZ4/edit?usp=sharing
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 62161: Update boost version.

[Benjamin Bannier]

> On Nov. 7, 2017, 4:26 p.m., Benjamin Bannier wrote:
> > 3rdparty/versions.am
> > Line 22 (original), 22 (patched)
> > 
> >
> > This patch in general seems to trigger the issue fixed in 
> > https://reviews.apache.org/r/63622/. Could you move that one before this 
> > patch to keep all patches compilable.

I meant https://reviews.apache.org/r/62444/ of course.


- Benjamin


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


On Oct. 6, 2017, 4:02 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62161/
> ---
> 
> (Updated Oct. 6, 2017, 4:02 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update boost version.
> 
> 
> Diffs
> -
> 
>   3rdparty/boost-1.53.0.tar.gz 175f373c330ff69bdb0d44fc75a29982e68554ab 
>   3rdparty/boost-1.65.0.tar.gz PRE-CREATION 
>   3rdparty/cmake/Versions.cmake 958629642 
>   3rdparty/versions.am 82d66be7d 
> 
> 
> Diff: https://reviews.apache.org/r/62161/diff/4/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> 0001-Update-boost-version.patch
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/06/6101ab66-98c3-45f3-a91b-dc046e24dca5__0001-Update-boost-version.patch
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 62161: Update boost version.

[Benjamin Bannier]

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




3rdparty/versions.am
Line 22 (original), 22 (patched)


This patch in general seems to trigger the issue fixed in 
https://reviews.apache.org/r/63622/. Could you move that one before this patch 
to keep all patches compilable.


- Benjamin Bannier


On Oct. 6, 2017, 4:02 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62161/
> ---
> 
> (Updated Oct. 6, 2017, 4:02 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update boost version.
> 
> 
> Diffs
> -
> 
>   3rdparty/boost-1.53.0.tar.gz 175f373c330ff69bdb0d44fc75a29982e68554ab 
>   3rdparty/boost-1.65.0.tar.gz PRE-CREATION 
>   3rdparty/cmake/Versions.cmake 958629642 
>   3rdparty/versions.am 82d66be7d 
> 
> 
> Diff: https://reviews.apache.org/r/62161/diff/4/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> 0001-Update-boost-version.patch
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/06/6101ab66-98c3-45f3-a91b-dc046e24dca5__0001-Update-boost-version.patch
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 63292: Added lamdba::zip.

[Alexander Rojas]

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




3rdparty/stout/include/stout/lambda.hpp
Lines 168 (patched)


why not use a `std::tuple` instead of `std::pair`? that will allow to 
expand this function eventually to any number of inputs.



3rdparty/stout/include/stout/lambda.hpp
Lines 209-221 (patched)


Why not replace this with:

```c++
  std::unordered_map output;
  auto contents = zipto(input1, input2);
  std::copy(
  std::begin(contents),
  std::end(contents),
  std::inserter(output, std::begin(output)));
  return output;
```

If we had ranges we could even get away without `contents`.


- Alexander Rojas


On Oct. 25, 2017, 12:03 p.m., Benjamin Hindman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63292/
> ---
> 
> (Updated Oct. 25, 2017, 12:03 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added lamdba::zip.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/lambda.hpp 
> a61d97b69e7eebd057c94148d39c6e1fc3066017 
>   3rdparty/stout/tests/lambda_tests.cpp 
> ad8c2efddb6b64184670d0cfb33188ef843351ab 
> 
> 
> Diff: https://reviews.apache.org/r/63292/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>

Re: Review Request 63604: Removed an unnecessary serialization in protobuf process.

[Benjamin Hindman]

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


Ship it!




Ship It!

- Benjamin Hindman


On Nov. 7, 2017, 3:40 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63604/
> ---
> 
> (Updated Nov. 7, 2017, 3:40 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Jiang Yan Xu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/include/process/protobuf.hpp 
> fe152f273332470ac50f9715291897bb04cf95b9 
> 
> 
> Diff: https://reviews.apache.org/r/63604/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 62161: Update boost version.

[Benjamin Bannier]

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


Fix it, then Ship it!





3rdparty/cmake/Versions.cmake
Lines 1-2 (original), 1-2 (patched)


Let's please make sure to add this bundle to 
https://github.com/mesos/3rdparty before landing this patch. Otherwise we break 
cmake builds with `REBUNDLED=OFF`.


- Benjamin Bannier


On Oct. 6, 2017, 4:02 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62161/
> ---
> 
> (Updated Oct. 6, 2017, 4:02 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Till Toenshoff.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update boost version.
> 
> 
> Diffs
> -
> 
>   3rdparty/boost-1.53.0.tar.gz 175f373c330ff69bdb0d44fc75a29982e68554ab 
>   3rdparty/boost-1.65.0.tar.gz PRE-CREATION 
>   3rdparty/cmake/Versions.cmake 958629642 
>   3rdparty/versions.am 82d66be7d 
> 
> 
> Diff: https://reviews.apache.org/r/62161/diff/4/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> 0001-Update-boost-version.patch
>   
> https://reviews.apache.org/media/uploaded/files/2017/10/06/6101ab66-98c3-45f3-a91b-dc046e24dca5__0001-Update-boost-version.patch
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 63621: Added a devolve function for 'OfferOperationStatus'.

[Benjamin Bannier]

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


Fix it, then Ship it!





src/internal/devolve.hpp
Lines 70 (patched)


Let's put this right after `devolve(const v1::Offer&)`.



src/internal/devolve.cpp
Lines 163-166 (patched)


Let's put this right behind `devolve(const v1::Offer&)`'s implementation.


- Benjamin Bannier


On Nov. 7, 2017, 3:19 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63621/
> ---
> 
> (Updated Nov. 7, 2017, 3:19 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/internal/devolve.hpp 48d9c33fff376f7842f9b72bf84056482cefadcd 
>   src/internal/devolve.cpp e05fcd4d0f423a2ba13bdcf6ba6561b43e31977d 
> 
> 
> Diff: https://reviews.apache.org/r/63621/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 63620: Updated offer operation handling to set resource versions.

[Benjamin Bannier]

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




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


nit: Do we require the `Some` here? I would have guessed that this would 
implicitly construct some provider id since `Option`'s constructor is implicit.



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


nit: `Some` is not needed here.



src/master/master.cpp
Lines 9987-9995 (patched)


Since we `apply` above, we here need to instead have a hard 
`CHECK_SOME(uuid)`, rollback, or apply later.


- Benjamin Bannier


On Nov. 7, 2017, 3:18 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63620/
> ---
> 
> (Updated Nov. 7, 2017, 3:18 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Resource version UUIDs are used to establish a relationship between
> operation and the resource they are operating on. For each agent the
> master keeps track of its resource version and the resource version of
> the agents' resource providers. As resource versions are required in a
> 'ApplyOfferOperationMessage', the master has to find the resource
> version that's matching the resources an operation should apply to and
> set it accordingly.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp ee212c1a21b432d3aa8e90d748d12cb0a754d57c 
> 
> 
> Diff: https://reviews.apache.org/r/63620/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 63434: Added IPv6 capabilities for TCP and HTTP healthchecks.

[Qian Zhang]

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



I think you need to do the similar changes to the `CheckInfo` message.

- Qian Zhang


On Nov. 3, 2017, 8:38 a.m., Avinash sridharan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63434/
> ---
> 
> (Updated Nov. 3, 2017, 8:38 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Qian Zhang, and Vinod Kone.
> 
> 
> Bugs: MESOS-8050
> https://issues.apache.org/jira/browse/MESOS-8050
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added IPv6 capabilities for TCP and HTTP healthchecks.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 12652fe2804efdd2228c8e8c95b88e1f4b762f7b 
>   include/mesos/v1/mesos.proto e0797aea6ffe200aab5b0bde5450dbd7c332618b 
> 
> 
> Diff: https://reviews.apache.org/r/63434/diff/2/
> 
> 
> Testing
> ---
> 
> make
> 
> 
> Thanks,
> 
> Avinash sridharan
> 
>

Review Request 63622: Provided handling for offer operation updates.

[Jan Schlicht]

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

Review request for mesos, Benjamin Bannier and Jie Yu.


Repository: mesos


Description
---

When a resource provider has finished an offer operation, it will send
a status update to the resource provider manager and subsequently to an
agent. The agent then updates its internal bookkeeping and forwards the
status update to the master.


Diffs
-

  src/resource_provider/manager.cpp a878507d71a09a415d8a4573cf2b33c26c985451 
  src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
  src/slave/slave.hpp df1b0205124555dcb6a0efa5c237f5e77fa2bdf7 
  src/slave/slave.cpp c10823985154bac19f8952b94311a03b2b9b4ea1 
  src/tests/resource_provider_manager_tests.cpp 
4008b1c751d6227b99adef756e95174d7d8a62f2 


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


Testing
---

make check


Thanks,

Jan Schlicht

Review Request 63621: Added a devolve function for 'OfferOperationStatus'.

[Jan Schlicht]

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

Review request for mesos, Benjamin Bannier and Jie Yu.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/internal/devolve.hpp 48d9c33fff376f7842f9b72bf84056482cefadcd 
  src/internal/devolve.cpp e05fcd4d0f423a2ba13bdcf6ba6561b43e31977d 


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


Testing
---

make check


Thanks,

Jan Schlicht

Review Request 63620: Updated offer operation handling to set resource versions.

[Jan Schlicht]

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

Review request for mesos, Benjamin Bannier and Jie Yu.


Repository: mesos


Description
---

Resource version UUIDs are used to establish a relationship between
operation and the resource they are operating on. For each agent the
master keeps track of its resource version and the resource version of
the agents' resource providers. As resource versions are required in a
'ApplyOfferOperationMessage', the master has to find the resource
version that's matching the resources an operation should apply to and
set it accordingly.


Diffs
-

  src/master/master.cpp ee212c1a21b432d3aa8e90d748d12cb0a754d57c 


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


Testing
---

make check


Thanks,

Jan Schlicht

Re: Review Request 63495: Added comparison operators for 'ResourceVersionUUID'.

[Benjamin Bannier]

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

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


Review request for mesos, Jie Yu and Jan Schlicht.


Summary (updated)
-

Added comparison operators for 'ResourceVersionUUID'.


Repository: mesos


Description (updated)
---

See summary.


Diffs
-

  src/messages/messages.hpp 59e670532c8eb1ab1ec6cb063ace03481cd6a33d 
  src/messages/messages.cpp 7f1da63ae67b3918851c2c1dd225bfd2d0044387 


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


Testing
---

`make check`, additional testing as part of https://reviews.apache.org/r/63496/.


Thanks,

Benjamin Bannier

Re: Review Request 63538: Removed expectation for scheduler connected events in some tests.

[Mesos Reviewbot Windows]

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



FAIL: Some Mesos libprocess-tests failed.

Reviews applied: `['63538']`

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

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

Relevant logs:

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

```
[   OK ] Scheme/HTTPTest.WWWAuthenticateHeader/0 (447 ms)
[ RUN  ] Scheme/HTTPTest.WWWAuthenticateHeader/1
[   OK ] Scheme/HTTPTest.WWWAuthenticateHeader/1 (66 ms)
[ RUN  ] Scheme/HTTPTest.Accepts/0
[   OK ] Scheme/HTTPTest.Accepts/0 (276 ms)
[ RUN  ] Scheme/HTTPTest.Accepts/1
[   OK ] Scheme/HTTPTest.Accepts/1 (90 ms)
[--] 34 tests from Scheme/HTTPTest (14891 ms total)

[--] 4 tests from SSLVerifyIPAdd/SSLTest
[ RUN  ] SSLVerifyIPAdd/SSLTest.BasicSameProcess/0
[   OK ] SSLVerifyIPAdd/SSLTest.BasicSameProcess/0 (1059 ms)
[ RUN  ] SSLVerifyIPAdd/SSLTest.BasicSameProcess/1
[   OK ] SSLVerifyIPAdd/SSLTest.BasicSameProcess/1 (356 ms)
[ RUN  ] SSLVerifyIPAdd/SSLTest.RequireCertificate/0
[   OK ] SSLVerifyIPAdd/SSLTest.RequireCertificate/0 (940 ms)
[ RUN  ] SSLVerifyIPAdd/SSLTest.RequireCertificate/1
[   OK ] SSLVerifyIPAdd/SSLTest.RequireCertificate/1 (1396 ms)
[--] 4 tests from SSLVerifyIPAdd/SSLTest (3899 ms total)

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

 1 FAILED TEST
  YOU HAVE 19 DISABLED TESTS

```

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

```
I1107 14:06:55.826413  4332 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1107 14:06:55.829547  4332 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peWARNING: Logging before 
InitGoogleLogging() is written to STDERR
I1107 14:06:56.149559  1292 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1107 14:06:56.153584  1292 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I1107 14:06:56.154577  1292 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1107 14:06:56.17  1292 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\UkTJRF\cert.pem
WARNING: Logging before InitGoogleLogging() is written to STDERR
I1107 14:06:57.503604  4236 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1107 14:06:57.504606  4236 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I1107 14:06:57.504606  4236 openssl.cpp:526] Will use IP address verification 
in subject alternative name certificate extension.
I1107 14:06:57.504606  4236 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1107 14:06:57.505607  4236 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\cILrcM\cert.pem
er certificate verification
I1107 14:06:55.849546  4332 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1107 14:06:55.849546  4332 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\UkTJRF\cert.pem
I1107 14:06:57.202595  4332 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1107 14:06:57.202595  4332 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I1107 14:06:57.202595  4332 openssl.cpp:526] Will use IP address verification 
in subject alternative name certificate extension.
I1107 14:06:57.202595  4332 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1107 14:06:57.203595  4332 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\cILrcM\cert.pem
I1107 14:06:57.763766  1084 process.cpp:1067] Failed to accept socket: future 
discarded
```

- Mesos Reviewbot Windows


On Nov. 3, 2017, 3:32 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> 

Re: Review Request 63538: Removed expectation for scheduler connected events in some tests.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [63538]

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 Nov. 3, 2017, 10:32 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63538/
> ---
> 
> (Updated Nov. 3, 2017, 10:32 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-8093
> https://issues.apache.org/jira/browse/MESOS-8093
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cgroups_isolator_tests.cpp 
> ee1e5e67b9eacb5909964b2951f659d1bd2233e5 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> 0d68dd634771c016553313d67ddb33504d1a13c2 
>   src/tests/containerizer/runtime_isolator_tests.cpp 
> 81c0d825c689b3d00cbc21c7567c8c0c8836bf75 
> 
> 
> Diff: https://reviews.apache.org/r/63538/diff/4/
> 
> 
> Testing
> ---
> 
> ```
> sudo ./mesos-tests.sh 
> --gtest_filter="CgroupsIsolatorTest.ROOT_CGROUPS_LimitSwap" 
> --gtest_break_on_failure --gtest_repeat=100
> sudo ./mesos-tests.sh 
> --gtest_filter="DockerRuntimeIsolatorTest.ROOT_INTERNET_CURL_NestedSimpleCommand"
>  --gtest_break_on_failure --gtest_repeat=100
> ```
> And `make check`.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 63538: Removed expectation for scheduler connected events in some tests.

[Alexander Rukletsov]

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


Fix it, then Ship it!




I'll fix the issue and commit shortly. Thanks!


src/tests/containerizer/cni_isolator_tests.cpp
Line 1497 (original), 1486-1489 (patched)


Let's wrap it like we did in the test above.


- Alexander Rukletsov


On Nov. 3, 2017, 2:32 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63538/
> ---
> 
> (Updated Nov. 3, 2017, 2:32 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-8093
> https://issues.apache.org/jira/browse/MESOS-8093
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cgroups_isolator_tests.cpp 
> ee1e5e67b9eacb5909964b2951f659d1bd2233e5 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> 0d68dd634771c016553313d67ddb33504d1a13c2 
>   src/tests/containerizer/runtime_isolator_tests.cpp 
> 81c0d825c689b3d00cbc21c7567c8c0c8836bf75 
> 
> 
> Diff: https://reviews.apache.org/r/63538/diff/4/
> 
> 
> Testing
> ---
> 
> ```
> sudo ./mesos-tests.sh 
> --gtest_filter="CgroupsIsolatorTest.ROOT_CGROUPS_LimitSwap" 
> --gtest_break_on_failure --gtest_repeat=100
> sudo ./mesos-tests.sh 
> --gtest_filter="DockerRuntimeIsolatorTest.ROOT_INTERNET_CURL_NestedSimpleCommand"
>  --gtest_break_on_failure --gtest_repeat=100
> ```
> And `make check`.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 62880: Marked the agent as RESOURCE_PROVIDER capable.

[Benjamin Bannier]

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



I'd suggest to discard this patch in favor of the feature flag introduced in 
https://reviews.apache.org/r/63519/.

- Benjamin Bannier


On Oct. 18, 2017, 1:25 a.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62880/
> ---
> 
> (Updated Oct. 18, 2017, 1:25 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Joseph Wu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8071
> https://issues.apache.org/jira/browse/MESOS-8071
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Marked the agent as RESOURCE_PROVIDER capable.
> 
> 
> Diffs
> -
> 
>   src/slave/constants.cpp 9b60bd0e808aab272039239db95bee71a3d910ab 
>   src/tests/master_tests.cpp 4c776012e84c34dda7410841e063faab2a84f2df 
>   src/tests/slave_tests.cpp 6d1e98d59ad0b8a02f4696ca9cc663d048109ca6 
> 
> 
> Diff: https://reviews.apache.org/r/62880/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 63615: Passed scheduler as a shared pointer into the callback.

[Alexander Rukletsov]

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

Review request for mesos, Michael Park and Vinod Kone.


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


Repository: mesos


Description
---

To ensure the lifetime of the scheduler is longer than the lifetime
of the scheduler library driver, pass scheduler as a shared_ptr.


Diffs
-

  src/tests/mesos.hpp e25defeb55608136e77363aa48cf820092a13a59 


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


Testing
---

make check on Mac OS 10.11.6 with Apple LLVM version 8.0.0 (clang-800.0.42.1)
make check on various linux distributions


Thanks,

Alexander Rukletsov

Review Request 63614: Removed unnecessary check in v1 scheduler library.

[Alexander Rukletsov]

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

Review request for mesos, Michael Park and Vinod Kone.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/scheduler/scheduler.cpp fe374541621015d04a594f68d009ce50ec751d30 


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


Testing
---

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


Thanks,

Alexander Rukletsov

Review Request 63612: Modified formatting of v1 scheduler test driver for consistency.

[Alexander Rukletsov]

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

Review request for mesos, Michael Park and Vinod Kone.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/tests/mesos.hpp e25defeb55608136e77363aa48cf820092a13a59 


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


Testing
---

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


Thanks,

Alexander Rukletsov