Re: Review Request 65556: Made the default executor treat agent disconnections more gracefully.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65556 was successfully built and tested.

Reviews applied: `['65548', '65549', '65550', '65551', '65552', '65556']`

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

- Mesos Reviewbot Windows


On Feb. 7, 2018, 9:55 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65556/
> ---
> 
> (Updated Feb. 7, 2018, 9:55 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Greg Mann, Qian Zhang, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-8468
> https://issues.apache.org/jira/browse/MESOS-8468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch makes the default executor not shutdown if there it there are
> active child containers, and it fails to connect or is not subscribed to
> the agent when starting to launch a task group.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp 4a619859095cc2d30f4806813f64a2e48c83b3ea 
> 
> 
> Diff: https://reviews.apache.org/r/65556/diff/2/
> 
> 
> Testing
> ---
> 
> `sudo make check` on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 65551: Stopped shutting down the whole default executor on task launch failure.

[Gaston Kleiman]

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

(Updated Feb. 9, 2018, 10:35 p.m.)


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


Changes
---

Rebased.


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


Repository: mesos


Description
---

The default executor would be completely shutdown on a
`LAUNCH_NESTED_CONTAINER` failure.

This patch makes it kill the affected task group instead of shutting
down and killing all task groups.


Diffs (updated)
-

  src/launcher/default_executor.cpp 4a619859095cc2d30f4806813f64a2e48c83b3ea 


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

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


Testing
---

`sudo make check` on GNU/Linux

Regression test on https://reviews.apache.org/r/65552/


Thanks,

Gaston Kleiman

Re: Review Request 65550: Made default executor not shutdown if unsubscribed during task launch.

[Gaston Kleiman]

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

(Updated Feb. 9, 2018, 10:35 p.m.)


Review request for mesos, Anand Mazumdar, Qian Zhang, and Vinod Kone.


Changes
---

Addressed feedback.


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


Repository: mesos


Description
---

The default executor would unnecessarily shutdown if, while launching a
task group, it gets unsubscribed after having successfully launched the
task group's containers.


Diffs (updated)
-

  src/launcher/default_executor.cpp 4a619859095cc2d30f4806813f64a2e48c83b3ea 


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

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


Testing
---

`make check` on GNU/Linux


Thanks,

Gaston Kleiman

Re: Review Request 65549: Improved some default executor log messages.

[Gaston Kleiman]

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

(Updated Feb. 9, 2018, 10:34 p.m.)


Review request for mesos, Greg Mann, Qian Zhang, and Vinod Kone.


Changes
---

Addressed comment.


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


Repository: mesos


Description
---

Improved some default executor log messages.


Diffs (updated)
-

  src/launcher/default_executor.cpp 4a619859095cc2d30f4806813f64a2e48c83b3ea 


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

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


Testing
---

None, this patch doesn't contain functional changes.


Thanks,

Gaston Kleiman

Re: Review Request 65549: Improved some default executor log messages.

[Gaston Kleiman]

> On Feb. 9, 2018, 6:06 p.m., Joseph Wu wrote:
> > Harmless enough.  What's the reason behind wanting to `s/launch/launch 
> > group/` ?

I wanted to clearly differentiate between launching groups, tasks, and children 
containers (this executor does it all).


- Gaston


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


On Feb. 7, 2018, 10:59 a.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65549/
> ---
> 
> (Updated Feb. 7, 2018, 10:59 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Qian Zhang, and Vinod Kone.
> 
> 
> Bugs: MESOS-8468
> https://issues.apache.org/jira/browse/MESOS-8468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved some default executor log messages.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp 4a619859095cc2d30f4806813f64a2e48c83b3ea 
> 
> 
> Diff: https://reviews.apache.org/r/65549/diff/1/
> 
> 
> Testing
> ---
> 
> None, this patch doesn't contain functional changes.
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 65550: Made default executor not shutdown if unsubscribed during task launch.

[Gaston Kleiman]

> On Feb. 9, 2018, 6:36 p.m., Joseph Wu wrote:
> > src/launcher/default_executor.cpp
> > Lines 539-546 (original), 537 (patched)
> > 
> >
> > Since the guard above was removed, this CHECK could potentially be hit 
> > now.

Good catch, I'll remove the CHECK.


> On Feb. 9, 2018, 6:36 p.m., Joseph Wu wrote:
> > src/launcher/default_executor.cpp
> > Line 558 (original), 549 (patched)
> > 
> >
> > What happens when the executor is disconnected (as is now allowed) and 
> > attempts to launch some health checks?
> > 
> > Any nested command checks would definitely fail.  But I suppose this is 
> > better than shutting down the executor.
> > 
> > Seems like you need to either delay the creation of the health checks 
> > or pause them immediately after creation.

The checker process will treat connection errors as transient failures, and 
reschedule the check: 
https://github.com/apache/mesos/blob/a86ff8c36532f97b6eb6b44c6f871de24afbcc4d/src/checks/checker_process.cpp#L531-L538


Transient failures are logged, but not treated as a health check failure:
https://github.com/apache/mesos/blob/a86ff8c36532f97b6eb6b44c6f871de24afbcc4d/src/checks/checker_process.cpp#L353-L356


> On Feb. 9, 2018, 6:36 p.m., Joseph Wu wrote:
> > src/launcher/default_executor.cpp
> > Lines 626-631 (original), 617-622 (patched)
> > 
> >
> > This will be dropped if the executor isn't subscribed.  And as far as I 
> > can tell, this status update is not sent in any other location

If the executor isn't subscribed, the status updates will be added to the 
`unacknowledgedUpdates` map, and sent by `doReliableRegistration()` in the next 
`SUBSCRIBE` call: 


https://github.com/apache/mesos/blob/a86ff8c36532f97b6eb6b44c6f871de24afbcc4d/src/launcher/default_executor.cpp#L309-L343


The executor doesn't wait for the updates to be ack'd before shutting down 
(https://github.com/apache/mesos/blob/a86ff8c36532f97b6eb6b44c6f871de24afbcc4d/src/launcher/default_executor.cpp#L1020-L1024),
 so there's a possibility that these updates will be dropped if the executor is 
not connected to the agent upon disconnection. This is tracked in 
https://issues.apache.org/jira/browse/MESOS-8537.


- Gaston


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


On Feb. 7, 2018, 11 a.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65550/
> ---
> 
> (Updated Feb. 7, 2018, 11 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Qian Zhang, and Vinod Kone.
> 
> 
> Bugs: MESOS-8468
> https://issues.apache.org/jira/browse/MESOS-8468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The default executor would unnecessarily shutdown if, while launching a
> task group, it gets unsubscribed after having successfully launched the
> task group's containers.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp 4a619859095cc2d30f4806813f64a2e48c83b3ea 
> 
> 
> Diff: https://reviews.apache.org/r/65550/diff/1/
> 
> 
> Testing
> ---
> 
> `make check` on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 65550: Made default executor not shutdown if unsubscribed during task launch.

[Gaston Kleiman]

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




src/launcher/default_executor.cpp
Lines 539-546 (original), 537 (patched)


Good catch, I'll remove the CHECK.



src/launcher/default_executor.cpp
Line 558 (original), 549 (patched)


The checker process will treat connection errors as transient failures, and 
reschedule the check: 
https://github.com/apache/mesos/blob/a86ff8c36532f97b6eb6b44c6f871de24afbcc4d/src/checks/checker_process.cpp#L531-L538

Transient failures are logged, but not treated as a health check failure:

https://github.com/apache/mesos/blob/a86ff8c36532f97b6eb6b44c6f871de24afbcc4d/src/checks/checker_process.cpp#L353-L356



src/launcher/default_executor.cpp
Lines 626-631 (original), 617-622 (patched)


If the executor isn't subscribed, the stauts updates will be added to the 
`unacknowledgedUpdates` map, and sent by `doReliableRegistration()` in the next 
`SUBSCRIBE` call: 


https://github.com/apache/mesos/blob/a86ff8c36532f97b6eb6b44c6f871de24afbcc4d/src/launcher/default_executor.cpp#L309-L343

The executor doesn't wait for the updates to be ack'd before shutting down 
(https://github.com/apache/mesos/blob/a86ff8c36532f97b6eb6b44c6f871de24afbcc4d/src/launcher/default_executor.cpp#L1020-L1024),
 so there's a possibility that these updates will be dropped if the executor is 
not connected to the agent upon disconnection. This is tracked in 
https://issues.apache.org/jira/browse/MESOS-8537.


- Gaston Kleiman


On Feb. 7, 2018, 11 a.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65550/
> ---
> 
> (Updated Feb. 7, 2018, 11 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Qian Zhang, and Vinod Kone.
> 
> 
> Bugs: MESOS-8468
> https://issues.apache.org/jira/browse/MESOS-8468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The default executor would unnecessarily shutdown if, while launching a
> task group, it gets unsubscribed after having successfully launched the
> task group's containers.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp 4a619859095cc2d30f4806813f64a2e48c83b3ea 
> 
> 
> Diff: https://reviews.apache.org/r/65550/diff/1/
> 
> 
> Testing
> ---
> 
> `make check` on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 65591: Explicitly tracked resource providers in master.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [65587, 65588, 65589, 65590, 65591]

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

- Mesos Reviewbot


On Feb. 9, 2018, 5:27 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65591/
> ---
> 
> (Updated Feb. 9, 2018, 5:27 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-8382
> https://issues.apache.org/jira/browse/MESOS-8382
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds explicit tracking of resource providers to the master
> process. While we already had explicitly send resource provider
> information in e.g., `UpdateSlaveMessage`, we only stored that
> information aggregated over the full agent in the master up to now.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.cpp b5c2997ada8362e42150fa3cfd762120e5ea715f 
>   src/master/master.hpp b434d2398b8815811345b6586ca586d2025cb2a2 
>   src/master/master.cpp d7d22866f7a4eb87bd8949efafc97e828e7d4b94 
> 
> 
> Diff: https://reviews.apache.org/r/65591/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 65545: Start from merge-base when posting reviews.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [65545]

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

- Mesos Reviewbot


On Feb. 7, 2018, 3:22 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65545/
> ---
> 
> (Updated Feb. 7, 2018, 3:22 p.m.)
> 
> 
> Review request for mesos, Armand Grillet and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When posting a review originally created from a branch B, it could happen 
> that the resulting review included garbage changes if B was updated between 
> revisions. (i.e., pulling new master changes)
> 
> This review changes the logic of the `post-reviews.py` script to only include 
> the changes between `HEAD` and the merge base of the tracking branch into the 
> review.
> 
> 
> Diffs
> -
> 
>   support/post-reviews.py 94b43253f25044a79009010e92363d1536c74299 
> 
> 
> Diff: https://reviews.apache.org/r/65545/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 65550: Made default executor not shutdown if unsubscribed during task launch.

[Joseph Wu]

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




src/launcher/default_executor.cpp
Lines 539-546 (original), 537 (patched)


Since the guard above was removed, this CHECK could potentially be hit now.



src/launcher/default_executor.cpp
Line 558 (original), 549 (patched)


What happens when the executor is disconnected (as is now allowed) and 
attempts to launch some health checks?

Any nested command checks would definitely fail.  But I suppose this is 
better than shutting down the executor.

Seems like you need to either delay the creation of the health checks or 
pause them immediately after creation.



src/launcher/default_executor.cpp
Lines 626-631 (original), 617-622 (patched)


This will be dropped if the executor isn't subscribed.  And as far as I can 
tell, this status update is not sent in any other location


- Joseph Wu


On Feb. 7, 2018, 11 a.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65550/
> ---
> 
> (Updated Feb. 7, 2018, 11 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Qian Zhang, and Vinod Kone.
> 
> 
> Bugs: MESOS-8468
> https://issues.apache.org/jira/browse/MESOS-8468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The default executor would unnecessarily shutdown if, while launching a
> task group, it gets unsubscribed after having successfully launched the
> task group's containers.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp 4a619859095cc2d30f4806813f64a2e48c83b3ea 
> 
> 
> Diff: https://reviews.apache.org/r/65550/diff/1/
> 
> 
> Testing
> ---
> 
> `make check` on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 65594: Checkpointed profiles in storage local resource provider.

[Chun-Hung Hsiao]

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




src/resource_provider/storage/uri_disk_profile.cpp
Line 248 (original), 242 (patched)


This should be removed to support missing profiles from upstream.


- Chun-Hung Hsiao


On Feb. 10, 2018, 12:47 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65594/
> ---
> 
> (Updated Feb. 10, 2018, 12:47 a.m.)
> 
> 
> Review request for mesos, James DeFelice, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8492
> https://issues.apache.org/jira/browse/MESOS-8492
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> SLRP now checkpoints profiles associated with storage pools, and no
> longer assumes that profiles won't be removed by the disk profile
> adaptor. It will reject volume/block creation requests that use
> missing profiles, but existing volumes/blocks will continue to work.
> When a volume/block with a missing profiles is destroyed, it will be
> converted to an empty resource, and SLRP will update the sizes of
> storage pools for currently known profiles.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/state.proto 8577b58b8cdb63b3daddf06ea5d12f80f9d42f2b 
>   src/resource_provider/storage/provider.cpp 
> 33abc0e05a804969ae14da9cb9c58698ba1aaea5 
>   src/resource_provider/storage/uri_disk_profile.cpp 
> 665798fdb085ea34f93bd287fe6f9ab29a265cbf 
> 
> 
> Diff: https://reviews.apache.org/r/65594/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 65518: Reaped the container process directly in Docker executor.

[Greg Mann]

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




src/docker/executor.cpp
Lines 273-302 (patched)


Could we validate this with a test? I think it would be possible to use a 
`MockDocker` which intercepts the call to `run()`, so that we can prevent it 
from returning when the container exits. I think something like the following 
should work:

```
  MockDocker* mockDocker =
new MockDocker(tests::flags.docker, tests::flags.docker_socket);

  Promise> runPromise;
  Future> runFuture;
  auto interceptedRun = [mockDocker, &runPromise, &runFuture](
  const Docker::RunOptions& runOptions,
  const process::Subprocess::IO& _stdout,
  const process::Subprocess::IO& _stderr) {
runFuture = mockDocker->_run(
runOptions,
_stdout,
_stderr);

return runPromise.future();
  };

  EXPECT_CALL(*mockDocker, run(_, _, _))
.WillRepeatedly(Invoke(interceptedRun));
```
Then we have control over when the `Future` associated with the 'docker 
run' call is satisfied. See the Docker containerizer tests for examples of how 
we use the MockDocker and MockDockerContainerizer.

What do you think?



src/docker/executor.cpp
Lines 275 (patched)


s/will/can/



src/docker/executor.cpp
Lines 276 (patched)


Nit:
s/"docker run"/'docker run'/



src/docker/executor.cpp
Lines 277 (patched)


s/returns/returning/



src/docker/executor.cpp
Lines 280 (patched)


Should we log an error in the case that `container.pid.isNone()`? AFAIK 
this means that the Docker daemon was not able to locate a process associated 
with the container.

This makes me wonder if we should actually do CHECK_SOME(container.pid)? It 
looks like the only place where we use `containerPid` is in the health checker, 
where the PID is used to enter the appropriate namespaces.

For now, I would suggest that we log an error to help in debugging.



src/docker/executor.cpp
Lines 283 (patched)


s/can not/cannot/


- Greg Mann


On Feb. 9, 2018, 1:03 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65518/
> ---
> 
> (Updated Feb. 9, 2018, 1:03 a.m.)
> 
> 
> Review request for mesos, Gaston Kleiman, Gilbert Song, Greg Mann, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-8488
> https://issues.apache.org/jira/browse/MESOS-8488
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Due to a Docker issue (https://github.com/moby/moby/issues/33820),
> Docker daemon will fail to catch a container exit, i.e., the container
> process has already exited but the command `docker ps` shows the
> container still running, this will lead to the "docker run" command
> that we execute in Docker executor never returns, and it will also
> cause the `docker stop` command takes no effect, i.e., it will return
> without error but `docker ps` shows the container still running, so
> the task will stuck in `TASK_KILLING` state.
> 
> To workaround this Docker issue, in this patch we made Docker executor
> reaps the container process directly so Docker executor will be notified
> once the container process exits.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp e4c53d558e414e50b1c429fba8e31e504c63744a 
> 
> 
> Diff: https://reviews.apache.org/r/65518/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 65549: Improved some default executor log messages.

[Joseph Wu]

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



Harmless enough.  What's the reason behind wanting to `s/launch/launch group/` ?


src/launcher/default_executor.cpp
Line 509 (original), 509 (patched)


What about this one?


- Joseph Wu


On Feb. 7, 2018, 10:59 a.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65549/
> ---
> 
> (Updated Feb. 7, 2018, 10:59 a.m.)
> 
> 
> Review request for mesos, Greg Mann, Qian Zhang, and Vinod Kone.
> 
> 
> Bugs: MESOS-8468
> https://issues.apache.org/jira/browse/MESOS-8468
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Improved some default executor log messages.
> 
> 
> Diffs
> -
> 
>   src/launcher/default_executor.cpp 4a619859095cc2d30f4806813f64a2e48c83b3ea 
> 
> 
> Diff: https://reviews.apache.org/r/65549/diff/1/
> 
> 
> Testing
> ---
> 
> None, this patch doesn't contain functional changes.
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 65548: Added `Event::Update` and `v1::scheduler::TaskStatus` ostream operators.

[Joseph Wu]

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


Ship it!




LGTM.


src/v1/mesos.cpp
Lines 604 (patched)


For debugging purposes, it may also help to include the `source` and 
`reason` fields (if present).  And maybe even `slave_id`.


- Joseph Wu


On Feb. 7, 2018, 10:57 a.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65548/
> ---
> 
> (Updated Feb. 7, 2018, 10:57 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Greg Mann, Qian Zhang, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This operators make gtest print a human-readable representation of the
> protos on test failures.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/mesos.hpp d4c354ab596a6ea361f2fe45afa46089f8c1a543 
>   include/mesos/v1/scheduler/scheduler.hpp 
> 2fdd8f265d8dd5e3a334055ab8777a175688f620 
>   src/v1/mesos.cpp 8abeae06e0fa87b50933df1c222ad4724a0b0116 
> 
> 
> Diff: https://reviews.apache.org/r/65548/diff/2/
> 
> 
> Testing
> ---
> 
> Example test failure without the patch:
> 
> ```
> Unexpected mock function call - returning directly.
> Function call: update(0x7ffedfa94eb0, @0x7f8638002c30 32-byte object 
> <50-92 EA-29 87-7F 00-00 00-00 00-00 00-00 00-00 01-00 00-00 00-00 00-00 
> 60-30 00-38 86-7F 00-00>)
> Google Mock tried the following 8 expectations, but none matched:
> 
> ../../src/tests/default_executor_tests.cpp:3315: tried expectation #0: 
> EXPECT_CALL(*scheduler, update(_, AllOf( 
> TaskStatusUpdateTaskIdEq(sleepTaskInfo1), 
> TaskStatusUpdateStateEq(v1::TASK_ERROR...
> [...]
> ) and (task status update state eq TASK_ERROR)
>Actual: 32-byte object <50-92 EA-29 87-7F 00-00 00-00 00-00 00-00 
> 00-00 01-00 00-00 00-00 00-00 60-30 00-38 86-7F 00-00>
>  Expected: to be called once
>Actual: never called - unsatisfied and active
> ```
> 
> After applying the patch:
> 
> ```
> Unexpected mock function call - returning directly.
> Function call: update(0x7ffc6b036a20, @0x7f4af4000bb0 TASK_STARTING 
> (Status UUID: f379eb50-1163-442a-8e30-a0c2f5247575) for task 'sleepTask1')
> Google Mock tried the following 8 expectations, but none matched:
> 
> ../../src/tests/default_executor_tests.cpp:3315: tried expectation #0: 
> EXPECT_CALL(*scheduler, update(_, AllOf( 
> TaskStatusUpdateTaskIdEq(sleepTaskInfo1), 
> TaskStatusUpdateStateEq(v1::TASK_ERROR...
> [...]
> ) and (task status update state eq TASK_ERROR)
>Actual: TASK_STARTING (Status UUID: 
> f379eb50-1163-442a-8e30-a0c2f5247575) for task 'sleepTask1'
> ```
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 65594: Checkpointed profiles in storage local resource provider.

[Mesos Reviewbot Windows]

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



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

Reviews applied: `['65594']`

Failed command: `Start-MesosCITesting`

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

Relevant logs:

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

```
 
D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(2601):
 warning C4267: '=': conversion from 'size_t' to 'int', possible loss of data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(166):
 warning C4716: 'pthread_cond_broadcast': must return a value 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3091):
 warning C4267: 'initializing': conversion from 'size_t' to 'int', possible 
loss of data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
d:\dcos\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\winport.c(205):
 warning C4716: 'pthread_cond_wait': must return a value 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3426):
 warning C4996: 'strerror': This function or variable may be unsafe. Consider 
using strerror_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. 
See online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3500):
 warning C4996: 'inet_ntoa': Use inet_ntop() or InetNtop() instead or define 
_WINSOCK_DEPRECATED_NO_WARNINGS to disable deprecated API warnings 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3501):
 warning C4996: 'sprintf': This function or variable may be unsafe. Consider 
using sprintf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. 
See online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\zookeeper.c(3479):
 warning C4101: 'addrstr': unreferenced local variable 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\zookeeper.vcxproj]
 [D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(124):
 warning C4996: 'fopen': This function or variable may be unsafe. Consider 
using fopen_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See 
online help for details. 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(128):
 warning C4267: 'initializing': conversion from 'size_t' to 'int', possible 
loss of data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(279):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(301):
 warning C4267: 'initializing': conversion from 'size_t' to 'int', possible 
loss of data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxproj]
 
D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8\src\c\src\cli.c(368):
 warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of 
data 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8\src\zookeeper-3.4.8-build\cli.vcxproj] 
[D:\DCOS\mesos\3rdparty\zookeeper-3.4.8.vcxp

Re: Review Request 65551: Stopped shutting down the whole default executor on task launch failure.

[Gaston Kleiman]

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

(Updated Feb. 9, 2018, 5:37 p.m.)


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


Changes
---

Minor style fix.


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


Repository: mesos


Description
---

The default executor would be completely shutdown on a
`LAUNCH_NESTED_CONTAINER` failure.

This patch makes it kill the affected task group instead of shutting
down and killing all task groups.


Diffs (updated)
-

  src/launcher/default_executor.cpp 4a619859095cc2d30f4806813f64a2e48c83b3ea 


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

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


Testing
---

`sudo make check` on GNU/Linux

Regression test on https://reviews.apache.org/r/65552/


Thanks,

Gaston Kleiman

Re: Review Request 65593: Added a test to ensure master removes fail-launched deafult executor.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65593 was successfully built and tested.

Reviews applied: `['65445', '65504', '65446', '65449', '65448', '65593']`

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

- Mesos Reviewbot Windows


On Feb. 10, 2018, 12:36 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65593/
> ---
> 
> (Updated Feb. 10, 2018, 12:36 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-1720
> https://issues.apache.org/jira/browse/MESOS-1720
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test ensures that agent sends ExitedExecutorMessage when the
> task group fails to launch due to unscheule GC failure. So that
> master's executor bookkeeping entry is removed.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 628b0d0fc862264c9553e1660c7df548df9cd4a1 
> 
> 
> Diff: https://reviews.apache.org/r/65593/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 65497: Added test to verify task-less executor is shutdown when re-subscribing.

[Benjamin Mahler]

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




src/tests/mesos.cpp
Line 391 (original), 400 (patched)


I realize this is a copy/paste, but do you know why we don't start() when 
mock is true?



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


Two spaces here?


- Benjamin Mahler


On Feb. 3, 2018, 1:18 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65497/
> ---
> 
> (Updated Feb. 3, 2018, 1:18 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8411
> https://issues.apache.org/jira/browse/MESOS-8411
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test verifies that the v1 executor is shutdown if all of its
> initial tasks could not be delivered when re-subscribing with
> a recovered agent. See MESOS-8411.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp c5593c25dde18c005f195a1885a8586bc72c849f 
>   src/tests/mesos.cpp ac789297d6aa034121b73e4efc0aafee1ee3b60f 
>   src/tests/mock_slave.cpp 597d7abef20dd5f89b16e4616233f02760b9d037 
>   src/tests/slave_tests.cpp b54688add9949b9c4ac2ce3a42503a411e6da09f 
> 
> 
> Diff: https://reviews.apache.org/r/65497/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 65497: Added test to verify task-less executor is shutdown when re-subscribing.

[Benjamin Mahler]

> On Feb. 8, 2018, 2:19 a.m., Benjamin Mahler wrote:
> > src/tests/mock_slave.cpp
> > Lines 107-108 (original), 107-109 (patched)
> > 
> >
> > What's going on here?
> 
> Meng Zhu wrote:
> For the agent failover test, I need the failed agent to start with the 
> same pid. There is a parameter `id` you can pass to the Slave constructor 
> above. But it turns out the original code ignore that argument and just call 
> `process::ID::generate("slave")` every time, changed it to take the argument.

I see, so this was a bug? Can you fix this in a separate patch?

I also didn't follow why you needed to add the `ProcessBase` constructor call, 
since that's done within the slave constructor already? What does it mean for 
it to happen twice?


> On Feb. 8, 2018, 2:19 a.m., Benjamin Mahler wrote:
> > src/tests/slave_tests.cpp
> > Lines 4926-4927 (patched)
> > 
> >
> > even after the exeutor has been registered..? I think you want to 
> > remove that?
> 
> Meng Zhu wrote:
> Why? The agent failover happens after the executor has been registered. 
> Then the executor re-registers and got killed.

Oh I read it as the executor being registered after failover rather than 
before, how about:

```
// This test verifies that if an agent fails over after registering
// a v1 executor but before delivering its initial task groups, the
// executor will be shut down since all of its initial task groups
// were dropped. See MESOS-8411.
```

This also avoids the hanging sentence at the end that mentions task group by 
stating it within the description.


> On Feb. 8, 2018, 2:19 a.m., Benjamin Mahler wrote:
> > src/tests/slave_tests.cpp
> > Lines 4945 (patched)
> > 
> >
> > "NotSlave"?
> > 
> > We should probably have found a way to use the logic from 
> > Master::newSlaveId here.
> 
> Meng Zhu wrote:
> Changed it to "slave". That we will need to change cluster::master, I do 
> not think it is important here. Let's just use "slave".

Oh, I mistook this for the SlaveID rather than the PID::id. I'm not sure I 
followed your response, but re-using the master code isn't applicable here 
anyway.

To avoid this confusion for other readers, can you do the following?

```
string processId = process::ID::generate("slave");

StartSlave(..., processId, ...);
```


> On Feb. 8, 2018, 2:19 a.m., Benjamin Mahler wrote:
> > src/tests/slave_tests.cpp
> > Lines 5032-5038 (patched)
> > 
> >
> > Instead of using a slave mock here, can we just expect the executorLost 
> > signal from the scheduler?
> 
> Meng Zhu wrote:
> This is v1 scheduler, I think the `Failure` message is not as clear as 
> the `_shutdownExecutor` call of the slave.

Ok! Another option is to expect the container is destroyed, but this looks 
good, marking as resolved.


- Benjamin


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


On Feb. 3, 2018, 1:18 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65497/
> ---
> 
> (Updated Feb. 3, 2018, 1:18 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8411
> https://issues.apache.org/jira/browse/MESOS-8411
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test verifies that the v1 executor is shutdown if all of its
> initial tasks could not be delivered when re-subscribing with
> a recovered agent. See MESOS-8411.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp c5593c25dde18c005f195a1885a8586bc72c849f 
>   src/tests/mesos.cpp ac789297d6aa034121b73e4efc0aafee1ee3b60f 
>   src/tests/mock_slave.cpp 597d7abef20dd5f89b16e4616233f02760b9d037 
>   src/tests/slave_tests.cpp b54688add9949b9c4ac2ce3a42503a411e6da09f 
> 
> 
> Diff: https://reviews.apache.org/r/65497/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 65570: WIP: Attached/detached volume directory for task which has volume specified.

[Qian Zhang]

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

(Updated Feb. 10, 2018, 9:01 a.m.)


Review request for mesos and Gilbert Song.


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


Repository: mesos


Description
---

Attached/detached volume directory for task which has volume specified.


Diffs
-

  src/slave/slave.cpp f98f37321872d090176b7cc50873fc3c627773f5 


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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 65518: Reaped the container process directly in Docker executor.

[Vinod Kone]

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




src/docker/executor.cpp
Lines 277 (patched)


s/never returns though/to never return although/



src/docker/executor.cpp
Lines 280 (patched)


when can this be None()?



src/docker/executor.cpp
Lines 288 (patched)


Add a LOG line here?



src/docker/executor.cpp
Lines 289 (patched)


s/lambda/lambda;/



src/docker/executor.cpp
Lines 530 (patched)


why not call `reaped` directly? sounds like `reaped` does a bunch of 
necessary cleanup that you are skipping by calling `_reaped`?



src/docker/executor.cpp
Line 490 (original), 535 (patched)


shouldn't we return here if `terminated` already is set in case the `run` 
returns after `reapedContainer` is called?


- Vinod Kone


On Feb. 9, 2018, 1:03 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65518/
> ---
> 
> (Updated Feb. 9, 2018, 1:03 a.m.)
> 
> 
> Review request for mesos, Gaston Kleiman, Gilbert Song, Greg Mann, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-8488
> https://issues.apache.org/jira/browse/MESOS-8488
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Due to a Docker issue (https://github.com/moby/moby/issues/33820),
> Docker daemon will fail to catch a container exit, i.e., the container
> process has already exited but the command `docker ps` shows the
> container still running, this will lead to the "docker run" command
> that we execute in Docker executor never returns, and it will also
> cause the `docker stop` command takes no effect, i.e., it will return
> without error but `docker ps` shows the container still running, so
> the task will stuck in `TASK_KILLING` state.
> 
> To workaround this Docker issue, in this patch we made Docker executor
> reaps the container process directly so Docker executor will be notified
> once the container process exits.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp e4c53d558e414e50b1c429fba8e31e504c63744a 
> 
> 
> Diff: https://reviews.apache.org/r/65518/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Review Request 65594: Checkpointed profiles in storage local resource provider.

[Chun-Hung Hsiao]

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

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


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


Repository: mesos


Description
---

SLRP now checkpoints profiles associated with storage pools, and no
longer assumes that profiles won't be removed by the disk profile
adaptor. It will reject volume/block creation requests that use
missing profiles, but existing volumes/blocks will continue to work.
When a volume/block with a missing profiles is destroyed, it will be
converted to an empty resource, and SLRP will update the sizes of
storage pools for currently known profiles.


Diffs
-

  src/resource_provider/state.proto 8577b58b8cdb63b3daddf06ea5d12f80f9d42f2b 
  src/resource_provider/storage/provider.cpp 
33abc0e05a804969ae14da9cb9c58698ba1aaea5 
  src/resource_provider/storage/uri_disk_profile.cpp 
665798fdb085ea34f93bd287fe6f9ab29a265cbf 


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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 62800: Created cgroups under systemd hierarchy in LinuxLauncher.

[James Peach]

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


Ship it!




Ship It!

- James Peach


On Feb. 7, 2018, 8:01 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62800/
> ---
> 
> (Updated Feb. 7, 2018, 8:01 p.m.)
> 
> 
> Review request for mesos, James Peach, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-7990
> https://issues.apache.org/jira/browse/MESOS-7990
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added the support for systemd hierarchy in LinuxLauncher.
> It created the same cgroup layout under the systemd hierarchy (if
> systemd is enabled) as that in the freezer hierarchy.
> 
> This can give us a bunch of benefits:
> 1) systemd-cgls can list mesos container processes.
> 2) systemd-cgtop can show stats for mesos containers.
> 3) Avoid the pid migration issue described in MESOS-3352.
> 
> For example:
> 
> ```
> [jie@core-dev ~]$ systemd-cgls
> |-1 /usr/lib/systemd/systemd --system --deserialize 20
> |-mesos
> |  |-8282b91a-5724-4964-a623-7c6bd68ff4ad
> |  |-31737 /usr/libexec/mesos/mesos-containerizer launch
> |  |-31739 mesos-default-executor --launcher_dir=/usr/libexec/mesos
> |  |-mesos
> | |-8555f4af-fa4f-4c9c-aeb3-0c9f72e6a2de
> |   |-31791 /usr/libexec/mesos/mesos-containerizer launch
> |   |-31793 sleep 1000
> ```
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/linux_launcher.cpp 
> c2e319894874e628beda6dc305462af0c274fd7b 
> 
> 
> Diff: https://reviews.apache.org/r/62800/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> I also use the steps describe in MESOS-3352 to repo the systemd pid migration 
> issue. Didn't observe it after applying the patch.
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 62799: Fixed an issue for the I/O switchboard process lifetime.

[James Peach]

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


Ship it!




Ship It!

- James Peach


On Feb. 7, 2018, 8:01 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62799/
> ---
> 
> (Updated Feb. 7, 2018, 8:01 p.m.)
> 
> 
> Review request for mesos, Joseph Wu and Kevin Klues.
> 
> 
> Bugs: MESOS-8056
> https://issues.apache.org/jira/browse/MESOS-8056
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We expect the I/O switchboard process to last across agent restarts
> (similar to log rotate process or executor processes). Therefore, we
> should put it into 'mesos_executor.slice' like others.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/io/switchboard.cpp 
> aeb0b3e4cf75b19efd9b3922cc4707d3c5cb 
> 
> 
> Diff: https://reviews.apache.org/r/62799/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Re: Review Request 62798: Added named cgroup hierarchy support.

[James Peach]

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


Ship it!




Ship It!

- James Peach


On Feb. 7, 2018, 8:01 p.m., Jie Yu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62798/
> ---
> 
> (Updated Feb. 7, 2018, 8:01 p.m.)
> 
> 
> Review request for mesos, James Peach, Joris Van Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-7990
> https://issues.apache.org/jira/browse/MESOS-7990
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch add a helper to get the cgroup associated with the given
> pid for a named cgroup hierarchy.
> 
> 
> Diffs
> -
> 
>   src/linux/cgroups.hpp 154e5b6c1adef4f4e77f1747c56dfd83a9967ed4 
>   src/linux/cgroups.cpp cdd0f40ec5748b909e529f21b15523122bf90d41 
> 
> 
> Diff: https://reviews.apache.org/r/62798/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Review Request 65593: Added a test to ensure master removes fail-launched deafult executor.

[Meng Zhu]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

This test ensures that agent sends ExitedExecutorMessage when the
task group fails to launch due to unscheule GC failure. So that
master's executor bookkeeping entry is removed.


Diffs
-

  src/tests/slave_tests.cpp 628b0d0fc862264c9553e1660c7df548df9cd4a1 


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


Testing
---

make check


Thanks,

Meng Zhu

Re: Review Request 65448: Added a test to ensure master removes executors that never launched.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65448 was successfully built and tested.

Reviews applied: `['65445', '65504', '65446', '65449', '65448']`

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

- Mesos Reviewbot Windows


On Feb. 1, 2018, 2:04 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65448/
> ---
> 
> (Updated Feb. 1, 2018, 2:04 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Chun-Hung Hsiao, and Vinod Kone.
> 
> 
> Bugs: MESOS-1720
> https://issues.apache.org/jira/browse/MESOS-1720
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test ensures that agent send exited executor message when the
> executor is never launched. So that master's executor bookkeeping
> entry is removed. See MESOS-1720.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp b54688add9949b9c4ac2ce3a42503a411e6da09f 
> 
> 
> Diff: https://reviews.apache.org/r/65448/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 65585: Improved new CLI bootstrap script, now runnable from anywhere.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [65584, 65585]

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

- Mesos Reviewbot


On Feb. 9, 2018, 2:54 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65585/
> ---
> 
> (Updated Feb. 9, 2018, 2:54 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Kevin Klues.
> 
> 
> Bugs: MESOS-8240
> https://issues.apache.org/jira/browse/MESOS-8240
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We can now call `src/python/cli_new/bootstrap` from anywhere and a
> virtual environement will be created in the current directory.
> From here, the CLI and its tests can be run.
> 
> No files or directories are created in the `src/` directory when
> the bootstrap script is called anymore, except if the script is
> run from `src/`. This is useful to build the CLI as part of Mesos,
> a process that should not modify the source directory.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/README.md 847141db9a1f9eb5c78b2d50367b599a5c72ce61 
>   src/python/cli_new/activate 65df76f4590caf8160435ccbc1d6b199b115f7f8 
>   src/python/cli_new/bin/mesos c5152a2ebf8704c804bb4f39e46580a512aecdea 
>   src/python/cli_new/bin/mesos-cli-tests 
> 07659e0b4551c2381828b256608d2c6ced3ae745 
>   src/python/cli_new/bootstrap 9329a61a2a1f05286b4ff6e5fe68cd86ed48859a 
>   src/python/cli_new/deactivate 3dbe16765cad6f10523b1a3824608005e81b0944 
> 
> 
> Diff: https://reviews.apache.org/r/65585/diff/1/
> 
> 
> Testing
> ---
> 
> On Fedora 25:
> ```
> apache-mesos (MESOS-8240)$ cd src/python/cli_new/
> cli_new (MESOS-8240)$ ./bootstrap
> cli_new (MESOS-8240)$ source .virtualenv/activate
> (mesos-cli) cli_new (MESOS-8240)$ mesos
> Mesos CLI
> 
> Usage:
>   mesos (-h | --help)
>   mesos --version
>   mesos  [...]
> 
> Options:
>   -h --help  Show this screen.
>   --version  Show version info.
> 
> Commands:
>   agent   Interacts with the Mesos agents
>   config  Interacts with the Mesos CLI configuration file
>   taskInteracts with the tasks running in a Mesos cluster
> 
> See 'mesos help ' for more information on a specific command.
> (mesos-cli) cli_new (MESOS-8240)$ source .virtualenv/deactivate
> cli_new (MESOS-8240)$ rm -rf .virtualenv/
> cli_new (MESOS-8240)$ cd ..
> python (MESOS-8240)$ ./cli_new/bootstrap
> python (MESOS-8240)$ source .virtualenv/activate
> (mesos-cli) python (MESOS-8240)$ mesos
> Mesos CLI
> 
> Usage:
>   mesos (-h | --help)
>   mesos --version
>   mesos  [...]
> 
> Options:
>   -h --help  Show this screen.
>   --version  Show version info.
> 
> Commands:
>   agent   Interacts with the Mesos agents
>   config  Interacts with the Mesos CLI configuration file
>   taskInteracts with the tasks running in a Mesos cluster
> 
> See 'mesos help ' for more information on a specific command.
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 65448: Added a test to ensure master removes executors that never launched.

[Meng Zhu]

> On Feb. 8, 2018, 5:28 p.m., Vinod Kone wrote:
> > src/tests/slave_tests.cpp
> > Lines 4725 (patched)
> > 
> >
> > Can you also add a test with default executor? And maybe instead of 
> > kill task path, try to exercise the authz failure path for that test? Feel 
> > free to do it in a subsequent review. More tests that exercise the code 
> > paths you changed the better.

OK, will add more tests in a separate patch.


- Meng


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


On Jan. 31, 2018, 6:04 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65448/
> ---
> 
> (Updated Jan. 31, 2018, 6:04 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Chun-Hung Hsiao, and Vinod Kone.
> 
> 
> Bugs: MESOS-1720
> https://issues.apache.org/jira/browse/MESOS-1720
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test ensures that agent send exited executor message when the
> executor is never launched. So that master's executor bookkeeping
> entry is removed. See MESOS-1720.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp b54688add9949b9c4ac2ce3a42503a411e6da09f 
> 
> 
> Diff: https://reviews.apache.org/r/65448/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 65571: Handle 'None' passed from the MasterDetector in 'Master::detect()'.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [65571]

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

- Mesos Reviewbot


On Feb. 9, 2018, 12:55 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65571/
> ---
> 
> (Updated Feb. 9, 2018, 12:55 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Vinod Kone.
> 
> 
> Bugs: MESOS-8550
> https://issues.apache.org/jira/browse/MESOS-8550
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The function `MasterDetector::detect()` returns a value of type
> `Future>`, which, according to its documentation,
> can be `None` if an election occured and no master is elected.
> 
> However, the code in `Master::detected()` was only handling the
> cases of a failed future or a valid `MasterInfo` object.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp cc2685a6bc14103c639ce776cf1c912361e93381 
>   src/tests/cluster.hpp ad2b80e658d2f8afcefe9969d62cd33f0c475ce9 
>   src/tests/master_tests.cpp 7112bb4efcc312dc4f68bdc44fb685c7624002b1 
> 
> 
> Diff: https://reviews.apache.org/r/65571/diff/2/
> 
> 
> Testing
> ---
> 
> `./mesos-tests`
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 65574: Windows: Fixed handle inheritance in `create_process` wrapper.

[Joseph Wu]

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



LGTM, but I'd like this part of the chain to be committed alongside a test 
which checks for fetcher output, along the same lines as these tests:

* `TEST_F(ContainerLoggerTest, DefaultToSandbox)` in 
`tests/container_logger_tests.cpp`
* `TEST_F(DockerContainerizerTest, ROOT_DOCKER_Logs)` in 
`tests/containerizer/docker_containerizer_tests.cpp`

- Joseph Wu


On Feb. 8, 2018, 11:55 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65574/
> ---
> 
> (Updated Feb. 8, 2018, 11:55 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-6838 and MESOS-8512
> https://issues.apache.org/jira/browse/MESOS-6838
> https://issues.apache.org/jira/browse/MESOS-8512
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The primary change in this patch is that `create_process` now enables
> inheritance for the `pipe` handles passed before starting the child
> process. This is required, otherwise the child process will behave
> incorrectly (for example, it will write to `stdout` but that will go
> nowhere, as the redirection silently failed). After the process is
> created, inheritance is disabled to prevent further calls to
> `create_process` from inheriting the wrong handles.
> 
> The `std::tuple` type was changed to a
> `std::array` as it is significantly easier to work
> with (it supports iteration).
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/windows/shell.hpp 
> 542039c31f94eda1af121335b12edf9b83725ae5 
> 
> 
> Diff: https://reviews.apache.org/r/65574/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65467: Windows: Added `internal::windows::set_inherit(WindowsFD, bool)`.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On Feb. 8, 2018, 2:35 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65467/
> ---
> 
> (Updated Feb. 8, 2018, 2:35 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8512
> https://issues.apache.org/jira/browse/MESOS-8512
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This function is used on Windows to explicitly enable or disable
> inheritance on a handle.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/internal/windows/inherit.hpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/65467/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65465: Windows: Fixed recovery of Mesos containerizer.

[Andrew Schwartzmeyer]

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




src/slave/containerizer/mesos/launch.cpp
Lines 534 (patched)


Good point.


- Andrew Schwartzmeyer


On Feb. 8, 2018, 11:54 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65465/
> ---
> 
> (Updated Feb. 8, 2018, 11:54 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8519
> https://issues.apache.org/jira/browse/MESOS-8519
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Windows OS deletes the job object created in the agent process when
> the agent dies, because no other process holds a handle to it (despite
> processes being assigned to the job object). While this is
> counter-intuitive, it is the observed behavior. So in order for recovery
> to succeed, the containerizer must also hold an otherwise unused handle
> to its job object to keep it alive in the kernel, and available for
> recovery to find.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> 91016ed417428e3a5b21a132a96b9d7760d13aa3 
> 
> 
> Diff: https://reviews.apache.org/r/65465/diff/2/
> 
> 
> Testing
> ---
> 
> ```
> [--] Global test environment tear-down
> [==] 874 tests from 85 test cases ran. (253311 ms total)
> [  PASSED  ] 874 tests.
> 
> I0201 12:46:58.159368  3116 slave.cpp:6921] Recovering framework 
> eb32cef4-c503-4ab7-85d4-8d4577e6a3bf-
> I0201 12:46:58.159368  3116 slave.cpp:8543] Recovering executor 
> 'notepad.01d79d48-0791-11e8-8f77-02421c3bc93c' of framework 
> eb32cef4-c503-4ab7-85d4-8d4577e6a3bf-
> I0201 12:46:58.162847  9456 task_status_update_manager.cpp:207] Recovering 
> task status update manager
> I0201 12:46:58.162847  9456 task_status_update_manager.cpp:215] Recovering 
> executor 'notepad.01d79d48-0791-11e8-8f77-02421c3bc93c' of framework 
> eb32cef4-c503-4ab7-85d4-8d4577e6a3bf-
> I0201 12:46:58.166851  7344 containerizer.cpp:674] Recovering containerizer
> I0201 12:46:58.167351  7344 containerizer.cpp:731] Recovering container 
> 69cefa53-61e0-444b-a808-e38ffb4cb18f for executor 
> 'notepad.01d79d48-0791-11e8-8f77-02421c3bc93c' of framework 
> eb32cef4-c503-4ab7-85d4-8d4577e6a3bf-
> I0201 12:46:58.183379 17088 provisioner.cpp:493] Provisioner recovery complete
> I0201 12:46:58.186367 16792 slave.cpp:6695] Sending reconnect request to 
> executor 'notepad.01d79d48-0791-11e8-8f77-02421c3bc93c' of framework 
> eb32cef4-c503-4ab7-85d4-8d4577e6a3bf- at executor(1)@10.123.7.41:52591
> I0201 12:46:58.194370  7344 slave.cpp:4519] Received re-registration message 
> from executor 'notepad.01d79d48-0791-11e8-8f77-02421c3bc93c' of framework 
> eb32cef4-c503-4ab7-85d4-8d4577e6a3bf-
> I0201 12:47:00.193958 16792 slave.cpp:4737] Cleaning up un-reregistered 
> executors
> I0201 12:47:00.193958 16792 slave.cpp:6824] Finished recovery
> I0201 12:47:00.200943  9456 task_status_update_manager.cpp:181] Pausing 
> sending task status updates
> I0201 12:47:00.200943  3116 slave.cpp:1146] New master detected at 
> master@10.123.6.78:5050
> I0201 12:47:00.200943  3116 slave.cpp:1190] No credentials provided. 
> Attempting to register without authentication
> I0201 12:47:00.200943  3116 slave.cpp:1201] Detecting new master
> I0201 12:47:00.214944 16792 slave.cpp:1471] Re-registered with master 
> master@10.123.6.78:5050
> I0201 12:47:00.214944 13180 task_status_update_manager.cpp:188] Resuming 
> sending task status updates
> I0201 12:47:00.215942 16792 slave.cpp:1516] Forwarding agent update 
> {"operations":{},"resource_version_uuid" 
> {"value":"jLIL1d\/PQnuwmFxpMf8CLQ=="},"slave_id":{"value":"7dc02270-a4e1-4f59-9ad7-56bad5182ea4S3"},"update_oversubscribed_resources":true}
> I0201 12:47:00.219952  3116 slave.cpp:3625] Updating info for framework 
> eb32cef4-c503-4ab7-85d4-8d4577e6a3bf- with pid updated to 
> scheduler-aaa62980-8b1b-4775-b8bb-c6890b41941e@10.123.6.78:45907
> I0201 12:47:00.233942  7344 task_status_update_manager.cpp:188] Resuming 
> sending task status updates
> ```
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65465: Windows: Fixed recovery of Mesos containerizer.

[Joseph Wu]

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


Ship it!





src/slave/containerizer/mesos/launch.cpp
Lines 534 (patched)


I wonder if we should `NOTE:` that this handle will not be destructed, even 
though it is a SharedHandle, because it never goes out of scope (i.e. `exec` 
will not trigger the destruction).


- Joseph Wu


On Feb. 8, 2018, 11:54 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65465/
> ---
> 
> (Updated Feb. 8, 2018, 11:54 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8519
> https://issues.apache.org/jira/browse/MESOS-8519
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Windows OS deletes the job object created in the agent process when
> the agent dies, because no other process holds a handle to it (despite
> processes being assigned to the job object). While this is
> counter-intuitive, it is the observed behavior. So in order for recovery
> to succeed, the containerizer must also hold an otherwise unused handle
> to its job object to keep it alive in the kernel, and available for
> recovery to find.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> 91016ed417428e3a5b21a132a96b9d7760d13aa3 
> 
> 
> Diff: https://reviews.apache.org/r/65465/diff/2/
> 
> 
> Testing
> ---
> 
> ```
> [--] Global test environment tear-down
> [==] 874 tests from 85 test cases ran. (253311 ms total)
> [  PASSED  ] 874 tests.
> 
> I0201 12:46:58.159368  3116 slave.cpp:6921] Recovering framework 
> eb32cef4-c503-4ab7-85d4-8d4577e6a3bf-
> I0201 12:46:58.159368  3116 slave.cpp:8543] Recovering executor 
> 'notepad.01d79d48-0791-11e8-8f77-02421c3bc93c' of framework 
> eb32cef4-c503-4ab7-85d4-8d4577e6a3bf-
> I0201 12:46:58.162847  9456 task_status_update_manager.cpp:207] Recovering 
> task status update manager
> I0201 12:46:58.162847  9456 task_status_update_manager.cpp:215] Recovering 
> executor 'notepad.01d79d48-0791-11e8-8f77-02421c3bc93c' of framework 
> eb32cef4-c503-4ab7-85d4-8d4577e6a3bf-
> I0201 12:46:58.166851  7344 containerizer.cpp:674] Recovering containerizer
> I0201 12:46:58.167351  7344 containerizer.cpp:731] Recovering container 
> 69cefa53-61e0-444b-a808-e38ffb4cb18f for executor 
> 'notepad.01d79d48-0791-11e8-8f77-02421c3bc93c' of framework 
> eb32cef4-c503-4ab7-85d4-8d4577e6a3bf-
> I0201 12:46:58.183379 17088 provisioner.cpp:493] Provisioner recovery complete
> I0201 12:46:58.186367 16792 slave.cpp:6695] Sending reconnect request to 
> executor 'notepad.01d79d48-0791-11e8-8f77-02421c3bc93c' of framework 
> eb32cef4-c503-4ab7-85d4-8d4577e6a3bf- at executor(1)@10.123.7.41:52591
> I0201 12:46:58.194370  7344 slave.cpp:4519] Received re-registration message 
> from executor 'notepad.01d79d48-0791-11e8-8f77-02421c3bc93c' of framework 
> eb32cef4-c503-4ab7-85d4-8d4577e6a3bf-
> I0201 12:47:00.193958 16792 slave.cpp:4737] Cleaning up un-reregistered 
> executors
> I0201 12:47:00.193958 16792 slave.cpp:6824] Finished recovery
> I0201 12:47:00.200943  9456 task_status_update_manager.cpp:181] Pausing 
> sending task status updates
> I0201 12:47:00.200943  3116 slave.cpp:1146] New master detected at 
> master@10.123.6.78:5050
> I0201 12:47:00.200943  3116 slave.cpp:1190] No credentials provided. 
> Attempting to register without authentication
> I0201 12:47:00.200943  3116 slave.cpp:1201] Detecting new master
> I0201 12:47:00.214944 16792 slave.cpp:1471] Re-registered with master 
> master@10.123.6.78:5050
> I0201 12:47:00.214944 13180 task_status_update_manager.cpp:188] Resuming 
> sending task status updates
> I0201 12:47:00.215942 16792 slave.cpp:1516] Forwarding agent update 
> {"operations":{},"resource_version_uuid" 
> {"value":"jLIL1d\/PQnuwmFxpMf8CLQ=="},"slave_id":{"value":"7dc02270-a4e1-4f59-9ad7-56bad5182ea4S3"},"update_oversubscribed_resources":true}
> I0201 12:47:00.219952  3116 slave.cpp:3625] Updating info for framework 
> eb32cef4-c503-4ab7-85d4-8d4577e6a3bf- with pid updated to 
> scheduler-aaa62980-8b1b-4775-b8bb-c6890b41941e@10.123.6.78:45907
> I0201 12:47:00.233942  7344 task_status_update_manager.cpp:188] Resuming 
> sending task status updates
> ```
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65409: Fixed `SlaveRecoveryTest.ReconcileTasksMissingFromSlave`.

[Joseph Wu]

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


Ship it!




Ship It!

- Joseph Wu


On Feb. 8, 2018, 11:53 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65409/
> ---
> 
> (Updated Feb. 8, 2018, 11:53 a.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-6713
> https://issues.apache.org/jira/browse/MESOS-6713
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Because it is not possible to delete a file (or a folder recursively)
> with open handles on Windows, we have to explicitly `reset()` the agent
> before removing the framework meta directory. Otherwise, the task status
> update manager will be destructed too late, and so an open handle for
> `task.updates` will cause the `os::rmdir` to fail.
> 
> This is safe because we previously destructed the agent anyway, just
> later in the test when it was reassigned.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_recovery_tests.cpp 77aa60c953bd0769eaba05f001755e4cec9ba028 
> 
> 
> Diff: https://reviews.apache.org/r/65409/diff/3/
> 
> 
> Testing
> ---
> 
> make check on CentOS 7, all passed
> ctest on Windows, all passed including new SlaveRecoveryTests
> 
> Note that while this chain enables recovery of Docker tasks on Windows, it 
> explicitly does not fix MESOS-8519 (recovery of job object tasks).
> 
> ```
> I0131 11:52:01.545505  8316 docker.cpp:898] Recovering Docker containers
> I0131 11:52:01.546005   660 containerizer.cpp:674] Recovering containerizer
> I0131 11:52:01.546505   660 containerizer.cpp:725] Skipping recovery of 
> executor 'iis.feae9d12-06ba-11e8-8f77-02421c3bc93c' of framework 
> eb32cef4-c503-4ab7-85d4-8d4577e6a3bf- because it was not launched from 
> mesos containerizer
> I0131 11:52:01.557006 11272 provisioner.cpp:493] Provisioner recovery complete
> I0131 11:52:02.521003  8720 docker.cpp:1008] Recovering container 
> 'f7978e90-32f5-458d-ad4e-3ffa25a7b190' for executor 
> 'iis.feae9d12-06ba-11e8-8f77-02421c3bc93c' of framework 
> eb32cef4-c503-4ab7-85d4-8d4577e6a3bf-
> I0131 11:52:02.530527  8316 slave.cpp:6695] Sending reconnect request to 
> executor 'iis.feae9d12-06ba-11e8-8f77-02421c3bc93c' of framework 
> eb32cef4-c503-4ab7-85d4-8d4577e6a3bf- at executor(1)@10.123.7.41:63903
> I0131 11:52:02.549062  8720 slave.cpp:4519] Received re-registration message 
> from executor 'iis.feae9d12-06ba-11e8-8f77-02421c3bc93c' of framework 
> eb32cef4-c503-4ab7-85d4-8d4577e6a3bf-
> I0131 11:52:04.548064 10556 slave.cpp:4737] Cleaning up un-reregistered 
> executors
> I0131 11:52:04.548064 10556 slave.cpp:6824] Finished recovery
> I0131 11:52:04.566066   660 task_status_update_manager.cpp:181] Pausing 
> sending task status updates
> I0131 11:52:04.567059 14636 slave.cpp:1146] New master detected at 
> master@10.123.6.78:5050
> I0131 11:52:04.567059 14636 slave.cpp:1190] No credentials provided. 
> Attempting to register without authentication
> I0131 11:52:04.568047 14636 slave.cpp:1201] Detecting new master
> I0131 11:52:04.604035  8720 slave.cpp:1471] Re-registered with master 
> master@10.123.6.78:5050
> I0131 11:52:04.605060   660 task_status_update_manager.cpp:188] Resuming 
> sending task status updates
> I0131 11:52:04.606036  8720 slave.cpp:1516] Forwarding agent update 
> {"operations":{},"resource_version_uuid":{"value":"mzwol7M6SrGxOml4zYlA8Q=="},"slave_id":{"value":"7dc02270-a4e1-4f59-9ad7-56bad5182ea4-S0"},"update_oversubscribed_resource
> s":true}
> I0131 11:52:04.612036  8720 slave.cpp:3625] Updating info for framework 
> eb32cef4-c503-4ab7-85d4-8d4577e6a3bf- with pid updated to 
> scheduler-aaa62980-8b1b-4775-b8bb-c6890b41941e@10.123.6.78:45907
> I0131 11:52:04.636543 13468 task_status_update_manager.cpp:188] Resuming 
> sending task status updates
> ```
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65569: Install ping for docker build.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [65569]

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

- Mesos Reviewbot


On Feb. 9, 2018, 2:51 a.m., Tomasz Janiszewski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65569/
> ---
> 
> (Updated Feb. 9, 2018, 2:51 a.m.)
> 
> 
> Review request for mesos, Jie Yu, Michael Park, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> By default Ubuntu docker image does not contain `ping`.
> We need to manually install it with `iputils-ping`.
> 
> Refs: https://stackoverflow.com/a/39901446/1387612
> 
> 
> Diffs
> -
> 
>   support/docker-build.sh 126d4e781086636a318afc9199a3566bdb578051 
> 
> 
> Diff: https://reviews.apache.org/r/65569/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tomasz Janiszewski
> 
>

Re: Review Request 65465: Windows: Fixed recovery of Mesos containerizer.

[Jie Yu]

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


Ship it!




LGTM

- Jie Yu


On Feb. 8, 2018, 7:54 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65465/
> ---
> 
> (Updated Feb. 8, 2018, 7:54 p.m.)
> 
> 
> Review request for mesos, Akash Gupta, Jie Yu, and Joseph Wu.
> 
> 
> Bugs: MESOS-8519
> https://issues.apache.org/jira/browse/MESOS-8519
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The Windows OS deletes the job object created in the agent process when
> the agent dies, because no other process holds a handle to it (despite
> processes being assigned to the job object). While this is
> counter-intuitive, it is the observed behavior. So in order for recovery
> to succeed, the containerizer must also hold an otherwise unused handle
> to its job object to keep it alive in the kernel, and available for
> recovery to find.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/launch.cpp 
> 91016ed417428e3a5b21a132a96b9d7760d13aa3 
> 
> 
> Diff: https://reviews.apache.org/r/65465/diff/2/
> 
> 
> Testing
> ---
> 
> ```
> [--] Global test environment tear-down
> [==] 874 tests from 85 test cases ran. (253311 ms total)
> [  PASSED  ] 874 tests.
> 
> I0201 12:46:58.159368  3116 slave.cpp:6921] Recovering framework 
> eb32cef4-c503-4ab7-85d4-8d4577e6a3bf-
> I0201 12:46:58.159368  3116 slave.cpp:8543] Recovering executor 
> 'notepad.01d79d48-0791-11e8-8f77-02421c3bc93c' of framework 
> eb32cef4-c503-4ab7-85d4-8d4577e6a3bf-
> I0201 12:46:58.162847  9456 task_status_update_manager.cpp:207] Recovering 
> task status update manager
> I0201 12:46:58.162847  9456 task_status_update_manager.cpp:215] Recovering 
> executor 'notepad.01d79d48-0791-11e8-8f77-02421c3bc93c' of framework 
> eb32cef4-c503-4ab7-85d4-8d4577e6a3bf-
> I0201 12:46:58.166851  7344 containerizer.cpp:674] Recovering containerizer
> I0201 12:46:58.167351  7344 containerizer.cpp:731] Recovering container 
> 69cefa53-61e0-444b-a808-e38ffb4cb18f for executor 
> 'notepad.01d79d48-0791-11e8-8f77-02421c3bc93c' of framework 
> eb32cef4-c503-4ab7-85d4-8d4577e6a3bf-
> I0201 12:46:58.183379 17088 provisioner.cpp:493] Provisioner recovery complete
> I0201 12:46:58.186367 16792 slave.cpp:6695] Sending reconnect request to 
> executor 'notepad.01d79d48-0791-11e8-8f77-02421c3bc93c' of framework 
> eb32cef4-c503-4ab7-85d4-8d4577e6a3bf- at executor(1)@10.123.7.41:52591
> I0201 12:46:58.194370  7344 slave.cpp:4519] Received re-registration message 
> from executor 'notepad.01d79d48-0791-11e8-8f77-02421c3bc93c' of framework 
> eb32cef4-c503-4ab7-85d4-8d4577e6a3bf-
> I0201 12:47:00.193958 16792 slave.cpp:4737] Cleaning up un-reregistered 
> executors
> I0201 12:47:00.193958 16792 slave.cpp:6824] Finished recovery
> I0201 12:47:00.200943  9456 task_status_update_manager.cpp:181] Pausing 
> sending task status updates
> I0201 12:47:00.200943  3116 slave.cpp:1146] New master detected at 
> master@10.123.6.78:5050
> I0201 12:47:00.200943  3116 slave.cpp:1190] No credentials provided. 
> Attempting to register without authentication
> I0201 12:47:00.200943  3116 slave.cpp:1201] Detecting new master
> I0201 12:47:00.214944 16792 slave.cpp:1471] Re-registered with master 
> master@10.123.6.78:5050
> I0201 12:47:00.214944 13180 task_status_update_manager.cpp:188] Resuming 
> sending task status updates
> I0201 12:47:00.215942 16792 slave.cpp:1516] Forwarding agent update 
> {"operations":{},"resource_version_uuid" 
> {"value":"jLIL1d\/PQnuwmFxpMf8CLQ=="},"slave_id":{"value":"7dc02270-a4e1-4f59-9ad7-56bad5182ea4S3"},"update_oversubscribed_resources":true}
> I0201 12:47:00.219952  3116 slave.cpp:3625] Updating info for framework 
> eb32cef4-c503-4ab7-85d4-8d4577e6a3bf- with pid updated to 
> scheduler-aaa62980-8b1b-4775-b8bb-c6890b41941e@10.123.6.78:45907
> I0201 12:47:00.233942  7344 task_status_update_manager.cpp:188] Resuming 
> sending task status updates
> ```
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 65545: Start from merge-base when posting reviews.

[Benjamin Bannier]

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


Fix it, then Ship it!





support/post-reviews.py
Lines 206-209 (patched)


This output sounds more scary than I thought, maybe we should prompt the 
user for input, especially since we ask them a question. Maybe something like

try:
raw_input('\nPress enter to continue or \'Ctrl-C\' to abort.\n)
except KeyboardInterrupt:
sys.exit()


- Benjamin Bannier


On Feb. 7, 2018, 4:22 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65545/
> ---
> 
> (Updated Feb. 7, 2018, 4:22 p.m.)
> 
> 
> Review request for mesos, Armand Grillet and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When posting a review originally created from a branch B, it could happen 
> that the resulting review included garbage changes if B was updated between 
> revisions. (i.e., pulling new master changes)
> 
> This review changes the logic of the `post-reviews.py` script to only include 
> the changes between `HEAD` and the merge base of the tracking branch into the 
> review.
> 
> 
> Diffs
> -
> 
>   support/post-reviews.py 94b43253f25044a79009010e92363d1536c74299 
> 
> 
> Diff: https://reviews.apache.org/r/65545/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 65591: Explicitly tracked resource providers in master.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65591 was successfully built and tested.

Reviews applied: `['65587', '65588', '65589', '65590', '65591']`

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

- Mesos Reviewbot Windows


On Feb. 9, 2018, 7:27 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65591/
> ---
> 
> (Updated Feb. 9, 2018, 7:27 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-8382
> https://issues.apache.org/jira/browse/MESOS-8382
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds explicit tracking of resource providers to the master
> process. While we already had explicitly send resource provider
> information in e.g., `UpdateSlaveMessage`, we only stored that
> information aggregated over the full agent in the master up to now.
> 
> 
> Diffs
> -
> 
>   src/common/protobuf_utils.cpp b5c2997ada8362e42150fa3cfd762120e5ea715f 
>   src/master/master.hpp b434d2398b8815811345b6586ca586d2025cb2a2 
>   src/master/master.cpp d7d22866f7a4eb87bd8949efafc97e828e7d4b94 
> 
> 
> Diff: https://reviews.apache.org/r/65591/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 52064: Support for multiple versions of docs.

[Benjamin Bannier]

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



I am wondering whether it wouldn't be simpler to have the site setup just 
generate output for the currently checked-out version and dump that into some 
version-specific output folder. We could then have some CI setup execute this 
for the different tags or branches we care about. This wouldn't only simplify 
the site setup, but probably also deal better with e.g., changed requirements 
to the base system (e.g., needed to build binaries generating the endpoint 
documentation) which are currently not managed in the rake file.

One difficulty with that approach would be to determine under what branch we 
are actually working on. My guess is that we wouldn't want to update to 
documentation of 1.3.0 until we have tagged a 1.3.0 version, so we'd likely 
build documentation for the n-latest tags. The `HEAD` of `master` is more 
tricky as it would change more frequently and not directly have a release tag 
as parent (these live on release branches) making it harder to work with say 
`git-describe`. Maybe we could parse this from source, e.g., `MESOS_VERSION` in 
`include/mesos/version.hpp`.

The other difficulty might be that we might move a lot of site-generation setup 
out of the repo into e.g., Jenkins config. There are probably ways to work 
around that, but I haven't thought that through.

- Benjamin Bannier


On Feb. 9, 2018, 2:46 a.m., Tim Anderegg wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52064/
> ---
> 
> (Updated Feb. 9, 2018, 2:46 a.m.)
> 
> 
> Review request for mesos, haosdent huang and Vinod Kone.
> 
> 
> Bugs: MESOS-3011
> https://issues.apache.org/jira/browse/MESOS-3011
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support for multiple versions of docs.
> 
> 
> Diffs
> -
> 
>   site/Gemfile 877fe914a9787c140848fdf9958571fec5fa58ff 
>   site/Gemfile.lock 909f3f3badeaa47c80929e243ce36307766edee4 
>   site/Rakefile 31ef6ffe225ce7ddc573054058af1070b9e96b09 
>   site/config.rb 04bc7aa1e0ac61ce5d89fd53d32f265532996913 
>   site/data/releases.yml e3edc308a5429585b3fc3f05564d695ba3217035 
>   site/source/assets/js/versions.js PRE-CREATION 
>   site/source/layouts/basic.erb 8a07488940f3793d6fdd291dbe896e098f321c96 
> 
> 
> Diff: https://reviews.apache.org/r/52064/diff/6/
> 
> 
> Testing
> ---
> 
> Testing was done manually to verify that the documentation was built for each 
> version of Mesos that is supported (some older versions do not have 
> compatible documentation).
> 
> 
> Thanks,
> 
> Tim Anderegg
> 
>

Re: Review Request 65448: Added a test to ensure master removes executors that never launched.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [65445, 65504, 65446, 65449, 65448]

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

- Mesos Reviewbot


On Feb. 1, 2018, 2:04 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65448/
> ---
> 
> (Updated Feb. 1, 2018, 2:04 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Chun-Hung Hsiao, and Vinod Kone.
> 
> 
> Bugs: MESOS-1720
> https://issues.apache.org/jira/browse/MESOS-1720
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test ensures that agent send exited executor message when the
> executor is never launched. So that master's executor bookkeeping
> entry is removed. See MESOS-1720.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp b54688add9949b9c4ac2ce3a42503a411e6da09f 
> 
> 
> Diff: https://reviews.apache.org/r/65448/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Review Request 65587: Added hash function for mesos::UUID.

[Benjamin Bannier]

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

Review request for mesos and Jan Schlicht.


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


Repository: mesos


Description
---

Added hash function for mesos::UUID.


Diffs
-

  include/mesos/type_utils.hpp af2b187b9b59552e4ba515ad640fd4419eaf5075 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Review Request 65590: Added helper function to determine provider ID of a conversion.

[Benjamin Bannier]

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

Review request for mesos, Jie Yu and Jan Schlicht.


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


Repository: mesos


Description
---

Added helper function to determine provider ID of a conversion.


Diffs
-

  src/common/resources_utils.hpp 73d070d48eedf30468305e96fc4ad0005584fc79 
  src/common/resources_utils.cpp 99b16e0d17b224eefa1e28f5f66c4284069c0e57 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Review Request 65591: Explicitly tracked resource providers in master.

[Benjamin Bannier]

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

Review request for mesos, Jie Yu and Jan Schlicht.


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


Repository: mesos


Description
---

This patch adds explicit tracking of resource providers to the master
process. While we already had explicitly send resource provider
information in e.g., `UpdateSlaveMessage`, we only stored that
information aggregated over the full agent in the master up to now.


Diffs
-

  src/common/protobuf_utils.cpp b5c2997ada8362e42150fa3cfd762120e5ea715f 
  src/master/master.hpp b434d2398b8815811345b6586ca586d2025cb2a2 
  src/master/master.cpp d7d22866f7a4eb87bd8949efafc97e828e7d4b94 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Review Request 65588: Used proto UUID instead stout UUID internally for operation IDs.

[Benjamin Bannier]

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

Review request for mesos, Jie Yu and Jan Schlicht.


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


Repository: mesos


Description
---

Used proto UUID instead stout UUID internally for operation IDs.


Diffs
-

  include/mesos/resource_provider/resource_provider.proto 
e96a40493c351a7242465c8591cae981abc92f24 
  src/common/protobuf_utils.hpp 9a940db188436b9c5a39b0637cb4c15ee2ab5266 
  src/common/protobuf_utils.cpp b5c2997ada8362e42150fa3cfd762120e5ea715f 
  src/master/master.hpp b434d2398b8815811345b6586ca586d2025cb2a2 
  src/master/master.cpp d7d22866f7a4eb87bd8949efafc97e828e7d4b94 
  src/resource_provider/manager.cpp cc195a3d35b93dd6493951de1ff8a1cb8a886377 
  src/resource_provider/message.hpp 2511af611edc94e3e3a78fce475bc8cd85ffc3f9 
  src/slave/slave.hpp 30151c4886e12e9183a971b86b854e28a8ca1b39 
  src/slave/slave.cpp f98f37321872d090176b7cc50873fc3c627773f5 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Review Request 65589: Added comparison operators for operations.

[Benjamin Bannier]

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

Review request for mesos, Greg Mann and Jie Yu.


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


Repository: mesos


Description
---

Added comparison operators for operations.


Diffs
-

  include/mesos/type_utils.hpp af2b187b9b59552e4ba515ad640fd4419eaf5075 
  include/mesos/v1/mesos.hpp d4c354ab596a6ea361f2fe45afa46089f8c1a543 
  src/common/type_utils.cpp a4d5dcb4e4445e307356d9b0c16dd39f00f6a8e2 
  src/v1/mesos.cpp 8abeae06e0fa87b50933df1c222ad4724a0b0116 


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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 65585: Improved new CLI bootstrap script, now runnable from anywhere.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65585 was successfully built and tested.

Reviews applied: `['65584', '65585']`

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

- Mesos Reviewbot Windows


On Feb. 9, 2018, 2:54 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65585/
> ---
> 
> (Updated Feb. 9, 2018, 2:54 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Kevin Klues.
> 
> 
> Bugs: MESOS-8240
> https://issues.apache.org/jira/browse/MESOS-8240
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> We can now call `src/python/cli_new/bootstrap` from anywhere and a
> virtual environement will be created in the current directory.
> From here, the CLI and its tests can be run.
> 
> No files or directories are created in the `src/` directory when
> the bootstrap script is called anymore, except if the script is
> run from `src/`. This is useful to build the CLI as part of Mesos,
> a process that should not modify the source directory.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/README.md 847141db9a1f9eb5c78b2d50367b599a5c72ce61 
>   src/python/cli_new/activate 65df76f4590caf8160435ccbc1d6b199b115f7f8 
>   src/python/cli_new/bin/mesos c5152a2ebf8704c804bb4f39e46580a512aecdea 
>   src/python/cli_new/bin/mesos-cli-tests 
> 07659e0b4551c2381828b256608d2c6ced3ae745 
>   src/python/cli_new/bootstrap 9329a61a2a1f05286b4ff6e5fe68cd86ed48859a 
>   src/python/cli_new/deactivate 3dbe16765cad6f10523b1a3824608005e81b0944 
> 
> 
> Diff: https://reviews.apache.org/r/65585/diff/1/
> 
> 
> Testing
> ---
> 
> On Fedora 25:
> ```
> apache-mesos (MESOS-8240)$ cd src/python/cli_new/
> cli_new (MESOS-8240)$ ./bootstrap
> cli_new (MESOS-8240)$ source .virtualenv/activate
> (mesos-cli) cli_new (MESOS-8240)$ mesos
> Mesos CLI
> 
> Usage:
>   mesos (-h | --help)
>   mesos --version
>   mesos  [...]
> 
> Options:
>   -h --help  Show this screen.
>   --version  Show version info.
> 
> Commands:
>   agent   Interacts with the Mesos agents
>   config  Interacts with the Mesos CLI configuration file
>   taskInteracts with the tasks running in a Mesos cluster
> 
> See 'mesos help ' for more information on a specific command.
> (mesos-cli) cli_new (MESOS-8240)$ source .virtualenv/deactivate
> cli_new (MESOS-8240)$ rm -rf .virtualenv/
> cli_new (MESOS-8240)$ cd ..
> python (MESOS-8240)$ ./cli_new/bootstrap
> python (MESOS-8240)$ source .virtualenv/activate
> (mesos-cli) python (MESOS-8240)$ mesos
> Mesos CLI
> 
> Usage:
>   mesos (-h | --help)
>   mesos --version
>   mesos  [...]
> 
> Options:
>   -h --help  Show this screen.
>   --version  Show version info.
> 
> Commands:
>   agent   Interacts with the Mesos agents
>   config  Interacts with the Mesos CLI configuration file
>   taskInteracts with the tasks running in a Mesos cluster
> 
> See 'mesos help ' for more information on a specific command.
> ```
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 52064: Support for multiple versions of docs.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [52064]

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

Error:
2018-02-09 15:06:42 URL:https://reviews.apache.org/r/52064/diff/raw/ 
[13073/13073] -> "52064.patch" [1]
error: patch failed: site/data/releases.yml:154
error: site/data/releases.yml: patch does not apply

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

- Mesos Reviewbot


On Feb. 9, 2018, 1:46 a.m., Tim Anderegg wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52064/
> ---
> 
> (Updated Feb. 9, 2018, 1:46 a.m.)
> 
> 
> Review request for mesos, haosdent huang and Vinod Kone.
> 
> 
> Bugs: MESOS-3011
> https://issues.apache.org/jira/browse/MESOS-3011
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Support for multiple versions of docs.
> 
> 
> Diffs
> -
> 
>   site/Gemfile 877fe914a9787c140848fdf9958571fec5fa58ff 
>   site/Gemfile.lock 909f3f3badeaa47c80929e243ce36307766edee4 
>   site/Rakefile 31ef6ffe225ce7ddc573054058af1070b9e96b09 
>   site/config.rb 04bc7aa1e0ac61ce5d89fd53d32f265532996913 
>   site/data/releases.yml e3edc308a5429585b3fc3f05564d695ba3217035 
>   site/source/assets/js/versions.js PRE-CREATION 
>   site/source/layouts/basic.erb 8a07488940f3793d6fdd291dbe896e098f321c96 
> 
> 
> Diff: https://reviews.apache.org/r/52064/diff/6/
> 
> 
> Testing
> ---
> 
> Testing was done manually to verify that the documentation was built for each 
> version of Mesos that is supported (some older versions do not have 
> compatible documentation).
> 
> 
> Thanks,
> 
> Tim Anderegg
> 
>

Review Request 65585: Improved new CLI bootstrap script, now runnable from anywhere.

[Armand Grillet]

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

Review request for mesos, Benjamin Bannier and Kevin Klues.


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


Repository: mesos


Description
---

We can now call `src/python/cli_new/bootstrap` from anywhere and a
virtual environement will be created in the current directory.
>From here, the CLI and its tests can be run.

No files or directories are created in the `src/` directory when
the bootstrap script is called anymore, except if the script is
run from `src/`. This is useful to build the CLI as part of Mesos,
a process that should not modify the source directory.


Diffs
-

  src/python/cli_new/README.md 847141db9a1f9eb5c78b2d50367b599a5c72ce61 
  src/python/cli_new/activate 65df76f4590caf8160435ccbc1d6b199b115f7f8 
  src/python/cli_new/bin/mesos c5152a2ebf8704c804bb4f39e46580a512aecdea 
  src/python/cli_new/bin/mesos-cli-tests 
07659e0b4551c2381828b256608d2c6ced3ae745 
  src/python/cli_new/bootstrap 9329a61a2a1f05286b4ff6e5fe68cd86ed48859a 
  src/python/cli_new/deactivate 3dbe16765cad6f10523b1a3824608005e81b0944 


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


Testing
---

On Fedora 25:
```
apache-mesos (MESOS-8240)$ cd src/python/cli_new/
cli_new (MESOS-8240)$ ./bootstrap
cli_new (MESOS-8240)$ source .virtualenv/activate
(mesos-cli) cli_new (MESOS-8240)$ mesos
Mesos CLI

