Re: Review Request 58361: Updated LICENSE information for protobuf 3.2.0.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [58361, 58360, 58359, 58515, 58514, 58513, 58358, 58357]

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

Error:
2017-04-19 05:14:27 URL:https://reviews.apache.org/r/58515/diff/raw/ 
[1521/1521] -> "58515.patch" [1]
error: patch failed: 3rdparty/CMakeLists.txt:316
error: 3rdparty/CMakeLists.txt: patch does not apply

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

- Mesos Reviewbot


On April 18, 2017, 10:48 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58361/
> ---
> 
> (Updated April 18, 2017, 10:48 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Joseph Wu.
> 
> 
> Bugs: MESOS-7228
> https://issues.apache.org/jira/browse/MESOS-7228
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Content copied from https://github.com/google/protobuf/blob/v3.2.0/LICENSE.
> 
> 
> Diffs
> -
> 
>   LICENSE 512b089c5dd2540f9f83afb559e5d0998d97ea22 
> 
> 
> Diff: https://reviews.apache.org/r/58361/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 58515: Update 3rdparty build to support protobuf 3.2.0.

[Anand Mazumdar]

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


Ship it!




Ship It!

- Anand Mazumdar


On April 18, 2017, 6:24 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58515/
> ---
> 
> (Updated April 18, 2017, 6:24 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Joseph Wu.
> 
> 
> Bugs: MESOS-7228
> https://issues.apache.org/jira/browse/MESOS-7228
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update 3rdparty build to support protobuf 3.2.0.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt bb61ef0514fb164f35b34bb6be1bbebb4d1a1861 
>   3rdparty/cmake/Versions.cmake 912726351ff744dd839b8d1c8d64dcc373d879be 
>   3rdparty/versions.am b8144702adca9dd41199f69cb6fb0c0c4b490dbd 
> 
> 
> Diff: https://reviews.apache.org/r/58515/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 58514: Update related tests in stout to support protobuf 3.2.0.

[Anand Mazumdar]

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


Ship it!




Ship It!

- Anand Mazumdar


On April 18, 2017, 6:23 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58514/
> ---
> 
> (Updated April 18, 2017, 6:23 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Joseph Wu.
> 
> 
> Bugs: MESOS-7228
> https://issues.apache.org/jira/browse/MESOS-7228
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Update related tests in stout to support protobuf 3.2.0.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/README.md f4e41aaacee4f88c708413e954553011229c75e4 
>   3rdparty/stout/tests/protobuf_tests.pb.h 
> 18870cde806c7bcb0a204de2a40555739adf777b 
>   3rdparty/stout/tests/protobuf_tests.pb.cc 
> b2458f0edcf679b2390f87ec8c4bee2b16e71d70 
> 
> 
> Diff: https://reviews.apache.org/r/58514/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 58513: Remove unnecessary patch after protobuf upgrade to 3.2.0.

[Anand Mazumdar]

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


Ship it!




Ship It!

- Anand Mazumdar


On April 18, 2017, 10:48 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58513/
> ---
> 
> (Updated April 18, 2017, 10:48 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Joseph Wu.
> 
> 
> Bugs: MESOS-7228
> https://issues.apache.org/jira/browse/MESOS-7228
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The patch should already be included in protobuf 3.0.0, so
> this is not necessary anymore.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt bb61ef0514fb164f35b34bb6be1bbebb4d1a1861 
>   3rdparty/Makefile.am 61d832b2d83cdeaf95341c062e8493ab72d0724e 
>   3rdparty/protobuf-2.6.1.patch bfd3bb2c600383ef05bdb4ea9df0188b4a560315 
> 
> 
> Diff: https://reviews.apache.org/r/58513/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 58358: Update vendored protobuf tar.gz to 3.2.0.

[Anand Mazumdar]

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


Ship it!




Ship It!

- Anand Mazumdar


On April 18, 2017, 6:22 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58358/
> ---
> 
> (Updated April 18, 2017, 6:22 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Joseph Wu.
> 
> 
> Bugs: MESOS-7228
> https://issues.apache.org/jira/browse/MESOS-7228
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The content of `3rdparty/protobuf-3.2.0.tar.gz` is generated by:
> - On a Mac OS, download and untar protobuf v3.2.0 source at
>   https://github.com/google/protobuf/archive/v3.2.0rc2.tar.gz;
> - Run `./autogen.sh`;
> - Recompress and tar by `tar -czvf`.
> 
> 
> Diffs
> -
> 
>   3rdparty/protobuf-2.6.1.tar.gz e1c2d541c1eb806e837ff5d1e9d3fdb678b40e06 
>   3rdparty/protobuf-3.2.0.tar.gz PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/58358/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 58305: Add some parameter validation to ReRegisterSlaveMessage.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58303, 58304, 58305]

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 April 18, 2017, 10:22 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58305/
> ---
> 
> (Updated April 18, 2017, 10:22 p.m.)
> 
> 
> Review request for mesos, Mesos Reviewbot and Neil Conway.
> 
> 
> Bugs: MESOS-7372.
> https://issues.apache.org/jira/browse/MESOS-7372.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The ReRegisterSlaveMessage message sends a number of fields which have
> internal consistency requirements. Add some simple validation checks
> to ensure that we have a minimally consistent re-registration request
> before proceeding.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 0f4c64c6b102ef201779a331c96b5d78a98281ad 
>   src/master/validation.hpp d96287de73ddb30ae2ed841c1b910b0ac6cfa74e 
>   src/master/validation.cpp 3f70875484bbd856ac79a7d6070ac313d69782fa 
> 
> 
> Diff: https://reviews.apache.org/r/58305/diff/5/
> 
> 
> Testing
> ---
> 
> Make check (Fedora 25). Internal fuzzing.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 58512: Avoid a crash in HTTP decoder.

[Anand Mazumdar]

> On April 19, 2017, 12:12 a.m., James Peach wrote:
> > AFAICT the same problem can occur in other places (eg. 
> > ``StreamingResponseDecoderon_headers_complete``).

James, the patch seems to address both the streaming request/response decoders.


- Anand


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


On April 18, 2017, 6:04 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58512/
> ---
> 
> (Updated April 18, 2017, 6:04 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, James Peach, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7400
> https://issues.apache.org/jira/browse/MESOS-7400
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If the callback `on_headers_complete()` fails, the decoder's writer
> object is `None()`. So, when the callback `on_message_complete()` is
> called, we should not crash in that case.
> Note that the `CHECK(decoder->writer)` is valid for the callback
> `on_body()` is valid since this callback is called only on a success
> in `on_headers_complete()` callback.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/decoder.hpp 
> 4c779d42548958e610142438a57529ccb4478053 
> 
> 
> Diff: https://reviews.apache.org/r/58512/diff/1/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 58512: Avoid a crash in HTTP decoder.

[Anand Mazumdar]

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



Looks good! Thanks for reporting the issue and also fixing it.

Can you add a test to go along with the fix? Otherwise, under the "Testing 
Done" section, the tests are _always_ expected to pass with/without the fix.


3rdparty/libprocess/src/decoder.hpp
Lines 699-701 (patched)


Kill this comment. 

In the decoder/encoder code using the http parser, it's fairly 
self-explanatory that the next callback is only invoked if the previous one 
succeeded. We do however, want to add a comment in the `on_message_complete()` 
handler though when this is not true.



3rdparty/libprocess/src/decoder.hpp
Line 711 (original), 714-716 (patched)


```cpp
// Note that `decoder->writer` can be `None()`.
```
hmm, this is evident from the conditional check later. 

Let's add a comment around when this can happen. How about:

```cpp
// This can happen if the callback `on_headers_complete()` had failed
// earlier (e.g., due to invalid query parameters).
```



3rdparty/libprocess/src/decoder.hpp
Lines 987-989 (patched)


Ditto as above.



3rdparty/libprocess/src/decoder.hpp
Line 1006 (original), 1017-1019 (patched)


Ditto as above.


- Anand Mazumdar


On April 18, 2017, 6:04 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58512/
> ---
> 
> (Updated April 18, 2017, 6:04 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, James Peach, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7400
> https://issues.apache.org/jira/browse/MESOS-7400
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If the callback `on_headers_complete()` fails, the decoder's writer
> object is `None()`. So, when the callback `on_message_complete()` is
> called, we should not crash in that case.
> Note that the `CHECK(decoder->writer)` is valid for the callback
> `on_body()` is valid since this callback is called only on a success
> in `on_headers_complete()` callback.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/decoder.hpp 
> 4c779d42548958e610142438a57529ccb4478053 
> 
> 
> Diff: https://reviews.apache.org/r/58512/diff/1/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 58099: Added authorization for frameworks in `GetRoles` v1 API.

[Jay Guo]

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

(Updated April 19, 2017, 11:38 a.m.)


Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler.


Changes
---

rebase


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


Repository: mesos


Description
---

Added authorization for frameworks in `GetRoles` v1 API.


Diffs (updated)
-

  src/master/http.cpp 5aae52870451d883ef1ea1fda5a27764d7f318e8 


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

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


Testing
---


Thanks,

Jay Guo

Re: Review Request 58097: Added a test to check framework filtering in /roles endpoint.

[Jay Guo]

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

(Updated April 19, 2017, 11:38 a.m.)


Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler.


Changes
---

rebase


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


Repository: mesos


Description
---

Added a test to check framework filtering in /roles endpoint.


Diffs (updated)
-

  src/tests/master_authorization_tests.cpp 
1a0285a3f345ef21a8256d4123d8bb684f538da4 


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

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


Testing
---

make check


Thanks,

Jay Guo

Re: Review Request 58096: Added authorization for frameworks in /roles endpoint.

[Jay Guo]

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

(Updated April 19, 2017, 11:37 a.m.)


Review request for mesos, Adam B, Alexander Rojas, and Benjamin Mahler.


Changes
---

rebase


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


Repository: mesos


Description
---

While /roles displays a list of frameworksIds that register with
a role, it did NOT filter them based on VIEW_FRAMEWORK ACL, which
impose a security risk. This patch fixed this issue by taking a
frameworksApprover in `Master::Http::roles()` which is used to
filter framework IDs.


Diffs (updated)
-

  src/master/http.cpp 5aae52870451d883ef1ea1fda5a27764d7f318e8 


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

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


Testing
---

see next patch in the chain.


Thanks,

Jay Guo

Re: Review Request 58095: Refactored functions that render /roles and GetRoles endpoints.

[Jay Guo]

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

(Updated April 19, 2017, 11:36 a.m.)


Review request for mesos, Adam B, Alexander Rojas, Benjamin Mahler, and Michael 
Park.


Changes
---

rebase


Bugs: MESOS-4732 and MESOS-7260
https://issues.apache.org/jira/browse/MESOS-4732
https://issues.apache.org/jira/browse/MESOS-7260


Repository: mesos


Description
---

Instead of generating JSON object, `Master::Http::roles()` now
leverages `jsonify` to compute output. Also approver is taken
out from its continuation function. This is for easier and cleaner
implementation of framework authorization in /roles endpoint,
see MESOS-7260.


Diffs (updated)
-

  src/master/http.cpp 5aae52870451d883ef1ea1fda5a27764d7f318e8 
  src/master/master.hpp d537933d0b467a6f9996951c601b31338bb9d034 


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

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


Testing
---

no functional changes
make check


Thanks,

Jay Guo

Re: Review Request 58487: Fix allocation quantities when shared resources are removed.

[Anindya Sinha]

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

(Updated April 19, 2017, 2:50 a.m.)


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


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


Repository: mesos


Description
---

When shared resources are removed from the `allocations` in the sorter,
we remove the scalar quantity only when the shared resource is actually
removed (i.e. shared count goes down to 0). Similarly, we increase the
scalar quantity only when this is a new shared resource that is being
added to the `allocations`.


Diffs (updated)
-

  src/master/allocator/sorter/drf/sorter.cpp 
ed54680cecb637931fc344fbcf8fd3b14cc24295 


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

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


Testing
---

Tests passed.


Thanks,

Anindya Sinha

Re: Review Request 58485: Avoid a corruption while rescinding offers.

[Anindya Sinha]

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

(Updated April 19, 2017, 2:49 a.m.)


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


Changes
---

Addressed review comments.


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


Repository: mesos


Description
---

When a `DESTROY` is processed, we rescind all pending offers to which
persistent volumes have been offered. As soon as we rescind an offer,
the `Offer` object is freed; and hence, we should move on to the next
offer (even though there are multiple volumes being destroyed in
the same `ACCEPT` call).


Diffs (updated)
-

  src/master/master.cpp 52de2f91bdacf46f913c27382ad50b4f278ad297 
  src/tests/persistent_volume_tests.cpp 
3eb7afe3de8e72ffb6502dfe12ef37ccd4ca2125 


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

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


Testing
---

Tests passed.


Thanks,

Anindya Sinha

Re: Review Request 58486: Fixed a race in `updateAllocation()` on DESTORY of a shared volume.

[Anindya Sinha]

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

(Updated April 19, 2017, 2:50 a.m.)


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


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


Repository: mesos


Description
---

In `updateAllocation()`, it is possible for the flattened scalar
quantites of original framework allocation to differ from the updated
framework allocation. This can happen when an offer with a shared
persistent volume is sent out after a DESTROY has been received. If
that is the case, we rescind the pending offers from all frameworks
which contain the volumes to be destroyed.


Diffs (updated)
-

  include/mesos/allocator/allocator.hpp 
6eda1b8619269c1501a935045b18b1deaf845b33 
  src/master/allocator/mesos/allocator.hpp 
57b54b86c43c7731e64d422d285c4b8ca7e27a60 
  src/master/allocator/mesos/hierarchical.hpp 
f84b0574ce9a392c9528c87b04b01dbb2053cff7 
  src/master/allocator/mesos/hierarchical.cpp 
051f749dd5921a322ca930a042c31814616d38f9 
  src/master/master.hpp d537933d0b467a6f9996951c601b31338bb9d034 
  src/master/master.cpp 52de2f91bdacf46f913c27382ad50b4f278ad297 
  src/tests/allocator.hpp 6b71c574e0e4facd1795ef50ee0869c03b87833d 
  src/tests/hierarchical_allocator_tests.cpp 
33e7b455f8664858eb4f03727b076a10c80cd6e0 


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

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


Testing
---

Tests passed.


Thanks,

Anindya Sinha

Re: Review Request 58224: Optionally verify the source IP address for libprocess messages.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58517, 58224]

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 April 18, 2017, 9:11 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58224/
> ---
> 
> (Updated April 18, 2017, 9:11 p.m.)
> 
> 
> Review request for mesos and Mesos Reviewbot.
> 
> 
> Bugs: MESOS-7401
> https://issues.apache.org/jira/browse/MESOS-7401
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In general, libprocess is unable to validate that a peer
> is a legitimate owner of the UPID it claims in a libprocess
> message. This change adds a check that the IP address in the
> UPID matches the peer address. This makes spoofing the UPID
> harder (eg. to send authenticated messages), but also breaks
> some legitimate configurations, particularly on multihomed
> hosts.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 92efa915414c2a38b18de99858c66b63e757f63c 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> bf90c7e78fd50ad7e16cc0a69a248ba71e2a7115 
>   3rdparty/libprocess/src/tests/test_linkee.cpp 
> 921d67695bc0e4d601e9f74fbc625d69bf36ba50 
> 
> 
> Diff: https://reviews.apache.org/r/58224/diff/2/
> 
> 
> Testing
> ---
> 
> make check (Fedora 25). Light manual testing.
> 
> With LIBPROCESS_pin_peer_address=true, all Mesos tests pass except 
> ``ExamplesTest.DiskFullFramework``, however enabling this will definitely 
> break some libprocess APIs (though not in the way that Mesos uses them) and 
> legitimate multi-homed configurations. Note that setting 
> LIBPROCESS_ip=127.0.0.1 makes you multihomed for this purpose, which is why 
> ``ExamplesTest.DiskFullFramework`` breaks.
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 56895: Allow agents to recover slave state post a reboot.

[Megha Sharma]

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

(Updated April 19, 2017, 1:10 a.m.)


Review request for mesos, Neil Conway and Jiang Yan Xu.


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


Repository: mesos


Description
---

With partition awareness, the agents are now allowed to re-register
after they have been marked Unreachable. The executors are anyway
terminated on the agent when it reboots so there is no harm in
letting the agent keep its SlaveID, re-register with the master
and reconcile the lost executors. This is a pre-requisite for
supporting persistent/restartable tasks in mesos.


Diffs (updated)
-

  src/slave/slave.hpp 77fb93abc701cd34b69c75b6219c219fdb784a67 
  src/slave/slave.cpp 4ff522e75bc8de34fe2e7720bdd8ce3d32cbf803 
  src/slave/state.hpp a497ce1f58fb8dc7718ee5bb10bc62dd7479efa5 
  src/slave/state.cpp 33dcc7a148f9a6b1a3216cce45710da8fd819ba6 
  src/tests/reservation_tests.cpp 4504831d77c1bfcf5f2ddf6d28cd45dea2c421ad 
  src/tests/slave_recovery_tests.cpp 53f33a2b0411c8158326074ce043c7b1dbeef5b4 


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

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


Testing
---

make check


Thanks,

Megha Sharma

Re: Review Request 58428: Added tests for failed executor authorization.

[Greg Mann]

> On April 19, 2017, 12:33 a.m., Vinod Kone wrote:
> > While these tests are good I'm wondering if they are realistic, because the 
> > assumption is that someone knows the agent's secret key but doesn't know 
> > the container id of the executor they want to attack. In reality it's the 
> > opposite; they know the container id of the executor they want to attack 
> > but not the agent's key. Just a thought.

True, it's a contrived scenario, but it's the only way for us to test the 
authorization logic. A token generated with an incorrect key will fail the 
authentication step. We don't have any end-to-end authentication tests for HTTP 
executors, so we could add those instead of/in addition to these tests.


- Greg


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


On April 14, 2017, 9:18 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58428/
> ---
> 
> (Updated April 14, 2017, 9:18 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-7339
> https://issues.apache.org/jira/browse/MESOS-7339
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds new tests to verify that HTTP executors cannot
> subscribe or launch nested containers when HTTP executor
> authentication is enabled, authorization is enabled, and they
> do not provide a valid executor authentication token
> 
> 
> Diffs
> -
> 
>   src/tests/slave_authorization_tests.cpp 
> 3657e0a3d19d75cef92e5bf90b65ef00c291b032 
> 
> 
> Diff: https://reviews.apache.org/r/58428/diff/3/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 58255: Added implicit authorization to the agent executor API.

[Greg Mann]

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

(Updated April 19, 2017, 12:34 a.m.)


Review request for mesos, Adam B, Alexander Rojas, and Till Toenshoff.


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


Repository: mesos


Description
---

This patch updates the agent handler for the executor API to
verify the FrameworkID and ExecutorID contained within the
executor's `Principal`, if present. This effectively performs
implicit authorization of executor calls.


Diffs (updated)
-

  src/slave/http.cpp 468cf332d79ed7315ecf51955235735dec0a2df1 


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

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


Testing
---

Testing details can be found at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 58458: Enabled authorization in default executor check tests.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On April 14, 2017, 4:08 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58458/
> ---
> 
> (Updated April 14, 2017, 4:08 p.m.)
> 
> 
> Review request for mesos, Adam B, Anand Mazumdar, Alexander Rojas, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-7339
> https://issues.apache.org/jira/browse/MESOS-7339
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch enables authorization in the check and health check
> tests which use the default executor. Simple permissive ACLs are
> set, forcing the local authorizer to be loaded which allows us
> to test the implicit executor authorization code for agent
> operator API calls.
> 
> 
> Diffs
> -
> 
>   src/tests/check_tests.cpp d7fcbf940102efe0841f07e8c4937a78361bc422 
>   src/tests/health_check_tests.cpp c5857b2415faaec4d0557e50cbeb42379f4550ac 
> 
> 
> Diff: https://reviews.apache.org/r/58458/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 58428: Added tests for failed executor authorization.

[Vinod Kone]

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


Fix it, then Ship it!




While these tests are good I'm wondering if they are realistic, because the 
assumption is that someone knows the agent's secret key but doesn't know the 
container id of the executor they want to attack. In reality it's the opposite; 
they know the container id of the executor they want to attack but not the 
agent's key. Just a thought.


src/tests/slave_authorization_tests.cpp
Lines 28-30 (original), 30-34 (patched)


order alphabetically.



src/tests/slave_authorization_tests.cpp
Line 32 (original), 36-37 (patched)


order alphabetically.



src/tests/slave_authorization_tests.cpp
Lines 739 (patched)


s/badRequest/error/



src/tests/slave_authorization_tests.cpp
Lines 1070 (patched)


don't need terminate/wait slave here like the above test?


- Vinod Kone


On April 14, 2017, 9:18 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58428/
> ---
> 
> (Updated April 14, 2017, 9:18 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-7339
> https://issues.apache.org/jira/browse/MESOS-7339
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds new tests to verify that HTTP executors cannot
> subscribe or launch nested containers when HTTP executor
> authentication is enabled, authorization is enabled, and they
> do not provide a valid executor authentication token
> 
> 
> Diffs
> -
> 
>   src/tests/slave_authorization_tests.cpp 
> 3657e0a3d19d75cef92e5bf90b65ef00c291b032 
> 
> 
> Diff: https://reviews.apache.org/r/58428/diff/3/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 58258: Added a new agent authorization test which runs a task group.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On April 13, 2017, 8:19 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58258/
> ---
> 
> (Updated April 13, 2017, 8:19 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Till Toenshoff, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-7339
> https://issues.apache.org/jira/browse/MESOS-7339
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a new test,
> `SlaveAuthorizerTest.AuthorizeRunTaskGroup`, which
> verifies that task groups can be launched when
> executor authentication is required and the local
> authorizer is loaded.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_authorization_tests.cpp 
> 3657e0a3d19d75cef92e5bf90b65ef00c291b032 
> 
> 
> Diff: https://reviews.apache.org/r/58258/diff/6/
> 
> 
> Testing
> ---
> 
> `make check` when configured with SSL enabled.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 58512: Avoid a crash in HTTP decoder.

[James Peach]

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



AFAICT the same problem can occur in other places (eg. 
``StreamingResponseDecoderon_headers_complete``).

- James Peach


On April 18, 2017, 6:04 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58512/
> ---
> 
> (Updated April 18, 2017, 6:04 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, James Peach, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7400
> https://issues.apache.org/jira/browse/MESOS-7400
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If the callback `on_headers_complete()` fails, the decoder's writer
> object is `None()`. So, when the callback `on_message_complete()` is
> called, we should not crash in that case.
> Note that the `CHECK(decoder->writer)` is valid for the callback
> `on_body()` is valid since this callback is called only on a success
> in `on_headers_complete()` callback.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/decoder.hpp 
> 4c779d42548958e610142438a57529ccb4478053 
> 
> 
> Diff: https://reviews.apache.org/r/58512/diff/1/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 58255: Added implicit authorization to the agent executor API.

[Vinod Kone]

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




src/slave/http.cpp
Lines 629-650 (patched)


Can you make the response status codes consistent or add a comment on why 
they are inconsistent?


- Vinod Kone


On April 18, 2017, 7:58 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58255/
> ---
> 
> (Updated April 18, 2017, 7:58 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Till Toenshoff.
> 
> 
> Bugs: MESOS-7339
> https://issues.apache.org/jira/browse/MESOS-7339
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the agent handler for the executor API to
> verify the FrameworkID and ExecutorID contained within the
> executor's `Principal`, if present. This effectively performs
> implicit authorization of executor calls.
> 
> 
> Diffs
> -
> 
>   src/slave/http.cpp 468cf332d79ed7315ecf51955235735dec0a2df1 
> 
> 
> Diff: https://reviews.apache.org/r/58255/diff/6/
> 
> 
> Testing
> ---
> 
> Testing details can be found at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 58355: Removed unnecessary Registry copying.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58355]

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 April 18, 2017, 9:11 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58355/
> ---
> 
> (Updated April 18, 2017, 9:11 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7376
> https://issues.apache.org/jira/browse/MESOS-7376
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When the number of agents is large every `Registry` copy operation takes
> a lot of time (~0.4 sec with 55k agents), because it involves deep
> copying a big object tree. Because of that, the use of `protobuf::State`
> in `Registrar` incurs a dramatic performance cost from multiple protobuf
> copying.
> 
> This patch drops the use of `protobuf::State` in `Registrar` in favor of
> "untyped" `State` and manual serialization/deserialization in order to
> minimize `Registry` copying and keep registry update timings at
> acceptable values.
> 
> Performance improvements to `protobuf::State` should be explored in
> order to make it usable in the registrar without regressing on the
> performance of this approach.
> 
> 
> Diffs
> -
> 
>   src/master/main.cpp 90d159e248924f6806271dda1ced6d37dfa5f5c9 
>   src/master/registrar.hpp a70132b0901aedf2c562a561a9d140b58fdfb4a0 
>   src/master/registrar.cpp 0029cc77628b5bb2a7b1ff551fb42b3eaf7b4fb1 
>   src/tests/cluster.hpp 250b12fcffd035834817ff6060eb80c2cf3e0246 
>   src/tests/cluster.cpp 02590a27aadd0583177f21a57ec3d05fe1542f42 
>   src/tests/mock_registrar.hpp cdcc6990a183e8c4911d3353584ed808ee88dd72 
>   src/tests/mock_registrar.cpp 8643e4cb06cef44fa0e342df0aa0f00823e1e268 
>   src/tests/registrar_tests.cpp 5c6fb565ee48378224bab091ccdf9d5d8f3c7614 
> 
> 
> Diff: https://reviews.apache.org/r/58355/diff/2/
> 
> 
> Testing
> ---
> 
> Benchmark
> =
> Ran `MESOS_VERBOSE=1 ./src/mesos-tests --benchmark 
> --gtest_filter='*Registrar_BENCHMARK_Test.Performance/3'`.
> 
> Before:
> ```
> I0411 10:04:11.726016 11802 registrar.cpp:508] Successfully updated the 
> registry in 89.478144ms
> I0411 10:04:13.860827 11803 registrar.cpp:508] Successfully updated the 
> registry in 216.688896ms
> I0411 10:04:15.167768 11803 registrar.cpp:508] Successfully updated the 
> registry in 1.29364096secs
> I0411 10:04:18.967394 11803 registrar.cpp:508] Successfully updated the 
> registry in 3.696552192secs
> I0411 10:04:25.631009 11803 registrar.cpp:508] Successfully updated the 
> registry in 6.267425024secs
> I0411 10:04:42.625507 11803 registrar.cpp:508] Successfully updated the 
> registry in 15.876419072secs
> I0411 10:04:44.209377 11787 registrar_tests.cpp:1262] Admitted 5 agents 
> in 30.479743816secs
> I0411 10:05:04.446650 11820 registrar.cpp:508] Successfully updated the 
> registry in 18.338545152secs
> I0411 10:05:21.171001 11820 registrar.cpp:508] Successfully updated the 
> registry in 15.31903872secs
> I0411 10:05:37.592319 11820 registrar.cpp:508] Successfully updated the 
> registry in 14.863101952secs
> I0411 10:05:39.099174 11787 registrar_tests.cpp:1276] Marked 5 agents 
> reachable in 53.593596102secs
> ../../src/tests/registrar_tests.cpp:1287: Failure
> Failed to wait 15secs for registry
> ```
> After:
> ```
> I0411 15:19:12.228904 40643 registrar.cpp:524] Successfully updated the 
> registry in 91.262208ms
> I0411 15:19:14.543190 40660 registrar.cpp:524] Successfully updated the 
> registry in 377.45408ms
> I0411 15:19:15.707006 40660 registrar.cpp:524] Successfully updated the 
> registry in 1.138724096secs
> I0411 15:19:18.267305 40660 registrar.cpp:524] Successfully updated the 
> registry in 2.466145792secs
> I0411 15:19:19.092073 40660 registrar.cpp:524] Successfully updated the 
> registry in 523.11296ms
> I0411 15:19:20.809330 40648 registrar.cpp:524] Successfully updated the 
> registry in 892.141824ms
> I0411 15:19:21.194135 40622 registrar_tests.cpp:1262] Admitted 5 agents 
> in 6.938952085secs
> I0411 15:19:26.973904 40637 registrar.cpp:524] Successfully updated the 
> registry in 3.938064128secs
> I0411 15:19:28.631865 40637 registrar.cpp:524] Successfully updated the 
> registry in 1.116326144secs
> I0411 15:19:30.222944 40660 registrar.cpp:524] Successfully updated the 
> registry in 911.86688ms
> I0411 15:19:30.678509 40622 registrar_tests.cpp:1276] Marked 5 agents 
> reachable in 8.249523305secs
> I0411 15:19:35.138797 40645 registrar.cpp:524] Successfully 

Re: Review Request 58361: Updated LICENSE information for protobuf 3.2.0.

[Zhitao Li]

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

(Updated April 18, 2017, 10:48 p.m.)


Review request for mesos, Anand Mazumdar and Joseph Wu.


Changes
---

Rerun reviewbot.


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


Repository: mesos


Description
---

Content copied from https://github.com/google/protobuf/blob/v3.2.0/LICENSE.


Diffs (updated)
-

  LICENSE 512b089c5dd2540f9f83afb559e5d0998d97ea22 


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

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 58513: Remove unnecessary patch after protobuf upgrade to 3.2.0.

[Zhitao Li]

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

(Updated April 18, 2017, 10:48 p.m.)


Review request for mesos, Anand Mazumdar and Joseph Wu.


Changes
---

Remove patch commands in make file.


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


Repository: mesos


Description
---

The patch should already be included in protobuf 3.0.0, so
this is not necessary anymore.


Diffs (updated)
-

  3rdparty/CMakeLists.txt bb61ef0514fb164f35b34bb6be1bbebb4d1a1861 
  3rdparty/Makefile.am 61d832b2d83cdeaf95341c062e8493ab72d0724e 
  3rdparty/protobuf-2.6.1.patch bfd3bb2c600383ef05bdb4ea9df0188b4a560315 


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

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 58254: Added implicit executor authorization to the agent operator API.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On April 14, 2017, 9:14 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58254/
> ---
> 
> (Updated April 14, 2017, 9:14 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, Till Toenshoff, and Vinod 
> Kone.
> 
> 
> Bugs: MESOS-7014
> https://issues.apache.org/jira/browse/MESOS-7014
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch updates the agent handlers for the LAUNCH_, WAIT_,
> and KILL_NESTED_CONTAINER calls of the operator API to set the
> `container_id` field within the authorization object,
> facilitating implicit executor authorization.
> 
> 
> Diffs
> -
> 
>   include/mesos/authorizer/authorizer.proto 
> 736f76d552956f2351ffd40fc51d088dff83f8c8 
>   src/authorizer/local/authorizer.cpp 
> 1c1f912794cfe61112a0e513b217ba3a755f35f1 
>   src/slave/http.cpp 468cf332d79ed7315ecf51955235735dec0a2df1 
> 
> 
> Diff: https://reviews.apache.org/r/58254/diff/6/
> 
> 
> Testing
> ---
> 
> Testing details can be found at the end of this chain.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 58507: Added a TODO for implicit scheduler authorization.

[Vinod Kone]

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


Ship it!




Ship It!

- Vinod Kone


On April 18, 2017, 3:19 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58507/
> ---
> 
> (Updated April 18, 2017, 3:19 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Vinod Kone.
> 
> 
> Bugs: MESOS-7399
> https://issues.apache.org/jira/browse/MESOS-7399
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a TODO to the master's HTTP scheduler API related
> to MESOS-7399.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 0e84d363b346de1ef05101dac066c2f0cf31609d 
> 
> 
> Diff: https://reviews.apache.org/r/58507/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 58305: Add some parameter validation to ReRegisterSlaveMessage.

[James Peach]

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

(Updated April 18, 2017, 10:22 p.m.)


Review request for mesos, Mesos Reviewbot and Neil Conway.


Changes
---

Rebases and addresse review feedback.


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


Repository: mesos


Description
---

The ReRegisterSlaveMessage message sends a number of fields which have
internal consistency requirements. Add some simple validation checks
to ensure that we have a minimally consistent re-registration request
before proceeding.


Diffs (updated)
-

  src/master/master.cpp 0f4c64c6b102ef201779a331c96b5d78a98281ad 
  src/master/validation.hpp d96287de73ddb30ae2ed841c1b910b0ac6cfa74e 
  src/master/validation.cpp 3f70875484bbd856ac79a7d6070ac313d69782fa 


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

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


Testing
---

Make check (Fedora 25). Internal fuzzing.


Thanks,

James Peach

Re: Review Request 58304: Remove unnecessary hashmap lookups.

[James Peach]

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

(Updated April 18, 2017, 10:15 p.m.)


Review request for mesos, Adam B, Mesos Reviewbot, and Neil Conway.


Changes
---

Rebased.


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


Repository: mesos


Description
---

In a number of places, we tested whether the slaves hashmap contains
the desired element before indexing it. It is safe to just index it and
check for a NULL result, which saves us some hashing and lookups.


Diffs (updated)
-

  src/master/master.cpp 0f4c64c6b102ef201779a331c96b5d78a98281ad 


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

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


Testing
---

Make check (Fedora 25). Internal fuzzing.


Thanks,

James Peach

Re: Review Request 58303: Pass the slave pointer directly to Master::removeTask.

[James Peach]

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

(Updated April 18, 2017, 10:14 p.m.)


Review request for mesos, Adam B, Mesos Reviewbot, and Neil Conway.


Changes
---

Rebased and addressed review feedback.


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


Repository: mesos


Description
---

In all cases where we call Master::removeTask, we have the correct slave
pointer in hand. We can just pass it down, avoiding the need to look it
up again with the SlaveID.


Diffs (updated)
-

  src/master/master.hpp d537933d0b467a6f9996951c601b31338bb9d034 
  src/master/master.cpp 0f4c64c6b102ef201779a331c96b5d78a98281ad 


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

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


Testing
---

Make check (Fedora 25). Internal fuzzing.


Thanks,

James Peach

Re: Review Request 58485: Avoid a corruption while rescinding offers.

[Jiang Yan Xu]

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


Fix it, then Ship it!





src/master/master.cpp
Lines 4334-4335 (patched)


s/can/should/ ?

Also move a little bit of the commit description here to justify this 
comment. Something like "This offer may contain other volumes to be destroyed 
but since we have already rescinded it, we should move on to the next offer."?



src/tests/persistent_volume_tests.cpp
Line 1084 (original), 1096 (patched)


s/volume/volumes/?



src/tests/persistent_volume_tests.cpp
Line 1154 (original), 1170 (patched)


s/offers/offer/?


- Jiang Yan Xu


On April 17, 2017, 4:14 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58485/
> ---
> 
> (Updated April 17, 2017, 4:14 p.m.)
> 
> 
> Review request for mesos, James Peach and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7308
> https://issues.apache.org/jira/browse/MESOS-7308
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When a `DESTROY` is processed, we rescind all pending offers to which
> persistent volumes have been offered. As soon as we rescind an offer,
> the `Offer` object is freed; and hence, we should move on to the next
> offer (even though there are multiple volumes being destroyed in
> the same `ACCEPT` call).
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 0f4c64c6b102ef201779a331c96b5d78a98281ad 
>   src/tests/persistent_volume_tests.cpp 
> 3eb7afe3de8e72ffb6502dfe12ef37ccd4ca2125 
> 
> 
> Diff: https://reviews.apache.org/r/58485/diff/1/
> 
> 
> Testing
> ---
> 
> Tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 58355: Removed unnecessary Registry copying.

[Ilya Pronin]

> On April 18, 2017, 12:21 a.m., Benjamin Mahler wrote:
> > Thanks for taking this on Ilya! Just a few suggestions below per our 
> > offline discussion. It would be great to update the description to include 
> > the context, mainly that we avoid the `protobuf::State` wrapper in favor of 
> > performing manual serialization/deserialization in the registrar, in order 
> > to avoid expensive protobuf copying. Also that, we should explore 
> > performance improvements to `protobuf::State` in order to make it usable in 
> > the registrar without regressing on the performance of this approach.

Thanks for the review! Done.


> On April 18, 2017, 12:21 a.m., Benjamin Mahler wrote:
> > src/master/registrar.cpp
> > Line 215 (original), 217-218 (patched)
> > 
> >
> > It would be nice to store the `State` class alongside these and put the 
> > TODO here:
> > 
> > ```
> >   // TODO(): We use the "untyped" `State` class here and 
> > perform
> >   // the protobuf (de)serialization manually within the Registrar, 
> > because
> >   // the use of `protobuf::State` incurs a dramatic peformance cost from
> >   // protobuf copying. We should explore using `protobuf::State`, which 
> > will
> >   // require move support and other copy elimination to maintain the
> >   // performance of the current approach.
> >   State* state;
> >   
> >   // Per the TODO above, we store both serialized and deserialized 
> > versions
> >   // of the `Registry` protobuf. If we're able to move to 
> > `protobuf::State`,
> >   // we could just store a single `protobuf::state::Variable`.
> >   Option variable;
> >   Option registry;
> > ```
> > 
> > By the way, would be nice if these both use or don't use underscores 
> > consistently.

Done.

`registry` without underscore conflicted with private `registry()` method. 
Renamed the method.


> On April 18, 2017, 12:21 a.m., Benjamin Mahler wrote:
> > src/master/registrar.cpp
> > Line 334 (original), 337 (patched)
> > 
> >
> > Chatted with Ilya offline about using the raw untyped `State` class 
> > rather than using the protobuf class and bypassing the overridden method 
> > here.
> > 
> > We can then frame this patch as being an avoidance of the protobuf 
> > `State` wrapper due to the extra expense incurred from protobuf copying, 
> > and leave a TODO in the code that captures this for posterity.

Avoided `protobuf::State` in favor of "untyped" `State`.


> On April 18, 2017, 12:21 a.m., Benjamin Mahler wrote:
> > src/master/registrar.cpp
> > Lines 449-450 (original), 464-465 (patched)
> > 
> >
> > Should we say we use an Owned here to avoid copying, since protobuf 
> > doesn't support move construction?

Added a comment.


- Ilya


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


On April 18, 2017, 10:11 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58355/
> ---
> 
> (Updated April 18, 2017, 10:11 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7376
> https://issues.apache.org/jira/browse/MESOS-7376
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When the number of agents is large every `Registry` copy operation takes
> a lot of time (~0.4 sec with 55k agents), because it involves deep
> copying a big object tree. Because of that, the use of `protobuf::State`
> in `Registrar` incurs a dramatic performance cost from multiple protobuf
> copying.
> 
> This patch drops the use of `protobuf::State` in `Registrar` in favor of
> "untyped" `State` and manual serialization/deserialization in order to
> minimize `Registry` copying and keep registry update timings at
> acceptable values.
> 
> Performance improvements to `protobuf::State` should be explored in
> order to make it usable in the registrar without regressing on the
> performance of this approach.
> 
> 
> Diffs
> -
> 
>   src/master/main.cpp 90d159e248924f6806271dda1ced6d37dfa5f5c9 
>   src/master/registrar.hpp a70132b0901aedf2c562a561a9d140b58fdfb4a0 
>   src/master/registrar.cpp 0029cc77628b5bb2a7b1ff551fb42b3eaf7b4fb1 
>   src/tests/cluster.hpp 250b12fcffd035834817ff6060eb80c2cf3e0246 
>   src/tests/cluster.cpp 02590a27aadd0583177f21a57ec3d05fe1542f42 
>   src/tests/mock_registrar.hpp cdcc6990a183e8c4911d3353584ed808ee88dd72 
>   src/tests/mock_registrar.cpp 8643e4cb06cef44fa0e342df0aa0f00823e1e268 
>   

Re: Review Request 58355: Removed unnecessary Registry copying.

[Ilya Pronin]

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

(Updated April 18, 2017, 10:11 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Addressed review comments


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


Repository: mesos


Description (updated)
---

When the number of agents is large every `Registry` copy operation takes
a lot of time (~0.4 sec with 55k agents), because it involves deep
copying a big object tree. Because of that, the use of `protobuf::State`
in `Registrar` incurs a dramatic performance cost from multiple protobuf
copying.

This patch drops the use of `protobuf::State` in `Registrar` in favor of
"untyped" `State` and manual serialization/deserialization in order to
minimize `Registry` copying and keep registry update timings at
acceptable values.

Performance improvements to `protobuf::State` should be explored in
order to make it usable in the registrar without regressing on the
performance of this approach.


Diffs (updated)
-

  src/master/main.cpp 90d159e248924f6806271dda1ced6d37dfa5f5c9 
  src/master/registrar.hpp a70132b0901aedf2c562a561a9d140b58fdfb4a0 
  src/master/registrar.cpp 0029cc77628b5bb2a7b1ff551fb42b3eaf7b4fb1 
  src/tests/cluster.hpp 250b12fcffd035834817ff6060eb80c2cf3e0246 
  src/tests/cluster.cpp 02590a27aadd0583177f21a57ec3d05fe1542f42 
  src/tests/mock_registrar.hpp cdcc6990a183e8c4911d3353584ed808ee88dd72 
  src/tests/mock_registrar.cpp 8643e4cb06cef44fa0e342df0aa0f00823e1e268 
  src/tests/registrar_tests.cpp 5c6fb565ee48378224bab091ccdf9d5d8f3c7614 


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

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


Testing
---

Benchmark
=
Ran `MESOS_VERBOSE=1 ./src/mesos-tests --benchmark 
--gtest_filter='*Registrar_BENCHMARK_Test.Performance/3'`.

Before:
```
I0411 10:04:11.726016 11802 registrar.cpp:508] Successfully updated the 
registry in 89.478144ms
I0411 10:04:13.860827 11803 registrar.cpp:508] Successfully updated the 
registry in 216.688896ms
I0411 10:04:15.167768 11803 registrar.cpp:508] Successfully updated the 
registry in 1.29364096secs
I0411 10:04:18.967394 11803 registrar.cpp:508] Successfully updated the 
registry in 3.696552192secs
I0411 10:04:25.631009 11803 registrar.cpp:508] Successfully updated the 
registry in 6.267425024secs
I0411 10:04:42.625507 11803 registrar.cpp:508] Successfully updated the 
registry in 15.876419072secs
I0411 10:04:44.209377 11787 registrar_tests.cpp:1262] Admitted 5 agents in 
30.479743816secs
I0411 10:05:04.446650 11820 registrar.cpp:508] Successfully updated the 
registry in 18.338545152secs
I0411 10:05:21.171001 11820 registrar.cpp:508] Successfully updated the 
registry in 15.31903872secs
I0411 10:05:37.592319 11820 registrar.cpp:508] Successfully updated the 
registry in 14.863101952secs
I0411 10:05:39.099174 11787 registrar_tests.cpp:1276] Marked 5 agents 
reachable in 53.593596102secs
../../src/tests/registrar_tests.cpp:1287: Failure
Failed to wait 15secs for registry
```
After:
```
I0411 15:19:12.228904 40643 registrar.cpp:524] Successfully updated the 
registry in 91.262208ms
I0411 15:19:14.543190 40660 registrar.cpp:524] Successfully updated the 
registry in 377.45408ms
I0411 15:19:15.707006 40660 registrar.cpp:524] Successfully updated the 
registry in 1.138724096secs
I0411 15:19:18.267305 40660 registrar.cpp:524] Successfully updated the 
registry in 2.466145792secs
I0411 15:19:19.092073 40660 registrar.cpp:524] Successfully updated the 
registry in 523.11296ms
I0411 15:19:20.809330 40648 registrar.cpp:524] Successfully updated the 
registry in 892.141824ms
I0411 15:19:21.194135 40622 registrar_tests.cpp:1262] Admitted 5 agents in 
6.938952085secs
I0411 15:19:26.973904 40637 registrar.cpp:524] Successfully updated the 
registry in 3.938064128secs
I0411 15:19:28.631865 40637 registrar.cpp:524] Successfully updated the 
registry in 1.116326144secs
I0411 15:19:30.222944 40660 registrar.cpp:524] Successfully updated the 
registry in 911.86688ms
I0411 15:19:30.678509 40622 registrar_tests.cpp:1276] Marked 5 agents 
reachable in 8.249523305secs
I0411 15:19:35.138797 40645 registrar.cpp:524] Successfully updated the 
registry in 815.439104ms
I0411 15:19:41.783651 40622 registrar_tests.cpp:1288] Recovered 5 agents 
(8238915B) in 10.963297677secs
I0411 15:19:47.431670 40657 registrar.cpp:524] Successfully updated the 
registry in 3.960920064secs
I0411 15:20:13.769872 40657 registrar.cpp:524] Successfully updated the 
registry in 1.169234944secs
I0411 15:21:49.685801 40657 registrar.cpp:524] Successfully updated the 
registry in 264.850688ms
Removed 5 agents in 2.12256788111667mins
```
Agents removal seems to be O(n^2) because of linear lookup, hence 2 mins.

Unit

Ran `make check`.


Thanks,

Ilya Pronin

Re: Review Request 58517: Update SlaveRecoveryTest to send libprocess messages from a real UPID.

[James Peach]

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

(Updated April 18, 2017, 9:11 p.m.)


Review request for mesos and Mesos Reviewbot.


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


Repository: mesos


Description
---

Update SlaveRecoveryTest to send libprocess messages from a real UPID.


Diffs
-

  src/tests/slave_recovery_tests.cpp 53f33a2b0411c8158326074ce043c7b1dbeef5b4 


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


Testing
---

make check (Fedora 25)


Thanks,

James Peach

Re: Review Request 58224: Optionally verify the source IP address for libprocess messages.

[James Peach]

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

(Updated April 18, 2017, 9:11 p.m.)


Review request for mesos and Mesos Reviewbot.


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


Repository: mesos


Description
---

In general, libprocess is unable to validate that a peer
is a legitimate owner of the UPID it claims in a libprocess
message. This change adds a check that the IP address in the
UPID matches the peer address. This makes spoofing the UPID
harder (eg. to send authenticated messages), but also breaks
some legitimate configurations, particularly on multihomed
hosts.


Diffs
-

  3rdparty/libprocess/src/process.cpp 92efa915414c2a38b18de99858c66b63e757f63c 
  3rdparty/libprocess/src/tests/process_tests.cpp 
bf90c7e78fd50ad7e16cc0a69a248ba71e2a7115 
  3rdparty/libprocess/src/tests/test_linkee.cpp 
921d67695bc0e4d601e9f74fbc625d69bf36ba50 


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


Testing
---

make check (Fedora 25). Light manual testing.

With LIBPROCESS_pin_peer_address=true, all Mesos tests pass except 
``ExamplesTest.DiskFullFramework``, however enabling this will definitely break 
some libprocess APIs (though not in the way that Mesos uses them) and 
legitimate multi-homed configurations. Note that setting 
LIBPROCESS_ip=127.0.0.1 makes you multihomed for this purpose, which is why 
``ExamplesTest.DiskFullFramework`` breaks.


Thanks,

James Peach

Re: Review Request 58224: Optionally verify the source IP address for libprocess messages.

[James Peach]

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

(Updated April 18, 2017, 9:02 p.m.)


Review request for mesos and Mesos Reviewbot.


Changes
---

Rebased, added options, fixed tests and addressed review feedback.


Summary (updated)
-

Optionally verify the source IP address for libprocess messages.


Repository: mesos


Description (updated)
---

In general, libprocess is unable to validate that a peer
is a legitimate owner of the UPID it claims in a libprocess
message. This change adds a check that the IP address in the
UPID matches the peer address. This makes spoofing the UPID
harder (eg. to send authenticated messages), but also breaks
some legitimate configurations, particularly on multihomed
hosts.


Diffs (updated)
-

  3rdparty/libprocess/src/process.cpp 92efa915414c2a38b18de99858c66b63e757f63c 
  3rdparty/libprocess/src/tests/process_tests.cpp 
bf90c7e78fd50ad7e16cc0a69a248ba71e2a7115 
  3rdparty/libprocess/src/tests/test_linkee.cpp 
921d67695bc0e4d601e9f74fbc625d69bf36ba50 


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

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


Testing (updated)
---

make check (Fedora 25). Light manual testing.

With LIBPROCESS_pin_peer_address=true, all Mesos tests pass except 
``ExamplesTest.DiskFullFramework``, however enabling this will definitely break 
some libprocess APIs (though not in the way that Mesos uses them) and 
legitimate multi-homed configurations. Note that setting 
LIBPROCESS_ip=127.0.0.1 makes you multihomed for this purpose, which is why 
``ExamplesTest.DiskFullFramework`` breaks.


Thanks,

James Peach

Re: Review Request 58517: Update SlaveRecoveryTest to send libprocess messages from a real UPID.

[James Peach]

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

(Updated April 18, 2017, 8:58 p.m.)


Review request for mesos and Mesos Reviewbot.


Repository: mesos


Description
---

Update SlaveRecoveryTest to send libprocess messages from a real UPID.


Diffs
-

  src/tests/slave_recovery_tests.cpp 53f33a2b0411c8158326074ce043c7b1dbeef5b4 


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


Testing
---

make check (Fedora 25)


Thanks,

James Peach

Review Request 58517: Update SlaveRecoveryTest to send libprocess messages from a real UPID.

[James Peach]

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

Review request for mesos.


Repository: mesos


Description
---

Update SlaveRecoveryTest to send libprocess messages from a real UPID.


Diffs
-

  src/tests/slave_recovery_tests.cpp 53f33a2b0411c8158326074ce043c7b1dbeef5b4 


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


Testing
---

make check (Fedora 25)


Thanks,

James Peach

Re: Review Request 58512: Avoid a crash in HTTP decoder.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58512]

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 April 18, 2017, 6:04 p.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58512/
> ---
> 
> (Updated April 18, 2017, 6:04 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, James Peach, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7400
> https://issues.apache.org/jira/browse/MESOS-7400
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> If the callback `on_headers_complete()` fails, the decoder's writer
> object is `None()`. So, when the callback `on_message_complete()` is
> called, we should not crash in that case.
> Note that the `CHECK(decoder->writer)` is valid for the callback
> `on_body()` is valid since this callback is called only on a success
> in `on_headers_complete()` callback.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/decoder.hpp 
> 4c779d42548958e610142438a57529ccb4478053 
> 
> 
> Diff: https://reviews.apache.org/r/58512/diff/1/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 58448: Windows: Updated ZooKeeper to use CMake.

[Jeff Coffler]

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



Big change, but important for us.

I'd like you to comment on using cmake for zookeeper, specifically in terms of 
rolling that back to Zookeeper themselves for them to maintain it.

Have you looked at contributing this to the Zookeeper folks for them to use and 
maintain? What was their response?

This feels like something that is easily broken (mentioning specific sources, 
etc) if they aren't maintaining it, and I'd rather not own this for life. If 
the Zookeeper folks don't want to maintain this, what do they say about VS 2017 
support? Even if it's not cmake, but a hand-generated solution for V/S, at 
least they maintain it, not us.

- Jeff Coffler


On April 14, 2017, 4:12 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58448/
> ---
> 
> (Updated April 14, 2017, 4:12 a.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This unblocks us from building exclusively with VS 2017. The previous
> patch to ZooKeeper only added VS 2015 support. This patch replaces it
> with a CMake build system that will generate whichever solution we need
> for Windows (and can replace the Autotools system on Linux).
> 
> We're updating to 3.5.2-alpha as the existing 3.5.1 rebundle was a
> source tarball, and so missing the necessary generated files. The most
> currently used version was based off a random commit. 3.5.2-alpha is the
> latest 3.5.x release of ZooKeeper (3.5.x itself is alpha, 3.5.2 is
> semi-stable, in comparison to 3.5.3 which is in RC).
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt bb61ef0514fb164f35b34bb6be1bbebb4d1a1861 
>   3rdparty/cmake/Mesos3rdpartyConfigure.cmake 
> c60652688a23f8628f133b7890ff39e38fc8ae94 
>   3rdparty/cmake/Versions.cmake 912726351ff744dd839b8d1c8d64dcc373d879be 
>   3rdparty/zookeeper-3.5.2-alpha.patch PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/58448/diff/1/
> 
> 
> Testing
> ---
> 
> cmake and make check on Linux
> stout-tests, libprocess-tests, and mesos-tests on Windows: all tests passed. 
> (Note: my machine updated to the Creators Update today, disabling long path 
> support; the previous way to enable it doesn't seem to work, so I re-ran the 
> failed tests on a machine that hadn't updated and still had long path 
> support, they passed).
> 
> Also tested the build on a clean machine with nothing but the VS 2017 build 
> tools (no IDE, no prior VS installations); mesos-tests builds no problem.
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 58446: Windows: Set CMake generator for Protobuf correctly.

[Jeff Coffler]

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



This looks okay. But again: You didn't fill out anything for testing, leading 
me to believe that no testing was done.

Please clarify the testing that you completed here, so I can understand that. 
Thanks.

- Jeff Coffler


On April 14, 2017, 2:03 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58446/
> ---
> 
> (Updated April 14, 2017, 2:03 a.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Reuse the variable rather than hardcode it.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt bb61ef0514fb164f35b34bb6be1bbebb4d1a1861 
> 
> 
> Diff: https://reviews.apache.org/r/58446/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 58447: CMake: Cleaned up 3rdparty dependencies.

[Jeff Coffler]

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



I'd like to understand the tests that you ran before you posted the review. 
What, specifically, did you test on to insure that nothing was broken by this 
change?

- Jeff Coffler


On April 14, 2017, 2:03 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58447/
> ---
> 
> (Updated April 14, 2017, 2:03 a.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit removes duplicate code from `3rdparty/CMakeLists.txt`,
> and consolidates platform-specific versions into `Versions.cmake`.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt bb61ef0514fb164f35b34bb6be1bbebb4d1a1861 
>   3rdparty/cmake/Mesos3rdpartyConfigure.cmake 
> c60652688a23f8628f133b7890ff39e38fc8ae94 
>   3rdparty/cmake/Versions.cmake 912726351ff744dd839b8d1c8d64dcc373d879be 
>   cmake/CompilationConfigure.cmake 1c5466960f5ac73b8fc81edf7950cc68ed744301 
> 
> 
> Diff: https://reviews.apache.org/r/58447/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 58449: CMake: Bump minimum version to 3.7.0 on Windows.

[Jeff Coffler]

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


Ship it!




Ship It!

- Jeff Coffler


On April 14, 2017, 2:10 a.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58449/
> ---
> 
> (Updated April 14, 2017, 2:10 a.m.)
> 
> 
> Review request for mesos, Jeff Coffler, John Kordich, Joseph Wu, and Li Li.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The `SOURCE_SUBDIR` command to `ExternalProject_Add` was added in CMake
> 3.7.0, and is necessary to most cleanly build an external CMake built
> project where the `CMakeLists.txt` is in a subfolder of the project.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt ea529ec2d5c2b9be4f19c67c2033c3f4b9073c1f 
> 
> 
> Diff: https://reviews.apache.org/r/58449/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 58255: Added implicit authorization to the agent executor API.

[Greg Mann]

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

(Updated April 18, 2017, 7:58 p.m.)


Review request for mesos, Adam B, Alexander Rojas, and Till Toenshoff.


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


Repository: mesos


Description
---

This patch updates the agent handler for the executor API to
verify the FrameworkID and ExecutorID contained within the
executor's `Principal`, if present. This effectively performs
implicit authorization of executor calls.


Diffs (updated)
-

  src/slave/http.cpp 468cf332d79ed7315ecf51955235735dec0a2df1 


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

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


Testing
---

Testing details can be found at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 58255: Added implicit authorization to the agent executor API.

[Greg Mann]

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

(Updated April 18, 2017, 7:29 p.m.)


Review request for mesos, Adam B, Alexander Rojas, and Till Toenshoff.


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


Repository: mesos


Description
---

This patch updates the agent handler for the executor API to
verify the FrameworkID and ExecutorID contained within the
executor's `Principal`, if present. This effectively performs
implicit authorization of executor calls.


Diffs (updated)
-

  src/slave/http.cpp 468cf332d79ed7315ecf51955235735dec0a2df1 


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

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


Testing
---

Testing details can be found at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 58510: Added two tests for hierarchical reservation.

[Mesos Reviewbot]

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



Bad review!

Reviews applied: [58510, 58509, 58508, 57516, 57254, 58112, 58110, 57564, 
57528, 57527, 57788, 57167, 57166, 56805, 57165, 57164]

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos Reviewbot


On April 18, 2017, 3:45 p.m., Jay Guo wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58510/
> ---
> 
> (Updated April 18, 2017, 3:45 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Bugs: MESOS-7149
> https://issues.apache.org/jira/browse/MESOS-7149
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added two tests for hierarchical reservation.
> 
> 
> Diffs
> -
> 
>   src/tests/hierarchical_allocator_tests.cpp 
> 33e7b455f8664858eb4f03727b076a10c80cd6e0 
>   src/tests/reservation_tests.cpp 4504831d77c1bfcf5f2ddf6d28cd45dea2c421ad 
> 
> 
> Diff: https://reviews.apache.org/r/58510/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jay Guo
> 
>

Re: Review Request 58357: Added more language support in test-upgrade script.

[Greg Mann]

> On April 18, 2017, 5:20 p.m., Greg Mann wrote:
> > support/test-upgrade.py
> > Lines 76-85 (patched)
> > 
> >
> > A high level comment: insteads of parametrizing the framework by 
> > "language", perhaps we could parametrize it by something more generic like 
> > "framework name". You could use names like "cpp-test-framework", etc. This 
> > would make it easier to expand this list to include other test frameworks 
> > from the codebase in the future. What do you think?
> 
> Zhitao Li wrote:
> Sounds good. I'll go with this list in the diff:
> 
> - `cpp-test-framework`, 
> - `java-test-framework`, 
> - `python-test-framework`
> 
> and we can add more in the future. How does this plan sound?

Sounds good


> On April 18, 2017, 5:20 p.m., Greg Mann wrote:
> > support/test-upgrade.py
> > Lines 164 (patched)
> > 
> >
> > Perhaps the default should be to run all of the available test 
> > frameworks, rather than just one?
> 
> Zhitao Li wrote:
> My only concern here is that certain frameworks are more flaky and 
> subjective to environments broken. For example, both python and java based 
> test frameworks have been broken from time to time in either my MacOS or 
> Linux environment (which might be different from the author's).
> 
> As long as the expectation is that user will look into why things are 
> broken, I'm fine changing this to run all by default, and change the flag to 
> "exclude some frameworks by name".

Yea, fair point. I'd say let's err on the side of caution and run them all by 
default - this will help expose broken test frameworks and push us to fix them 
more quickly.


- Greg


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


On April 12, 2017, 12:02 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58357/
> ---
> 
> (Updated April 12, 2017, 12:02 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Xiaojian Huang, and Greg Mann.
> 
> 
> Bugs: MESOS-7228
> https://issues.apache.org/jira/browse/MESOS-7228
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added support to java and python based test framework in
> `test-upgrade.py` script by adding a new option `--lang`.
> 
> 
> Diffs
> -
> 
>   support/test-upgrade.py 84df21dd6e63653e1a18e700ef904aa3a04b2b45 
> 
> 
> Diff: https://reviews.apache.org/r/58357/diff/2/
> 
> 
> Testing
> ---
> 
> Ran this on all three languages options for cpp, java and python.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 58357: Added more language support in test-upgrade script.

[Zhitao Li]

> On April 18, 2017, 5:20 p.m., Greg Mann wrote:
> > support/test-upgrade.py
> > Lines 76-85 (patched)
> > 
> >
> > A high level comment: insteads of parametrizing the framework by 
> > "language", perhaps we could parametrize it by something more generic like 
> > "framework name". You could use names like "cpp-test-framework", etc. This 
> > would make it easier to expand this list to include other test frameworks 
> > from the codebase in the future. What do you think?

Sounds good. I'll go with this list in the diff:

- `cpp-test-framework`, 
- `java-test-framework`, 
- `python-test-framework`

and we can add more in the future. How does this plan sound?


- Zhitao


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


On April 12, 2017, 12:02 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58357/
> ---
> 
> (Updated April 12, 2017, 12:02 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Xiaojian Huang, and Greg Mann.
> 
> 
> Bugs: MESOS-7228
> https://issues.apache.org/jira/browse/MESOS-7228
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added support to java and python based test framework in
> `test-upgrade.py` script by adding a new option `--lang`.
> 
> 
> Diffs
> -
> 
>   support/test-upgrade.py 84df21dd6e63653e1a18e700ef904aa3a04b2b45 
> 
> 
> Diff: https://reviews.apache.org/r/58357/diff/2/
> 
> 
> Testing
> ---
> 
> Ran this on all three languages options for cpp, java and python.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 58357: Added more language support in test-upgrade script.

[Zhitao Li]

> On April 18, 2017, 5:20 p.m., Greg Mann wrote:
> > support/test-upgrade.py
> > Lines 164 (patched)
> > 
> >
> > Perhaps the default should be to run all of the available test 
> > frameworks, rather than just one?

My only concern here is that certain frameworks are more flaky and subjective 
to environments broken. For example, both python and java based test frameworks 
have been broken from time to time in either my MacOS or Linux environment 
(which might be different from the author's).

As long as the expectation is that user will look into why things are broken, 
I'm fine changing this to run all by default, and change the flag to "exclude 
some frameworks by name".


- Zhitao


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


On April 12, 2017, 12:02 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58357/
> ---
> 
> (Updated April 12, 2017, 12:02 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Xiaojian Huang, and Greg Mann.
> 
> 
> Bugs: MESOS-7228
> https://issues.apache.org/jira/browse/MESOS-7228
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added support to java and python based test framework in
> `test-upgrade.py` script by adding a new option `--lang`.
> 
> 
> Diffs
> -
> 
>   support/test-upgrade.py 84df21dd6e63653e1a18e700ef904aa3a04b2b45 
> 
> 
> Diff: https://reviews.apache.org/r/58357/diff/2/
> 
> 
> Testing
> ---
> 
> Ran this on all three languages options for cpp, java and python.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 58359: Update Mesos build library to use protobuf 3.2.0.

[Zhitao Li]

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

(Updated April 18, 2017, 6:25 p.m.)


Review request for mesos, Anand Mazumdar and Joseph Wu.


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


Repository: mesos


Description (updated)
---

Update Mesos build library to use protobuf 3.2.0.


Diffs (updated)
-

  configure.ac 090783abe98f40c2929ecb4fc025c75936a3d8ef 
  src/java/mesos.pom.in 080767f011782f048ab0244f93daea189326587c 
  src/python/interface/setup.py.in 8e38f3feb705f08737cff93e0ed442a1f48dcbf9 
  src/python/native_common/ext_modules.py.in 
e0bb335e5ae663f398cea904a0e53e3c5fb210d7 
  src/python/protocol/setup.py.in b7de10a2a099d21d6eba551ba071b6c31a1460be 
  support/mesos-tidy/entrypoint.sh 5dbaa6086621934a7313dfa5c8008b6a6f5496f1 


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

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 58361: Updated LICENSE information for protobuf 3.2.0.

[Zhitao Li]

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

(Updated April 18, 2017, 6:25 p.m.)


Review request for mesos, Anand Mazumdar and Joseph Wu.


Changes
---

Rebase.


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


Repository: mesos


Description
---

Content copied from https://github.com/google/protobuf/blob/v3.2.0/LICENSE.


Diffs (updated)
-

  LICENSE 512b089c5dd2540f9f83afb559e5d0998d97ea22 


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

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 58360: Added a test for evolving large protobuf message.

[Zhitao Li]

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

(Updated April 18, 2017, 6:25 p.m.)


Review request for mesos and Anand Mazumdar.


Changes
---

Rebase.


Bugs: MESOS-6644 and MESOS-7228
https://issues.apache.org/jira/browse/MESOS-6644
https://issues.apache.org/jira/browse/MESOS-7228


Repository: mesos


Description
---

Before protobuf 3.2.0, this test would fail because the 64MB limit imposed by
older version of protobuf.


Diffs (updated)
-

  src/tests/protobuf_utils_tests.cpp 5239182812835b93a28e85146b2df2b20ae77328 


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

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


Testing
---


Thanks,

Zhitao Li

Review Request 58515: Update 3rdparty build to support protobuf 3.2.0.

[Zhitao Li]

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

Review request for mesos, Anand Mazumdar and Joseph Wu.


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


Repository: mesos


Description
---

Update 3rdparty build to support protobuf 3.2.0.


Diffs
-

  3rdparty/CMakeLists.txt bb61ef0514fb164f35b34bb6be1bbebb4d1a1861 
  3rdparty/cmake/Versions.cmake 912726351ff744dd839b8d1c8d64dcc373d879be 
  3rdparty/versions.am b8144702adca9dd41199f69cb6fb0c0c4b490dbd 


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


Testing
---


Thanks,

Zhitao Li

Review Request 58514: Update related tests in stout to support protobuf 3.2.0.

[Zhitao Li]

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

Review request for mesos, Anand Mazumdar and Joseph Wu.


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


Repository: mesos


Description
---

Update related tests in stout to support protobuf 3.2.0.


Diffs
-

  3rdparty/stout/README.md f4e41aaacee4f88c708413e954553011229c75e4 
  3rdparty/stout/tests/protobuf_tests.pb.h 
18870cde806c7bcb0a204de2a40555739adf777b 
  3rdparty/stout/tests/protobuf_tests.pb.cc 
b2458f0edcf679b2390f87ec8c4bee2b16e71d70 


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


Testing
---


Thanks,

Zhitao Li

Review Request 58513: Remove unnecessary patch after protobuf upgrade to 3.2.0.

[Zhitao Li]

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

Review request for mesos, Anand Mazumdar and Joseph Wu.


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


Repository: mesos


Description
---

The patch should already be included in protobuf 3.0.0, so
this is not necessary anymore.


Diffs
-

  3rdparty/protobuf-2.6.1.patch bfd3bb2c600383ef05bdb4ea9df0188b4a560315 


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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 58358: Update vendored protobuf tar.gz to 3.2.0.

[Zhitao Li]

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

(Updated April 18, 2017, 6:22 p.m.)


Review request for mesos, Anand Mazumdar and Joseph Wu.


Changes
---

Only include binary files.


Summary (updated)
-

Update vendored protobuf tar.gz to 3.2.0.


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


Repository: mesos


Description (updated)
---

The content of `3rdparty/protobuf-3.2.0.tar.gz` is generated by:
- On a Mac OS, download and untar protobuf v3.2.0 source at
  https://github.com/google/protobuf/archive/v3.2.0rc2.tar.gz;
- Run `./autogen.sh`;
- Recompress and tar by `tar -czvf`.


Diffs (updated)
-

  3rdparty/protobuf-2.6.1.tar.gz e1c2d541c1eb806e837ff5d1e9d3fdb678b40e06 
  3rdparty/protobuf-3.2.0.tar.gz PRE-CREATION 


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

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 58507: Added a TODO for implicit scheduler authorization.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58507]

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 April 18, 2017, 3:19 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58507/
> ---
> 
> (Updated April 18, 2017, 3:19 p.m.)
> 
> 
> Review request for mesos, Alexander Rojas and Vinod Kone.
> 
> 
> Bugs: MESOS-7399
> https://issues.apache.org/jira/browse/MESOS-7399
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch adds a TODO to the master's HTTP scheduler API related
> to MESOS-7399.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 0e84d363b346de1ef05101dac066c2f0cf31609d 
> 
> 
> Diff: https://reviews.apache.org/r/58507/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Review Request 58512: Avoid a crash in HTTP decoder.

[Anindya Sinha]

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

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


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


Repository: mesos


Description
---

If the callback `on_headers_complete()` fails, the decoder's writer
object is `None()`. So, when the callback `on_message_complete()` is
called, we should not crash in that case.
Note that the `CHECK(decoder->writer)` is valid for the callback
`on_body()` is valid since this callback is called only on a success
in `on_headers_complete()` callback.


Diffs
-

  3rdparty/libprocess/src/decoder.hpp 4c779d42548958e610142438a57529ccb4478053 


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


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 58357: Added more language support in test-upgrade script.

[Greg Mann]

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



Thanks for this, Zhitao! Glad to see this script getting some new 
functionality. :)


support/test-upgrade.py
Lines 76-85 (patched)


A high level comment: insteads of parametrizing the framework by 
"language", perhaps we could parametrize it by something more generic like 
"framework name". You could use names like "cpp-test-framework", etc. This 
would make it easier to expand this list to include other test frameworks from 
the codebase in the future. What do you think?



support/test-upgrade.py
Lines 164 (patched)


Perhaps the default should be to run all of the available test frameworks, 
rather than just one?


- Greg Mann


On April 12, 2017, 12:02 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58357/
> ---
> 
> (Updated April 12, 2017, 12:02 a.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Xiaojian Huang, and Greg Mann.
> 
> 
> Bugs: MESOS-7228
> https://issues.apache.org/jira/browse/MESOS-7228
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch added support to java and python based test framework in
> `test-upgrade.py` script by adding a new option `--lang`.
> 
> 
> Diffs
> -
> 
>   support/test-upgrade.py 84df21dd6e63653e1a18e700ef904aa3a04b2b45 
> 
> 
> Diff: https://reviews.apache.org/r/58357/diff/2/
> 
> 
> Testing
> ---
> 
> Ran this on all three languages options for cpp, java and python.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 58361: Updated LICENSE information for protobuf 3.2.0.

[Zhitao Li]

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

(Updated April 18, 2017, 4:33 p.m.)


Review request for mesos, Anand Mazumdar and Joseph Wu.


Changes
---

Rebase to run again.


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


Repository: mesos


Description
---

Content copied from https://github.com/google/protobuf/blob/v3.2.0/LICENSE.


Diffs (updated)
-

  LICENSE 512b089c5dd2540f9f83afb559e5d0998d97ea22 


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

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 58358: Upgrade vendored 3rdparty protobuf to 3.2.0.

[Zhitao Li]

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

(Updated April 18, 2017, 4:32 p.m.)


Review request for mesos, Anand Mazumdar and Joseph Wu.


Changes
---

Try binary upload.


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


Repository: mesos


Description (updated)
---

Update vendored protobuf tar.gz to 3.2.0.

The content of `3rdparty/protobuf-3.2.0.tar.gz` is generated by:
- On a Mac OS, download and untar protobuf v3.2.0 source at
  https://github.com/google/protobuf/archive/v3.2.0rc2.tar.gz;
- Run `./autogen.sh`;
- Recompress and tar by `tar -czvf`.


Diffs (updated)
-

  3rdparty/CMakeLists.txt bb61ef0514fb164f35b34bb6be1bbebb4d1a1861 
  3rdparty/cmake/Versions.cmake 912726351ff744dd839b8d1c8d64dcc373d879be 
  3rdparty/protobuf-2.6.1.patch bfd3bb2c600383ef05bdb4ea9df0188b4a560315 
  3rdparty/protobuf-2.6.1.tar.gz e1c2d541c1eb806e837ff5d1e9d3fdb678b40e06 
  3rdparty/protobuf-3.2.0.tar.gz PRE-CREATION 
  3rdparty/stout/README.md f4e41aaacee4f88c708413e954553011229c75e4 
  3rdparty/stout/tests/protobuf_tests.pb.h 
18870cde806c7bcb0a204de2a40555739adf777b 
  3rdparty/stout/tests/protobuf_tests.pb.cc 
b2458f0edcf679b2390f87ec8c4bee2b16e71d70 
  3rdparty/versions.am b8144702adca9dd41199f69cb6fb0c0c4b490dbd 


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

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


Testing
---


Thanks,

Zhitao Li

Review Request 58511: Update vendored protobuf tar.gz to 3.2.0.

[Zhitao Li]

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

Review request for mesos.


Repository: mesos


Description
---

The content of `3rdparty/protobuf-3.2.0.tar.gz` is generated by:
- On a Mac OS, download and untar protobuf v3.2.0 source at
  https://github.com/google/protobuf/archive/v3.2.0rc2.tar.gz;
- Run `./autogen.sh`;
- Recompress and tar by `tar -czvf`.


Diffs
-

  3rdparty/protobuf-2.6.1.tar.gz e1c2d541c1eb806e837ff5d1e9d3fdb678b40e06 
  3rdparty/protobuf-3.2.0.tar.gz PRE-CREATION 


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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 58506: Updated the high availability doc about ZK session timeout.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58421, 58506]

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 April 18, 2017, 2:23 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58506/
> ---
> 
> (Updated April 18, 2017, 2:23 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-7387
> https://issues.apache.org/jira/browse/MESOS-7387
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated the high availability doc about ZK session timeout.
> 
> 
> Diffs
> -
> 
>   docs/high-availability.md 46bd78987f37aa2c024a6277deab8404232782e2 
> 
> 
> Diff: https://reviews.apache.org/r/58506/diff/1/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Ilya Pronin
> 
>

Review Request 58510: Added two tests for hierarchical reservation.

[Jay Guo]

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

Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

Added two tests for hierarchical reservation.


Diffs
-

  src/tests/hierarchical_allocator_tests.cpp 
33e7b455f8664858eb4f03727b076a10c80cd6e0 
  src/tests/reservation_tests.cpp 4504831d77c1bfcf5f2ddf6d28cd45dea2c421ad 


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


Testing
---

make check


Thanks,

Jay Guo

Review Request 58509: Enabled allocator to handle hierarchical reservation.

[Jay Guo]

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

Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

Other than looking at reservation made for a role, now allocator also
aggregates reservations of all its ancestor in the hierarchy.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
051f749dd5921a322ca930a042c31814616d38f9 


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


Testing
---

see next patch in chain


Thanks,

Jay Guo

Review Request 58508: Introduced method `Resources::inheritable` to aggregate reservations.

[Jay Guo]

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

Review request for mesos and Michael Park.


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


Repository: mesos


Description
---

A reservation is inheritable by 'role' iff:
 - it is reserved to ancestor of 'role' in hierarchy, OR
 - it is reserved to 'role' itself.

This is a helper function for reservation delegate, where reserved
resources can be 'delegated' downwards in the hierarchy.


Diffs
-

  include/mesos/resources.hpp 74583e3c784f22b45cce81f07c5a69b339e1684d 
  src/common/resources.cpp 77bac0c4390da905016a942991b14a79877f8455 
  src/tests/resources_tests.cpp 4cf320d802a749f1419ac5b9f63b6c73b0c974be 


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


Testing
---

make check


Thanks,

Jay Guo

Review Request 42937: Added helper which returns flag map to 'FlagsBase'.

[Greg Mann]

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

Review request for mesos.


Repository: mesos


Description
---

Added helper which returns flag map to 'FlagsBase'.


Diffs
-

  3rdparty/libprocess/3rdparty/stout/include/stout/flags/flags.hpp 
2a188459465a5203c56d788a74e69d403790c5bf 


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


Testing
---


Thanks,

Greg Mann

Review Request 42938: Added automatic generation of the config flag docs.

[Greg Mann]

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

Review request for mesos.


Repository: mesos


Description
---

Added automatic generation of the config flag docs.


Diffs
-

  src/Makefile.am fac17f4bac3b2ddda0384dec8b0ed6f960bd24d1 
  support/configuration-docs.cpp PRE-CREATION 


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


Testing
---


Thanks,

Greg Mann

Review Request 58507: Added a TODO for implicit scheduler authorization.

[Greg Mann]

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

Review request for mesos, Alexander Rojas and Vinod Kone.


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


Repository: mesos


Description
---

This patch adds a TODO to the master's HTTP scheduler API related
to MESOS-7399.


Diffs
-

  src/master/http.cpp 0e84d363b346de1ef05101dac066c2f0cf31609d 


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


Testing
---


Thanks,

Greg Mann

Re: Review Request 58255: Added implicit authorization to the agent executor API.

[Greg Mann]

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

(Updated April 18, 2017, 3:17 p.m.)


Review request for mesos, Adam B, Alexander Rojas, and Till Toenshoff.


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


Repository: mesos


Description
---

This patch updates the agent handler for the executor API to
verify the FrameworkID and ExecutorID contained within the
executor's `Principal`, if present. This effectively performs
implicit authorization of executor calls.


Diffs (updated)
-

  src/slave/http.cpp 468cf332d79ed7315ecf51955235735dec0a2df1 


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

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


Testing
---

Testing details can be found at the end of this chain.


Thanks,

Greg Mann

Re: Review Request 58196: Implemented TCP check support in command and default executors.

[Alexander Rukletsov]

> On April 7, 2017, 1:58 a.m., Vinod Kone wrote:
> > src/checks/checker.cpp
> > Lines 1081 (patched)
> > 
> >
> > Does this work even when executor is running with its own file system?
> 
> Alexander Rukletsov wrote:
> This is a good question. My understanding is that an executor always have 
> access to `launcherDir` because it's necessary for `mesos-containerizer` 
> command, which we, e.g., put into `pre_exec_commands`. However, this might 
> not be the case for the default executor, because launch is delegated to the 
> agent in this case. @Jie, @Gilbert, any comments?
> 
> Jie Yu wrote:
> Both default executor and command executor can see agent's filesystem. Is 
> the expectation here that the checker will only be used by these two 
> executors?
> 
> Vinod Kone wrote:
> Yea checker is only used by default and command executors. So I think we 
> are ok here.

We might add support for docker executor in the future. I'll add a note here.


- Alexander


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


On April 7, 2017, 3:25 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58196/
> ---
> 
> (Updated April 7, 2017, 3:25 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman and Vinod Kone.
> 
> 
> Bugs: MESOS-7275
> https://issues.apache.org/jira/browse/MESOS-7275
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/checks/checker.hpp fb939d85dbec2bf7e81e0c11518ccecddc5a7a11 
>   src/checks/checker.cpp 7510bf23977e007d101fab635865b7160c3a5af6 
>   src/launcher/default_executor.cpp 9cc40c6391a931f9cdf6965e64be231980537299 
>   src/launcher/executor.cpp d14fbfb65d1cc44c954c8cdbbf36d7ddf6ede19e 
>   src/tests/check_tests.cpp d7fcbf940102efe0841f07e8c4937a78361bc422 
> 
> 
> Diff: https://reviews.apache.org/r/58196/diff/2/
> 
> 
> Testing
> ---
> 
> make check on Mac OS 10.11.6 and various linux variants.
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Review Request 58506: Updated the high availability doc about ZK session timeout.

[Ilya Pronin]

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

Review request for mesos and Vinod Kone.


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


Repository: mesos


Description
---

Updated the high availability doc about ZK session timeout.


Diffs
-

  docs/high-availability.md 46bd78987f37aa2c024a6277deab8404232782e2 


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


Testing
---

N/A


Thanks,

Ilya Pronin

Re: Review Request 58503: Removed containerizer flag logging to prevent leak of sensitive data.

[Benjamin Bannier]

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


Ship it!




Ship It!

- Benjamin Bannier


On April 18, 2017, 4:06 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58503/
> ---
> 
> (Updated April 18, 2017, 4:06 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, and Benjamin Bannier.
> 
> 
> Bugs: MESOS-7265
> https://issues.apache.org/jira/browse/MESOS-7265
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> * backported for 1.1.x *
> 
> 
> Diffs
> -
> 
>   src/launcher/posix/executor.cpp da0081c0e470aebb16d2e78031d276f5d7d2c726 
> 
> 
> Diff: https://reviews.apache.org/r/58503/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 58502: Removed containerizer flag logging to prevent leak of sensitive data.

[Benjamin Bannier]

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


Ship it!




Ship It!

- Benjamin Bannier


On April 18, 2017, 3:50 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58502/
> ---
> 
> (Updated April 18, 2017, 3:50 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, and Benjamin Bannier.
> 
> 
> Bugs: MESOS-7265
> https://issues.apache.org/jira/browse/MESOS-7265
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> * backported for 1.0.x *
> 
> 
> Diffs
> -
> 
>   src/launcher/posix/executor.cpp 1c423f07b2c1b22b4d8aded274cae1295fe02a9d 
> 
> 
> Diff: https://reviews.apache.org/r/58502/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 57474: Added test for authorization of hierarchical roles.

[Mesos Reviewbot]

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



Bad review!

Reviews applied: [57474, 57473, 58292, 57166, 56805, 57165, 57164]

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos Reviewbot


On April 18, 2017, 1:28 p.m., Alexander Rojas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57474/
> ---
> 
> (Updated April 18, 2017, 1:28 p.m.)
> 
> 
> Review request for mesos, Adam B and Benjamin Bannier.
> 
> 
> Bugs: MESOS-7026
> https://issues.apache.org/jira/browse/MESOS-7026
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds tests for each of the actions which support hierarchical roles.
> 
> 
> Diffs
> -
> 
>   src/tests/authorization_tests.cpp 3e18c70738b6b7098f37fadebb799a596e76452d 
> 
> 
> Diff: https://reviews.apache.org/r/57474/diff/5/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>

Re: Review Request 58503: Removed containerizer flag logging to prevent leak of sensitive data.

[Till Toenshoff]

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

(Updated April 18, 2017, 2:06 p.m.)


Review request for mesos, Adam B, Alexander Rukletsov, and Benjamin Bannier.


Changes
---

Moved log output to be inline with original commit.


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


Repository: mesos


Description (updated)
---

See summary.

* backported for 1.1.x *


Diffs (updated)
-

  src/launcher/posix/executor.cpp da0081c0e470aebb16d2e78031d276f5d7d2c726 


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

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


Testing
---

make check


Thanks,

Till Toenshoff

Re: Review Request 58421: Passed `--zk_session_timeout` to ZK master contender and detector.

[Ilya Pronin]

> On April 14, 2017, 6:45 p.m., Vinod Kone wrote:
> > Can you update one of the tests in master_contender_detector_tests to use a 
> > configured session timeout value rather than the constant?

Done. Modified 
`ZooKeeperMasterContenderDetectorTest.ContenderDetectorShutdownNetwork` to use 
a configured timeout.


> On April 14, 2017, 6:45 p.m., Vinod Kone wrote:
> > src/master/contender/contender.cpp
> > Line 61 (original), 62-65 (patched)
> > 
> >
> > How about?
> > 
> > ```
> > return new ZooKeeperMasterContender(
> >   url.get(),
> >   zkSessionTimeout_.get(MASTER_CONTENDER_ZK_SESSION_TIMEOUT));
> > ```

Fixed. Forgot about `Option::getOrElse()`.


> On April 14, 2017, 6:45 p.m., Vinod Kone wrote:
> > src/master/contender/zookeeper.hpp
> > Lines 49 (patched)
> > 
> >
> > no need for default?

Constructors without `sessionTimeout` are used in tests. Maybe leave default 
for convenience?


> On April 14, 2017, 6:45 p.m., Vinod Kone wrote:
> > src/master/detector/zookeeper.hpp
> > Lines 51 (patched)
> > 
> >
> > ditto. see above.

As with `ZooKeeperMasterContender` constructors without `sessionTimeout` are 
used in tests. Default value can be left for convenience.


- Ilya


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


On April 18, 2017, 3 p.m., Ilya Pronin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58421/
> ---
> 
> (Updated April 18, 2017, 3 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Vinod Kone.
> 
> 
> Bugs: MESOS-7387
> https://issues.apache.org/jira/browse/MESOS-7387
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Currently `ZooKeeperMasterContender` and `ZooKeeperMasterDetector` use
> hardcoded session timeouts and do not respect `--zk_session_timeout`
> option. This patch fixes it.
> 
> 
> Diffs
> -
> 
>   include/mesos/master/contender.hpp 2c485ef2f24b3aa1b36b764b4780248835a1d9d5 
>   include/mesos/master/detector.hpp f53c7c0bbeaed0b263d2634dc6eb15481d2d90b2 
>   src/master/contender/contender.cpp cc486a176d14e61a2434ce3677c8741963a39057 
>   src/master/contender/zookeeper.hpp becb93fbc04031c133f42f44f3cf406a27262444 
>   src/master/contender/zookeeper.cpp 1aa8f53fc9b0733ac3edb2b4288b7225fac25828 
>   src/master/detector/detector.cpp 0ba29ca9455b85f73cee6787e251b9442c6f97ea 
>   src/master/detector/zookeeper.hpp 5b531e0f1fed7297bb73c5b02e8ef51d0c27ca38 
>   src/master/detector/zookeeper.cpp a737d2403e480d14f1cd345b48af2833d2fa3284 
>   src/master/main.cpp 90d159e248924f6806271dda1ced6d37dfa5f5c9 
>   src/tests/master_contender_detector_tests.cpp 
> c8c1ea0bf14909541e331ba026c754092d0e8fdd 
> 
> 
> Diff: https://reviews.apache.org/r/58421/diff/2/
> 
> 
> Testing
> ---
> 
> Ran `make check`.
> 
> Manually verified that all 4 ZK session negotiate 20 sec timeout when 
> `--zk_session_timeout=20secs`.
> ```
> 2017-04-13 15:29:21,313:67863(0x7028d000):ZOO_INFO@zookeeper_init@800: 
> Initiating client connection, host=192.168.99.100:2181 sessionTimeout=2 
> watcher=0x107617130 sessionId=0 sessionPasswd= context=0x7f999ad15f90 
> flags=0
> 2017-04-13 15:29:21,314:67863(0x7031):ZOO_INFO@zookeeper_init@800: 
> Initiating client connection, host=192.168.99.100:2181 sessionTimeout=2 
> watcher=0x107617130 sessionId=0 sessionPasswd= context=0x7f999ad164f0 
> flags=0
> 2017-04-13 15:29:21,316:67863(0x7020a000):ZOO_INFO@zookeeper_init@800: 
> Initiating client connection, host=192.168.99.100:2181 sessionTimeout=2 
> watcher=0x107617130 sessionId=0 sessionPasswd= context=0x7f999ae05900 
> flags=0
> 2017-04-13 15:29:21,316:67863(0x70104000):ZOO_INFO@zookeeper_init@800: 
> Initiating client connection, host=192.168.99.100:2181 sessionTimeout=2 
> watcher=0x107617130 sessionId=0 sessionPasswd= context=0x7f999ae05a80 
> flags=0
> 2017-04-13 15:29:21,327:67863(0x70622000):ZOO_INFO@check_events@1775: 
> session establishment complete on server [192.168.99.100:2181], 
> sessionId=0x15b67b0efd3000c, negotiated timeout=2
> 2017-04-13 15:29:21,333:67863(0x70728000):ZOO_INFO@check_events@1775: 
> session establishment complete on server [192.168.99.100:2181], 
> sessionId=0x15b67b0efd3000d, negotiated timeout=2
> 2017-04-13 15:29:21,337:67863(0x7082e000):ZOO_INFO@check_events@1775: 
> session establishment complete on server [192.168.99.100:2181], 
> sessionId=0x15b67b0efd3000e, negotiated timeout=2
> 

