Re: Review Request 56721: Implemented pruneImages with a mark and sweep in docker store.

2017-11-13 Thread Zhitao Li

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

(Updated Nov. 14, 2017, 6:21 a.m.)


Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.


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


Repository: mesos


Description
---

This includes the following changes:
- add a `pruneImages()` function on the chain of relevant classes;
- implement prune in docker store;
- fix mock interface to keep existing tests pass.


Diffs (updated)
-

  src/slave/containerizer/composing.hpp 
06d68eef5de7745e32f0e808f11016bcc285dd8f 
  src/slave/containerizer/composing.cpp 
587f009384f0c7ef87482686578dc822d3d5b208 
  src/slave/containerizer/containerizer.hpp 
449bb5d0902936faae7bf9bae9c703b219aed842 
  src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270 
  src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67 
  src/slave/containerizer/mesos/containerizer.hpp 
6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b 
  src/slave/containerizer/mesos/containerizer.cpp 
100e3bbda543d87808da9ff6bea42da5099ea8c5 
  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
954da1681778878c8aff6150002e52ecb648d1bb 
  src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
d86afd2a6ff0bf87e624db1c99255c85068bf6ab 
  src/slave/containerizer/mesos/provisioner/docker/paths.hpp 
232c027f8f96da0cb30b957bce4607d3695050d2 
  src/slave/containerizer/mesos/provisioner/docker/paths.cpp 
cd684b33eb308ce1eeb4539a5b2d51985d835db7 
  src/slave/containerizer/mesos/provisioner/docker/store.hpp 
1cf68665d33bd40a7605d26c96fb7b618407fdd0 
  src/slave/containerizer/mesos/provisioner/docker/store.cpp 
f357710cb19aec3654b0604f7909d068eaf20095 
  src/slave/containerizer/mesos/provisioner/provisioner.hpp 
7cba54ce490d1e6e17081cd7e04fd6759ceddb8e 
  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
450a3b32d69d2882973a6ed4e94e169a0256056b 
  src/slave/containerizer/mesos/provisioner/store.hpp 
01ab83dca79e51b8c96d18ee65705912b0ac8324 
  src/slave/containerizer/mesos/provisioner/store.cpp 
cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f 
  src/tests/containerizer.hpp a778b8581904bacea9eec3ff50c3c009959b5dac 
  src/tests/containerizer.cpp cd140f4263621a0a33a34b7e062a9ca6cf426e7a 
  src/tests/containerizer/mock_containerizer.hpp 
0adcb01e6c12d6cc4abed1f14fa2df833ffc6569 


Diff: https://reviews.apache.org/r/56721/diff/16/

Changes: https://reviews.apache.org/r/56721/diff/15-16/


Testing
---


Thanks,

Zhitao Li



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

2017-11-13 Thread Zhitao Li

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