Usage:
  mesos (-h | --help)
  mesos --version
  mesos  [...]

Options:
  -h --help  Show this screen.
  --version  Show version info.

Commands:
  agent   Interacts with the Mesos agents
  config  Interacts with the Mesos CLI configuration file
  taskInteracts with the tasks running in a Mesos cluster

See 'mesos help ' for more information on a specific command.
(mesos-cli) cli_new (MESOS-8240)$ source .virtualenv/deactivate
cli_new (MESOS-8240)$ rm -rf .virtualenv/
cli_new (MESOS-8240)$ cd ..
python (MESOS-8240)$ ./cli_new/bootstrap
python (MESOS-8240)$ source .virtualenv/activate
(mesos-cli) python (MESOS-8240)$ mesos
Mesos CLI

Usage:
  mesos (-h | --help)
  mesos --version
  mesos  [...]

Options:
  -h --help  Show this screen.
  --version  Show version info.

Commands:
  agent   Interacts with the Mesos agents
  config  Interacts with the Mesos CLI configuration file
  taskInteracts with the tasks running in a Mesos cluster

See 'mesos help ' for more information on a specific command.
```


Thanks,

Armand Grillet

Re: Review Request 65518: Reaped the container process directly in Docker executor.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [65518]

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

- Mesos Reviewbot


On Feb. 9, 2018, 1:03 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65518/
> ---
> 
> (Updated Feb. 9, 2018, 1:03 a.m.)
> 
> 
> Review request for mesos, Gaston Kleiman, Gilbert Song, Greg Mann, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-8488
> https://issues.apache.org/jira/browse/MESOS-8488
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Due to a Docker issue (https://github.com/moby/moby/issues/33820),
> Docker daemon will fail to catch a container exit, i.e., the container
> process has already exited but the command `docker ps` shows the
> container still running, this will lead to the "docker run" command
> that we execute in Docker executor never returns, and it will also
> cause the `docker stop` command takes no effect, i.e., it will return
> without error but `docker ps` shows the container still running, so
> the task will stuck in `TASK_KILLING` state.
> 
> To workaround this Docker issue, in this patch we made Docker executor
> reaps the container process directly so Docker executor will be notified
> once the container process exits.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp e4c53d558e414e50b1c429fba8e31e504c63744a 
> 
> 
> Diff: https://reviews.apache.org/r/65518/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 65571: Handle 'None' passed from the MasterDetector in 'Master::detect()'.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65571 was successfully built and tested.

Reviews applied: `['65571']`

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

- Mesos Reviewbot Windows


On Feb. 9, 2018, 1:55 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65571/
> ---
> 
> (Updated Feb. 9, 2018, 1:55 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Vinod Kone.
> 
> 
> Bugs: MESOS-8550
> https://issues.apache.org/jira/browse/MESOS-8550
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The function `MasterDetector::detect()` returns a value of type
> `Future>`, which, according to its documentation,
> can be `None` if an election occured and no master is elected.
> 
> However, the code in `Master::detected()` was only handling the
> cases of a failed future or a valid `MasterInfo` object.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp cc2685a6bc14103c639ce776cf1c912361e93381 
>   src/tests/cluster.hpp ad2b80e658d2f8afcefe9969d62cd33f0c475ce9 
>   src/tests/master_tests.cpp 7112bb4efcc312dc4f68bdc44fb685c7624002b1 
> 
> 
> Diff: https://reviews.apache.org/r/65571/diff/2/
> 
> 
> Testing
> ---
> 
> `./mesos-tests`
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

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

[Benno Evers]

> On Feb. 6, 2018, 5:48 p.m., Alexander Rukletsov wrote:
> > 3rdparty/libprocess/src/memory_profiler.cpp
> > Lines 243-249 (patched)
> > 
> >
> > I think it is safe to use this function because it is only accessed 
> > from `MemoryProfiler::DiskArtifact::path()` which is in turn only accessed 
> > from `MemoryProfiler` methods, which are always serialized. You should call 
> > this in a comment or even employ `process::Once` now to make sure it is 
> > thread-safe and hence future-proof.

I will add a comment. I think using `Once` can be counter-productive, because 
the serialization provided by `libprocess` is fundamental enough that most 
likely everything would break if it wouldn't exist - leading the reader to 
wonder why it isn't enough in this function so that `Once` needs to be used.


- Benno


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


On Feb. 6, 2018, 5:45 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63368/
> ---
> 
> (Updated Feb. 6, 2018, 5:45 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 3071b7ce2b82a4ce0ea62e4d2b3518a6f5114803 
>   3rdparty/libprocess/include/process/memory_profiler.hpp PRE-CREATION 
>   3rdparty/libprocess/include/process/process.hpp 
> 8661706cb058efb26f5bfbcc84972f9930d3670f 
>   3rdparty/libprocess/src/CMakeLists.txt 
> f002c157dc2ca64da66bc4e61f5095f2b533ae1f 
>   3rdparty/libprocess/src/memory_profiler.cpp PRE-CREATION 
>   3rdparty/libprocess/src/process.cpp 
> ba9bc291bb6741e32b3a066fe90771311d21852a 
> 
> 
> Diff: https://reviews.apache.org/r/63368/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 65571: Handle 'None' passed from the MasterDetector in 'Master::detect()'.

[Benno Evers]

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

(Updated Feb. 9, 2018, 12:55 p.m.)


Review request for mesos, Andrei Budnik and Vinod Kone.


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


Repository: mesos


Description
---

The function `MasterDetector::detect()` returns a value of type
`Future>`, which, according to its documentation,
can be `None` if an election occured and no master is elected.

However, the code in `Master::detected()` was only handling the
cases of a failed future or a valid `MasterInfo` object.


Diffs (updated)
-

  src/master/master.cpp cc2685a6bc14103c639ce776cf1c912361e93381 
  src/tests/cluster.hpp ad2b80e658d2f8afcefe9969d62cd33f0c475ce9 
  src/tests/master_tests.cpp 7112bb4efcc312dc4f68bdc44fb685c7624002b1 


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

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


Testing
---

`./mesos-tests`


Thanks,

Benno Evers

Re: Review Request 65571: Handle 'None' passed from the MasterDetector in 'Master::detect()'.

[Benno Evers]

> On Feb. 8, 2018, 8:22 p.m., Vinod Kone wrote:
> > src/master/master.cpp
> > Lines 2186 (patched)
> > 
> >
> > s/Leader detector indicated no master elected/No master was elected/
> > 
> > More importantly, this changes the semantics a bit. Previously if this 
> > master was the current leader it committed suicide even in this case. But 
> > we don't anymore. Is that what we want?
> > 
> > 
> > Also, where in the interface does it say that None() is retryable. It 
> > says retryable errors are handled internally by the detector?

Ok, I guess I misinterpreted the documentation on `MasterDetector::detect()`:

```
   * Returns MasterInfo after an election has occurred and the elected
   * master is different than that specified (if any), or NONE if an
   * election occurs and no master is elected (e.g., all masters are
   * lost). A failed future is returned if the detector is unable to
   * detect the leading master due to a non-retryable error.