Re: Review Request 58421: Passed `--zk_session_timeout` to ZK master contender and detector.

[Ilya Pronin]

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

(Updated April 18, 2017, 3 p.m.)


Review request for mesos, Benjamin Mahler and Vinod Kone.


Changes
---

Addressed review comments.


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


Repository: mesos


Description
---

Currently `ZooKeeperMasterContender` and `ZooKeeperMasterDetector` use
hardcoded session timeouts and do not respect `--zk_session_timeout`
option. This patch fixes it.


Diffs (updated)
-

  include/mesos/master/contender.hpp 2c485ef2f24b3aa1b36b764b4780248835a1d9d5 
  include/mesos/master/detector.hpp f53c7c0bbeaed0b263d2634dc6eb15481d2d90b2 
  src/master/contender/contender.cpp cc486a176d14e61a2434ce3677c8741963a39057 
  src/master/contender/zookeeper.hpp becb93fbc04031c133f42f44f3cf406a27262444 
  src/master/contender/zookeeper.cpp 1aa8f53fc9b0733ac3edb2b4288b7225fac25828 
  src/master/detector/detector.cpp 0ba29ca9455b85f73cee6787e251b9442c6f97ea 
  src/master/detector/zookeeper.hpp 5b531e0f1fed7297bb73c5b02e8ef51d0c27ca38 
  src/master/detector/zookeeper.cpp a737d2403e480d14f1cd345b48af2833d2fa3284 
  src/master/main.cpp 90d159e248924f6806271dda1ced6d37dfa5f5c9 
  src/tests/master_contender_detector_tests.cpp 