(Updated Nov. 14, 2017, 6:20 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 (updated)
-

  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/5/

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


Testing
---


Thanks,

Zhitao Li



Re: Review Request 56721: Implemented pruneImages with a mark and sweep in docker store.

2017-11-13 Thread Zhitao Li


> On Nov. 11, 2017, 6:57 p.m., Jason Lai wrote:
> > src/slave/containerizer/mesos/provisioner/docker/paths.hpp
> > Lines 98 (patched)
> > 
> >
> > Nit: I'd suggest using `GC` over `Gc`

The other example of `Gc` for registry in mesos master is also in this case so 
not changing.


- Zhitao


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


On Nov. 10, 2017, 7:34 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56721/
> ---
> 
> (Updated Nov. 10, 2017, 7:34 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This includes the following changes:
> - add a `pruneImages()` function on the chain of relevant classes;
> - implement prune in docker store;
> - fix mock interface to keep existing tests pass.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.hpp 
> 06d68eef5de7745e32f0e808f11016bcc285dd8f 
>   src/slave/containerizer/composing.cpp 
> 587f009384f0c7ef87482686578dc822d3d5b208 
>   src/slave/containerizer/containerizer.hpp 
> 449bb5d0902936faae7bf9bae9c703b219aed842 
>   src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270 
>   src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 100e3bbda543d87808da9ff6bea42da5099ea8c5 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
> 954da1681778878c8aff6150002e52ecb648d1bb 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
> d86afd2a6ff0bf87e624db1c99255c85068bf6ab 
>   src/slave/containerizer/mesos/provisioner/docker/paths.hpp 
> 232c027f8f96da0cb30b957bce4607d3695050d2 
>   src/slave/containerizer/mesos/provisioner/docker/paths.cpp 
> cd684b33eb308ce1eeb4539a5b2d51985d835db7 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 
> 1cf68665d33bd40a7605d26c96fb7b618407fdd0 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> f357710cb19aec3654b0604f7909d068eaf20095 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 450a3b32d69d2882973a6ed4e94e169a0256056b 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> 01ab83dca79e51b8c96d18ee65705912b0ac8324 
>   src/slave/containerizer/mesos/provisioner/store.cpp 
> cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f 
>   src/tests/containerizer.hpp a778b8581904bacea9eec3ff50c3c009959b5dac 
>   src/tests/containerizer.cpp cd140f4263621a0a33a34b7e062a9ca6cf426e7a 
>   src/tests/containerizer/mock_containerizer.hpp 
> 0adcb01e6c12d6cc4abed1f14fa2df833ffc6569 
> 
> 
> Diff: https://reviews.apache.org/r/56721/diff/15/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>



Re: Review Request 56721: Implemented pruneImages with a mark and sweep in docker store.

2017-11-13 Thread Zhitao Li


> On Nov. 11, 2017, 6:57 p.m., Jason Lai wrote:
> > src/slave/containerizer/mesos/provisioner/docker/store.cpp
> > Lines 594 (patched)
> > 
> >
> > Will there be a race here?

`rmdirs` is a lambda which is sent to the `executor` and executed in a 
different thread, so I don't see how a race condition can happen. Can you 
elaborate?


- Zhitao


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


On Nov. 10, 2017, 7:34 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56721/
> ---
> 
> (Updated Nov. 10, 2017, 7:34 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jason Lai, and Jie Yu.
> 
> 
> Bugs: MESOS-4945
> https://issues.apache.org/jira/browse/MESOS-4945
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This includes the following changes:
> - add a `pruneImages()` function on the chain of relevant classes;
> - implement prune in docker store;
> - fix mock interface to keep existing tests pass.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.hpp 
> 06d68eef5de7745e32f0e808f11016bcc285dd8f 
>   src/slave/containerizer/composing.cpp 
> 587f009384f0c7ef87482686578dc822d3d5b208 
>   src/slave/containerizer/containerizer.hpp 
> 449bb5d0902936faae7bf9bae9c703b219aed842 
>   src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270 
>   src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 100e3bbda543d87808da9ff6bea42da5099ea8c5 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.hpp 
> 954da1681778878c8aff6150002e52ecb648d1bb 
>   src/slave/containerizer/mesos/provisioner/docker/metadata_manager.cpp 
> d86afd2a6ff0bf87e624db1c99255c85068bf6ab 
>   src/slave/containerizer/mesos/provisioner/docker/paths.hpp 
> 232c027f8f96da0cb30b957bce4607d3695050d2 
>   src/slave/containerizer/mesos/provisioner/docker/paths.cpp 
> cd684b33eb308ce1eeb4539a5b2d51985d835db7 
>   src/slave/containerizer/mesos/provisioner/docker/store.hpp 
> 1cf68665d33bd40a7605d26c96fb7b618407fdd0 
>   src/slave/containerizer/mesos/provisioner/docker/store.cpp 
> f357710cb19aec3654b0604f7909d068eaf20095 
>   src/slave/containerizer/mesos/provisioner/provisioner.hpp 
> 7cba54ce490d1e6e17081cd7e04fd6759ceddb8e 
>   src/slave/containerizer/mesos/provisioner/provisioner.cpp 
> 450a3b32d69d2882973a6ed4e94e169a0256056b 
>   src/slave/containerizer/mesos/provisioner/store.hpp 
> 01ab83dca79e51b8c96d18ee65705912b0ac8324 
>   src/slave/containerizer/mesos/provisioner/store.cpp 
> cc5cc81e05f29bb0e11ffa13cdb8d63d4397114f 
>   src/tests/containerizer.hpp a778b8581904bacea9eec3ff50c3c009959b5dac 
>   src/tests/containerizer.cpp cd140f4263621a0a33a34b7e062a9ca6cf426e7a 
>   src/tests/containerizer/mock_containerizer.hpp 
> 0adcb01e6c12d6cc4abed1f14fa2df833ffc6569 
> 
> 
> Diff: https://reviews.apache.org/r/56721/diff/15/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>



Review Request 63779: Windows: Fixed `SlaveReregisterTaskExecutorIds` test.

2017-11-13 Thread Andrew Schwartzmeyer

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

Review request for mesos, Akash Gupta, Alexander Rukletsov, Jie Yu, John 
Kordich, Joseph Wu, and Li Li.


Repository: mesos


Description
---

Similar to the check tests, with the `cpu` hard cap enabled, the task in
this test failed with an OOM error because PowerShell requires more
resources than were being allocated. The `defaultTaskResourcesString`
was moved into the `MesosTests` base class so that any test creating a
task can use these default resources, instead of a hard-coded string.
Additionally, the resources allocated by default for an agent were
increased so that multiple tasks can be run simultaneously (as this test
does).


Diffs
-

  src/tests/check_tests.cpp f14b81aa06f6e0acbc2c3d4c65b0443d09127e78 
  src/tests/master_slave_reconciliation_tests.cpp 
6acf694538c61ce46e513b7b2805da0e0f7758dc 
  src/tests/mesos.hpp f25173232df4a9b460194e5e041230e307b80e36 


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


Testing
---

Tests pass on Windows, waiting on Linux tests.


Thanks,

Andrew Schwartzmeyer



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

2017-11-13 Thread Andrew Schwartzmeyer

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

(Updated Nov. 13, 2017, 5:57 p.m.)


Review request for mesos, Aaron Wood, Akash Gupta, Jeff Coffler, Jie Yu, John 
Kordich, Joseph Wu, Li Li, and Michael Park.


Changes
---

Refactored based on Jie's comments.


Bugs: MESOS-5462 and MESOS-6690
https://issues.apache.org/jira/browse/MESOS-5462
https://issues.apache.org/jira/browse/MESOS-6690


Repository: mesos


Description (updated)
---

Instead of deriving from the POSIX isolators, we now have two real
Windows isolators that can be used together or separately. The `Cpu`
isolator enables a hard-cap CPU limit, and the `Mem` isolator enables a
hard-cap memory limit on the job object for the container.

These classes are separate derivations of `MesosIsolatorProcess`,
because introducing a `WindowsIsolatorProcess` base class would be
abstraction for the sole purpose of code deduplication.


Diffs (updated)
-

  src/CMakeLists.txt b0e223121f24ac0dc604dc2c639158b817eec535 
  src/Makefile.am 955f01a665019d7750980ff16f126ad63b524594 
  src/slave/containerizer/mesos/containerizer.cpp 
100e3bbda543d87808da9ff6bea42da5099ea8c5 
  src/slave/containerizer/mesos/isolators/windows.hpp 
b0621a5fc411b8812722f7fcf6580ed64dac5382 
  src/slave/containerizer/mesos/isolators/windows/cpu.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/windows/cpu.cpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/windows/mem.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/windows/mem.cpp PRE-CREATION 
  src/slave/flags.cpp 0eeecdc6023443a99c8e8fe29c2f7bf791c0a36e 


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

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


Testing
---


Thanks,

Andrew Schwartzmeyer



Re: Review Request 63274: Windows: Added `os::get_job_mem` to stout.

2017-11-13 Thread Andrew Schwartzmeyer

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

(Updated Nov. 13, 2017, 5:56 p.m.)


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


Changes
---

Same rename, same reason.


Summary (updated)
-

Windows: Added `os::get_job_mem` to stout.


Repository: mesos


Description
---

This function sums the working set size for each process in a job object
represented by the PID. The Windows API does not support this directly.
Instead, the list of processes in a job object must be obtained, and
then for each process, the memory usage must be queried.


Diffs (updated)
-

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


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

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


Testing
---


Thanks,

Andrew Schwartzmeyer



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

2017-11-13 Thread Joseph Wu

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

(Updated Nov. 13, 2017, 5:41 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Changes
---

Changed port_mapping isolator too... but now the name of the review can't fit 
the isolator names :(


Summary (updated)
-

Updated some isolators' usage of ExecutorInfo.


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


Repository: mesos


Description (updated)
---

These four isolators refered to the ExecutorInfo field of the given
ContainerConfig to determine the total resources to isolate.  This
changes these isolators to refer instead to the Resources field of the
given ContainerConfig because the resource amounts should be identical
(enforced by the caller to Containerizer::launch, such as the agent)
and in order to support standalone containers, which specify resources
but no ExecutorInfo.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
549a4551736d1b88a79f275a2d6e19c586c49f26 
  src/slave/containerizer/mesos/isolators/gpu/isolator.cpp 
25636b50314cb4ea53d1931c97c87c65ecb658a8 
  src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
55672b14b44005673214aa49aa5ea7be8e7bffb8 
  src/slave/containerizer/mesos/isolators/xfs/disk.cpp 
2113f8665e4e84bdf12868f605b480522816a9ba 


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

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


Testing
---

make check
sudo src/mesos-tests

On... 
OSX and Ubuntu 16


Thanks,

Joseph Wu



Re: Review Request 63514: Updated tests with Containerizer::launch interface change.

2017-11-13 Thread Joseph Wu

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

(Updated Nov. 13, 2017, 5:39 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Changes
---

Rebase.


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


Repository: mesos


Description
---

This commit contains the test changes required due to the interface
change in https://reviews.apache.org/r/63063 .

Instead of `AWAIT_ASSERT_TRUE(...)`, the affected test lines now use
`AWAIT_ASSERT_EQ(Containerizer::LaunchResult::SUCCESS, ...)`.


Diffs (updated)
-

  src/tests/containerizer.hpp a778b8581904bacea9eec3ff50c3c009959b5dac 
  src/tests/containerizer.cpp cd140f4263621a0a33a34b7e062a9ca6cf426e7a 
  src/tests/containerizer/composing_containerizer_tests.cpp 
61e47e9cffb565e4621babb28e9bf8736945c386 
  src/tests/containerizer/io_switchboard_tests.cpp 
be1078e8e92a8bb42062842ef249859c5fc39ef4 
  src/tests/containerizer/isolator_tests.cpp 
4ad42bc427acebc7fe7e4d8d0d346537db23c9b1 
  src/tests/containerizer/linux_filesystem_isolator_tests.cpp 
4dfd90bee600f3b91183c60a0216d3990f0fba10 
  src/tests/containerizer/mesos_containerizer_tests.cpp 
ef424150183c782551a3019afcd4b9d1eb94b863 
  src/tests/containerizer/mock_containerizer.hpp 
0adcb01e6c12d6cc4abed1f14fa2df833ffc6569 
  src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
3e2a2d151759e78686e90e84322c9fd348c86a04 
  src/tests/containerizer/volume_host_path_isolator_tests.cpp 
1d006727e5bf76cc659e862cb64fe3facd608c8c 
  src/tests/containerizer/volume_image_isolator_tests.cpp 
2f91730075c0eca455be84bc1f1b01b4395fb382 
  src/tests/containerizer/volume_sandbox_path_isolator_tests.cpp 
b36c8df4df346096ed3f0ca3411b3ab5d9a8c74e 
  src/tests/containerizer/volume_secret_isolator_tests.cpp 
a55af954bdacfcc7b629fe6e5c47759dc0e8a709 
  src/tests/hook_tests.cpp dc8d87f824925252fb23bfc40afa9dfdf851afdf 
  src/tests/mock_docker.hpp 59873646be494c8fe6aebf5ede595d77e3ac4cae 
  src/tests/slave_recovery_tests.cpp 64bba047c6eaee563126a8bd1c6fa048f18172e1 
  src/tests/slave_tests.cpp cf2fbac4cc53d632c385eb72adb0d80ef942e8a6 


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

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


Testing
---

make check

On... 
OSX and Ubuntu 16


Thanks,

Joseph Wu



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

2017-11-13 Thread Joseph Wu

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

Review request for mesos, Gilbert Song and Jie Yu.


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


Repository: mesos


Description
---

The change in the return type for Containerizer::launch has less of an
impact on the Docker containerizer, as it (currently) will not return
the "new" enum 'ALREADY_LAUNCHED'.

Note that in changing Containerizer::launch, a private helper of the
Docker containerizer `reapExecutor`s return value was changed.
The return value was originally `Future` because `reapExecutor`
is the final continuation in the launch path so it needed to match
the return value of Containerizer::launch.  However, `reapExecutor`
never returns `false` (only `true` or a Failure).


Diffs
-

  src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270 
  src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67 


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


Testing
---

Requires the next review in the chain (test changes).


Thanks,

Joseph Wu



Re: Review Request 63063: Modified Containerizer::launch interface to allow repeated launch.

2017-11-13 Thread Joseph Wu

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

(Updated Nov. 13, 2017, 5:36 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Changes
---

* Fixup some comments in composing containerizer talking about return `false`, 
which they don't anymore.
* Add an UNREACHABLE to a switch statement (compiler warning).
* Pull out the docker changes into the next review.


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


Repository: mesos


Description
---

There is some existing tech debt around requiring the caller of
`Containerizer::launch` to call `Containerizer::destroy` if the launch
fails (see MESOS-6214).  For nested and standalone containers, the
side effect of this results in accidentally destroying running
containers if you make the same call an even number of times.

For example, suppose the user launches a valid nested container
with an ID of 'parent.child'. If the user issues the same call to
launch 'parent.child' again, this second call will fail *and* will
also destroy the first container.

This commit prevents repeated launch calls from destroying containers
by changing the return value of `Containerizer::launch`.  There are
now four possible return values:
  * The launch succeeded.
  * The standalone/nested container already exists.
  * The given ContainerConfig is not supported.
  * The launch failed.


Diffs (updated)
-

  src/slave/containerizer/composing.hpp 
06d68eef5de7745e32f0e808f11016bcc285dd8f 
  src/slave/containerizer/composing.cpp 
587f009384f0c7ef87482686578dc822d3d5b208 
  src/slave/containerizer/containerizer.hpp 
449bb5d0902936faae7bf9bae9c703b219aed842 
  src/slave/containerizer/mesos/containerizer.hpp 
6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b 
  src/slave/containerizer/mesos/containerizer.cpp 
100e3bbda543d87808da9ff6bea42da5099ea8c5 
  src/slave/http.cpp 22cdac97e1a8a28ca149043ffa1d0646073d7fdb 
  src/slave/slave.hpp c0acaa639a2bacaa6955ae6c5ab41e75dc1d11f7 
  src/slave/slave.cpp d8bacebc74790e955490a158c37ac0d9e75fd6b5 
  src/tests/agent_container_api_tests.cpp PRE-CREATION 


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

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


Testing
---

Requires the next review in the chain (test changes).


Thanks,

Joseph Wu



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

2017-11-13 Thread Joseph Wu

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

(Updated Nov. 13, 2017, 5:34 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Changes
---

Moved comment up.  And shortened some `std::`s


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


Repository: mesos


Description
---

This introduces a new test class that is parameterized based
on the type of API used to launch (1) the parent container,
(2) the nested container, and (3) the content type (like all
other API tests).  The test is also parameterized based
on the enabled set of isolators.

Because this change overlaps with existing nested container
API tests, those test(s) will be removed.


Diffs (updated)
-

  src/Makefile.am 955f01a665019d7750980ff16f126ad63b524594 
  src/tests/CMakeLists.txt 81c85d9bb1b50c6897526d5c207d09042883d771 
  src/tests/agent_container_api_tests.cpp PRE-CREATION 
  src/tests/api_tests.cpp 4e2cbe1b12b6d6e44e1d774397a78011beff0f95 


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

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


Testing
---

make check

On... 
OSX and Ubuntu 16


Thanks,

Joseph Wu



Re: Review Request 62145: Implemented Standalone Container API.

2017-11-13 Thread Joseph Wu

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

(Updated Nov. 13, 2017, 5:32 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Changes
---

* Changed AuthZ to use the AuthorizationAcceptor pattern.
* Removed the `user` fields from AuthZ objects due to decision to reduce 
granularity of AuthZ.
* Combined `__launchContainer` into `_launchContainer`.
* Made failure to chown standalone sandbox into a 500 error.


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


Repository: mesos


Description
---

The Standalone and Nested Container APIs are very similar.
This commit combines the two API implementations by adding a
translation function (i.e. `launchNestedContainer` and 
`launchContainer`) which unpacks the V1 protobuf into fields
which can be passed into a common function (i.e. `_launchContainer`).

The common functions authorize based on the type of container being
launched and it is possible to use both Standalone and Nested
Container APIs interchangably for nested containers.

This approach is somewhat messy for for the `WAIT_(NESTED_)CONTAINER`
calls, as these methods require different return protobufs based on
the original call.


Diffs (updated)
-

  src/slave/http.hpp 44a95dec4c9b8bb65d712c5538bbd7afffe2cf7b 
  src/slave/http.cpp 22cdac97e1a8a28ca149043ffa1d0646073d7fdb 


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

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


Testing
---

See later in chain.


Thanks,

Joseph Wu



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

2017-11-13 Thread Joseph Wu

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

(Updated Nov. 13, 2017, 5:27 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Changes
---

Changed some of the resource validation to use the common helpers.  
Functionality should remain the same.


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

  src/slave/validation.cpp a575d88dffd2714447d7780a7433506a6e8c085f 


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

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


Testing
---

See later in chain.


Thanks,

Joseph Wu



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

2017-11-13 Thread Joseph Wu


> On Oct. 17, 2017, 8:40 p.m., Jie Yu wrote:
> > include/mesos/authorizer/acls.proto
> > Lines 435-437 (patched)
> > 
> >
> > hum, i got confused. How do you get the user of a container? And it's 
> > not consistent with below?
> 
> Joseph Wu wrote:
> Note: The user is specified in the call to launch standalone/nested 
> containers.
> 
> Jie Yu wrote:
> but this is kill standalone container. Do we set 'user' in the 
> `ObjectApprover::Object` for kill action?

As discussed offline, the `user` is no longer a factor in AuthZ for standalone 
containers.  Instead, principals can either use the APIs... or they can't.


- Joseph


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


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



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

2017-11-13 Thread Joseph Wu

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

(Updated Nov. 13, 2017, 5:24 p.m.)


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


Changes
---

Removed granularity from ACLs.  Now the permissions are ANY or NONE.  i.e. A 
given principal can either launch standalone containers or not.
This includes an ACL validation addition.


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


Repository: mesos


Description
---

This defines some coarse-grained AuthZ for launching and managing
standalone containers.  Each HTTP principal can be given the right
to Launch, Wait upon, Kill, or Remove standalone containers under
a given (posix) user.


Diffs (updated)
-

  include/mesos/authorizer/acls.proto 587b71489730f9a1252c73c0239e3d9892b3ae8e 
  include/mesos/authorizer/authorizer.proto 
87a805794f430fc8b2e47de6d624b95deef162b4 
  src/authorizer/local/authorizer.cpp 2fe7b879e649b13322cfcb300c21ef1ed0fea410 


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

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


Testing
---

See later in chain.


Thanks,

Joseph Wu



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

2017-11-13 Thread Joseph Wu

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

(Updated Nov. 13, 2017, 5:22 p.m.)


Review request for mesos, Gilbert Song and Jie Yu.


Changes
---

Comment changes.  Added a beefy section for `RemoveContainer` and deleted some 
comments in the deprecated calls to remove some duplication.


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

  include/mesos/agent/agent.proto 4df3dcef4bd1d42cb8b4955e290bd38038355e46 
  include/mesos/v1/agent/agent.proto e99d23d55a0951f2ed728360e103d83ea5a1ad7f 


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

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


Testing
---

make

See later patches in chain.


Thanks,

Joseph Wu



Re: Review Request 63278: Windows: Documented the `cpu` and `mem` isolators.

2017-11-13 Thread Mesos Reviewbot Windows

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



PASS: Mesos patch 63278 was successfully built and tested.

Reviews applied: `['63268', '63269', '63271', '63272', '63273', '63274', 
'63275', '63276', '63277', '63278']`

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

- Mesos Reviewbot Windows


On Nov. 6, 2017, 11:19 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63278/
> ---
> 
> (Updated Nov. 6, 2017, 11:19 p.m.)
> 
> 
> Review request for mesos, Aaron Wood, Akash Gupta, Jeff Coffler, Jie Yu, John 
> Kordich, Joseph Wu, Li Li, and Michael Park.
> 
> 
> Bugs: MESOS-6690
> https://issues.apache.org/jira/browse/MESOS-6690
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds documentation on the usage of the job object isolators, which
> enable task limits, as well as the statistics they report.
> 
> 
> Diffs
> -
> 
>   docs/isolators/windows.md PRE-CREATION 
>   docs/mesos-containerizer.md a194eb05703650dbce6b9e2fb84c9ff4aa9d9522 
> 
> 
> Diff: https://reviews.apache.org/r/63278/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



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

2017-11-13 Thread Joseph Wu


> On Nov. 8, 2017, 11:06 a.m., Jie Yu wrote:
> > src/tests/agent_container_api_tests.cpp
> > Lines 88 (patched)
> > 
> >
> > Can you comment on what each field means?

I'll move the comment blob (found after the test class is defined) up above the 
class.


> On Nov. 8, 2017, 11:06 a.m., Jie Yu wrote:
> > src/tests/agent_container_api_tests.cpp
> > Lines 251-291 (patched)
> > 
> >
> > Do you still need this? Can you just use LAUNCH_CONTAINER for both 
> > cases? Ditto below. Or you just want to exercise that path?

The point is to exercise the four potential code paths:

A parent container (launched via a framework, or launched via 
`LAUNCH_CONTAINER`)
 X (cartesian product)
A nested container (launched via `LAUNCH_NESTED_CONTAINER`, or via 
`LAUNCH_CONTAINER`).

It's actually a bit messier to disallow certain cases (like parent 
`LAUNCH_CONTAINER` with nested `LAUNCH_NESTED_CONTAINER`).  And the code paths 
are slightly different for each combination, so it's good to test them all.


- Joseph


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


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



Re: Review Request 62145: Implemented Standalone Container API.

2017-11-13 Thread Joseph Wu


> On Nov. 8, 2017, 10:28 a.m., Jie Yu wrote:
> > src/slave/http.cpp
> > Line 2359 (original), 2420-2423 (patched)
> > 
> >
> > I think we should only try to set 'user' if agent `switch_user` flag is 
> > set.
> > 
> > ```
> > if (slave->flags.switch_user) {
> >   if (commandInfo.has_user()) {
> > containerUser = commandInfo.user();
> >   }
> >   
> >   // If 'user' is not specified for standalone container,
> >   // it'll be the same as not switching user in containerizer.
> >   // No need to call os::user() here.
> > }
> > ```

As discussed offline, since we're making the AuthZ into ANY or NONE, some of 
this can go away.


> On Nov. 8, 2017, 10:28 a.m., Jie Yu wrote:
> > src/slave/http.cpp
> > Lines 2510 (patched)
> > 
> >
> > maybe try to remove the sandbox created above and return a 
> > InternalServerError()?

This is mirroring the logic of sandbox creation for normal top-level containers:
https://github.com/apache/mesos/blob/a595bcbf7afd9783e15f3e32cd9e70fa979531df/src/slave/paths.cpp#L613-L617

We don't fail the launch if the sandbox chown fails.

However, since this API is more user facing, it makes sense to fail here.


- Joseph


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


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



Re: Review Request 62579: Added a test `CgroupsIsolatorTest.ROOT_CGROUPS_BlkioUsage`.

2017-11-13 Thread Gilbert Song

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


Ship it!




Ship It!

- Gilbert Song


On Nov. 8, 2017, 6:03 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62579/
> ---
> 
> (Updated Nov. 8, 2017, 6:03 p.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Bugs: MESOS-8013
> https://issues.apache.org/jira/browse/MESOS-8013
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test `CgroupsIsolatorTest.ROOT_CGROUPS_BlkioUsage`.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cgroups_isolator_tests.cpp 
> aad3f53c87af5e87f1b85b746d16d483ca632f83 
> 
> 
> Diff: https://reviews.apache.org/r/62579/diff/4/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>



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

2017-11-13 Thread Andrew Schwartzmeyer

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

(Updated Nov. 13, 2017, 4:20 p.m.)


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


Changes
---

Removed 'ory' from the name for consistency with 'cpu' length.


Summary (updated)
-

Windows: Added `os::set_job_mem_limit` to stout.


Repository: mesos


Description
---

This is used to set a hard cap on the memory usage for a job object.


Diffs (updated)
-

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


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

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


Testing
---


Thanks,

Andrew Schwartzmeyer



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

2017-11-13 Thread Jie Yu

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


Ship it!




Ship It!

- Jie Yu


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



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

2017-11-13 Thread Greg Mann

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

(Updated Nov. 13, 2017, 11:25 p.m.)


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


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


Repository: mesos


Description (updated)
---

Added 'ReconcileOfferOperations' reponse to scheduler API.


Diffs (updated)
-

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


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

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


Testing
---

make check


Thanks,

Greg Mann



Re: Review Request 63063: Modified Containerizer::launch interface to allow repeated launch.

2017-11-13 Thread Joseph Wu


> On Nov. 8, 2017, 11:23 a.m., Jie Yu wrote:
> > src/slave/containerizer/docker.cpp
> > Lines 1259-1260 (original), 1259-1271 (patched)
> > 
> >
> > This is a bit weird. I think the interface of `reapExecutor` is made in 
> > such a way so that it can be chained.
> > 
> > Can you change `reapExecutor`'s interface to return Nothing, and chain 
> > a lambda that returns a SUCCESS
> > ```
> > return container->launch = fetch(containerId)
> >   ...
> >   .then(defer(self(), [=](pid_t pid) {
> > return reapExecutor(containerId, pid);
> >   }))
> >   .then([]() {
> > return Containerizer::LaunchResult::SUCCESS;
> >   }));
> > ```

I will apply this change, but because it touches some other parts that I want 
to call out in the commit message, I'll pull it into a separate patch.


- Joseph


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


On Nov. 2, 2017, 9:02 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63063/
> ---
> 
> (Updated Nov. 2, 2017, 9:02 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-7305
> https://issues.apache.org/jira/browse/MESOS-7305
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> There is some existing tech debt around requiring the caller of
> `Containerizer::launch` to call `Containerizer::destroy` if the launch
> fails (see MESOS-6214).  For nested and standalone containers, the
> side effect of this results in accidentally destroying running
> containers if you make the same call an even number of times.
> 
> For example, suppose the user launches a valid nested container
> with an ID of 'parent.child'. If the user issues the same call to
> launch 'parent.child' again, this second call will fail *and* will
> also destroy the first container.
> 
> This commit prevents repeated launch calls from destroying containers
> by changing the return value of `Containerizer::launch`.  There are
> now four possible return values:
>   * The launch succeeded.
>   * The standalone/nested container already exists.
>   * The given ContainerConfig is not supported.
>   * The launch failed.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/composing.hpp 
> 06d68eef5de7745e32f0e808f11016bcc285dd8f 
>   src/slave/containerizer/composing.cpp 
> 587f009384f0c7ef87482686578dc822d3d5b208 
>   src/slave/containerizer/containerizer.hpp 
> 449bb5d0902936faae7bf9bae9c703b219aed842 
>   src/slave/containerizer/docker.hpp b602a5698cae12686f51c4b9370a06042cda6270 
>   src/slave/containerizer/docker.cpp 292eecbca246edf068ec8c262aff4f3ce9cd8c67 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 6d356ccf82f36df8c6f558fb0ace7d9f982a3d6b 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 100e3bbda543d87808da9ff6bea42da5099ea8c5 
>   src/slave/http.cpp f2e06aff95e0628624b6ed25de222fd3f3577a0b 
>   src/slave/slave.hpp df1b0205124555dcb6a0efa5c237f5e77fa2bdf7 
>   src/slave/slave.cpp 337083dbe60bba2d3773b785bdceeaf0b8fcd070 
>   src/tests/agent_container_api_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/63063/diff/2/
> 
> 
> Testing
> ---
> 
> Requires the next review in the chain (test changes).
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>



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

2017-11-13 Thread Greg Mann

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

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


Repository: mesos


Description
---

The new `OfferOperationUpdate` event type was added to the unversioned
scheduler API protobuf definitions, but the corresponding field was not
added to the `Event` message.


Diffs
-

  include/mesos/scheduler/scheduler.proto 
f5dda10a076ecae53a56875d0ed4d201a1c92f12 


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


Testing
---

make check


Thanks,

Greg Mann



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

2017-11-13 Thread Greg Mann

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

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


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


Repository: mesos


Description
---

This patch adds the top-level `Response` message to the scheduler API,
along with the first response, `ReconcileOfferOperations`.


Diffs
-

  include/mesos/scheduler/scheduler.proto 
f5dda10a076ecae53a56875d0ed4d201a1c92f12 
  include/mesos/v1/scheduler/scheduler.proto 
4fa6606b0db701aebe208b962b7768da68b180ab 


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


Testing
---

make check


Thanks,

Greg Mann



Re: Review Request 63765: Fixed an issue with the scheduler driver subscribe backoff time.

2017-11-13 Thread Mesos Reviewbot Windows

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



FAIL: Some Mesos libprocess-tests failed.

Reviews applied: `['63765']`

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/63765

Relevant logs:

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

```
[   OK ] Scheme/HTTPTest.WWWAuthenticateHeader/0 (837 ms)
[ RUN  ] Scheme/HTTPTest.WWWAuthenticateHeader/1
[   OK ] Scheme/HTTPTest.WWWAuthenticateHeader/1 (71 ms)
[ RUN  ] Scheme/HTTPTest.Accepts/0
[   OK ] Scheme/HTTPTest.Accepts/0 (400 ms)
[ RUN  ] Scheme/HTTPTest.Accepts/1
[   OK ] Scheme/HTTPTest.Accepts/1 (95 ms)
[--] 34 tests from Scheme/HTTPTest (12856 ms total)

[--] 4 tests from SSLVerifyIPAdd/SSLTest
[ RUN  ] SSLVerifyIPAdd/SSLTest.BasicSameProcess/0
[   OK ] SSLVerifyIPAdd/SSLTest.BasicSameProcess/0 (718 ms)
[ RUN  ] SSLVerifyIPAdd/SSLTest.BasicSameProcess/1
[   OK ] SSLVerifyIPAdd/SSLTest.BasicSameProcess/1 (255 ms)
[ RUN  ] SSLVerifyIPAdd/SSLTest.RequireCertificate/0
[   OK ] SSLVerifyIPAdd/SSLTest.RequireCertificate/0 (728 ms)
[ RUN  ] SSLVerifyIPAdd/SSLTest.RequireCertificate/1
[   OK ] SSLVerifyIPAdd/SSLTest.RequireCertificate/1 (1154 ms)
[--] 4 tests from SSLVerifyIPAdd/SSLTest (3015 ms total)

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

 1 FAILED TEST
  YOU HAVE 19 DISABLED TESTS

```

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

```
I1113 22:35:08.356146   388 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1113 22:35:08.356146   388 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
I1113 22:35:08.664160  2056 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1113 22:35:08.665160  2056 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I1113 22:35:08.665160  2056 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1113 22:35:08.666160  2056 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\7tcras\cert.pem
WARNING: Logging before InitGoogleLogging() is written to STDERR
I1113 22:35:09.757180  4952 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1113 22:35:09.759182  4952 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I1113 22:35:09.759182  4952 openssl.cpp:526] Will use IP address verification 
in subject alternative name certificate extension.
I1113 22:35:09.759182  4952 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1113 22:35:09.760185  4952 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\eGNHTo\cert.pem
er certificate verification
I1113 22:35:08.368152   388 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1113 22:35:08.368152   388 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\7tcras\cert.pem
I1113 22:35:09.456182   388 openssl.cpp:509] CA directory path unspecified! 
NOTE: Set CA directory path with LIBPROCESS_SSL_CA_DIR=
I1113 22:35:09.457181   388 openssl.cpp:514] Will not verify peer certificate!
NOTE: Set LIBPROCESS_SSL_VERIFY_CERT=1 to enable peer certificate verification
I1113 22:35:09.457181   388 openssl.cpp:526] Will use IP address verification 
in subject alternative name certificate extension.
I1113 22:35:09.457181   388 openssl.cpp:534] LIBPROCESS_SSL_REQUIRE_CERT 
implies peer certificate verification.
LIBPROCESS_SSL_VERIFY_CERT set to true
I1113 22:35:09.457181   388 openssl.cpp:563] Using CA file: 
C:\Users\mesos\AppData\Local\Temp\eGNHTo\cert.pem
I1113 22:35:10.005447  4220 process.cpp:1052] Failed to accept socket: future 
discarded
```

- Mesos Reviewbot Windows


On Nov. 13, 2017, 1:32 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> 

Review Request 63766: Clarified log message when selecting backend.

2017-11-13 Thread Meng Zhu

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

Review request for mesos and Gilbert Song.


Repository: mesos


Description
---

Clarified log message when selecting backend.


Diffs
-

  src/slave/containerizer/mesos/provisioner/provisioner.cpp 
450a3b32d69d2882973a6ed4e94e169a0256056b 


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


Testing
---


Thanks,

Meng Zhu



Re: Review Request 63731: Reconciled pending resource provider operations in agent.

2017-11-13 Thread Greg Mann

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




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


`operation_uuid` is a required field in 
`OfferOperationUpdateAcknowledgementMessage`.



src/slave/slave.cpp
Lines 3800-3803 (patched)


Am I correct in thinking that we usually don't use quotes around IDs that 
we generate ourselves in Mesos? We may be able to remove the quotes around 
`operation_uuid` here.



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


This currently assumes that all operation update acknowledgements are for 
terminal operation updates - is the plan to write it this way for now, and then 
update later if we add intermediate, non-terminal operation states?



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


Note that in `addOfferOperation()`, we do `CHECK_SOME(uuid)` after grabbing 
`operation.operation_uuid`.

I'm not sure that always checking the error case is necessary when the 
messages are generated by Mesos, but I mention it here because in the 
`offerOperationUpdateAcknowledgement` handler, you're performing an `if 
(operationUuid.isError())` check on a UUID out of a message from master, so the 
treatment of errors seems inconsistent.


- Greg Mann


On Nov. 13, 2017, 6:33 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63731/
> ---
> 
> (Updated Nov. 13, 2017, 6:33 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-8207
> https://issues.apache.org/jira/browse/MESOS-8207
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When resource providers update their state they send a list of
> pending or unacknowledged operations to the agent. This patch add
> tracking for such operations to the agent. The agent can then forward
> these operations to the master, e.g., for calculating the unused
> resources behind an agent.
> 
> We track an operation until we either receive a updated list of
> pending or unacknowledged operations from a resource provider, or
> until we see an acknowledgement from a framework. This keeps the list
> of operations bounded and ensures that we maintain the latest
> information in the agent.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 7cb6661b55fb5437a1ffc447f974076aadd1eced 
> 
> 
> Diff: https://reviews.apache.org/r/63731/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`, still need to implement dedicated tests.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



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

2017-11-13 Thread Meng Zhu

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

(Updated Nov. 13, 2017, 2:03 p.m.)


Review request for mesos, Gilbert Song and James Peach.


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


Repository: mesos


Description (updated)
---

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


Diffs (updated)
-

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


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

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


Testing
---

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


Thanks,

Meng Zhu



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

2017-11-13 Thread Meng Zhu


> On Nov. 10, 2017, 9:07 a.m., James Peach wrote:
> > src/slave/containerizer/mesos/provisioner/provisioner.cpp
> > Lines 134 (patched)
> > 
> >
> > You don't need to make a temporary directory, you can use the one you 
> > already have (i.e. the `.` entry should be `DT_DIR`).
> > 
> > I'm assuming that filesystems won't fake up special cases for `.` and 
> > `..`; is that true?

A temporary directory is needed. My tests show that . and .. entries always 
return DT_DIR regardless of the XFS mount option.


- Meng


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


On Nov. 9, 2017, 5:07 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63652/
> ---
> 
> (Updated Nov. 9, 2017, 5:07 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and James Peach.
> 
> 
> Bugs: MESOS-8121
> https://issues.apache.org/jira/browse/MESOS-8121
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In unified containerizer, the d_type cannot be 1
> if we are using the overlay fs backend.
> 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/2/
> 
> 
> 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 62997: Added checkpoint and recover capability for layers in provisioner.

2017-11-13 Thread Gilbert Song


> On Nov. 9, 2017, 9:25 p.m., Zhitao Li wrote:
> > src/slave/containerizer/mesos/provisioner/provisioner.cpp
> > Lines 531 (patched)
> > 
> >
> > This is the layerRootfs from store.
> > 
> > However, I do not think we should checkpoint "layer dir" instead here, 
> > as it is second guessing how store manages its internal directory 
> > structure, which breaks the current provisioner and store abstraction.

make sense to me!


- Gilbert


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


On Nov. 10, 2017, 11:33 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62997/
> ---
> 
> (Updated Nov. 10, 2017, 11:33 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/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>



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

2017-11-13 Thread Gilbert Song

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


Fix it, then Ship it!




LGTM!


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


Move it right below `slave/paths.hpp`



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


one more space after `foreach`



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


kill this line



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


return Failure("Failed to checkpoint layers to '" + path + "': " + 
checkpoint.error());


- Gilbert Song


On Nov. 10, 2017, 11:33 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62997/
> ---
> 
> (Updated Nov. 10, 2017, 11:33 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/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>



Re: Review Request 63278: Windows: Documented the `cpu` and `mem` isolators.

2017-11-13 Thread Akash Gupta

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


Ship it!




Ship It!

- Akash Gupta


On Nov. 6, 2017, 11:19 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63278/
> ---
> 
> (Updated Nov. 6, 2017, 11:19 p.m.)
> 
> 
> Review request for mesos, Aaron Wood, Akash Gupta, Jeff Coffler, Jie Yu, John 
> Kordich, Joseph Wu, Li Li, and Michael Park.
> 
> 
> Bugs: MESOS-6690
> https://issues.apache.org/jira/browse/MESOS-6690
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds documentation on the usage of the job object isolators, which
> enable task limits, as well as the statistics they report.
> 
> 
> Diffs
> -
> 
>   docs/isolators/windows.md PRE-CREATION 
>   docs/mesos-containerizer.md a194eb05703650dbce6b9e2fb84c9ff4aa9d9522 
> 
> 
> Diff: https://reviews.apache.org/r/63278/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Re: Review Request 63275: Windows: Abstracted out `os::name_job` in stout.

2017-11-13 Thread Akash Gupta

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


Ship it!




Ship It!

- Akash Gupta


On Nov. 2, 2017, 8:40 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63275/
> ---
> 
> (Updated Nov. 2, 2017, 8:40 p.m.)
> 
> 
> Review request for mesos, Aaron Wood, Akash Gupta, Jeff Coffler, Jie Yu, John 
> Kordich, Joseph Wu, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Almost all uses identify a job object by the PID, but the job object
> handle must be obtained by name. So this overloads `os::open_job` to
> call `os::name_job` when given a PID.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/windows/os.hpp 
> 09ddec6d69472cd13b453fe1a77fdbe343fc23c8 
> 
> 
> Diff: https://reviews.apache.org/r/63275/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>



Review Request 63765: Fixed an issue with the scheduler driver subscribe backoff time.

2017-11-13 Thread Meng Zhu

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

Review request for mesos, Gilbert Song and Vinod Kone.


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


Repository: mesos


Description
---

When framework failover time is set to zero (which is the
default value), the scheduler driver subscribe backoff time
will also become zero. Ignore failover time if it is zero
when deciding the subscribe backoff time.
Also added a dedicated test.


Diffs
-

  src/sched/sched.cpp 6028499285ad092ffd252e842c5d9835dd4442f8 
  src/tests/scheduler_driver_tests.cpp 14d872b8fadfd4ef16d8923fb0df924331534bc3 


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


Testing
---

make check, and the new dedicated test.


Thanks,

Meng Zhu



Re: Review Request 63589: Fixed bug in tests leading to orphaned containers.

2017-11-13 Thread Alexander Rukletsov

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


Ship it!




Ship It!

- Alexander Rukletsov


On Nov. 6, 2017, 6:41 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63589/
> ---
> 
> (Updated Nov. 6, 2017, 6:41 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benno Evers, and Gilbert Song.
> 
> 
> Bugs: MESOS-7506
> https://issues.apache.org/jira/browse/MESOS-7506
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, some tests tried to advance the clock until task status
> update was sent, while task's container was destroying. Container
> destruction consists of multiple steps, where some steps have a timeout
> specified, e.g. `cgroups::DESTROY_TIMEOUT`. So, there was a race
> between container destruction process and the loop that advanced the
> clock, leading to the following outcomes:
> 
>   (1) Container destroyed, before clock advancing reaches timeout.
> 
>   (2) Triggered timeout due to clock advancing, before container
>   destruction completes. That results in leaving orphaned
>   containers that will be detected by Slave destructor in
>   `tests/cluster.cpp`, so the test will fail.
> 
> This change gets rid of the loop and resumes clock after a single
> advancing of the clock.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_recovery_tests.cpp 64bba047c6eaee563126a8bd1c6fa048f18172e1 
>   src/tests/slave_tests.cpp cf2fbac4cc53d632c385eb72adb0d80ef942e8a6 
> 
> 
> Diff: https://reviews.apache.org/r/63589/diff/2/
> 
> 
> Testing
> ---
> 
> 1. make check
> 2. internal ci (5x)
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>



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

2017-11-13 Thread Alexander Rukletsov

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

(Updated Nov. 13, 2017, 8:51 p.m.)


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



Re: Review Request 63613: Refactored executor test driver to avoid using uninitialized object.

2017-11-13 Thread Alexander Rukletsov

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

(Updated Nov. 13, 2017, 8:51 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
---

The shared pointer to MockHTTPExecutor is initialized after the library,
while the library may start using it right after its initialization via
the `events` callback. This change removes the shared pointer altogether
and hence prevents possible segfaults.


Diffs
-

  src/tests/mesos.hpp e25defeb55608136e77363aa48cf820092a13a59 


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


Testing
---

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


Thanks,

Alexander Rukletsov



Re: Review Request 63269: Windows: Added `os::set_job_cpu_limit` to stout.

2017-11-13 Thread Andrew Schwartzmeyer


> On Nov. 10, 2017, 5:14 p.m., Akash Gupta wrote:
> > 3rdparty/stout/include/stout/windows/os.hpp
> > Lines 745 (patched)
> > 
> >
> > Just realized this, but I don't think function works if your machine 
> > has > 64 CPUs. It calls GetSystemInfo, which returns the cpus of the 
> > "current" group, which is 64 max. Call 
> > GetMaximumProcessorCount(ALL_PROCESSOR_GROUPS) to get the right number. 
> > There was a bug fix in python with a similar issue to this: 
> > https://github.com/python/cpython/commit/c67bae04780f9d7590f9f91b4ee5f31c5d75b3c3
> 
> Andrew Schwartzmeyer wrote:
> That's a good point, but it's separate issue I'll file.

https://issues.apache.org/jira/browse/MESOS-8216


- Andrew


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


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



Re: Review Request 63269: Windows: Added `os::set_job_cpu_limit` to stout.

2017-11-13 Thread Andrew Schwartzmeyer


> On Nov. 10, 2017, 5:14 p.m., Akash Gupta wrote:
> > 3rdparty/stout/include/stout/windows/os.hpp
> > Lines 745 (patched)
> > 
> >
> > Just realized this, but I don't think function works if your machine 
> > has > 64 CPUs. It calls GetSystemInfo, which returns the cpus of the 
> > "current" group, which is 64 max. Call 
> > GetMaximumProcessorCount(ALL_PROCESSOR_GROUPS) to get the right number. 
> > There was a bug fix in python with a similar issue to this: 
> > https://github.com/python/cpython/commit/c67bae04780f9d7590f9f91b4ee5f31c5d75b3c3

That's a good point, but it's separate issue I'll file.


- Andrew


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


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



Re: Review Request 63269: Windows: Added `os::set_job_cpu_limit` to stout.

2017-11-13 Thread Andrew Schwartzmeyer


> On Nov. 10, 2017, 4:56 p.m., Akash Gupta wrote:
> > 3rdparty/stout/include/stout/windows/os.hpp
> > Lines 760 (patched)
> > 
> >
> > small nit, but for consistency, change false to FALSE.

`FALSE` is used for the Windows APIs, and this is a `stout` API which takes 
`false`.


- Andrew


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


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



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

2017-11-13 Thread Alexander Rukletsov

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

(Updated Nov. 13, 2017, 8:45 p.m.)


Review request for mesos, Anand Mazumdar, Greg Mann, and Vinod Kone.


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/master/master.cpp 49dbaa979d692061a7b479b1db4511e8357b8baf 


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

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


Testing
---

Tested in repetition on Mac OS 10.11.6

`GTEST_FILTER="*SchedulerTest.MasterFailover*" ./bin/mesos-tests.sh --verbose 
--gtest_repeat=500 --gtest_break_on_failure`


Thanks,

Alexander Rukletsov



Re: Review Request 63673: Allowed multiple master detections in MasterFailover test.

2017-11-13 Thread Alexander Rukletsov

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

(Updated Nov. 13, 2017, 8:44 p.m.)


Review request for mesos, Greg Mann and Vinod Kone.


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


Repository: mesos


Description
---

After master failover, the scheduler library can fire new master
detection more than once. We are not interested in sporadic
connected events, but in the end result: the scheduler can
resubscribe with the same framework id.


Diffs (updated)
-

  src/tests/scheduler_tests.cpp c3ac7063f217e57465a5115e03c900b8142e21b4 


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

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


Testing
---

https://reviews.apache.org/r/63674/


Thanks,

Alexander Rukletsov



Re: Review Request 63672: Added a two-parameter SendSubscribe action in test.

2017-11-13 Thread Alexander Rukletsov

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

(Updated Nov. 13, 2017, 8:44 p.m.)


Review request for mesos, Greg Mann and Vinod Kone.


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


Repository: mesos


Description
---

See summary.


Diffs (updated)
-

  src/tests/mesos.hpp 4e2513fc8da22e97a0de84188e79790c18f9a139 


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

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


Testing
---

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


Thanks,

Alexander Rukletsov



Re: Review Request 63214: Fixed GPU tests.

2017-11-13 Thread Benno Evers

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

(Updated Nov. 13, 2017, 8:20 p.m.)


Review request for mesos, Alexander Rukletsov and Jie Yu.


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


Repository: mesos


Description
---

This set of tests was broken because they didn't expect the
TASK_STARTING update from the executor.


Diffs
-

  src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 
b97b9a41b5b3ac042b72689c49933a225a4c2b9a 


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


Testing
---

`sudo ./mesos-tests --gtest_filter="*GPU*" --gtest_repeat=5` on `core-dev`.


Thanks,

Benno Evers



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

2017-11-13 Thread Joseph Wu


> On Aug. 28, 2017, 8:26 p.m., Jie Yu wrote:
> > include/mesos/agent/agent.proto
> > Line 65 (original), 67 (patched)
> > 
> >
> > We should deprecate this as well. `REMOVE_CONTAINER` will be useful for 
> > standalone container as well (to remove the runtime dir which won't be 
> > deleted until this call).

Re-opening this because I'm undo-ing the addition of a `RemoveContainer` call 
(and keeping the `RemoveNestedContainer` call non-deprecated).  Consider 
dropping the issue if you agree with my reasoning:

As it currently stands, `Containerizer::remove` is only intended for nested 
containers launched in a loop under a long-lived parent container.  The runtime 
artifacts of standalone containers will be removed upon destruction like any 
other parent container.  (Note that nested containers under a standalone 
container are considered nested containers.)


- Joseph


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


On Nov. 2, 2017, 8:50 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60890/
> ---
> 
> (Updated Nov. 2, 2017, 8:50 a.m.)
> 
> 
> Review request for mesos, Gilbert Song and Jie Yu.
> 
> 
> Bugs: MESOS-7305
> https://issues.apache.org/jira/browse/MESOS-7305
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> 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 63366: Added jemalloc release tarball and build rules.

2017-11-13 Thread Alexander Rukletsov

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




configure.ac
Lines 211-213 (patched)


Additionally to a more descriptive name, could you please leave a comment 
saying that jemalloc is not linked into project binaries by default, but only 
linked by a specific set of binaries?


- Alexander Rukletsov


On Oct. 27, 2017, 7:03 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63366/
> ---
> 
> (Updated Oct. 27, 2017, 7:03 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This allows building and linking against the bundled
> version of jemalloc when the --enable-jemalloc
> configuration flag is specified.
> 
> 
> Diffs
> -
> 
>   3rdparty/Makefile.am 687f52d9299bf9c6425c2ba7c35c050ff45a33cd 
>   3rdparty/jemalloc-5.0.1.tar.bz2 PRE-CREATION 
>   3rdparty/versions.am cbab26d3e303180c3f210f0c5f45613a5c8c96ba 
>   configure.ac 067005e1be3a09e8320be120c8784f195e30ac5d 
>   src/Makefile.am b60a54a031260de6f1fb43584ae5083df2dc7e31 
> 
> 
> Diff: https://reviews.apache.org/r/63366/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



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

2017-11-13 Thread Alexander Rukletsov

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




3rdparty/libprocess/src/memory_profiler.cpp
Lines 345-346 (patched)


Please add `memory_profiling` to Libprocess flags instead.



3rdparty/libprocess/src/memory_profiler.cpp
Lines 360-361 (patched)


Let's always a default for now. Feel free to leave a TODO to make it 
configurable.


- Alexander Rukletsov


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



Re: Review Request 63370: Added new --memory_profiling flag to agent and master binaries.

2017-11-13 Thread Alexander Rukletsov

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




src/master/flags.cpp
Lines 630-635 (patched)


Let's mention here (and below) that --memory_profiling enables profiling 
but requires jemalloc loaded into the address space of the binary. This can ba 
achieved by either configurting mesos appropriately or using LD_PRELOD. Other 
malloc profiling libraries are not supported.



src/master/flags.cpp
Lines 632 (patched)


Let's call it "inactive" for clarity.


- Alexander Rukletsov


On Oct. 27, 2017, 7:04 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63370/
> ---
> 
> (Updated Oct. 27, 2017, 7:04 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This flag allows explicit disabling of the memory profiler
> endpoint in the master and agent binaries.
> 
> 
> Diffs
> -
> 
>   src/master/flags.hpp edda71af41a8d199bb24b84487300e9c85b12fec 
>   src/master/flags.cpp 18f405b7e9cc6bd0011284b631974eee52dd2bf3 
>   src/master/main.cpp f65ce637d77ce183f83b70dce6da8d0b4b8b8e71 
>   src/slave/flags.hpp d02edbfd68266c9f2d5c78fdbd5c2ba5f497adba 
>   src/slave/flags.cpp 789b45b8d84fc01733a885754204fe2fdd6d6807 
>   src/slave/main.cpp 94431b036524ef4db16a594c80f45b64a45728f7 
> 
> 
> Diff: https://reviews.apache.org/r/63370/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



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

2017-11-13 Thread Michael Park

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


Ship it!




Ship It!

- Michael Park


On Nov. 7, 2017, 4:58 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63615/
> ---
> 
> (Updated Nov. 7, 2017, 4:58 a.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 63614: Removed unnecessary check in v1 scheduler library.

2017-11-13 Thread Michael Park

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


Ship it!




Ship It!

- Michael Park


On Nov. 7, 2017, 4:58 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63614/
> ---
> 
> (Updated Nov. 7, 2017, 4:58 a.m.)
> 
> 
> 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
> 
>



Re: Review Request 63613: Refactored executor test driver to avoid using uninitialized object.

2017-11-13 Thread Michael Park

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


Ship it!




Ship It!

- Michael Park


On Nov. 7, 2017, 4:58 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63613/
> ---
> 
> (Updated Nov. 7, 2017, 4:58 a.m.)
> 
> 
> Review request for mesos, Michael Park and Vinod Kone.
> 
> 
> Bugs: MESOS-8096
> https://issues.apache.org/jira/browse/MESOS-8096
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The shared pointer to MockHTTPExecutor is initialized after the library,
> while the library may start using it right after its initialization via
> the `events` callback. This change removes the shared pointer altogether
> and hence prevents possible segfaults.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp e25defeb55608136e77363aa48cf820092a13a59 
> 
> 
> Diff: https://reviews.apache.org/r/63613/diff/1/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/63615/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>



Re: Review Request 63611: Refactored scheduler test driver to avoid using uninitialized object.

2017-11-13 Thread Michael Park

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


Ship it!




Ship It!

- Michael Park


On Nov. 7, 2017, 4:58 a.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63611/
> ---
> 
> (Updated Nov. 7, 2017, 4:58 a.m.)
> 
> 
> Review request for mesos, Michael Park and Vinod Kone.
> 
> 
> Bugs: MESOS-8096
> https://issues.apache.org/jira/browse/MESOS-8096
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The shared pointer to MockHTTPScheduler is initialized after the
> library, while the library may start using it right after its
> initialization via the `events` callback. This change removes the
> shared pointer altogether and hence prevents possible segfaults.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp e25defeb55608136e77363aa48cf820092a13a59 
> 
> 
> Diff: https://reviews.apache.org/r/63611/diff/1/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/63615/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>



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

2017-11-13 Thread Jan Schlicht

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


Ship it!




Ship It!

- Jan Schlicht


On Nov. 10, 2017, 6:55 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63737/
> ---
> 
> (Updated Nov. 10, 2017, 6:55 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When a terminal status update for an offer operaiton is received, we
> need to apply the conversion for the offer operation to the master's
> view about the total resources of the corresponding agent.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 49dbaa979d692061a7b479b1db4511e8357b8baf 
> 
> 
> Diff: https://reviews.apache.org/r/63737/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>



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

2017-11-13 Thread Mesos Reviewbot Windows

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



FAIL: Failed to apply the dependent review: 63730.

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

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

Relevant logs:

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

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

- Mesos Reviewbot Windows


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



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

2017-11-13 Thread Benjamin Bannier

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

(Updated Nov. 13, 2017, 7:33 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/resource_provider/manager.cpp bcc833b6c96344762a693dd6d4e0a9129aa735f8 
  src/resource_provider/message.hpp a1a84c1fd374b740c56ed2175ee5a2dbc8f96dbc 


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


Testing
---

`make check`, still need to implement dedicated tests.


Thanks,

Benjamin Bannier



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

2017-11-13 Thread Benjamin Bannier

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

(Updated Nov. 13, 2017, 7:33 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  src/master/master.cpp e1381a9a4f0183548a2b8ea8431bf52b0739629f 


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


Testing
---

`make check`, still need to implement dedicated tests.


Thanks,

Benjamin Bannier



Re: Review Request 63731: Reconciled pending resource provider operations in agent.

2017-11-13 Thread Benjamin Bannier

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

(Updated Nov. 13, 2017, 7:33 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


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


Repository: mesos


Description
---

When resource providers update their state they send a list of
pending or unacknowledged operations to the agent. This patch add
tracking for such operations to the agent. The agent can then forward
these operations to the master, e.g., for calculating the unused
resources behind an agent.

We track an operation until we either receive a updated list of
pending or unacknowledged operations from a resource provider, or
until we see an acknowledgement from a framework. This keeps the list
of operations bounded and ensures that we maintain the latest
information in the agent.


Diffs
-

  src/slave/slave.cpp 7cb6661b55fb5437a1ffc447f974076aadd1eced 


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


Testing
---

`make check`, still need to implement dedicated tests.


Thanks,

Benjamin Bannier



Re: Review Request 63728: Fix IoSwitchboard tests.

2017-11-13 Thread Alexander Rukletsov

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


Fix it, then Ship it!





src/tests/containerizer/io_switchboard_tests.cpp
Line 1104 (original), 1115 (patched)


It is not clear that this comment is related to the next two blocks in a 
whole.


- Alexander Rukletsov


On Nov. 13, 2017, 4:24 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63728/
> ---
> 
> (Updated Nov. 13, 2017, 4:24 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fix IoSwitchboard tests.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/io_switchboard_tests.cpp 
> 0828119f88e3d74a48f904b9aeb6b401c1766e8d 
> 
> 
> Diff: https://reviews.apache.org/r/63728/diff/1/
> 
> 
> Testing
> ---
> 
> `sudo ./src/mesos-tests --gtest_filter="*IOSwitchboard*"`
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Re: Review Request 63761: Replaced std::shared_ptr with std::unique_ptr in dispatch.

2017-11-13 Thread Mesos Reviewbot Windows

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



FAIL: mesos-java failed to build.

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

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

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

Relevant logs:

- 
[mesos-java-build-cmake-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63761/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(354): 
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 63673: Allowed multiple master detections in MasterFailover test.

2017-11-13 Thread James Peach

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


Ship it!




Ship It!

- James Peach


On Nov. 8, 2017, 5:31 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63673/
> ---
> 
> (Updated Nov. 8, 2017, 5:31 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Bugs: MESOS-6949
> https://issues.apache.org/jira/browse/MESOS-6949
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> After master failover, the scheduler library can fire new master
> detection more than once. We are not interested in sporadic
> connected events, but in the end result: the scheduler can
> resubscribe with the same framework id.
> 
> 
> Diffs
> -
> 
>   src/tests/scheduler_tests.cpp 60f60ccd721a5362f827e284f6de6d6e80816ed8 
> 
> 
> Diff: https://reviews.apache.org/r/63673/diff/1/
> 
> 
> Testing
> ---
> 
> https://reviews.apache.org/r/63674/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>



Re: Review Request 63672: Added a two-parameter SendSubscribe action in test.

2017-11-13 Thread James Peach

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


Ship it!




Ship It!

- James Peach


On Nov. 8, 2017, 5:31 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63672/
> ---
> 
> (Updated Nov. 8, 2017, 5:31 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Bugs: MESOS-6949
> https://issues.apache.org/jira/browse/MESOS-6949
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp e25defeb55608136e77363aa48cf820092a13a59 
> 
> 
> Diff: https://reviews.apache.org/r/63672/diff/1/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/63674/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>



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

2017-11-13 Thread James Peach

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


Ship it!




Ship It!

- James Peach


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 63614: Removed unnecessary check in v1 scheduler library.

2017-11-13 Thread James Peach

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


Ship it!




Ship It!

- James Peach


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/63614/
> ---
> 
> (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
> ---
> 
> 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
> 
>



Re: Review Request 63613: Refactored executor test driver to avoid using uninitialized object.

2017-11-13 Thread James Peach

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


Ship it!




Ship It!

- James Peach


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/63613/
> ---
> 
> (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
> ---
> 
> The shared pointer to MockHTTPExecutor is initialized after the library,
> while the library may start using it right after its initialization via
> the `events` callback. This change removes the shared pointer altogether
> and hence prevents possible segfaults.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp e25defeb55608136e77363aa48cf820092a13a59 
> 
> 
> Diff: https://reviews.apache.org/r/63613/diff/1/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/63615/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>



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

2017-11-13 Thread Mesos Reviewbot Windows

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



PASS: Mesos patch 63751 was successfully built and tested.

Reviews applied: `['63622', '63679', '63625', '63751']`

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

- Mesos Reviewbot Windows


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



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

2017-11-13 Thread James Peach

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



AFAICT this was already formatted consistently with Mesos conventions. Now the 
`inverseOffers` and `rescindInverseOffers` declarations are different from the 
`offerOperationUpdate`, which makes it look weird.

- James Peach


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/63612/
> ---
> 
> (Updated Nov. 7, 2017, 12:58 p.m.)
> 
> 
> 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
> 
>



Re: Review Request 63611: Refactored scheduler test driver to avoid using uninitialized object.

2017-11-13 Thread James Peach

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


Ship it!




Ship It!

- James Peach


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/63611/
> ---
> 
> (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
> ---
> 
> The shared pointer to MockHTTPScheduler is initialized after the
> library, while the library may start using it right after its
> initialization via the `events` callback. This change removes the
> shared pointer altogether and hence prevents possible segfaults.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp e25defeb55608136e77363aa48cf820092a13a59 
> 
> 
> Diff: https://reviews.apache.org/r/63611/diff/1/
> 
> 
> Testing
> ---
> 
> See https://reviews.apache.org/r/63615/
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>



Review Request 63761: Replaced std::shared_ptr with std::unique_ptr in dispatch.

2017-11-13 Thread Dmitry Zhuk

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

Review request for mesos, Benjamin Mahler and Michael Park.


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


Repository: mesos


Description
---

Since `dispatch` can now handle movable only parameters, `Promise` and
functor can be safely wrapped into `std::unique_ptr` for efficiency.


Diffs
-

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


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


Testing
---

make check


Thanks,

Dmitry Zhuk



Re: Review Request 63710: Fixed flaky RescindRevocableOfferWithIncreasedRevocable test.

2017-11-13 Thread James Peach

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


Ship it!




Ship It!

- James Peach


On Nov. 9, 2017, 11:45 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63710/
> ---
> 
> (Updated Nov. 9, 2017, 11:45 p.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7519
> https://issues.apache.org/jira/browse/MESOS-7519
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This is addendum to review https://reviews.apache.org/r/55893/.
> The loop that merges outstanding offers can miss some because it
> removes the items from the collection it iterates over.
> 
> 
> Diffs
> -
> 
>   src/tests/oversubscription_tests.cpp 
> d262bbe06787f70427f4f7820e64891c7d1b5552 
> 
> 
> Diff: https://reviews.apache.org/r/63710/diff/1/
> 
> 
> Testing
> ---
> 
> `GLOG_v=2 GTEST_FILTER="*RescindRevocableOfferWithIncreasedRevocable*" 
> ./bin/mesos-tests.sh --verbose --gtest_repeat=1000 --gtest_break_on_failure` 
> on Mac OS 10.11.6
> does not fail with the patch while consistently fails without.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>



Re: Review Request 63728: Fix IoSwitchboard tests.

2017-11-13 Thread Benno Evers

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

(Updated Nov. 13, 2017, 4:24 p.m.)


Review request for mesos and Alexander Rukletsov.


Repository: mesos


Description
---

Fix IoSwitchboard tests.


Diffs
-

  src/tests/containerizer/io_switchboard_tests.cpp 
0828119f88e3d74a48f904b9aeb6b401c1766e8d 


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


Testing (updated)
---

`sudo ./src/mesos-tests --gtest_filter="*IOSwitchboard*"`


Thanks,

Benno Evers



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

2017-11-13 Thread Jan Schlicht

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

(Updated Nov. 13, 2017, 3:21 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Fixed an issues with allocation info being set.


Summary (updated)
-

Triggered 'ApplyOfferOperationMessage' for agent local resources.


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


Repository: mesos


Description
---

Agents the have the 'RESOURCE_PROVIDER' capability set, should get sent
'ApplyOfferOperationMessage' instead of 'CheckpointResourcesMessage'.
The agent will then figure out how to apply the operation. For agent
local resources the agent will checkpoint resources.


Diffs (updated)
-

  src/common/protobuf_utils.hpp 0ca4c6d689bf7d2c477174b039432ed7b6d0b650 
  src/common/protobuf_utils.cpp 5739a63f8d87923c034375b88c4f0b3b19f4b521 
  src/master/master.cpp 49dbaa979d692061a7b479b1db4511e8357b8baf 
  src/slave/slave.cpp 7cb6661b55fb5437a1ffc447f974076aadd1eced 
  src/tests/reservation_tests.cpp 470f7341686e69d0a71fb234a26b277c45c29780 


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

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


Testing
---

make check


Thanks,

Jan Schlicht



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

2017-11-13 Thread Jan Schlicht

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

(Updated Nov. 13, 2017, 3:18 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
---

Added JSON string.


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

  src/common/protobuf_utils.hpp 0ca4c6d689bf7d2c477174b039432ed7b6d0b650 
  src/common/protobuf_utils.cpp 5739a63f8d87923c034375b88c4f0b3b19f4b521 
  src/resource_provider/manager.cpp bcc833b6c96344762a693dd6d4e0a9129aa735f8 
  src/resource_provider/message.hpp a1a84c1fd374b740c56ed2175ee5a2dbc8f96dbc 
  src/slave/slave.hpp 0124df44256685689b593dfbf962e4482e4495c6 
  src/slave/slave.cpp 7cb6661b55fb5437a1ffc447f974076aadd1eced 
  src/tests/resource_provider_manager_tests.cpp 
4008b1c751d6227b99adef756e95174d7d8a62f2 


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

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


Testing
---

make check


Thanks,

Jan Schlicht



Review Request 63752: Updated documentation on protobuf version requirements.

2017-11-13 Thread Dmitry Zhuk

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

Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description
---

Updated documentation on protobuf version requirements.


Diffs
-

  3rdparty/stout/README.md 2637f1408403815bc7cd861b08796286118474b2 


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


Testing
---


Thanks,

Dmitry Zhuk



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

2017-11-13 Thread Jan Schlicht

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

(Updated Nov. 13, 2017, 11:49 a.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description
---

Agents the have the 'RESOURCE_PROVIDER' capability set, should get sent
'ApplyOfferOperationMessage' instead of 'CheckpointResourcesMessage'.
The agent will then figure out how to apply the operation. For agent
local resources the agent will checkpoint resources.


Diffs
-

  src/common/protobuf_utils.hpp 0ca4c6d689bf7d2c477174b039432ed7b6d0b650 
  src/common/protobuf_utils.cpp 5739a63f8d87923c034375b88c4f0b3b19f4b521 
  src/master/master.cpp 49dbaa979d692061a7b479b1db4511e8357b8baf 
  src/slave/slave.cpp 7cb6661b55fb5437a1ffc447f974076aadd1eced 
  src/tests/reservation_tests.cpp 470f7341686e69d0a71fb234a26b277c45c29780 


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


Testing
---

make check


Thanks,

Jan Schlicht



Review Request 63751: Triggered 'ApplyOfferOperatioMessage' for agent local resources.

2017-11-13 Thread Jan Schlicht

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

Review request for mesos, Benjamin Bannier and Jie Yu.


Repository: mesos


Description
---

Agents the have the 'RESOURCE_PROVIDER' capability set, should get sent
'ApplyOfferOperationMessage' instead of 'CheckpointResourcesMessage'.
The agent will then figure out how to apply the operation. For agent
local resources the agent will checkpoint resources.


Diffs
-

  src/common/protobuf_utils.hpp 0ca4c6d689bf7d2c477174b039432ed7b6d0b650 
  src/common/protobuf_utils.cpp 5739a63f8d87923c034375b88c4f0b3b19f4b521 
  src/master/master.cpp 49dbaa979d692061a7b479b1db4511e8357b8baf 
  src/slave/slave.cpp 7cb6661b55fb5437a1ffc447f974076aadd1eced 
  src/tests/reservation_tests.cpp 470f7341686e69d0a71fb234a26b277c45c29780 


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


Testing
---

make check


Thanks,

Jan Schlicht



Re: Review Request 62893: Expand description of the --recovery_timeout flag.

2017-11-13 Thread Alexander Rukletsov

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


Ship it!




Ship It!

- Alexander Rukletsov


On Nov. 10, 2017, 3:45 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62893/
> ---
> 
> (Updated Nov. 10, 2017, 3:45 p.m.)
> 
> 
> Review request for mesos and Alexander Rukletsov.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Expand description of the --recovery_timeout flag.
> 
> 
> Diffs
> -
> 
>   src/slave/flags.cpp c5078a85f4d3b30ffd8a9fcc1fbb87980d7d2a03 
> 
> 
> Diff: https://reviews.apache.org/r/62893/diff/2/
> 
> 
> Testing
> ---
> 
> No.
> 
> 
> Thanks,
> 
> Benno Evers
> 
>