```

Since electing no master sounds like an error, and the future is not failed, I 
assumed that this case was implicitly classifid as retryable error.

Anyways, for the semantics I assume that not aborting is at least what the 
original author intended, otherwise there would be no point to differentiate 
between `Error` and `None` in the API.

Additionally, the code that causes the master to crash in this situation was 
introduced relatively recently (11 July 2017, a8c7ae44c8), before that the 
`detected()`-handler would have just set `leader` to `None` and quietly 
continued. So I would argue that this fix is actually restoring the previously 
existing behaviour, not changing it.


- Benno


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


On Feb. 8, 2018, 4:50 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65571/
> ---
> 
> (Updated Feb. 8, 2018, 4:50 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Vinod Kone.
> 
> 
> Bugs: MESOS-8550
> https://issues.apache.org/jira/browse/MESOS-8550
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The function `MasterDetector::detect()` returns a value of type
> `Future>`, which, according to its documentation,
> can be `None` if an election occured and no master is elected.
> 
> However, the code in `Master::detected()` was only handling the
> cases of a failed future or a valid `MasterInfo` object.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp cc2685a6bc14103c639ce776cf1c912361e93381 
>   src/tests/cluster.hpp ad2b80e658d2f8afcefe9969d62cd33f0c475ce9 
>   src/tests/master_tests.cpp 7112bb4efcc312dc4f68bdc44fb685c7624002b1 
> 
> 
> Diff: https://reviews.apache.org/r/65571/diff/1/
> 
> 
> Testing
> ---
> 
> `./mesos-tests`
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 65572: Fixed two slave recovery tests.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [65572]

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

Error:
2018-02-09 11:30:49 URL:https://reviews.apache.org/r/65572/diff/raw/ 
[2007/2007] -> "65572.patch" [1]
error: patch failed: src/tests/slave_recovery_tests.cpp:5156
error: src/tests/slave_recovery_tests.cpp: patch does not apply

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

- Mesos Reviewbot


On Feb. 9, 2018, 1:12 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65572/
> ---
> 
> (Updated Feb. 9, 2018, 1:12 a.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Bugs: MESOS-8552
> https://issues.apache.org/jira/browse/MESOS-8552
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These two tests are affected by the lingering executor fix
> (#65109). We should wait to make sure tasks are actually launched
> on the executor before trigger the agent failover. Otherwise, the
> container will be shutdown upon re-registration because the executor
> has never received any task.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_recovery_tests.cpp 77aa60c953bd0769eaba05f001755e4cec9ba028 
> 
> 
> Diff: https://reviews.apache.org/r/65572/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 65584: Improved new CLI README regarding bash completion.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 65584 was successfully built and tested.

Reviews applied: `['65584']`

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

- Mesos Reviewbot Windows


On Feb. 9, 2018, 10:31 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65584/
> ---
> 
> (Updated Feb. 9, 2018, 10:31 a.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/README.md 847141db9a1f9eb5c78b2d50367b599a5c72ce61 
> 
> 
> Diff: https://reviews.apache.org/r/65584/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Review Request 65584: Improved new CLI README regarding bash completion.

[Armand Grillet]

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

Review request for mesos and Benjamin Bannier.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/python/cli_new/README.md 847141db9a1f9eb5c78b2d50367b599a5c72ce61 


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


Testing
---


Thanks,

Armand Grillet

Re: Review Request 65314: Removed code which is not used.

[Alexander Rojas]

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



Failed test `DiskQuotaTest.SlaveRecovery` is known to be flaky, see 
[MESOS-3968](https://issues.apache.org/jira/browse/MESOS-3968)

- Alexander Rojas


On Feb. 8, 2018, 1:59 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65314/
> ---
> 
> (Updated Feb. 8, 2018, 1:59 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Greg Mann.
> 
> 
> Bugs: MESOS-8434
> https://issues.apache.org/jira/browse/MESOS-8434
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The introduction of the `ObjectApprovers` abstraction rendered the
> `AuthorizationAcceptor` class oboslete. After the refactor of the code
> the acceptor class was no longer used, nor were the helper functions
> built around it.
> 
> This patch removes that obsolete code.
> 
> 
> Diffs
> -
> 
>   src/common/http.hpp 750d3bfba1647624983108bdd23295a3b3091fe4 
>   src/common/http.cpp 728fc554917ed031f9cb3d811fbbc064307b3e32 
> 
> 
> Diff: https://reviews.apache.org/r/65314/diff/5/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 65497: Added test to verify task-less executor is shutdown when re-subscribing.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [65109, 65110, 65111, 65369, 65496, 65497]

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

- Mesos Reviewbot


On Feb. 3, 2018, 9:18 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65497/
> ---
> 
> (Updated Feb. 3, 2018, 9:18 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8411
> https://issues.apache.org/jira/browse/MESOS-8411
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test verifies that the v1 executor is shutdown if all of its
> initial tasks could not be delivered when re-subscribing with
> a recovered agent. See MESOS-8411.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.hpp c5593c25dde18c005f195a1885a8586bc72c849f 
>   src/tests/mesos.cpp ac789297d6aa034121b73e4efc0aafee1ee3b60f 
>   src/tests/mock_slave.cpp 597d7abef20dd5f89b16e4616233f02760b9d037 
>   src/tests/slave_tests.cpp b54688add9949b9c4ac2ce3a42503a411e6da09f 
> 
> 
> Diff: https://reviews.apache.org/r/65497/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>