c8c1ea0bf14909541e331ba026c754092d0e8fdd 


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

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


Testing
---

Ran `make check`.

Manually verified that all 4 ZK session negotiate 20 sec timeout when 
`--zk_session_timeout=20secs`.
```
2017-04-13 15:29:21,313:67863(0x7028d000):ZOO_INFO@zookeeper_init@800: 
Initiating client connection, host=192.168.99.100:2181 sessionTimeout=2 
watcher=0x107617130 sessionId=0 sessionPasswd= context=0x7f999ad15f90 
flags=0
2017-04-13 15:29:21,314:67863(0x7031):ZOO_INFO@zookeeper_init@800: 
Initiating client connection, host=192.168.99.100:2181 sessionTimeout=2 
watcher=0x107617130 sessionId=0 sessionPasswd= context=0x7f999ad164f0 
flags=0
2017-04-13 15:29:21,316:67863(0x7020a000):ZOO_INFO@zookeeper_init@800: 
Initiating client connection, host=192.168.99.100:2181 sessionTimeout=2 
watcher=0x107617130 sessionId=0 sessionPasswd= context=0x7f999ae05900 
flags=0
2017-04-13 15:29:21,316:67863(0x70104000):ZOO_INFO@zookeeper_init@800: 
Initiating client connection, host=192.168.99.100:2181 sessionTimeout=2 
watcher=0x107617130 sessionId=0 sessionPasswd= context=0x7f999ae05a80 
flags=0
2017-04-13 15:29:21,327:67863(0x70622000):ZOO_INFO@check_events@1775: 
session establishment complete on server [192.168.99.100:2181], 
sessionId=0x15b67b0efd3000c, negotiated timeout=2
2017-04-13 15:29:21,333:67863(0x70728000):ZOO_INFO@check_events@1775: 
session establishment complete on server [192.168.99.100:2181], 
sessionId=0x15b67b0efd3000d, negotiated timeout=2
2017-04-13 15:29:21,337:67863(0x7082e000):ZOO_INFO@check_events@1775: 
session establishment complete on server [192.168.99.100:2181], 
sessionId=0x15b67b0efd3000e, negotiated timeout=2
2017-04-13 15:29:21,347:67863(0x70934000):ZOO_INFO@check_events@1775: 
session establishment complete on server [192.168.99.100:2181], 
sessionId=0x15b67b0efd3000f, negotiated timeout=2
```


