Re: Review Request 62326: Always send TASK_KILLED when the task is killed.

[Qian Zhang]

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

(Updated Sept. 29, 2017, 2:55 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


Changes
---

Updated Docker executor as well.


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


Repository: mesos


Description
---

Always send TASK_KILLED when the task is killed.


Diffs (updated)
-

  src/docker/executor.cpp 3b0767ffbbe9982639d0b87fbb0573accdd48559 
  src/launcher/default_executor.cpp 136c000917b2c4fdaf68fd460764c3a15fdf3bf0 
  src/launcher/executor.cpp 0131577d5b9018e7094815f1377b2ee59a4493d4 


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

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


Testing
---

sudo make check


Thanks,

Qian Zhang

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

[Benjamin Bannier]

> On Sept. 29, 2017, 5:33 a.m., Jie Yu wrote:
> > src/tests/oversubscription_tests.cpp
> > Lines 324-329 (original), 333-338 (patched)
> > 
> >
> > Do you need to update those tests?

This particular test case checks that no update of oversubscribed agent 
resources is sent before the first `oversubscribed_resources_interval` has 
elapsed. The agent will however send its latest total on registration. We need 
check these independently to make sure they contain what we expect.


- Benjamin


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


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

Re: Review Request 61174: Added a test `ProtobufTest.ParseJSONUnrecognizedEnum`.

[Qian Zhang]

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

(Updated Sept. 29, 2017, 4:47 p.m.)


Review request for mesos, Benjamin Mahler and James Peach.


Changes
---

Updated commit message.


Summary (updated)
-

Added a test `ProtobufTest.ParseJSONUnrecognizedEnum`.


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


Repository: mesos


Description (updated)
---

Added a test `ProtobufTest.ParseJSONUnrecognizedEnum`.


Diffs (updated)
-

  3rdparty/stout/tests/protobuf_tests.cpp 
8877e8934e0f7875bfedcfa88b491ce4b13ca44f 
  3rdparty/stout/tests/protobuf_tests.proto 
d16726aa8060aea2b830040b20dbdd467c801483 


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

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


Testing
---


Thanks,

Qian Zhang

Review Request 62685: Updated the comments of TASK_FINISHED to make it more clear.

[Qian Zhang]

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

Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

Updated the comments of TASK_FINISHED to make it more clear.


Diffs
-

  include/mesos/mesos.proto 3b2d6bb7034c3157347c8ad3c32aa38c642762f8 
  include/mesos/v1/mesos.proto 4ca48868c033c7285662d7eb182ac3d40f550833 


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


Testing
---


Thanks,

Qian Zhang

Re: Review Request 62326: Always send TASK_KILLED when the task is killed.

[Qian Zhang]

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

(Updated Sept. 29, 2017, 3:33 p.m.)


Review request for mesos, Anand Mazumdar and Vinod Kone.


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


Repository: mesos


Description
---

Always send TASK_KILLED when the task is killed.


Diffs
-

  src/docker/executor.cpp 3b0767ffbbe9982639d0b87fbb0573accdd48559 
  src/launcher/default_executor.cpp 136c000917b2c4fdaf68fd460764c3a15fdf3bf0 
  src/launcher/executor.cpp 0131577d5b9018e7094815f1377b2ee59a4493d4 


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


Testing
---

sudo make check


Thanks,

Qian Zhang

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

[Benjamin Bannier]

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

(Updated Sept. 29, 2017, 11:14 a.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Removed redundant updates.


Repository: mesos


Description
---

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

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

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

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

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


Diffs (updated)
-

  src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
  src/slave/slave.hpp aea1e948209c7c8945665915bc2f6d8eb47814ef 
  src/slave/slave.cpp 75e2e25c438637e2b1731f9fd01b6fe297687c50 
  src/tests/oversubscription_tests.cpp 02b10d6689dd6f01510cd2d5db2bb76b4b190eca 
  src/tests/slave_tests.cpp e9bcfefd52e1e99a7a5877b2e7c30af958ca1723 


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

Changes: https://reviews.apache.org/r/61183/diff/14-15/


Testing
---

Tested on a number of platforms on internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 62655: Made 'UpdateSlaveMessage' a union of possible updates.

[Benjamin Bannier]

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

(Updated Sept. 29, 2017, 11:14 a.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Added an empty line.


Repository: mesos


Description
---

The 'UpdateSlaveMessage' could either transport an update to an
agent's total, or to its oversubscribed resources. In certain
scenarios this required the agent to send two messages where before
one was sufficient. When talking to a master which did not understand
the 'type' field this would have let the master to rescind all offers
for revocable resources from that agent.

This patch changes the message to be a union of possible updates,
i.e., it is now possible to send updates to both the total and the
oversubscribed resources simultaneously on agent registration and
reregistration without the master rescinding offers for revocable
resources on the agent.


Diffs (updated)
-

  src/master/master.cpp 6d84a26bff970b842b58dfb69dbf232ba5c16a20 
  src/messages/messages.proto dc4e19c08d671743f08223dcdf1dbc336b3940bd 
  src/slave/slave.cpp 75e2e25c438637e2b1731f9fd01b6fe297687c50 
  src/tests/oversubscription_tests.cpp 02b10d6689dd6f01510cd2d5db2bb76b4b190eca 


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

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


Testing
---

`make check`; also tested as part of https://reviews.apache.org/r/61183/.


Thanks,

Benjamin Bannier

Re: Review Request 62158: Rescinded offers possibly affected by updates to agent total resources.

[Benjamin Bannier]

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

(Updated Sept. 29, 2017, 11:14 a.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Rebased.


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


Repository: mesos


Description
---

When an agent changes its resources, the master should rescind any
offers affected by the change. We already performed the rescind for
updates to the agent's oversubscribed resources; this patch adds offer
rescinding when an update an agent's total resources is processed.

While for updates to an agent's oversubscribed resources we currently
only rescind offers containing revocable resources to e.g., reduce
offer churn, we for updates to the total we here currently rescind all
offers for resources on the agent.


Diffs (updated)
-

  src/master/master.cpp 6d84a26bff970b842b58dfb69dbf232ba5c16a20 


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

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


Testing
---

Tested on a number of platforms on internal CI.


Thanks,

Benjamin Bannier

Re: Review Request 59987: Added protobuf map support.

[Qian Zhang]

> On Sept. 23, 2017, 7:19 a.m., Chun-Hung Hsiao wrote:
> > 3rdparty/stout/include/stout/protobuf.hpp
> > Lines 433 (patched)
> > 
> >
> > The key can also be of an integral type.

Yeah, I had the same concern before. But in [our 
code](https://github.com/apache/mesos/blob/1.4.0/3rdparty/stout/include/stout/json.hpp#L190),
 we define `JSON::Object::values` as a `string` to `JSON::Value` map, so I am 
not sure how we can support non-string key.


> On Sept. 23, 2017, 7:19 a.m., Chun-Hung Hsiao wrote:
> > 3rdparty/stout/include/stout/protobuf.hpp
> > Lines 925 (patched)
> > 
> >
> > The key can also be of an integral type. Same below.

Ditto.


- Qian


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


On July 4, 2017, 10:43 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59987/
> ---
> 
> (Updated July 4, 2017, 10:43 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Zhitao Li.
> 
> 
> Bugs: MESOS-7656
> https://issues.apache.org/jira/browse/MESOS-7656
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Map is a feature of proto2 syntax, but it can only be compiled
> with proto3 compiler, see the following discussion for details:
> https://groups.google.com/forum/#!topic/protobuf/p4WxcplrlA4
> We have already upgraded the compiler from 2.6.1 to 3.3.0 in
> MESOS-7228, however, to use protobuf map in Mesos code, we also
> need to add the protobuf map support to the code in Mesos for
> converting protobuf message to JSON object and parsing JSON
> object as protobuf message, that is what I have done in this patch.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/protobuf.hpp 
> 15690b66cc4ae0c1bf2c2176d73c385ca75d3c20 
> 
> 
> Diff: https://reviews.apache.org/r/59987/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 62687: Added utility function to get data from a master endpoint to CLI.

[Mesos Reviewbot Windows]

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



FAIL: Failed to apply the dependent review: 62663.

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

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

Relevant logs:

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

```
error: patch failed: src/python/pylint.config:28
error: src/python/pylint.config: patch does not apply
```

- Mesos Reviewbot Windows


On Sept. 29, 2017, 10:16 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62687/
> ---
> 
> (Updated Sept. 29, 2017, 10:16 a.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This removes the similar code that had CLI plugins.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/lib/cli/plugins/agent/main.py PRE-CREATION 
>   src/python/cli_new/lib/cli/plugins/task/main.py PRE-CREATION 
>   src/python/cli_new/lib/cli/util.py 7371f83543ed527bea8dbf2fe4e20d92ef8e4492 
> 
> 
> Diff: https://reviews.apache.org/r/62687/diff/1/
> 
> 
> Testing
> ---
> 
> To test with one master:
> ```
> $ ./bootstrap
> $ source activate
> $ mesos-cli-tests
> ```
> I also checked that the Python linter was still working.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 62661: Added --disable-libtool-wrapper configuration to Mesos.

[Benjamin Bannier]

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


Fix it, then Ship it!





configure.ac
Lines 234-238 (patched)


We currently have two such comment blocks which are not identical, one in 
`configure.ac` and one in `src/Makefile.am`. I'd suggest to consolidate them 
into a single block, e.g., in `src/Makefile.am`, and remove the other one.



configure.ac
Lines 671 (patched)


Let's break this line to not extend beyond 80 chars, e.g.,

AM_CONDITIONAL([DISABLE_LIBTOOL_WRAPPERS],
   [test x"$enable_libtool_wrappers" = "xno"])



src/Makefile.am
Line 2551 (original), 2558 (patched)


This will lead to `mesos-tests` not picking up `AM_LDFLAGS`. We need to 
remove this line so it is automatically inherited. Otherwise `mesos-tests` is 
still a wrapper script.


- Benjamin Bannier


On Sept. 28, 2017, 1:37 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62661/
> ---
> 
> (Updated Sept. 28, 2017, 1:37 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, and Kapil 
> Arya.
> 
> 
> Bugs: MESOS-7500
> https://issues.apache.org/jira/browse/MESOS-7500
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This flag is used to force libtool to generate executables instead of
> wrapper scripts. A wrapper script might trigger relinking, which takes
> quite a while on slow machines, thus causing failure of tests.
> 
> 
> Diffs
> -
> 
>   configure.ac 92bc1aa5f9604e3b2b678225a57622cd2eb8679a 
>   src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 
> 
> 
> Diff: https://reviews.apache.org/r/62661/diff/3/
> 
> 
> Testing
> ---
> 
> 1. sudo make check (fedora)
> 2. Apache CI (centos 7, ubuntu 14.04)
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 61174: Added a test `ProtobufTest.ParseJSONUnrecognizedEnum`.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 61174 was successfully built and tested.

Reviews applied: `['61109', '61174']`

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

- Mesos Reviewbot Windows


On Sept. 29, 2017, 8:47 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61174/
> ---
> 
> (Updated Sept. 29, 2017, 8:47 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and James Peach.
> 
> 
> Bugs: MESOS-7828
> https://issues.apache.org/jira/browse/MESOS-7828
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test `ProtobufTest.ParseJSONUnrecognizedEnum`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/protobuf_tests.cpp 
> 8877e8934e0f7875bfedcfa88b491ce4b13ca44f 
>   3rdparty/stout/tests/protobuf_tests.proto 
> d16726aa8060aea2b830040b20dbdd467c801483 
> 
> 
> Diff: https://reviews.apache.org/r/61174/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Review Request 62687: Added utility function to get data from a master endpoint to CLI.

[Armand Grillet]

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

Review request for mesos and Kevin Klues.


Repository: mesos


Description
---

This removes the similar code that had CLI plugins.


Diffs
-

  src/python/cli_new/lib/cli/plugins/agent/main.py PRE-CREATION 
  src/python/cli_new/lib/cli/plugins/task/main.py PRE-CREATION 
  src/python/cli_new/lib/cli/util.py 7371f83543ed527bea8dbf2fe4e20d92ef8e4492 


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


Testing
---

To test with one master:
```
$ ./bootstrap
$ source activate
$ mesos-cli-tests
```
I also checked that the Python linter was still working.


Thanks,

Armand Grillet

Re: Review Request 62661: Added --disable-libtool-wrapper configuration to Mesos.

[Andrei Budnik]

> On Sept. 28, 2017, 5:22 p.m., Benjamin Bannier wrote:
> > configure.ac
> > Lines 243 (patched)
> > 
> >
> > Let's not explicitly specify the different branches here and instead 
> > let autoconf handle it for us, see e.g., the handling of `werror` below.

Fixed.


- Andrei


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


On Sept. 28, 2017, 11:37 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62661/
> ---
> 
> (Updated Sept. 28, 2017, 11:37 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, and Kapil 
> Arya.
> 
> 
> Bugs: MESOS-7500
> https://issues.apache.org/jira/browse/MESOS-7500
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This flag is used to force libtool to generate executables instead of
> wrapper scripts. A wrapper script might trigger relinking, which takes
> quite a while on slow machines, thus causing failure of tests.
> 
> 
> Diffs
> -
> 
>   configure.ac 92bc1aa5f9604e3b2b678225a57622cd2eb8679a 
>   src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 
> 
> 
> Diff: https://reviews.apache.org/r/62661/diff/3/
> 
> 
> Testing
> ---
> 
> 1. sudo make check (fedora)
> 2. Apache CI (centos 7, ubuntu 14.04)
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 62214: Added JavaScript linter.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [62333, 62214]

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

- Mesos Reviewbot


On Sept. 29, 2017, 9:28 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62214/
> ---
> 
> (Updated Sept. 29, 2017, 9:28 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Kevin Klues.
> 
> 
> Bugs: MESOS-7924
> https://issues.apache.org/jira/browse/MESOS-7924
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This linter runs when changes on a JavaScript file are being committed.
> The linter used is ESLint with a configuration based on our current JS
> code base. The linter and its dependencies (i.e. Node.js) are installed
> in a environment using Virtualenv and then Nodeenv.
> 
> 
> Diffs
> -
> 
>   src/webui/.eslintrc.js PRE-CREATION 
>   src/webui/.gitignore PRE-CREATION 
>   src/webui/bootstrap PRE-CREATION 
>   src/webui/pip-requirements.txt PRE-CREATION 
>   support/mesos-style.py 9cd1623c48623e558dcd5b80b4bbf5a2162c57cb 
> 
> 
> Diff: https://reviews.apache.org/r/62214/diff/4/
> 
> 
> Testing
> ---
> 
> Following this commit, I have tried to commit a change on a JavaScript file 
> and checked that ESLinter was correctly running.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 62661: Added --disable-libtool-wrapper configuration to Mesos.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 62661 was successfully built and tested.

Reviews applied: `['62661']`

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

- Mesos Reviewbot Windows


On Sept. 28, 2017, 11:37 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62661/
> ---
> 
> (Updated Sept. 28, 2017, 11:37 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, and Kapil 
> Arya.
> 
> 
> Bugs: MESOS-7500
> https://issues.apache.org/jira/browse/MESOS-7500
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This flag is used to force libtool to generate executables instead of
> wrapper scripts. A wrapper script might trigger relinking, which takes
> quite a while on slow machines, thus causing failure of tests.
> 
> 
> Diffs
> -
> 
>   configure.ac 92bc1aa5f9604e3b2b678225a57622cd2eb8679a 
>   src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 
> 
> 
> Diff: https://reviews.apache.org/r/62661/diff/3/
> 
> 
> Testing
> ---
> 
> 1. sudo make check (fedora)
> 2. Apache CI (centos 7, ubuntu 14.04)
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 62214: Added JavaScript linter.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 62214 was successfully built and tested.

Reviews applied: `['62333', '62214']`

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

- Mesos Reviewbot Windows


On Sept. 29, 2017, 1:28 p.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62214/
> ---
> 
> (Updated Sept. 29, 2017, 1:28 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Kevin Klues.
> 
> 
> Bugs: MESOS-7924
> https://issues.apache.org/jira/browse/MESOS-7924
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This linter runs when changes on a JavaScript file are being committed.
> The linter used is ESLint with a configuration based on our current JS
> code base. The linter and its dependencies (i.e. Node.js) are installed
> in a environment using Virtualenv and then Nodeenv.
> 
> 
> Diffs
> -
> 
>   src/webui/.eslintrc.js PRE-CREATION 
>   src/webui/.gitignore PRE-CREATION 
>   src/webui/bootstrap PRE-CREATION 
>   src/webui/pip-requirements.txt PRE-CREATION 
>   support/mesos-style.py 9cd1623c48623e558dcd5b80b4bbf5a2162c57cb 
> 
> 
> Diff: https://reviews.apache.org/r/62214/diff/4/
> 
> 
> Testing
> ---
> 
> Following this commit, I have tried to commit a change on a JavaScript file 
> and checked that ESLinter was correctly running.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 62554: Added the ability to prune the gone agent list from the registry.

[Vinod Kone]

> On Sept. 26, 2017, 10:11 p.m., Mesos Reviewbot Windows wrote:
> > FAIL: Some Mesos tests failed.
> > 
> > Reviews applied: `['62475', '62476', '62477', '62581', '62478', '62479', 
> > '62480', '62481', '62507', '62531', '62554']`
> > 
> > Failed command: `C:\mesos\src\mesos-tests.exe --verbose 
> > --gtest_filter="-ContentType/MasterAPITest.EventAuthorizationFiltering/1"`
> > 
> > All the build artifacts available at: 
> > http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62554
> > 
> > Relevant logs:
> > 
> > - 
> > [mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62554/logs/mesos-tests-stdout.log):
> > 
> > ```
> > [   OK ] FilesTest.DetachTest (5 ms)
> > [ RUN  ] FilesTest.ReadTest
> > [   OK ] FilesTest.ReadTest (292 ms)
> > [ RUN  ] FilesTest.DownloadTest
> > [   OK ] FilesTest.DownloadTest (126 ms)
> > [ RUN  ] FilesTest.DebugTest
> > [   OK ] FilesTest.DebugTest (123 ms)
> > [ RUN  ] FilesTest.AuthenticationTest
> > [   OK ] FilesTest.AuthenticationTest (99 ms)
> > [--] 6 tests from FilesTest (807 ms total)
> > 
> > [--] 3 tests from GarbageCollectorTest
> > [ RUN  ] GarbageCollectorTest.Schedule
> > [   OK ] GarbageCollectorTest.Schedule (123 ms)
> > [ RUN  ] GarbageCollectorTest.Unschedule
> > [   OK ] GarbageCollectorTest.Unschedule (36 ms)
> > [ RUN  ] GarbageCollectorTest.Prune
> > [   OK ] GarbageCollectorTest.Prune (37 ms)
> > [--] 3 tests from GarbageCollectorTest (299 ms total)
> > 
> > [--] 5 tests from GarbageCollectorIntegrationTest
> > [ RUN  ] GarbageCollectorIntegrationTest.Restart
> > 
> > C:\mesos\mesos\src\tests\gc_tests.cpp(332): ERROR: this mock object (used 
> > in test GarbageCollectorIntegrationTest.Restart) should be deleted but 
> > never is. Its address is @00752FB59910.
> > C:\mesos\mesos\src\tests\containerizer.cpp(414): ERROR: this mock object 
> > (used in test GarbageCollectorIntegrationTest.Restart) should be deleted 
> > but never is. Its address is @00752FB59DE0.
> > C:\mesos\mesos\src\tests\gc_tests.cpp(317): ERROR: this mock object (used 
> > in test GarbageCollectorIntegrationTest.Restart) should be deleted but 
> > never is. Its address is @00752FB5B370.
> > C:\mesos\mesos\src\tests\mock_registrar.cpp(54): ERROR: this mock object 
> > (used in test GarbageCollectorIntegrationTest.Restart) should be deleted 
> > but never is. Its address is @01E405FCC520.
> > C:\mesos\mesos\3rdparty\libprocess\include\process/gmock.hpp(235): ERROR: 
> > this mock object (used in test GarbageCollectorIntegrationTest.Restart) 
> > should be deleted but never is. Its address is @01E406498898.
> > ERROR: 5 leaked mock objects found at program exit.
> > ```
> > 
> > - 
> > [mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62554/logs/mesos-tests-stderr.log):
> > 
> > ```
> >   scalar {
> > value: 1024
> >   }
> > }
> > resources {
> >   name: "disk"
> >   type: SCALAR
> >   scalar {
> > value: 1024
> >   }
> > }
> > resources {
> >   name: "ports"
> >   type: RANGES
> >   ranges {
> > range {
> >   begin: 31000
> >   end: 32000
> > }
> >   }
> > }
> > checkpoint: true
> > port: 56595
> > 
> > 
> > To remedy this do as follows:
> > Step 1: rm -f C:\Users\mesos\AppData\Local\Temp\2\ppnykI\meta\slaves\latest
> > This ensures agent doesn't recover old live executors.
> > Step 2: Restart the agent.
> > ```

this seems problematic, though i can't easily tell which test is breaking. 
GarbageCollectorIntegrationTest.Restart? Anand, can you please triage this?


- Vinod


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


On Sept. 26, 2017, 8:30 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62554/
> ---
> 
> (Updated Sept. 26, 2017, 8:30 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7448
> https://issues.apache.org/jira/browse/MESOS-7448
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces two master options `registry_max_gone_agent_age`
> (age based) and `registry_max_gone_agent_count` (count based) for
> GC'ing the list of gone agents in the registry.
> 
> Review: https://reviews.apache.org/r/62554
> 
> 
> Diffs
> -
> 
>   src/master/flags.cpp fa6d274cb52448269a8108ffdc5f07bad8b222f2 
>   src/master/master.hpp 05f88111afb4fa0e2baf57106e1479914c16a113 
>   src/master/master.cpp 6d84a26bff970b842b58dfb69dbf232ba5c16a20 
>   src/tests/master_tests.cpp 

Re: Review Request 62479: Removed the logic for removing the latest symlink on the agent.

[Vinod Kone]

> On Sept. 26, 2017, 6:58 p.m., Vinod Kone wrote:
> > Can you also add a blurb to CHANGELOG about the new semantics? I think it's 
> > worth calling out.

don't forget this!


- Vinod


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


On Sept. 21, 2017, 8:28 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62479/
> ---
> 
> (Updated Sept. 21, 2017, 8:28 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7445
> https://issues.apache.org/jira/browse/MESOS-7445
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change removes the logic of removing the latest symlink on
> receiving the shutdown message from the Mesos master. This ensures
> that agents come back with the same agent ID upon a successful
> shutdown similar to the behavior when they come back post a reboot.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 01020a624bdd67ee6371f607d897ad71a2cdcc82 
> 
> 
> Diff: https://reviews.apache.org/r/62479/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 62326: Always send TASK_KILLED when the task is killed.

[Vinod Kone]

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


Ship it!




Summary "Always send TASK_KILLED when the task is killed by a framework."

Description: This change is done for command, docker and default executors.

- Vinod Kone


On Sept. 29, 2017, 7:33 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62326/
> ---
> 
> (Updated Sept. 29, 2017, 7:33 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-7975
> https://issues.apache.org/jira/browse/MESOS-7975
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Always send TASK_KILLED when the task is killed.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 3b0767ffbbe9982639d0b87fbb0573accdd48559 
>   src/launcher/default_executor.cpp 136c000917b2c4fdaf68fd460764c3a15fdf3bf0 
>   src/launcher/executor.cpp 0131577d5b9018e7094815f1377b2ee59a4493d4 
> 
> 
> Diff: https://reviews.apache.org/r/62326/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

[Kevin Klues]

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




src/python/lib/mesos/exceptions.py
Lines 21 (patched)


We don't need this because we don't import anything.



src/python/lib/mesos/exceptions.py
Lines 50 (patched)


Please reword as "Could not fetch..." All exceptions will be wrapped with 
an "Error" when caught at a higher level.



src/python/lib/mesos/exceptions.py
Lines 69 (patched)


Ditto



src/python/lib/mesos/http.py
Lines 33 (patched)


Typically we separate these out one per line for consistency



src/python/lib/mesos/http.py
Lines 249 (patched)


What is mesos specific about this class to justify calling it 
`MesosResource`?



src/python/lib/tests/conftest.py
Lines 27 (patched)


Until now we haven't really been using python decorators anywhere. It seems 
weird to introduce them here without having them anywhere else. Independently, 
why not run this test using the other test infrastructure we have for actually 
spinning up a master / agent?



src/python/lib/tests/test_exceptions.py
Lines 27 (patched)


Again, it seems weird to introduce this without introducing it everywhere.



src/python/lib/tests/test_http.py
Lines 41 (patched)


Ditto



src/python/lib/tests/test_http.py
Lines 124 (patched)


In general, all of these tests should have the same look and feel as the 
existing tests. 

If we think the way you have organized these is the right way to go, we can 
change the exisiting ones to follow this pattern, but we should strive for 
consistency in whatever we finally commit back.

As of now, it's hard for me to glance through these patches and knwo 
whether everything is correct or not because I am not familiar with the style 
you are going for - it's different than the style used throughout the rest of 
the code base.


- Kevin Klues


On Sept. 21, 2017, 4:27 a.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61172/
> ---
> 
> (Updated Sept. 21, 2017, 4:27 a.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
> modules, which provides a Resource class and its descendants for
> abstracting away common operations over http connectioins with JSON
> serialization.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
>   src/python/cli_new/pylint.config 63fbb94fbf47cccd1053b4c1eafac18b88dbc818 
>   src/python/lib/mesos/exceptions.py PRE-CREATION 
>   src/python/lib/mesos/http.py PRE-CREATION 
>   src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   src/python/lib/tests/conftest.py PRE-CREATION 
>   src/python/lib/tests/test_exceptions.py PRE-CREATION 
>   src/python/lib/tests/test_http.py PRE-CREATION 
>   src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
> 
> 
> Diff: https://reviews.apache.org/r/61172/diff/5/
> 
> 
> Testing
> ---
> 
> install tox
> cd src/python/lib
> tox
> 
> 
> Thanks,
> 
> Eric Chung
> 
>

Re: Review Request 62685: Updated the comments of TASK_FINISHED to make it more clear.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Sept. 29, 2017, 7:32 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62685/
> ---
> 
> (Updated Sept. 29, 2017, 7:32 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-7975
> https://issues.apache.org/jira/browse/MESOS-7975
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the comments of TASK_FINISHED to make it more clear.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 3b2d6bb7034c3157347c8ad3c32aa38c642762f8 
>   include/mesos/v1/mesos.proto 4ca48868c033c7285662d7eb182ac3d40f550833 
> 
> 
> Diff: https://reviews.apache.org/r/62685/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 62327: Checked TASK_KILLED in the test `ROOT_INTERNET_CURL_PortMapper`.

[Vinod Kone]

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



Can you also add tests for command and default executors to test this behavior?

- Vinod Kone


On Sept. 14, 2017, 10:45 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62327/
> ---
> 
> (Updated Sept. 14, 2017, 10:45 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-7975
> https://issues.apache.org/jira/browse/MESOS-7975
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Checked TASK_KILLED in the test `ROOT_INTERNET_CURL_PortMapper`.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/cni_isolator_tests.cpp 
> 636695f368d1c8acd9ed4ccfd1a449f375af8282 
> 
> 
> Diff: https://reviews.apache.org/r/62327/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 62214: Added JavaScript linter.

[Armand Grillet]

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

(Updated Sept. 29, 2017, 1:28 p.m.)


Review request for mesos, Benjamin Mahler and Kevin Klues.


Changes
---

Added comments to bootstrap script and changed nodeenv version to not have 
`OSError: [Errno 17] File exists` errors.


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


Repository: mesos


Description
---

This linter runs when changes on a JavaScript file are being committed.
The linter used is ESLint with a configuration based on our current JS
code base. The linter and its dependencies (i.e. Node.js) are installed
in a environment using Virtualenv and then Nodeenv.


Diffs (updated)
-

  src/webui/.eslintrc.js PRE-CREATION 
  src/webui/.gitignore PRE-CREATION 
  src/webui/bootstrap PRE-CREATION 
  src/webui/pip-requirements.txt PRE-CREATION 
  support/mesos-style.py 9cd1623c48623e558dcd5b80b4bbf5a2162c57cb 


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

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


Testing
---

Following this commit, I have tried to commit a change on a JavaScript file and 
checked that ESLinter was correctly running.


Thanks,

Armand Grillet

Re: Review Request 62661: Added --disable-libtool-wrapper configuration to Mesos.

[Andrei Budnik]

> On Sept. 29, 2017, 12:30 p.m., Benjamin Bannier wrote:
> > configure.ac
> > Lines 234-238 (patched)
> > 
> >
> > We currently have two such comment blocks which are not identical, one 
> > in `configure.ac` and one in `src/Makefile.am`. I'd suggest to consolidate 
> > them into a single block, e.g., in `src/Makefile.am`, and remove the other 
> > one.

Fixed.


> On Sept. 29, 2017, 12:30 p.m., Benjamin Bannier wrote:
> > configure.ac
> > Lines 671 (patched)
> > 
> >
> > Let's break this line to not extend beyond 80 chars, e.g.,
> > 
> > AM_CONDITIONAL([DISABLE_LIBTOOL_WRAPPERS],
> >[test x"$enable_libtool_wrappers" = "xno"])

Fixed.


> On Sept. 29, 2017, 12:30 p.m., Benjamin Bannier wrote:
> > src/Makefile.am
> > Line 2551 (original), 2558 (patched)
> > 
> >
> > This will lead to `mesos-tests` not picking up `AM_LDFLAGS`. We need to 
> > remove this line so it is automatically inherited. Otherwise `mesos-tests` 
> > is still a wrapper script.

Fixed.


- Andrei


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


On Sept. 28, 2017, 11:37 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62661/
> ---
> 
> (Updated Sept. 28, 2017, 11:37 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, and Kapil 
> Arya.
> 
> 
> Bugs: MESOS-7500
> https://issues.apache.org/jira/browse/MESOS-7500
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This flag is used to force libtool to generate executables instead of
> wrapper scripts. A wrapper script might trigger relinking, which takes
> quite a while on slow machines, thus causing failure of tests.
> 
> 
> Diffs
> -
> 
>   configure.ac 92bc1aa5f9604e3b2b678225a57622cd2eb8679a 
>   src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 
> 
> 
> Diff: https://reviews.apache.org/r/62661/diff/4/
> 
> 
> Testing
> ---
> 
> 1. sudo make check (fedora)
> 2. Apache CI (centos 7, ubuntu 14.04)
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

[Kevin Klues]

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



Looks like this needs a rebase. I can't get it to apply cleanly.

- Kevin Klues


On Sept. 21, 2017, 4:27 a.m., Eric Chung wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61172/
> ---
> 
> (Updated Sept. 21, 2017, 4:27 a.m.)
> 
> 
> Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
> modules, which provides a Resource class and its descendants for
> abstracting away common operations over http connectioins with JSON
> serialization.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
>   src/python/cli_new/pylint.config 63fbb94fbf47cccd1053b4c1eafac18b88dbc818 
>   src/python/lib/mesos/exceptions.py PRE-CREATION 
>   src/python/lib/mesos/http.py PRE-CREATION 
>   src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   src/python/lib/tests/conftest.py PRE-CREATION 
>   src/python/lib/tests/test_exceptions.py PRE-CREATION 
>   src/python/lib/tests/test_http.py PRE-CREATION 
>   src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
> 
> 
> Diff: https://reviews.apache.org/r/61172/diff/5/
> 
> 
> Testing
> ---
> 
> install tox
> cd src/python/lib
> tox
> 
> 
> Thanks,
> 
> Eric Chung
> 
>

Re: Review Request 62333: Added helper functions for linters using a virtual environment.

[Armand Grillet]

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

(Updated Sept. 29, 2017, 1:23 p.m.)


Review request for mesos, Benjamin Mahler and Kevin Klues.


Changes
---

Removed `VirtualenvLinterBase` class.


Summary (updated)
-

Added helper functions for linters using a virtual environment.


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


Repository: mesos


Description
---

This is used by the Python linter and
will be used by the Javascript linter.


Diffs (updated)
-

  support/mesos-style.py 9cd1623c48623e558dcd5b80b4bbf5a2162c57cb 


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

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


Testing
---

Manual updates of `.py` and `.js` files then test commits to see if the linters 
before and afer removing their `.virtualenv` were still working as expected.


Thanks,

Armand Grillet

Re: Review Request 59987: Added protobuf map support.

[Qian Zhang]

> On Sept. 23, 2017, 7:14 a.m., Chun-Hung Hsiao wrote:
> > 3rdparty/stout/include/stout/protobuf.hpp
> > Lines 413 (patched)
> > 
> >
> > Not sure if it is cleaner to parse the map here  because we're 
> > duplicating the `apply_visitor` call here.
> > 
> > Another possible implementation is to move this into `parse()` with an 
> > additional `bool mapField = false` parameter. In `parse()`, if `mapFiled` 
> > is set to true, we can set up the key and replace `message` by a `Message` 
> > pointer to the value before passing it into `apply_visitor`.
> > 
> > Thoughts?

I think calling `apply_visitor` here should not be a problem. Actually in the 
code 
[here](https://github.com/apache/mesos/blob/1.4.0/3rdparty/stout/include/stout/protobuf.hpp#L538),
 we already call `apply_visitor` to handle array, so I think it should be OK 
for us call `apply_visitor` here to handle map.


> On Sept. 23, 2017, 7:14 a.m., Chun-Hung Hsiao wrote:
> > 3rdparty/stout/include/stout/protobuf.hpp
> > Lines 421 (patched)
> > 
> >
> > Although it seems that `*entry` would not be managed by `repeated_ref` 
> > after `repeated_ref.Add(*entry)`, I cannot find such a guarantee from 
> > protobuf's documentation. How about doing `entry = 
> > reflection->AddMessage(message, field)` here instead, so that we don't need 
> > `repeated_ref`, and the memory management is consistent with the other 
> > cases?

I am not sure how we can do `entry = reflection->AddMessage(message, field)` 
here, do you mean calling `AddMessage()` to add a map message? Then what we 
should do afterward? I think we still need to parse each entry in the map one 
by one.

Actually, using `GetMutableRepeatedFieldRef()` to handle the map field is a way 
that a Google guy told me, please see the following mail thread for details:
https://groups.google.com/d/msg/protobuf/nNZ_ItflbLE/x7hLZ1GtAAAJ


> On Sept. 23, 2017, 7:14 a.m., Chun-Hung Hsiao wrote:
> > 3rdparty/stout/include/stout/protobuf.hpp
> > Line 607 (original), 640 (patched)
> > 
> >
> > How about a partial specialization for `google::protobuf::Map`?

Why do we need to do that? What do we want to achieve with such a partial 
specialization?


> On Sept. 23, 2017, 7:14 a.m., Chun-Hung Hsiao wrote:
> > 3rdparty/stout/include/stout/protobuf.hpp
> > Line 868 (original), 901 (patched)
> > 
> >
> > How about iterating through `reflection->GetRepeatedMessage()` here 
> > instead?

Can you elaborate a bit?


- Qian


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


On July 4, 2017, 10:43 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59987/
> ---
> 
> (Updated July 4, 2017, 10:43 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Zhitao Li.
> 
> 
> Bugs: MESOS-7656
> https://issues.apache.org/jira/browse/MESOS-7656
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Map is a feature of proto2 syntax, but it can only be compiled
> with proto3 compiler, see the following discussion for details:
> https://groups.google.com/forum/#!topic/protobuf/p4WxcplrlA4
> We have already upgraded the compiler from 2.6.1 to 3.3.0 in
> MESOS-7228, however, to use protobuf map in Mesos code, we also
> need to add the protobuf map support to the code in Mesos for
> converting protobuf message to JSON object and parsing JSON
> object as protobuf message, that is what I have done in this patch.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/protobuf.hpp 
> 15690b66cc4ae0c1bf2c2176d73c385ca75d3c20 
> 
> 
> Diff: https://reviews.apache.org/r/59987/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 62326: Always send TASK_KILLED when the task is killed.

[Gaston Kleiman]

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


Ship it!




Ship It!

- Gaston Kleiman


On Sept. 29, 2017, 12:33 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62326/
> ---
> 
> (Updated Sept. 29, 2017, 12:33 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-7975
> https://issues.apache.org/jira/browse/MESOS-7975
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Always send TASK_KILLED when the task is killed.
> 
> 
> Diffs
> -
> 
>   src/docker/executor.cpp 3b0767ffbbe9982639d0b87fbb0573accdd48559 
>   src/launcher/default_executor.cpp 136c000917b2c4fdaf68fd460764c3a15fdf3bf0 
>   src/launcher/executor.cpp 0131577d5b9018e7094815f1377b2ee59a4493d4 
> 
> 
> Diff: https://reviews.apache.org/r/62326/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 61473: Do not kill non partition aware tasks.

[Jiang Yan Xu]

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




src/master/http.cpp
Lines 324 (patched)


Leave a blank line between these if blocks. Here and below.



src/master/http.cpp
Lines 350 (patched)


Leave a blank line between these if blocks.



src/master/http.cpp
Line 4203 (original), 4220-4226 (patched)


NPA tasks already have the state `TASK_LOST` right? Could you check that?

The difference is that previously, if a framework was PA (and its 
unreachable tasks were stored in  `unreachableTasks` as `UNREACHABLE`) and 
later it changed to NPA again, the code here made no attempt to double check to 
change it to `TASK_LOST`. The new code would do it though.

It's debatable which is more right but I'd say it's safe to maintain the 
existing behavior?



src/master/master.hpp
Lines 2475-2477 (patched)


Because this method takes a pointer, this mutation could affect future uses 
of it. Even though right now nothing that cares about the state follows the 
call of `addUnreachableTask`, it may still be good to not propagate the change.

How about doing it in `addUnreachableTask`?

```
void addUnreachableTask(const Task& _task)
{
  Task* task = new Task(_task);

  // We have to use TASK_LOST for non-partition-aware frameworks
  // for backwards compatibility.
  if (!capabilities.partitionAware) {
task->set_state(TASK_LOST);
  }

  unreachableTasks.set(task.task_id(), process::Owned(task));
}
```



src/master/master.hpp
Line 2844 (original), 2840 (patched)


s/non-/Non-/ to be consistent with other instances of `NOTE`s.



src/master/master.cpp
Line 6448 (original), 6439 (patched)


The decision is pretty simple right now: all tasks are added unless the 
framework is completed. So this separate introduction paragraph seems redudant 
and can be removed now:

```
// Decide how to handle the tasks running on the agent:
//
```



src/master/master.cpp
Line 6450 (original), 6441 (patched)


"All tasks" except for those belonging to completed frameworks, so to 
summarize the block of code below, we should probably mention this.



src/master/master.cpp
Lines 6453-6456 (original), 6443-6446 (patched)


"no further cleanup is required" is true for all  circumstances now so I 
think this comment is not important anymore.

If we comment about the code below ignoring the history of how we got here 
but just as it is, we could probably just remove the whole sentence:

```
  // If the master has failed
  // over since the agent was marked unreachable then the master shouldn't
  // have any record of the tasks running on the agent, so no further
  // cleanup is required.
```



src/master/master.cpp
Lines 7188-7190 (original), 7144-7146 (patched)


Our handling of `TASK_UNREACHABLE` vs. `TASK_LOST` here is a little 
different than elsewhere so I think this warrants a bit of explanation.

e.g., 
```
// Transition tasks to TASK_UNREACHABLE and remove (archive) them.
// We convert the task state to TASK_LOST if is the framework is not 
partition aware.
// However we only do the conversion right before the status update is sent 
out or the
// task is archived because the processing prior to then requires tasks to 
be of the 
// correct state TASK_UNREACHABLE.
```

Does this sound right?



src/master/master.cpp
Lines 7169-7173 (patched)


You have created this `newUpdate` but are not immediately using it, and I 
have to pay attention to notice that it is `update` instead of `newUpdate` that 
is passed to `updateTask()`.

So perhaps defer this change to after `updateTask` and `removeTask` are 
called? At that point because the only remaining use of the `update` is to send 
it out, you don't even need to make a copy any more. Just change the field.



src/master/master.cpp
Lines 7171-7172 (patched)


This could be shortened to one line.

```
update.mutable_status()->set_state(TASK_LOST);
```



src/master/master.cpp
Line 7218 (original), 7174 (patched)


Looking into the method `updateTask`, 

Re: Review Request 62479: Removed the logic for removing the latest symlink on the agent.

[Anand Mazumdar]

> On Sept. 26, 2017, 6:58 p.m., Vinod Kone wrote:
> > Can you also add a blurb to CHANGELOG about the new semantics? I think it's 
> > worth calling out.
> 
> Vinod Kone wrote:
> don't forget this!

Yep!  I need to add the Agent Lifecycle feature to the `CHANGELOG`; so would do 
it as part of that. Haven't got to it yet. :)


- Anand


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


On Sept. 21, 2017, 8:28 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62479/
> ---
> 
> (Updated Sept. 21, 2017, 8:28 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7445
> https://issues.apache.org/jira/browse/MESOS-7445
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change removes the logic of removing the latest symlink on
> receiving the shutdown message from the Mesos master. This ensures
> that agents come back with the same agent ID upon a successful
> shutdown similar to the behavior when they come back post a reboot.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 01020a624bdd67ee6371f607d897ad71a2cdcc82 
> 
> 
> Diff: https://reviews.apache.org/r/62479/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>

Re: Review Request 62638: Removed support for platforms without O_CLOEXEC.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [62638]

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

- Mesos Reviewbot


On Sept. 27, 2017, 3:20 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62638/
> ---
> 
> (Updated Sept. 27, 2017, 3:20 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Benjamin Bannier, and Jie Yu.
> 
> 
> Bugs: MESOS-8027
> https://issues.apache.org/jira/browse/MESOS-8027
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `O_CLOEXEC` fallback code was broken since it did not guarantee
> to include `fcntl.h` before checking for the `O_CLOEXEC` symbol.
> O_CLOEXEC is supported by all reasonably current service platforms,
> so we should not need to fall back to compatibility code. So rather
> than fixing the fallback code, we can just eliminate it.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/os/open.hpp 
> c9346c62e01688f0f55811f7acbe63321b084355 
>   3rdparty/stout/include/stout/os/windows/fcntl.hpp 
> ac90bf08ccf5b594e70310e9843475502b3603a5 
>   3rdparty/stout/include/stout/windows.hpp 
> 1d865f8fd23aba0198017f0bf4be8471cfb714ed 
> 
> 
> Diff: https://reviews.apache.org/r/62638/diff/3/
> 
> 
> Testing
> ---
> 
> make check (Fedora 26)
> 
> Manually verified that sandbox files opened through the webui get the 
> `O_CLOEXEC` flag applied.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 59987: Added protobuf map support.

[Chun-Hung Hsiao]

> On Sept. 22, 2017, 11:14 p.m., Chun-Hung Hsiao wrote:
> > 3rdparty/stout/include/stout/protobuf.hpp
> > Lines 413 (patched)
> > 
> >
> > Not sure if it is cleaner to parse the map here  because we're 
> > duplicating the `apply_visitor` call here.
> > 
> > Another possible implementation is to move this into `parse()` with an 
> > additional `bool mapField = false` parameter. In `parse()`, if `mapFiled` 
> > is set to true, we can set up the key and replace `message` by a `Message` 
> > pointer to the value before passing it into `apply_visitor`.
> > 
> > Thoughts?
> 
> Qian Zhang wrote:
> I think calling `apply_visitor` here should not be a problem. Actually in 
> the code 
> [here](https://github.com/apache/mesos/blob/1.4.0/3rdparty/stout/include/stout/protobuf.hpp#L538),
>  we already call `apply_visitor` to handle array, so I think it should be OK 
> for us call `apply_visitor` here to handle map.

Yeah it's fine to call `apply_visitor` here. I was just thinking if the code is 
more maintainable if we put the parsing logic together. But I don't know the 
answer here so I didn't open an issue. I'll just leave this to you and Anand ;)


> On Sept. 22, 2017, 11:14 p.m., Chun-Hung Hsiao wrote:
> > 3rdparty/stout/include/stout/protobuf.hpp
> > Lines 421 (patched)
> > 
> >
> > Although it seems that `*entry` would not be managed by `repeated_ref` 
> > after `repeated_ref.Add(*entry)`, I cannot find such a guarantee from 
> > protobuf's documentation. How about doing `entry = 
> > reflection->AddMessage(message, field)` here instead, so that we don't need 
> > `repeated_ref`, and the memory management is consistent with the other 
> > cases?
> 
> Qian Zhang wrote:
> I am not sure how we can do `entry = reflection->AddMessage(message, 
> field)` here, do you mean calling `AddMessage()` to add a map message? Then 
> what we should do afterward? I think we still need to parse each entry in the 
> map one by one.
> 
> Actually, using `GetMutableRepeatedFieldRef()` to handle the map field is 
> a way that a Google guy told me, please see the following mail thread for 
> details:
> https://groups.google.com/d/msg/protobuf/nNZ_ItflbLE/x7hLZ1GtAAAJ

The wire format of a map is the same as a repeated field, so we should be able 
to use either `GetMutableRepatedFieldRef.Add(...)` or `AddMessage` to add a map 
entry. Once you get an entry you can just do what you have done in Line 
424--440. It seems to me that we cloud avoid an addition 
construction/destruction of the entry protobuf object with this way.


> On Sept. 22, 2017, 11:14 p.m., Chun-Hung Hsiao wrote:
> > 3rdparty/stout/include/stout/protobuf.hpp
> > Line 607 (original), 640 (patched)
> > 
> >
> > How about a partial specialization for `google::protobuf::Map`?
> 
> Qian Zhang wrote:
> Why do we need to do that? What do we want to achieve with such a partial 
> specialization?

For providing the `parse>(const JSON::Value& 
value)` function. Say if we want to create a protobuf message `m` that contains 
a `map map_field`, we cannot just do 
`m.mutable_map_field()->CopyFrom(parse(json))`.
 Instead we probably need something like 
`m.mutable_map_field()->swap(parse On Sept. 22, 2017, 11:14 p.m., Chun-Hung Hsiao wrote:
> > 3rdparty/stout/include/stout/protobuf.hpp
> > Line 868 (original), 901 (patched)
> > 
> >
> > How about iterating through `reflection->GetRepeatedMessage()` here 
> > instead?
> 
> Qian Zhang wrote:
> Can you elaborate a bit?

Since a proto2 map is the same as a repearted field in its wire format, we can 
do
```
for (int i = 0; i < fieldSize; ++i) {
  const google::protobuf::Message& entry = 
reflection->GetRepeatedMessage(message, field, i);
  const google::protobuf::Reflection* entryReflection = entry.GetReflection();
  ...
}
```


- Chun-Hung


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


On July 4, 2017, 2:43 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59987/
> ---
> 
> (Updated July 4, 2017, 2:43 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Zhitao Li.
> 
> 
> Bugs: MESOS-7656
> https://issues.apache.org/jira/browse/MESOS-7656
> 
> 
> Repository: mesos
> 
> 
> Description

Re: Review Request 61109: Used the default value when parsing an optional enum field from JSON.

[Benjamin Mahler]

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


Fix it, then Ship it!




Thanks guys, let's just clarify the commit summary and description a bit. The 
description is inaccurate in that it states that MESOS-4997 consisted of making 
inexistent enum values get parsed to the default. But this is just the behavior 
that protobuf de-serialization has always had and MESOS-4997 was just about 
coming up with pattern for our enums that reflect this when dealing with 
upgrades.

How about:

```
Fixed JSON protobuf deserialization to ignore unknown enum values.

Protobuf deserialization will discard any unknown enum values. This
patch fixes our custom JSON -> protobuf conversion code to be
consistent with this behavior.

See MESOS-4997 for why this matters when dealing with upgrades.

Fixes MESOS-7828.
```


3rdparty/stout/include/stout/protobuf.hpp
Lines 450-455 (original), 450-458 (patched)


Can you reference the quote from the protobuf documentation, for posterity?

> Notably, unrecognized enum values are discarded when the message is 
deserialized, which makes the field's has.. accessor return false and its 
getter return the first value listed in the enum definition.

From: https://developers.google.com/protocol-buffers/docs/proto#updating


- Benjamin Mahler


On Sept. 29, 2017, 12:24 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61109/
> ---
> 
> (Updated Sept. 29, 2017, 12:24 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and James Peach.
> 
> 
> Bugs: MESOS-7828
> https://issues.apache.org/jira/browse/MESOS-7828
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously in MESOS-4997, we have made any inexistent enum value will
> be parsed to the default enum value when parsing protobuf message from
> a serialized string. Now in this patch, I made parsing protobuf message
> from a JSON have the same behavior.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/protobuf.hpp 
> 15690b66cc4ae0c1bf2c2176d73c385ca75d3c20 
> 
> 
> Diff: https://reviews.apache.org/r/61109/diff/3/
> 
> 
> Testing
> ---
> 
> With this patch, when accessing master endpoint with an inexistent enum `xxx` 
> in a JSON:
> ```
> curl -X POST -H "Content-Type: application/json" -d '{"type": "xxx"}' 
> 127.0.0.1:5050/api/v1
> ```
> The master log will be:
> ```
> I0725 23:09:53.097790   665 http.cpp:1133] HTTP POST for /master/api/v1 from 
> 127.0.0.1:49566 with User-Agent='curl/7.47.0'
> I0725 23:09:53.098006   665 http.cpp:669] Processing call UNKNOWN
> ```
> This proves when parsing an inexistent enum the default enum value (i.e., 
> `UNKNOWN`) will be used.
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 62661: Added --disable-libtool-wrapper configuration to Mesos.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 62661 was successfully built and tested.

Reviews applied: `['62661']`

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

- Mesos Reviewbot Windows


On Sept. 28, 2017, 11:37 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62661/
> ---
> 
> (Updated Sept. 28, 2017, 11:37 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Bannier, and Kapil 
> Arya.
> 
> 
> Bugs: MESOS-7500
> https://issues.apache.org/jira/browse/MESOS-7500
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This flag is used to force libtool to generate executables instead of
> wrapper scripts. A wrapper script might trigger relinking, which takes
> quite a while on slow machines, thus causing failure of tests.
> 
> 
> Diffs
> -
> 
>   configure.ac 92bc1aa5f9604e3b2b678225a57622cd2eb8679a 
>   src/Makefile.am 93ed2bf55447e3e470d9bea8a0b61ce78aad1900 
> 
> 
> Diff: https://reviews.apache.org/r/62661/diff/4/
> 
> 
> Testing
> ---
> 
> 1. sudo make check (fedora)
> 2. Apache CI (centos 7, ubuntu 14.04)
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 62554: Added the ability to prune the gone agent list from the registry.

[Anand Mazumdar]

> On Sept. 26, 2017, 10:11 p.m., Mesos Reviewbot Windows wrote:
> > FAIL: Some Mesos tests failed.
> > 
> > Reviews applied: `['62475', '62476', '62477', '62581', '62478', '62479', 
> > '62480', '62481', '62507', '62531', '62554']`
> > 
> > Failed command: `C:\mesos\src\mesos-tests.exe --verbose 
> > --gtest_filter="-ContentType/MasterAPITest.EventAuthorizationFiltering/1"`
> > 
> > All the build artifacts available at: 
> > http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62554
> > 
> > Relevant logs:
> > 
> > - 
> > [mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62554/logs/mesos-tests-stdout.log):
> > 
> > ```
> > [   OK ] FilesTest.DetachTest (5 ms)
> > [ RUN  ] FilesTest.ReadTest
> > [   OK ] FilesTest.ReadTest (292 ms)
> > [ RUN  ] FilesTest.DownloadTest
> > [   OK ] FilesTest.DownloadTest (126 ms)
> > [ RUN  ] FilesTest.DebugTest
> > [   OK ] FilesTest.DebugTest (123 ms)
> > [ RUN  ] FilesTest.AuthenticationTest
> > [   OK ] FilesTest.AuthenticationTest (99 ms)
> > [--] 6 tests from FilesTest (807 ms total)
> > 
> > [--] 3 tests from GarbageCollectorTest
> > [ RUN  ] GarbageCollectorTest.Schedule
> > [   OK ] GarbageCollectorTest.Schedule (123 ms)
> > [ RUN  ] GarbageCollectorTest.Unschedule
> > [   OK ] GarbageCollectorTest.Unschedule (36 ms)
> > [ RUN  ] GarbageCollectorTest.Prune
> > [   OK ] GarbageCollectorTest.Prune (37 ms)
> > [--] 3 tests from GarbageCollectorTest (299 ms total)
> > 
> > [--] 5 tests from GarbageCollectorIntegrationTest
> > [ RUN  ] GarbageCollectorIntegrationTest.Restart
> > 
> > C:\mesos\mesos\src\tests\gc_tests.cpp(332): ERROR: this mock object (used 
> > in test GarbageCollectorIntegrationTest.Restart) should be deleted but 
> > never is. Its address is @00752FB59910.
> > C:\mesos\mesos\src\tests\containerizer.cpp(414): ERROR: this mock object 
> > (used in test GarbageCollectorIntegrationTest.Restart) should be deleted 
> > but never is. Its address is @00752FB59DE0.
> > C:\mesos\mesos\src\tests\gc_tests.cpp(317): ERROR: this mock object (used 
> > in test GarbageCollectorIntegrationTest.Restart) should be deleted but 
> > never is. Its address is @00752FB5B370.
> > C:\mesos\mesos\src\tests\mock_registrar.cpp(54): ERROR: this mock object 
> > (used in test GarbageCollectorIntegrationTest.Restart) should be deleted 
> > but never is. Its address is @01E405FCC520.
> > C:\mesos\mesos\3rdparty\libprocess\include\process/gmock.hpp(235): ERROR: 
> > this mock object (used in test GarbageCollectorIntegrationTest.Restart) 
> > should be deleted but never is. Its address is @01E406498898.
> > ERROR: 5 leaked mock objects found at program exit.
> > ```
> > 
> > - 
> > [mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/62554/logs/mesos-tests-stderr.log):
> > 
> > ```
> >   scalar {
> > value: 1024
> >   }
> > }
> > resources {
> >   name: "disk"
> >   type: SCALAR
> >   scalar {
> > value: 1024
> >   }
> > }
> > resources {
> >   name: "ports"
> >   type: RANGES
> >   ranges {
> > range {
> >   begin: 31000
> >   end: 32000
> > }
> >   }
> > }
> > checkpoint: true
> > port: 56595
> > 
> > 
> > To remedy this do as follows:
> > Step 1: rm -f C:\Users\mesos\AppData\Local\Temp\2\ppnykI\meta\slaves\latest
> > This ensures agent doesn't recover old live executors.
> > Step 2: Restart the agent.
> > ```
> 
> Vinod Kone wrote:
> this seems problematic, though i can't easily tell which test is 
> breaking. GarbageCollectorIntegrationTest.Restart? Anand, can you please 
> triage this?

We need to add this for the test to pass on windows similar to what we did for 
the other new tests that involve restarting an agent again

```cpp
// TODO(andschwa): Enable this when MESOS-7604 is fixed.
```


- Anand


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


On Sept. 26, 2017, 8:30 p.m., Anand Mazumdar wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62554/
> ---
> 
> (Updated Sept. 26, 2017, 8:30 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7448
> https://issues.apache.org/jira/browse/MESOS-7448
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This change introduces two master options `registry_max_gone_agent_age`
> (age based) and `registry_max_gone_agent_count` (count based) for
> GC'ing the list of gone agents in the registry.
> 
> Review: https://reviews.apache.org/r/62554
> 
> 
> Diffs
> -
> 
>   

Re: Review Request 61174: Added a test `ProtobufTest.ParseJSONUnrecognizedEnum`.

[Benjamin Mahler]

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


Fix it, then Ship it!





3rdparty/stout/tests/protobuf_tests.cpp
Lines 423-425 (patched)


Let's clarify that the field will be unset (similar to how this is handled 
by protobuf de-serialization).


- Benjamin Mahler


On Sept. 29, 2017, 8:47 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61174/
> ---
> 
> (Updated Sept. 29, 2017, 8:47 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and James Peach.
> 
> 
> Bugs: MESOS-7828
> https://issues.apache.org/jira/browse/MESOS-7828
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test `ProtobufTest.ParseJSONUnrecognizedEnum`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/protobuf_tests.cpp 
> 8877e8934e0f7875bfedcfa88b491ce4b13ca44f 
>   3rdparty/stout/tests/protobuf_tests.proto 
> d16726aa8060aea2b830040b20dbdd467c801483 
> 
> 
> Diff: https://reviews.apache.org/r/61174/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Review Request 62696: Updated error messages in `getMountNamespaceTarget()`.

[Andrei Budnik]

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

Review request for mesos, Alexander Rukletsov, Gilbert Song, and Jan Schlicht.


Repository: mesos


Description
---

Previously, `getMountNamespaceTarget()` ignored error messages
returned by `ns::getns()` and `os::children()`.


Diffs
-

  src/slave/containerizer/mesos/utils.cpp 
be8126da00b690e83ec379393e78457b5ff0acee 


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


Testing
---

make check (fedora)


Thanks,

Andrei Budnik

Re: Review Request 59987: Added protobuf map support.

[Chun-Hung Hsiao]

> On Sept. 22, 2017, 11:19 p.m., Chun-Hung Hsiao wrote:
> > 3rdparty/stout/include/stout/protobuf.hpp
> > Lines 433 (patched)
> > 
> >
> > The key can also be of an integral type.
> 
> Qian Zhang wrote:
> Yeah, I had the same concern before. But in [our 
> code](https://github.com/apache/mesos/blob/1.4.0/3rdparty/stout/include/stout/json.hpp#L190),
>  we define `JSON::Object::values` as a `string` to `JSON::Value` map, so I am 
> not sure how we can support non-string key.

I think that's because JSON cannos use a numeric value as a key. But we can do 
things like the following in JSON:
```
{
  "1": 1.0,
  "2": 2.0
}
```
Given that ve already know the key type of the map from the reflection, we 
could parse the string into the appropriate integral key type.


- Chun-Hung


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


On July 4, 2017, 2:43 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59987/
> ---
> 
> (Updated July 4, 2017, 2:43 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Zhitao Li.
> 
> 
> Bugs: MESOS-7656
> https://issues.apache.org/jira/browse/MESOS-7656
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Map is a feature of proto2 syntax, but it can only be compiled
> with proto3 compiler, see the following discussion for details:
> https://groups.google.com/forum/#!topic/protobuf/p4WxcplrlA4
> We have already upgraded the compiler from 2.6.1 to 3.3.0 in
> MESOS-7228, however, to use protobuf map in Mesos code, we also
> need to add the protobuf map support to the code in Mesos for
> converting protobuf message to JSON object and parsing JSON
> object as protobuf message, that is what I have done in this patch.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/protobuf.hpp 
> 15690b66cc4ae0c1bf2c2176d73c385ca75d3c20 
> 
> 
> Diff: https://reviews.apache.org/r/59987/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

[Eric Chung]

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

(Updated Sept. 29, 2017, 9:25 p.m.)


Review request for mesos, Armand Grillet, Jason Lai, and Kevin Klues.


Changes
---

rebase


Repository: mesos


Description
---

Part of MESOS-7310, this patch adds the mesos.http and mesos.exceptions
modules, which provides a Resource class and its descendants for
abstracting away common operations over http connectioins with JSON
serialization.


Diffs (updated)
-

  src/python/cli_new/bootstrap c84e549e59bc7a07b5b70c24c63bc0c16881e442 
  src/python/lib/mesos/exceptions.py PRE-CREATION 
  src/python/lib/mesos/http.py PRE-CREATION 
  src/python/lib/requirements.in e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
  src/python/lib/tests/conftest.py PRE-CREATION 
  src/python/lib/tests/test_exceptions.py PRE-CREATION 
  src/python/lib/tests/test_http.py PRE-CREATION 
  src/python/lib/tox.ini 8ad030d2dbf1fb7a04f1eaadb587462b4ef3f054 
  src/python/pylint.config 9f0361d160fa82038ccefcbc146e52f0af8f99e8 


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

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


Testing
---

install tox
cd src/python/lib
tox


Thanks,

Eric Chung

Re: Review Request 62212: Send TASK_STARTING from the built-in executors.

[Benno Evers]

> On Sept. 15, 2017, 11:36 a.m., Andrei Budnik wrote:
> > src/launcher/default_executor.cpp
> > Line 1379 (original), 1396 (patched)
> > 
> >
> > If a default executor is allowed to call `launchGroup` more than once, 
> > then there may be a case when some containers are launched already, while 
> > others not.

I think if this is possible, the error was already in the existing code?


- Benno


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


On Sept. 13, 2017, 2:52 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62212/
> ---
> 
> (Updated Sept. 13, 2017, 2:52 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7941
> https://issues.apache.org/jira/browse/MESOS-7941
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gives schedulers more information about a tasks status,
> in particular it gives a better estimate of a tasks start time
> and helps differentiating between tasks stuck in TASK_STAGING
> and tasks stuck in TASK_STARTING.
> 
> 
> Diffs
> -
> 
>   docs/high-availability-framework-guide.md 
> 73743aba31f9d0ca827d318e2ecb4752a91b1be0 
>   src/docker/executor.cpp e9949f652cd8527991ebfdfbf14e68b4c958fe79 
>   src/launcher/default_executor.cpp 106b7f2e0244d211c66b237b5d1c51f43fc6e529 
>   src/launcher/executor.cpp 951597b576b4912541dd87d52dcb981393e58082 
> 
> 
> Diff: https://reviews.apache.org/r/62212/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 62632: Added a test using Docker, a file URI, and the DefaultExecutor.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On Sept. 27, 2017, 9:50 p.m., Gaston Kleiman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62632/
> ---
> 
> (Updated Sept. 27, 2017, 9:50 p.m.)
> 
> 
> Review request for mesos, Greg Mann and Vinod Kone.
> 
> 
> Bugs: MESOS-7916
> https://issues.apache.org/jira/browse/MESOS-7916
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This test verifies that URIs set on Docker tasks are fetched and made
> available to them when started by the DefaultExecutor.
> 
> 
> Diffs
> -
> 
>   src/tests/default_executor_tests.cpp 
> dd1a6ad0d4e2bf74972e15b478652196ee9cd927 
> 
> 
> Diff: https://reviews.apache.org/r/62632/diff/1/
> 
> 
> Testing
> ---
> 
> `sudo GLOG_v=1 bin/mesos-tests.sh 
> --gtest_filter="*DefaultExecutor*DockerTaskWithFileURI*" --verbose 
> --gtest_repeat=200` passed on GNU/Linux
> 
> 
> Thanks,
> 
> Gaston Kleiman
> 
>

Re: Review Request 62168: Added a test using a file URI and the DefaultExecutor.

[Gaston Kleiman]

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

(Updated Sept. 29, 2017, 12:06 p.m.)


Review request for mesos, Greg Mann and Vinod Kone.


Changes
---

Rebased.


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


Repository: mesos


Description
---

This test verifies that URIs set on tasks are fetched and made available
to them when started by the DefaultExecutor.


Diffs (updated)
-

  src/tests/default_executor_tests.cpp dd1a6ad0d4e2bf74972e15b478652196ee9cd927 


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

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


Testing
---

The test succeeded for over 500 iterations on GNU/Linux.


Thanks,

Gaston Kleiman

Re: Review Request 61174: Added a test `ProtobufTest.ParseJSONUnrecognizedEnum`.

[James Peach]

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


Fix it, then Ship it!





3rdparty/stout/tests/protobuf_tests.cpp
Lines 430 (patched)


I think it is the same case, but could you add a case where the enum has an 
empty value?

```
"empty": ""
```

This ought to behave the same as the "XXX"



3rdparty/stout/tests/protobuf_tests.cpp
Lines 432 (patched)


Just a suggestion ... consider a separate patch to use raw string literals 
for all the JSON in this file so we can remove all the escaping.


- James Peach


On Sept. 29, 2017, 8:47 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61174/
> ---
> 
> (Updated Sept. 29, 2017, 8:47 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and James Peach.
> 
> 
> Bugs: MESOS-7828
> https://issues.apache.org/jira/browse/MESOS-7828
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test `ProtobufTest.ParseJSONUnrecognizedEnum`.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/protobuf_tests.cpp 
> 8877e8934e0f7875bfedcfa88b491ce4b13ca44f 
>   3rdparty/stout/tests/protobuf_tests.proto 
> d16726aa8060aea2b830040b20dbdd467c801483 
> 
> 
> Diff: https://reviews.apache.org/r/61174/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 62212: Send TASK_STARTING from the built-in executors.

[Benno Evers]

> On Sept. 13, 2017, 5:29 p.m., Andrei Budnik wrote:
> > src/launcher/default_executor.cpp
> > Line 326 (original), 332 (patched)
> > 
> >
> > Should we update the comment above?

I think the comment is still correct.


> On Sept. 13, 2017, 5:29 p.m., Andrei Budnik wrote:
> > src/launcher/default_executor.cpp
> > Lines 412 (patched)
> > 
> >
> > Adding default constructor for `DefaultExecutor::Container` will help 
> > us to get rid of repeating `None()` and `false`.

I'm not sure about adding a function that will be called only once.


- Benno


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


On Sept. 13, 2017, 2:52 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62212/
> ---
> 
> (Updated Sept. 13, 2017, 2:52 p.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Alexander Rukletsov.
> 
> 
> Bugs: MESOS-7941
> https://issues.apache.org/jira/browse/MESOS-7941
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This gives schedulers more information about a tasks status,
> in particular it gives a better estimate of a tasks start time
> and helps differentiating between tasks stuck in TASK_STAGING
> and tasks stuck in TASK_STARTING.
> 
> 
> Diffs
> -
> 
>   docs/high-availability-framework-guide.md 
> 73743aba31f9d0ca827d318e2ecb4752a91b1be0 
>   src/docker/executor.cpp e9949f652cd8527991ebfdfbf14e68b4c958fe79 
>   src/launcher/default_executor.cpp 106b7f2e0244d211c66b237b5d1c51f43fc6e529 
>   src/launcher/executor.cpp 951597b576b4912541dd87d52dcb981393e58082 
> 
> 
> Diff: https://reviews.apache.org/r/62212/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Benno Evers
> 
>

Re: Review Request 62213: Fix unit tests that were broken by the additional TASK_STARTING update.

[Benno Evers]

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

(Updated Sept. 29, 2017, 7:48 p.m.)


Review request for mesos, Andrei Budnik and Alexander Rukletsov.


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


Repository: mesos


Description
---

Fix unit tests that were broken by the additional TASK_STARTING update.


Diffs (updated)
-

  src/tests/api_tests.cpp d260a1c9560e8ff6b46eea7f2f4ddb11e18653e3 
  src/tests/check_tests.cpp fd15a47f1d67ad852d84d83aad54c9fa93c60bbb 
  src/tests/command_executor_tests.cpp f706f55b5bfab824268498d95d775b216561cd66 
  src/tests/container_logger_tests.cpp fb8e441bd3842b1840a384a025fa8e9ccbb22cf6 
  src/tests/containerizer/environment_secret_isolator_tests.cpp 
cf7b9eb6fd76dbdd3650ea2ad5b9097cf490e948 
  src/tests/containerizer/posix_rlimits_isolator_tests.cpp 
0030cd1e1f73422bef806bbc0453134e3d7840d8 
  src/tests/default_executor_tests.cpp dd1a6ad0d4e2bf74972e15b478652196ee9cd927 
  src/tests/disk_quota_tests.cpp 742b6e1a6340d488f4bae8acbd09a3e575c72ad2 
  src/tests/fault_tolerance_tests.cpp c34850aaa76289d822317f30e5da0e0cb7f115c6 
  src/tests/health_check_tests.cpp f4b50b1cb505084f64bf2dd279d9189ca65c8cdc 
  src/tests/master_tests.cpp 98908c00c2d5bfd08122fd317a2ce95332d9583d 
  src/tests/master_validation_tests.cpp 
f00dd9b6fcb963b995fa238766a531f073205ce9 
  src/tests/oversubscription_tests.cpp 02b10d6689dd6f01510cd2d5db2bb76b4b190eca 
  src/tests/partition_tests.cpp 0886f4890ac3fec6f38146946892769a99c3e68f 
  src/tests/persistent_volume_endpoints_tests.cpp 
7a24bf48e9d84bf31ad85397ac3e1b4e566cbc2c 
  src/tests/persistent_volume_tests.cpp 
1b35af427a2561f6094560c7bc9387dcb948b781 
  src/tests/reconciliation_tests.cpp 64a1d3da9f03b863551555c4734aef39e835b122 
  src/tests/reservation_endpoints_tests.cpp 
5a6e9a7f08a5306542830f359868b2c31f4820c6 
  src/tests/role_tests.cpp fc4c017dc59ab44a0ce0ea46f02eb6e1706ffb72 
  src/tests/scheduler_tests.cpp 4eda96e50a505eb36588bbcf9c3ba76d9ede7839 
  src/tests/slave_authorization_tests.cpp 
4c7d37fd14c8f3e7a52d35fb685fb8f05cba1e70 
  src/tests/slave_recovery_tests.cpp 0cd2b5d37e35ccc2fa2c14db750d1314238bc312 
  src/tests/slave_tests.cpp e9bcfefd52e1e99a7a5877b2e7c30af958ca1723 
  src/tests/teardown_tests.cpp 5eada4f4392b7c721f5efbd537b0e817d40c7a27 


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

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


Testing
---


Thanks,

Benno Evers

Re: Review Request 62212: Send TASK_STARTING from the built-in executors.

[Benno Evers]

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

(Updated Sept. 29, 2017, 7:48 p.m.)


Review request for mesos, Andrei Budnik and Alexander Rukletsov.


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


Repository: mesos


Description
---

This gives schedulers more information about a tasks status,
in particular it gives a better estimate of a tasks start time
and helps differentiating between tasks stuck in TASK_STAGING
and tasks stuck in TASK_STARTING.


Diffs (updated)
-

  docs/high-availability-framework-guide.md 
73743aba31f9d0ca827d318e2ecb4752a91b1be0 
  src/docker/executor.cpp 3b0767ffbbe9982639d0b87fbb0573accdd48559 
  src/launcher/default_executor.cpp 136c000917b2c4fdaf68fd460764c3a15fdf3bf0 
  src/launcher/executor.cpp 0131577d5b9018e7094815f1377b2ee59a4493d4 


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

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


Testing
---


Thanks,

Benno Evers

Re: Review Request 62696: Updated error messages in `getMountNamespaceTarget()`.

[Mesos Reviewbot Windows]

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



PASS: Mesos patch 62696 was successfully built and tested.

Reviews applied: `['62696']`

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

- Mesos Reviewbot Windows


On Sept. 29, 2017, 11:12 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62696/
> ---
> 
> (Updated Sept. 29, 2017, 11:12 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Gilbert Song, and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, `getMountNamespaceTarget()` ignored error messages
> returned by `ns::getns()` and `os::children()`.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/utils.cpp 
> be8126da00b690e83ec379393e78457b5ff0acee 
> 
> 
> Diff: https://reviews.apache.org/r/62696/diff/1/
> 
> 
> Testing
> ---
> 
> make check (fedora)
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Review Request 62698: Fixed the clang complation error for the executor tests.

[Chun-Hung Hsiao]

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

Review request for mesos, Benjamin Hindman and Vinod Kone.


Repository: mesos


Description
---

clang does not allow conversions from `Future` to
`Future`, so we specify the return type of the lambda
in the `THREADSAFE_Executor_Execute` test explicitly.


Diffs
-

  3rdparty/libprocess/src/tests/process_tests.cpp 
e49431873393e835bf0e0a9c240e5d54f06e0ab7 


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


Testing
---

make check with gcc 5.4.0 and clang 3.4.2.


Thanks,

Chun-Hung Hsiao

Re: Review Request 62213: Fix unit tests that were broken by the additional TASK_STARTING update.

[Mesos Reviewbot Windows]

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



FAIL: Some Mesos tests failed.

Reviews applied: `['62212', '62213']`

Failed command: `C:\mesos\src\mesos-tests.exe --verbose 
--gtest_filter="-ContentType/MasterAPITest.EventAuthorizationFiltering/1"`

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

Relevant logs:

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

```
[--] Global test environment tear-down
[==] 636 tests from 67 test cases ran. (588866 ms total)
[  PASSED  ] 615 tests.
[  FAILED  ] 21 tests, listed below:
[  FAILED  ] CommandExecutorCheckTest.CommandCheckStatusChange
[  FAILED  ] CommandExecutorCheckTest.CommandCheckTimeout
[  FAILED  ] CommandExecutorCheckTest.CommandCheckAndHealthCheckNoShadowing
[  FAILED  ] DefaultExecutorCheckTest.CommandCheckAndHealthCheckNoShadowing
[  FAILED  ] HTTPCommandExecutorTest.ExplicitAcknowledgements
[  FAILED  ] HealthCheckTest.HealthyTask
[  FAILED  ] HealthCheckTest.HealthyTaskNonShell
[  FAILED  ] HealthCheckTest.ConsecutiveFailures
[  FAILED  ] HealthCheckTest.EnvironmentSetup
[  FAILED  ] HealthCheckTest.GracePeriod
[  FAILED  ] HealthCheckTest.CheckCommandTimeout
[  FAILED  ] HealthCheckTest.HealthyToUnhealthyTransitionWithinGracePeriod
[  FAILED  ] HealthCheckTest.HealthyTaskViaTCP
[  FAILED  ] SlaveTest.CommandTaskWithKillPolicy
[  FAILED  ] SlaveTest.StatisticsEndpointRunningExecutor
[  FAILED  ] HTTPCommandExecutor/CommandExecutorTest.NoTaskKillingCapability/0, 
where GetParam() = false
[  FAILED  ] HTTPCommandExecutor/CommandExecutorTest.NoTaskKillingCapability/1, 
where GetParam() = true
[  FAILED  ] HTTPCommandExecutor/CommandExecutorTest.TaskKillingCapability/0, 
where GetParam() = false
[  FAILED  ] HTTPCommandExecutor/CommandExecutorTest.TaskKillingCapability/1, 
where GetParam() = true
[  FAILED  ] 
MesosContainerizer/DefaultExecutorTest.ROOT_ContainerStatusForTask/0, where 
GetParam() = "mesos"
[  FAILED  ] MesosContainerizer/DefaultExecutorTest.TaskWithFileURI/0, where 
GetParam() = "mesos"

21 FAILED TESTS
  YOU HAVE 174 DISABLED TESTS

```

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

```
I0929 22:19:53.078156 31456 master.cpp:8418] Removing framework 
469678d9-0630-4a2c-bea0-3fe1bf761c71- (default)
I0929 22:19:53.078156 31456 master.cpp:3267] Deactivating framework 
469678d9-0630-4a2c-bea0-3fe1bf761c71- (default)
I0929 22:19:53.079155 30516 hierarchical.cpp:412] Deactivated framework 
469678d9-0630-4a2c-bea0-3fe1bf761c71-
I0929 22:19:53.079155 30024 slave.cpp:3248] Shutting down framework 
469678d9-0630-4a2c-bea0-3fe1bf761c71-
I0929 22:19:53.079155 31456 master.cpp:8993] Updating the state of task 
0be475a3-9a01-457e-a42a-7ac80c78d758 of framework 
469678d9-0630-4a2c-bea0-3fe1bf761c71- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0929 22:19:53.096598 30024 slave.cpp:5755] Shutting down executor 'default' of 
framework 469678d9-0630-4a2c-bea0-3fe1bf761c71- (via HTTP)
I0929 22:19:53.099606 31456 master.cpp:9087] Removing task 
0be475a3-9a01-457e-a42a-7ac80c78d758 with resources 
[{"allocation_info":{"role":"*"},"name":"cpus","scalar":{"value":2.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"mem","scalar":{"value":1024.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"disk","scalar":{"value":1024.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"ports","ranges":{"range":[{"begin":31000,"end":32000}]},"type":"RANGES"}]
 of framework 469678d9-0630-4a2c-bea0-3fe1bf761c71- on agent 
469678d9-0630-4a2c-bea0-3fe1bf761c71-S0 at slave(255)@10.3.1.7:54694 
(mesos-bld-s2.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0929 22:19:53.117606 31456 master.cpp:9116] Removing executor 'default' with 
resources [] of framework 469678d9-0630-4a2c-bea0-3fe1bf761c71- on agent 
469678d9-0630-4a2c-bea0-3fe1bf761c71-S0 at slave(255)@10.3.1.7:54694 
(mesos-bld-s2.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0929 22:19:53.120606 29992 hierarchical.cpp:355] Removed framework 
469678d9-0630-4a2c-bea0-3fe1bf761c71-
E0929 22:19:53.121605 31440 scheduler.cpp:649] End-Of-File received from 
master. The master closed the event stream
I0929 22:19:53.126607 29992 scheduler.cpp:444] Re-detecting master
I0929 22:19:53.130606 29992 scheduler.cpp:470] New master detected at 
master@10.3.1.7:54694
I0929 22:19:53.140605 31456 slave.cpp:5420] Executor 'default' of framework 
469678d9-0630-4a2c-bea0-3fe1bf761c71- exited with status 0
I0929 22:19:53.155608 31456 slave.cpp:5524] Cleaning up executor 'default' of 
framework 469678d9-0630-4a2c-bea0-3fe1bf761c71- 

Re: Review Request 62698: Fixed the clang complation error for the executor tests.

[Benjamin Hindman]

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


Ship it!




Ship It!

- Benjamin Hindman


On Sept. 29, 2017, 9:45 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62698/
> ---
> 
> (Updated Sept. 29, 2017, 9:45 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> clang does not allow conversions from `Future` to
> `Future`, so we specify the return type of the lambda
> in the `THREADSAFE_Executor_Execute` test explicitly.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> e49431873393e835bf0e0a9c240e5d54f06e0ab7 
> 
> 
> Diff: https://reviews.apache.org/r/62698/diff/1/
> 
> 
> Testing
> ---
> 
> make check with gcc 5.4.0 and clang 3.4.2.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 62698: Fixed the clang complation problem for the executor tests.

[Chun-Hung Hsiao]

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

(Updated Sept. 29, 2017, 10:18 p.m.)


Review request for mesos, Benjamin Hindman and Vinod Kone.


Summary (updated)
-

Fixed the clang complation problem for the executor tests.


Repository: mesos


Description (updated)
---

It appears that g++ throws away the cv-qualifiers when doing the
lvalue-to-rvalue conversion for lambdas returning strings but clang does
not, so we make `f3` in `THREADSAFE_Executor_Execute` return a
non-constant string.


Diffs (updated)
-

  3rdparty/libprocess/src/tests/process_tests.cpp 
e49431873393e835bf0e0a9c240e5d54f06e0ab7 


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

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


Testing
---

make check with gcc 5.4.0 and clang 3.4.2.


Thanks,

Chun-Hung Hsiao

Re: Review Request 62687: Added utility function to get data from a master endpoint to CLI.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [62687, 62663]

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

Error:
2017-09-30 00:10:30 URL:https://reviews.apache.org/r/62687/diff/raw/ 
[6979/6979] -> "62687.patch" [1]
error: patch failed: src/python/cli_new/lib/cli/util.py:155
error: src/python/cli_new/lib/cli/util.py: patch does not apply

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

- Mesos Reviewbot


On Sept. 29, 2017, 10:16 a.m., Armand Grillet wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62687/
> ---
> 
> (Updated Sept. 29, 2017, 10:16 a.m.)
> 
> 
> Review request for mesos and Kevin Klues.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This removes the similar code that had CLI plugins.
> 
> 
> Diffs
> -
> 
>   src/python/cli_new/lib/cli/plugins/agent/main.py PRE-CREATION 
>   src/python/cli_new/lib/cli/plugins/task/main.py PRE-CREATION 
>   src/python/cli_new/lib/cli/util.py 7371f83543ed527bea8dbf2fe4e20d92ef8e4492 
> 
> 
> Diff: https://reviews.apache.org/r/62687/diff/1/
> 
> 
> Testing
> ---
> 
> To test with one master:
> ```
> $ ./bootstrap
> $ source activate
> $ mesos-cli-tests
> ```
> I also checked that the Python linter was still working.
> 
> 
> Thanks,
> 
> Armand Grillet
> 
>

Re: Review Request 62698: Fixed the clang complation problem for the executor tests.

[Mesos Reviewbot Windows]

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



FAIL: Some Mesos tests failed.

Reviews applied: `['62698']`

Failed command: `C:\mesos\src\mesos-tests.exe --verbose 
--gtest_filter="-ContentType/MasterAPITest.EventAuthorizationFiltering/1"`

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

Relevant logs:

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

```
[ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/0
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/0 (282 ms)
[ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/1
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/1 (249 ms)
[--] 30 tests from ContentType/SchedulerTest (26066 ms total)

[--] 2 tests from ContentTypeAndSSLConfig/SchedulerSSLTest
[ RUN  ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0
[   OK ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0 (923 
ms)
[ RUN  ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/1
[   OK ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/1 
(1093 ms)
[--] 2 tests from ContentTypeAndSSLConfig/SchedulerSSLTest (2126 ms 
total)

[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (138 
ms)
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (143 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (341 ms 
total)

[--] Global test environment tear-down
[==] 634 tests from 67 test cases ran. (373504 ms total)
[  PASSED  ] 633 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] MesosContainerizer/DefaultExecutorTest.TaskWithFileURI/0, where 
GetParam() = "mesos"

 1 FAILED TEST
  YOU HAVE 182 DISABLED TESTS

```

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

```
I0930 00:14:39.732174 36956 master.cpp:8438] Removing framework 
dd611862-78a5-4022-a45d-29889f26a903- (default)
I0930 00:14:39.732174 36956 master.cpp:3317] Deactivating framework 
dd611862-78a5-4022-a45d-29889f26a903- (default)
I0930 00:14:39.741178 37440 hierarchical.cpp:412] Deactivated framework 
dd611862-78a5-4022-a45d-29889f26a903-
I0930 00:14:39.742172 33496 slave.cpp:3239] Shutting down framework 
dd611862-78a5-4022-a45d-29889f26a903-
I0930 00:14:39.742172 36956 master.cpp:9136] Updating the state of task 
8dad71be-ce9c-42fe-9c61-a675180666d7 of framework 
dd611862-78a5-4022-a45d-29889f26a903- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0930 00:14:39.742172 33496 slave.cpp:5746] Shutting down executor 'default' of 
framework dd611862-78a5-4022-a45d-29889f26a903- (via HTTP)
I0930 00:14:39.746171 36956 master.cpp:9230] Removing task 
8dad71be-ce9c-42fe-9c61-a675180666d7 with resources 
[{"allocation_info":{"role":"*"},"name":"cpus","scalar":{"value":2.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"mem","scalar":{"value":1024.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"disk","scalar":{"value":1024.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"ports","ranges":{"range":[{"begin":31000,"end":32000}]},"type":"RANGES"}]
 of framework dd611862-78a5-4022-a45d-29889f26a903- on agent 
dd611862-78a5-4022-a45d-29889f26a903-S0 at slave(251)@10.3.1.5:53530 
(mesos-bld-s1.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0930 00:14:39.765171 36956 master.cpp:9259] Removing executor 'default' with 
resources [] of framework dd611862-78a5-4022-a45d-29889f26a903- on agent 
dd611862-78a5-4022-a45d-29889f26a903-S0 at slave(251)@10.3.1.5:53530 
(mesos-bld-s1.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0930 00:14:39.769170 36088 hierarchical.cpp:355] Removed framework 
dd611862-78a5-4022-a45d-29889f26a903-
E0930 00:14:39.769170 37092 scheduler.cpp:649] End-Of-File received from 
master. The master closed the event stream
I0930 00:14:39.770170 37440 scheduler.cpp:444] Re-detecting master
I0930 00:14:39.773172 37440 scheduler.cpp:470] New master detected at 
master@10.3.1.5:53530
I0930 00:14:39.794169 36820 slave.cpp:5411] Executor 'default' of framework 
dd611862-78a5-4022-a45d-29889f26a903- exited with status 0
I0930 00:14:39.795168 36820 slave.cpp:5515] Cleaning up executor 'default' of 
framework dd611862-78a5-4022-a45d-29889f26a903- (via HTTP)
W0930 00:14:39.795168 37092 master.cpp:7089] Ignoring unknown exited executor 
'default' of 

Re: Review Request 61172: Added mesos.http and mesos.exceptions for CLI.

[Mesos Reviewbot Windows]

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



FAIL: Some Mesos tests failed.

Reviews applied: `['61172']`

Failed command: `C:\mesos\src\mesos-tests.exe --verbose 
--gtest_filter="-ContentType/MasterAPITest.EventAuthorizationFiltering/1"`

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

Relevant logs:

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

```
[ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/0
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/0 (249 ms)
[ RUN  ] ContentType/SchedulerTest.SchedulerReconnect/1
[   OK ] ContentType/SchedulerTest.SchedulerReconnect/1 (253 ms)
[--] 30 tests from ContentType/SchedulerTest (26578 ms total)

[--] 2 tests from ContentTypeAndSSLConfig/SchedulerSSLTest
[ RUN  ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0
[   OK ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/0 (902 
ms)
[ RUN  ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/1
[   OK ] ContentTypeAndSSLConfig/SchedulerSSLTest.RunTaskAndTeardown/1 
(1023 ms)
[--] 2 tests from ContentTypeAndSSLConfig/SchedulerSSLTest (2053 ms 
total)

[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (133 
ms)
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (151 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (370 ms 
total)

[--] Global test environment tear-down
[==] 634 tests from 67 test cases ran. (358685 ms total)
[  PASSED  ] 633 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] MesosContainerizer/DefaultExecutorTest.TaskWithFileURI/0, where 
GetParam() = "mesos"

 1 FAILED TEST
  YOU HAVE 182 DISABLED TESTS

```

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

```
I0930 00:17:39.407632 31184 master.cpp:8438] Removing framework 
29bea81c-7cc9-4cae-8b13-4e61edd45a51- (default)
I0930 00:17:39.407632 31184 master.cpp:3317] Deactivating framework 
29bea81c-7cc9-4cae-8b13-4e61edd45a51- (default)
I0930 00:17:39.408632 30504 hierarchical.cpp:412] Deactivated framework 
29bea81c-7cc9-4cae-8b13-4e61edd45a51-
I0930 00:17:39.408632 28900 slave.cpp:3239] Shutting down framework 
29bea81c-7cc9-4cae-8b13-4e61edd45a51-
I0930 00:17:39.408632 31184 master.cpp:9136] Updating the state of task 
5213cf5b-e0d5-4579-bdcf-84cb469c141b of framework 
29bea81c-7cc9-4cae-8b13-4e61edd45a51- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0930 00:17:39.408632 28900 slave.cpp:5746] Shutting down executor 'default' of 
framework 29bea81c-7cc9-4cae-8b13-4e61edd45a51- (via HTTP)
I0930 00:17:39.412633 31184 master.cpp:9230] Removing task 
5213cf5b-e0d5-4579-bdcf-84cb469c141b with resources 
[{"allocation_info":{"role":"*"},"name":"cpus","scalar":{"value":2.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"mem","scalar":{"value":1024.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"disk","scalar":{"value":1024.0},"type":"SCALAR"},{"allocation_info":{"role":"*"},"name":"ports","ranges":{"range":[{"begin":31000,"end":32000}]},"type":"RANGES"}]
 of framework 29bea81c-7cc9-4cae-8b13-4e61edd45a51- on agent 
29bea81c-7cc9-4cae-8b13-4e61edd45a51-S0 at slave(251)@10.3.1.7:55700 
(mesos-bld-s2.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0930 00:17:39.426633 31184 master.cpp:9259] Removing executor 'default' with 
resources [] of framework 29bea81c-7cc9-4cae-8b13-4e61edd45a51- on agent 
29bea81c-7cc9-4cae-8b13-4e61edd45a51-S0 at slave(251)@10.3.1.7:55700 
(mesos-bld-s2.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
E0930 00:17:39.444922 31364 scheduler.cpp:649] End-Of-File received from 
master. The master closed the event stream
I0930 00:17:39.443634 28900 hierarchical.cpp:355] Removed framework 
29bea81c-7cc9-4cae-8b13-4e61edd45a51-
I0930 00:17:39.445634 31248 scheduler.cpp:444] Re-detecting master
I0930 00:17:39.452633 31248 scheduler.cpp:470] New master detected at 
master@10.3.1.7:55700
I0930 00:17:39.468634 30652 slave.cpp:5411] Executor 'default' of framework 
29bea81c-7cc9-4cae-8b13-4e61edd45a51- exited with status 0
I0930 00:17:39.469635 30652 slave.cpp:5515] Cleaning up executor 'default' of 
framework 29bea81c-7cc9-4cae-8b13-4e61edd45a51- (via HTTP)
W0930 00:17:39.469635 28900 master.cpp:7089] Ignoring unknown exited executor 
'default' of 

Re: Review Request 61473: Do not kill non partition aware tasks.

[Jiang Yan Xu]

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



As I commented on the JIRA, we should probably bring the work for MESOS-6406 
into this JIRA because this JIRA wouldn't be complete without it. It certainly 
should be a different patch though.

For this patch we should also update the comments above `PARTITION_AWARE` API 
to reflect the change.
https://github.com/apache/mesos/blob/6eefc685ccf304d0fb8ed4ff9bc314197d77f078/include/mesos/mesos.proto#L336

Also please search the docs for necessary changes.


src/tests/partition_tests.cpp
Line 526 (original), 526 (patched)


s/still/are still/



src/tests/partition_tests.cpp
Line 708 (original), 708 (patched)


s/the not PARTITION_AWARE framework/the non-PARTITION_AWARE framework/



src/tests/partition_tests.cpp
Lines 728-729 (original), 728-729 (patched)


Update comments?



src/tests/partition_tests.cpp
Lines 2045 (patched)


Perhaps add a comment: `// The agent may resend status updates.`


- Jiang Yan Xu


On Sept. 24, 2017, 3:46 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61473/
> ---
> 
> (Updated Sept. 24, 2017, 3:46 p.m.)
> 
> 
> Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7215
> https://issues.apache.org/jira/browse/MESOS-7215
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master will not kill the tasks for non-Partition aware frameworks
> when an unreachable agent re-registers with the master.
> Master used to send a ShutdownFrameworkMessages to the agent
> to kill the tasks from non partition aware frameworks including the
> ones that are still registered which was problematic because the offer
> from this agent could still go to the same framework which could then
> launch new tasks. The agent would then receive tasks of the same
> framework and ignore them because it thinks the framework is shutting
> down. The framework is not shutting down of course, so from the master
> and the scheduler’s perspective the task is pending in STAGING forever
> until the next agent reregistration, which could happen much later.
> This commit fixes the problem by not shutting down the non-partition
> aware frameworks on such an agent.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 28d0393fb5962df4d731521265efd81a54e1e655 
>   src/master/master.hpp 05f88111afb4fa0e2baf57106e1479914c16a113 
>   src/master/master.cpp 6d84a26bff970b842b58dfb69dbf232ba5c16a20 
>   src/tests/partition_tests.cpp 0886f4890ac3fec6f38146946892769a99c3e68f 
> 
> 
> Diff: https://reviews.apache.org/r/61473/diff/7/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>