Thanks,

Ilya Pronin

Re: Review Request 58502: Removed containerizer flag logging to prevent leak of sensitive data.

[Till Toenshoff]

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

(Updated April 18, 2017, 1:50 p.m.)


Review request for mesos, Adam B, Alexander Rukletsov, and Benjamin Bannier.


Changes
---

Moved log output to be inline with original commit.


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


Repository: mesos


Description (updated)
---

See summary.

* backported for 1.0.x *


Diffs (updated)
-

  src/launcher/posix/executor.cpp 1c423f07b2c1b22b4d8aded274cae1295fe02a9d 


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

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


Testing
---

make check


Thanks,

Till Toenshoff

Re: Review Request 58505: Added MESOS-7265 to 1.0.4 CHANGELOG.

[Benjamin Bannier]

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


Ship it!




Ship It!

- Benjamin Bannier


On April 18, 2017, 3:33 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58505/
> ---
> 
> (Updated April 18, 2017, 3:33 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, and Benjamin Bannier.
> 
> 
> Bugs: MESOS-7265
> https://issues.apache.org/jira/browse/MESOS-7265
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   CHANGELOG d9a09853e0c6915908f212ceec2fefe9032f26e2 
> 
> 
> Diff: https://reviews.apache.org/r/58505/diff/1/
> 
> 
> Testing
> ---
> 
> Visual inspection.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Re: Review Request 58504: Added MESOS-7265 to 1.1.2 CHANGELOG.

[Benjamin Bannier]

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


Ship it!




Ship It!

- Benjamin Bannier


On April 18, 2017, 3:32 p.m., Till Toenshoff wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58504/
> ---
> 
> (Updated April 18, 2017, 3:32 p.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rukletsov, and Benjamin Bannier.
> 
> 
> Bugs: MESOS-7265
> https://issues.apache.org/jira/browse/MESOS-7265
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   CHANGELOG d9a09853e0c6915908f212ceec2fefe9032f26e2 
> 
> 
> Diff: https://reviews.apache.org/r/58504/diff/1/
> 
> 
> Testing
> ---
> 
> Visual inspection.
> 
> 
> Thanks,
> 
> Till Toenshoff
> 
>

Review Request 58505: Added MESOS-7265 to 1.0.4 CHANGELOG.

[Till Toenshoff]

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

Review request for mesos, Adam B, Alexander Rukletsov, and Benjamin Bannier.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  CHANGELOG d9a09853e0c6915908f212ceec2fefe9032f26e2 


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


Testing
---

Visual inspection.


Thanks,

Till Toenshoff

Review Request 58504: Added MESOS-7265 to 1.1.2 CHANGELOG.

[Till Toenshoff]

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

Review request for mesos, Adam B, Alexander Rukletsov, and Benjamin Bannier.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  CHANGELOG d9a09853e0c6915908f212ceec2fefe9032f26e2 


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


Testing
---

Visual inspection.


Thanks,

Till Toenshoff

Re: Review Request 57474: Added test for authorization of hierarchical roles.

[Alexander Rojas]

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

(Updated April 18, 2017, 3:28 p.m.)


Review request for mesos, Adam B and Benjamin Bannier.


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


Repository: mesos


Description
---

Adds tests for each of the actions which support hierarchical roles.


Diffs (updated)
-

  src/tests/authorization_tests.cpp 3e18c70738b6b7098f37fadebb799a596e76452d 


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

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


Testing
---

`make check`


Thanks,

Alexander Rojas

Review Request 58503: Removed containerizer flag logging to prevent leak of sensitive data.

[Till Toenshoff]

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

Review request for mesos, Adam B, Alexander Rukletsov, and Benjamin Bannier.


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


Repository: mesos


Description
---

See summary.

* backported for 1.1.x *


Diffs
-

  src/launcher/posix/executor.cpp da0081c0e470aebb16d2e78031d276f5d7d2c726 


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


Testing
---

make check


Thanks,

Till Toenshoff

Review Request 58502: Removed containerizer flag logging to prevent leak of sensitive data.

[Till Toenshoff]

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

Review request for mesos, Adam B, Alexander Rukletsov, and Benjamin Bannier.


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


Repository: mesos


Description
---

See summary.

* backported for 1.0.x *


Diffs
-

  src/launcher/posix/executor.cpp 1c423f07b2c1b22b4d8aded274cae1295fe02a9d 


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


Testing
---

make check


Thanks,

Till Toenshoff

Re: Review Request 58502: Removed containerizer flag logging to prevent leak of sensitive data.

[Till Toenshoff]

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

(Updated April 18, 2017, 1:10 p.m.)


Review request for mesos, Adam B, Alexander Rukletsov, and Benjamin Bannier.


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


Repository: mesos


Description
---

See summary.

* backported for 1.0.x *


Diffs
-

  src/launcher/posix/executor.cpp 1c423f07b2c1b22b4d8aded274cae1295fe02a9d 


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


Testing
---

make check


Thanks,

Till Toenshoff

Re: Review Request 57824: Filtered stout tests with symlinks when unable to create symlinks.

[Jan Schlicht]

> On April 18, 2017, 1:56 p.m., Jan Schlicht wrote:
> > Because I need to implement similar filters for tests in `libprocess`, 
> > wouldn't it have been better to create a `Filter` abstraction in stout and 
> > using that here as well as in Mesos instead of copy-pasting the 
> > implementation of Mesos?
> > Also the copied Mesos code seems to not quite fit the stout code-style, 
> > e.g. it uses camel case instead of snake case.

Oh, seems like the same filters should be using in `libprocess` as well, as per 
https://reviews.apache.org/r/57971/. Not a good choice because the filters that 
need to be used for stout might differ from the ones used in libprocess. I'll 
probably have to refactor the current filter handling to support this use case 
(i.e. applying a filter only in `libprocess-tests`).


- Jan


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


On April 10, 2017, 11:41 p.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57824/
> ---
> 
> (Updated April 10, 2017, 11:41 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.
> 
> 
> Bugs: MESOS-6731
> https://issues.apache.org/jira/browse/MESOS-6731
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Filtered stout tests with symlinks when unable to create symlinks.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/tests/environment.hpp PRE-CREATION 
>   3rdparty/stout/tests/main.cpp f41b29c087402d5dbed1144504980746e0e9b6da 
>   3rdparty/stout/tests/os/filesystem_tests.cpp 
> e445daf9ed4e4c0d44dbb95e1cbebd0342c1acbc 
>   3rdparty/stout/tests/os/rmdir_tests.cpp 
> ed43b44663cbf04d7ddb449fd9f42b8de210bc6e 
>   3rdparty/stout/tests/os_tests.cpp 8b9531443f625ae0e1f00d29e9e45299f181880e 
> 
> 
> Diff: https://reviews.apache.org/r/57824/diff/4/
> 
> 
> Testing
> ---
> 
> Ran make check on Linux and the tests on Windows.
> 
> 
> Thanks,
> 
> John Kordich
> 
>

Re: Review Request 57824: Filtered stout tests with symlinks when unable to create symlinks.

[Jan Schlicht]

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



Because I need to implement similar filters for tests in `libprocess`, wouldn't 
it have been better to create a `Filter` abstraction in stout and using that 
here as well as in Mesos instead of copy-pasting the implementation of Mesos?
Also the copied Mesos code seems to not quite fit the stout code-style, e.g. it 
uses camel case instead of snake case.

- Jan Schlicht


On April 10, 2017, 11:41 p.m., John Kordich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57824/
> ---
> 
> (Updated April 10, 2017, 11:41 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Joseph Wu.
> 
> 
> Bugs: MESOS-6731
> https://issues.apache.org/jira/browse/MESOS-6731
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Filtered stout tests with symlinks when unable to create symlinks.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/tests/environment.hpp PRE-CREATION 
>   3rdparty/stout/tests/main.cpp f41b29c087402d5dbed1144504980746e0e9b6da 
>   3rdparty/stout/tests/os/filesystem_tests.cpp 
> e445daf9ed4e4c0d44dbb95e1cbebd0342c1acbc 
>   3rdparty/stout/tests/os/rmdir_tests.cpp 
> ed43b44663cbf04d7ddb449fd9f42b8de210bc6e 
>   3rdparty/stout/tests/os_tests.cpp 8b9531443f625ae0e1f00d29e9e45299f181880e 
> 
> 
> Diff: https://reviews.apache.org/r/57824/diff/4/
> 
> 
> Testing
> ---
> 
> Ran make check on Linux and the tests on Windows.
> 
> 
> Thanks,
> 
> John Kordich
> 
>

Re: Review Request 57254: Added support for hierarchical roles to DRFSorter.

[Jay Guo]

> On April 17, 2017, 11:09 p.m., Jay Guo wrote:
> > src/master/allocator/sorter/drf/sorter.cpp
> > Line 333 (original), 509 (patched)
> > 
> >
> > In case of `dirty`, how about insert logic of `listClients` there to 
> > avoid traversing tree twice for the sake of performance? I tried a 
> > benchmark test which does suggest a performance benefit.
> 
> Neil Conway wrote:
> That could be a useful improvement. When you benchmarked this, what % 
> performance improvement did you observe?

Approximately 10%


- Jay


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


On April 18, 2017, 12:58 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57254/
> ---
> 
> (Updated April 18, 2017, 12:58 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This commit replaces the sorter's flat list of clients with a tree; the
> tree represents the hierarchical relationship between sorter clients.
> Each node in the tree contains a vector of pointers to child nodes. The
> tree might contain nodes that do not correspond directly to sorter
> clients. For example, adding clients "a/b" and "c/d" results in the
> following tree:
> 
> root
>  -> a
>-> b
>  -> c
>-> d
> 
> The `sort` member function still only returns one result for each active
> client in the sorter. This is implemented by ensuring that each sorter
> client is associated with a leaf node in the tree. Note that it is
> possible for a leaf node to be transformed into an internal node by a
> subsequent insertion; to handle this case, we "implicitly" create an
> extra child node, which maintains the invariant that each client has a
> leaf node. For example, if the client "a/b/x" is added to the tree
> above, the result is:
> 
> root
>  -> a
>-> b
>  -> .
>  -> x
>  -> c
>-> d
> 
> The "." leaf node holds the allocation that has been made to the "a/b"
> client itself; the "a/b" node holds the sum of all the allocations that
> have been made to the subtree rooted at "a/b", which also includes
> "a/b/x".
> 
> This commit also introduces a new approach to sorting: rather than
> keeping an `std::set` of sorter clients, we now keep a tree of
> `std::vector`, which is sorted explicitly via `std::sort`. The previous
> implementation tried to optimize the sorting process by updating the
> sort order incrementally when a single sorter client was updated; this
> commit removes that optimization, and instead re-sorts the entire tree
> whenever the sort order is changed.
> 
> Re-introducing a version of this optimization would make sense in the
> future (MESOS-7390), but benchmarking suggests that this commit results
> in a net improvement in sorter performance anyway. The sorter perf
> improvement is likely due to the introduction of a secondary hashmap
> that allows the leaf node associated with a tree path to be find
> efficiently; the previous implementation required a linear scan of a
> `std::set`.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/sorter/drf/metrics.cpp 
> 15aab32db5ca1a7a14080e9bbb7c65283be3ec20 
>   src/master/allocator/sorter/drf/sorter.hpp 
> 76329220e1115c1de7810fb69b943c78c078be59 
>   src/master/allocator/sorter/drf/sorter.cpp 
> ed54680cecb637931fc344fbcf8fd3b14cc24295 
>   src/master/allocator/sorter/sorter.hpp 
> b3029fcf7342406955760da53f1ae736769f308c 
>   src/tests/hierarchical_allocator_tests.cpp 
> 33e7b455f8664858eb4f03727b076a10c80cd6e0 
>   src/tests/master_allocator_tests.cpp 
> 119e318f8a01d50e8dae5c30cf5fa6a017c3c625 
>   src/tests/sorter_tests.cpp 43bd85798aef0c89751b725ebf35308a5e9e997a 
> 
> 
> Diff: https://reviews.apache.org/r/57254/diff/19/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 58021: Added storage-related offer operations.

[Jie Yu]

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




src/common/protobuf_utils.cpp
Lines 435-448 (original), 435-452 (patched)


why pull those outside the function? Can we still keep them inside 
injectAllocationInfo?



src/common/protobuf_utils.cpp
Lines 577-581 (patched)


Ditto here.


- Jie Yu


On April 4, 2017, 12:47 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58021/
> ---
> 
> (Updated April 4, 2017, 12:47 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-7314
> https://issues.apache.org/jira/browse/MESOS-7314
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added storage-related offer operations.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto dd90465cc3da283c078d4e907cc6a4a0e50309ac 
>   include/mesos/v1/mesos.proto 228623155c7f68c0f24d173aacbc6eb734f1382f 
>   src/common/protobuf_utils.cpp 3fcaf786b29a00f003c10b0f1614a2c7eddc725d 
>   src/common/resources.cpp c26e0f995006dc6b2e70a491cea58fa90347e42a 
>   src/master/master.cpp 6a6a570e52d21bfb2443f981e3d7faf8c36f74bc 
>   src/v1/resources.cpp a53deafbea399a1bcf729d1c151bc46e9da04e11 
> 
> 
> Diff: https://reviews.apache.org/r/58021/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 58463: Overwriting Symbolic Links with Files in Copy Provisioner.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On April 17, 2017, 9:52 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58463/
> ---
> 
> (Updated April 17, 2017, 9:52 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-6327
> https://issues.apache.org/jira/browse/MESOS-6327
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When a layer overwrites a symbolic link with a regular file, the link
> must be removed first, otherwise 'cp' would follow the link and
> overwrite the target instead of the link itself.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/backends/copy.cpp 
> 584cc6524f81cc1bc231b105507dbfe51fec991d 
> 
> 
> Diff: https://reviews.apache.org/r/58463/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> Manually tested with the following docker images:
>   https://hub.docker.com/r/anldisr/mesos_layers/ (provided by the reporter)
>   https://hub.docker.com/r/chhsiao/mycrosslink/  (simplist image to reproduce 
> the same bug)
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 58408: Overwriting Directories with Files in Copy Provisioner.

[Jie Yu]

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


Fix it, then Ship it!





src/slave/containerizer/mesos/provisioner/backends/copy.cpp
Lines 200 (patched)


I would add extra parathesis for the second half:
```
if (os::exists(rootfsPath) &&
(os::stat::isdir(rootfsPath) != ftsIsDir)) {
  ...
}
```

This improves readability because the reader might not understand the 
operator priority.


- Jie Yu


On April 17, 2017, 9:44 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58408/
> ---
> 
> (Updated April 17, 2017, 9:44 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Bugs: MESOS-5028
> https://issues.apache.org/jira/browse/MESOS-5028
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> When a layer overwrites a directory with a regular file or symbolic link
> (or vice versa), the old dir/file need to be removed before copying the
> layer into the rootfs. This is processed together with whiteout:
> The copy provisioner find all files to remove, including files
> marked as whiteout and the files described above, and remove them
> before the copy process.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/provisioner/backends/copy.cpp 
> 584cc6524f81cc1bc231b105507dbfe51fec991d 
> 
> 
> Diff: https://reviews.apache.org/r/58408/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> Manually tested on the following images:
>   https://hub.docker.com/r/gilbertsong/whiteout/
>   https://hub.docker.com/r/chhsiao/overwrite/
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>