Re: Review Request 59320: Added test case for agent re-registration.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [59213, 59214, 59215, 59216, 59217, 59218, 59219, 59259, 
59301, 59302, 59320]

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 May 16, 2017, 8:19 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59320/
> ---
> 
> (Updated May 16, 2017, 8:19 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add an explanatory comment and a test case for a particular case in the
> master's logic for handling agent re-registration.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 4e7a161f431624bd78d3e9032eb8587687149cad 
>   src/tests/partition_tests.cpp 4ff428564d1fa6cb96e6f8ec8edc331da88a3eb6 
> 
> 
> Diff: https://reviews.apache.org/r/59320/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> Checked that if you add `CHECK_NOTNULL(framework)` to the master code (circa 
> the newly added comment), the incorrect `CHECK` is triggered by this test 
> case (but not by any existing test cases).
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

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 May 17, 2017, 3:50 a.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/4/

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


Testing
---


Thanks,

Zhitao Li

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

[Chun-Hung Hsiao]

> On April 19, 2017, 6:28 p.m., Zhitao Li wrote:
> > src/java/mesos.pom.in
> > Line 86 (original), 86 (patched)
> > 
> >
> > This is simply done by a sweeping grep and replace in the code base.
> > 
> > I don't know whether we want to force framework to move to protobuf 
> > 3.2.0. I guess if the framework author wants to use proto2, they should 
> > still be able to do it with the same `syntax=proto2` annotation as what 
> > Mesos does?
> > 
> > I'm fine to keep this out of the current changeset for now, although I 
> > don't know how compatible protobuf-java at lower version can handle newer 
> > java classes.
> > 
> > Do we have formal rules about compatibility on released libraries?
> 
> Chun-Hung Hsiao wrote:
> This needs to be set to the same version, otherwise maven won't be able 
> to find the generated `protobuf-java-3.3.1.jar` and will produce `package 
> com.google.protobuf.GeneratedMessageV3 does not exist` errors. I'm not 
> familiar with maven so not sure if this can be resolved without restricting 
> the dependency.
> 
> Zhitao Li wrote:
> Thanks. So we are going to force framework owners to upgrade to protobuf3 
> once they upgrade to Mesos 1.4 maven packages.

I guess we can either do this or do some autoconf stuff with maven. I'm new to 
here and not sure which is more appropriate ;p Personally don't have a strong 
feeling about staying with the current dependency.


- Chun-Hung


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


On April 18, 2017, 6:25 p.m., Zhitao Li wrote:
> 
> ---
> 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
> ---
> 
> Update Mesos build library to use protobuf 3.2.0.
> 
> 
> Diffs
> -
> 
>   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/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: 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/
---

(Updated May 17, 2017, 3:47 a.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
---

Update 3rdparty build to support protobuf 3.2.0.


Diffs (updated)
-

  3rdparty/cmake/Versions.cmake 728f88fe57aef4dd7be1d40c7c0227ad2db5015e 
  3rdparty/versions.am b8144702adca9dd41199f69cb6fb0c0c4b490dbd 


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

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


Testing
---


Thanks,

Zhitao Li

Re: 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/
---

(Updated May 17, 2017, 3:46 a.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
---

Update related tests in stout to support protobuf 3.2.0.


Diffs (updated)
-

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

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


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 May 17, 2017, 3:46 a.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
---

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


Diffs (updated)
-

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


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

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


Testing
---


Thanks,

Zhitao Li

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

[Chun-Hung Hsiao]

> On May 16, 2017, 10:27 p.m., Chun-Hung Hsiao wrote:
> > I personally don't like using an RC version. We should update protobuf to 
> > an official release, say 3.3.1?
> 
> Zhitao Li wrote:
> IIRC the rc2 tar was what's available when I looked at the download page. 
> It seems like they have fixed the symlink, so I can download this one more 
> time and get it updated, although I don't expect any difference.
> 
> For version, 3.3.x was not yet available when we looked at this. Now that 
> it's out, I'm personall okay if we want to go with latest, although it seems 
> like all issues between 3.2 and 3.3 is not critical to us?

3.2.x should be fine. I just looked up what's the difference between 3.2.0 and 
3.2.1. It seems that there're some fixes for ruby, objective-c and its CLI. 
Probably nothing critical.


- Chun-Hung


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


On May 17, 2017, 3:45 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58358/
> ---
> 
> (Updated May 17, 2017, 3:45 a.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.0.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/5/
> 
> 
> Testing
> ---
> 
> 1. make check;
> 2. used the modified `test-upgrade.py` in r/58357 to test upgrade/downgrade 
> between this chain and Mesos 1.2;
> 3. build this branch in Apache Aurora's vagrant based box, and verifies that 
> Aurora scheduler (using java) and executor (using python) still works with 
> `libmesos.so` built from this branch.
> 
> 
> 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 May 17, 2017, 3:45 a.m.)


Review request for mesos, Anand Mazumdar and Joseph Wu.


Changes
---

Rebase and use official source tar.gz instead.


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

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


Testing
---

1. make check;
2. used the modified `test-upgrade.py` in r/58357 to test upgrade/downgrade 
between this chain and Mesos 1.2;
3. build this branch in Apache Aurora's vagrant based box, and verifies that 
Aurora scheduler (using java) and executor (using python) still works with 
`libmesos.so` built from this branch.


Thanks,

Zhitao Li

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

[Zhitao Li]

> On April 19, 2017, 5:07 p.m., Anand Mazumdar wrote:
> > src/tests/protobuf_utils_tests.cpp
> > Lines 289 (patched)
> > 
> >
> > Can you add a comment around why we added this test?
> 
> Chun-Hung Hsiao wrote:
> It seems to me that if we don't want to enforce a hard dependency on 
> protobuf-3.x, we should not add it.

I don't mind taking out the test, but I definitely need to make sure we are 
committed to supporting large message (>64MB).

The only alternative I can see is to ensure we have proper API design to 
paginate any potential protobuf message which could go large within the 64MB 
limit imposed by protobuf2.

AFAIK both `GET_STATE` and `OFFERS` messages could exceed this and render the 
cluster not usable for an important use case.

Do you have any concern about a hard dependency to protobuf 3.x?


- Zhitao


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


On April 19, 2017, 4:23 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58360/
> ---
> 
> (Updated April 19, 2017, 4:23 p.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> 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
> -
> 
>   src/tests/protobuf_utils_tests.cpp 5239182812835b93a28e85146b2df2b20ae77328 
> 
> 
> Diff: https://reviews.apache.org/r/58360/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

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

[Zhitao Li]

> On April 19, 2017, 6:28 p.m., Zhitao Li wrote:
> > src/java/mesos.pom.in
> > Line 86 (original), 86 (patched)
> > 
> >
> > This is simply done by a sweeping grep and replace in the code base.
> > 
> > I don't know whether we want to force framework to move to protobuf 
> > 3.2.0. I guess if the framework author wants to use proto2, they should 
> > still be able to do it with the same `syntax=proto2` annotation as what 
> > Mesos does?
> > 
> > I'm fine to keep this out of the current changeset for now, although I 
> > don't know how compatible protobuf-java at lower version can handle newer 
> > java classes.
> > 
> > Do we have formal rules about compatibility on released libraries?
> 
> Chun-Hung Hsiao wrote:
> This needs to be set to the same version, otherwise maven won't be able 
> to find the generated `protobuf-java-3.3.1.jar` and will produce `package 
> com.google.protobuf.GeneratedMessageV3 does not exist` errors. I'm not 
> familiar with maven so not sure if this can be resolved without restricting 
> the dependency.

Thanks. So we are going to force framework owners to upgrade to protobuf3 once 
they upgrade to Mesos 1.4 maven packages.


> On April 19, 2017, 6:28 p.m., Zhitao Li wrote:
> > src/python/interface/setup.py.in
> > Line 29 (original), 29 (patched)
> > 
> >
> > Similar to the comment above on java.
> 
> Chun-Hung Hsiao wrote:
> The Python unit test works fine without changing the protobuf version 
> dependency FYI.

I still feel that that we should also ask any python framework owners to 
upgrade to proto3 once they go to Mesos 1.4, but this is not a strong feeling 
and I'm okay to keep this as old value.


- Zhitao


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


On April 18, 2017, 6:25 p.m., Zhitao Li wrote:
> 
> ---
> 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
> ---
> 
> Update Mesos build library to use protobuf 3.2.0.
> 
> 
> Diffs
> -
> 
>   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/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

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

[Zhitao Li]

> On May 16, 2017, 10:27 p.m., Chun-Hung Hsiao wrote:
> > I personally don't like using an RC version. We should update protobuf to 
> > an official release, say 3.3.1?

IIRC the rc2 tar was what's available when I looked at the download page. It 
seems like they have fixed the symlink, so I can download this one more time 
and get it updated, although I don't expect any difference.

For version, 3.3.x was not yet available when we looked at this. Now that it's 
out, I'm personall okay if we want to go with latest, although it seems like 
all issues between 3.2 and 3.3 is not critical to us?


- Zhitao


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


On April 25, 2017, 10:51 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58358/
> ---
> 
> (Updated April 25, 2017, 10:51 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`.
> 
> Note that reviewboard seems to have problem for uploading/dowloading binary 
> patches, so this particular commit is also available at 
> https://github.com/apache/mesos/pull/208
> 
> 
> 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
> ---
> 
> 1. make check;
> 2. used the modified `test-upgrade.py` in r/58357 to test upgrade/downgrade 
> between this chain and Mesos 1.2;
> 3. build this branch in Apache Aurora's vagrant based box, and verifies that 
> Aurora scheduler (using java) and executor (using python) still works with 
> `libmesos.so` built from this branch.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 58486: Update the allocator on a per offer operation.

[Anindya Sinha]

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

(Updated May 17, 2017, 2 a.m.)


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


Changes
---

Validate that the valid operations can be processed in `Master::__accept()`.


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


Repository: mesos


Description
---

The allocator is updated for each offer operation in the order specified
in the `ACCEPT` call. Once the allocator is updated successfully, we
handle subsequent operations as follows:

1) Launch or Launch Group: We launch the tasks or task group on the
   agent.
2) (Un)reservation or Create/Destroy of Persistent Volumes: We send
   the `CheckPointResourcesMessage` to the agent.

If allocation for any of the operations fail, those resources are not
recovered from the allocator.


Diffs (updated)
-

  src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732 
  src/master/master.cpp 4e7a161f431624bd78d3e9032eb8587687149cad 


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

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


Testing
---

Tests passed.


Thanks,

Anindya Sinha

Re: Review Request 59195: Ensure that allocator can be updated before committing changes.

[Anindya Sinha]

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

(Updated May 17, 2017, 2 a.m.)


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


Changes
---

Check if `slave.allocated.apply()` can be done before iterating through 
individual operations.


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


Repository: mesos


Description (updated)
---

In addition to `apply()` on `offeredResources`, we ensure all operations
can be applied on `slave.allocated`. It is is so because certain
operations require that they are applicable to the agent's allocated
resources as a whole.
Note that we return a failed future on any error.We do not
modify the state of the allocator if any of the operations fail.


Diffs (updated)
-

  include/mesos/allocator/allocator.hpp 
dc34a1b6f0c2bdf0d653bd53394364cfa9873ca7 
  src/master/allocator/mesos/allocator.hpp 
119b461136123229f85ed3c3cfd41137974a6b9b 
  src/master/allocator/mesos/hierarchical.hpp 
123f97cf495bff0f822838e09df0d88818f04da6 
  src/master/allocator/mesos/hierarchical.cpp 
b75ed9a20a9a42f958cebbacd91e5e15b0d21394 
  src/tests/allocator.hpp 4ea722491f63f5ceda5a47228aafddc020f643b0 
  src/tests/hierarchical_allocator_tests.cpp 
08180b9975869de328f0c095dd3cddf0c84fbecf 


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

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


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 59290: Added 'registrar/log/ensemble_size' metric.

[Jiang Yan Xu]

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

(Updated May 16, 2017, 6:02 p.m.)


Review request for mesos, Anindya Sinha and Benjamin Mahler.


Changes
---

Changed to a more descriptive name. NNFR.


Summary (updated)
-

Added 'registrar/log/ensemble_size' metric.


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


Repository: mesos


Description (updated)
---

Added 'registrar/log/ensemble_size' metric.

Will update monitoring.md next.


Diffs (updated)
-

  src/log/log.hpp a600025f4ca38e3fad0f64f48b007138da8e22d4 
  src/log/metrics.hpp PRE-CREATION 
  src/log/metrics.cpp PRE-CREATION 
  src/tests/log_tests.cpp 52b1bfedd8d3f8f2ab3308c4be9f167126ee694b 


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

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


Testing
---

make check.


Thanks,

Jiang Yan Xu

Re: Review Request 56681: Use glog to log EXIT() messages.

[Jiang Yan Xu]

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


Ship it!




Ship It!

- Jiang Yan Xu


On May 16, 2017, 4:19 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56681/
> ---
> 
> (Updated May 16, 2017, 4:19 p.m.)
> 
> 
> Review request for mesos, haosdent huang and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7115
> https://issues.apache.org/jira/browse/MESOS-7115
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Use glog to log EXIT() messages.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/stout/exit.hpp 
> 762864afd8f86f7e1f439ef6ea7e3965a5f147d5 
> 
> 
> Diff: https://reviews.apache.org/r/56681/diff/3/
> 
> 
> Testing
> ---
> 
> make check (Fedora 25)
> 
> **Example output after this change:**
> ```
> jpeach@jpeach src]$ sudo ./mesos-agent --work_dir=/nothing/here 
> --master=phoney:5050
> WARNING: Logging before InitGoogleLogging() is written to STDERR
> I0516 16:31:34.630625  4987 main.cpp:322] Build: 2017-05-11 11:40:39 by jpeach
> I0516 16:31:34.630748  4987 main.cpp:323] Version: 1.4.0
> I0516 16:31:34.630760  4987 main.cpp:330] Git SHA: 
> 62e0f726198e90d63e9e88156432d833ca88476c
> I0516 16:31:34.647601  4987 systemd.cpp:238] systemd version `231` detected
> I0516 16:31:34.647650  4987 main.cpp:432] Inializing systemd state
> I0516 16:31:34.656100  4987 systemd.cpp:326] Started systemd slice 
> `mesos_executors.slice`
> I0516 16:31:34.658421  4987 containerizer.cpp:221] Using isolation: 
> posix/cpu,posix/mem,filesystem/posix,network/cni
> I0516 16:31:34.668910  4987 linux_launcher.cpp:150] Using 
> /sys/fs/cgroup/freezer as the freezer hierarchy for the Linux launcher
> I0516 16:31:34.670874  4987 provisioner.cpp:249] Using default backend 
> 'overlay'
> E0516 16:31:34.684639  4987 main.cpp:461] EXIT with status 1: Failed to 
> create a master detector: Failed to parse 'phoney:5050'
> ```
> 
> **Example output before this change:**
> ```
> [jpeach@jpeach build]$ sudo /opt/mesos/sbin//mesos-agent 
> --work_dir=/nothing/here --master=phoney:5050
> WARNING: Logging before InitGoogleLogging() is written to STDERR
> I0320 11:03:43.868015  1188 main.cpp:243] Build: 2017-03-10 10:49:55 by jpeach
> I0320 11:03:43.868135  1188 main.cpp:244] Version: 1.3.0
> I0320 11:03:43.868151  1188 main.cpp:251] Git SHA: 
> 487c667260ec89127e18c77b9e8c8c243cf6ec57
> I0320 11:03:43.886561  1188 systemd.cpp:238] systemd version `231` detected
> I0320 11:03:43.886631  1188 main.cpp:342] Inializing systemd state
> I0320 11:03:43.896606  1188 systemd.cpp:326] Started systemd slice 
> `mesos_executors.slice`
> I0320 11:03:43.899202  1188 containerizer.cpp:221] Using isolation: 
> posix/cpu,posix/mem,filesystem/posix,network/cni
> I0320 11:03:43.911183  1188 linux_launcher.cpp:150] Using 
> /sys/fs/cgroup/freezer as the freezer hierarchy for the Linux launcher
> I0320 11:03:43.913216  1188 provisioner.cpp:249] Using default backend 
> 'overlay'
> Failed to create a master detector: Failed to parse 'phoney:5050'
> ```
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 59318: Replace an EXPECT with an ASSERT in master_tests to avoid segfaults.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [58923, 58924, 58925, 58759, 58760, 58999, 59000, 59001, 59318]

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 May 16, 2017, 7:28 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59318/
> ---
> 
> (Updated May 16, 2017, 7:28 p.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   src/tests/master_tests.cpp 37a54138b03229778bc62501532e698e87760218 
> 
> 
> Diff: https://reviews.apache.org/r/59318/diff/1/
> 
> 
> Testing
> ---
> 
> Make check with an without the patch.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 58351: Updated GMock/GTest in libprocess.

[Neil Conway]

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


Ship it!




Ship It!

- Neil Conway


On April 11, 2017, 12:31 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58351/
> ---
> 
> (Updated April 11, 2017, 12:31 p.m.)
> 
> 
> Review request for mesos, Neil Conway and Till Toenshoff.
> 
> 
> Bugs: MESOS-7364
> https://issues.apache.org/jira/browse/MESOS-7364
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will fix various issues that occured with version 1.7.0.
> As GTest is now using 'std::tuple' instead of 'std::tr1::tuple', some
> test cases have been updated accordingly.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/3rdparty/Makefile.am 
> 9dc5fb51d8120d0b26b3f158f69191e468694275 
>   3rdparty/libprocess/Makefile.am 80758d60cd2b391105c30151883c3c7dbf763a31 
>   3rdparty/libprocess/cmake/ProcessTestsConfigure.cmake 
> 31e20e475a4f51ea74eeebae2a026264589e8dda 
>   3rdparty/libprocess/configure.ac 1540006e6189cf6618d7f1df4d4b9544bd1c25ea 
>   3rdparty/libprocess/include/process/gmock.hpp 
> 05d8b8e55ab3aa526adb0f7df4683cbd535926f8 
> 
> 
> Diff: https://reviews.apache.org/r/58351/diff/1/
> 
> 
> Testing
> ---
> 
> make check (on macOS, Linux, Windows)
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 58348: Fixed use of 'GTEST_IS_THREADSAFE'.

[Neil Conway]

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


Ship it!




Ship It!

- Neil Conway


On May 15, 2017, 10:24 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58348/
> ---
> 
> (Updated May 15, 2017, 10:24 a.m.)
> 
> 
> Review request for mesos, Neil Conway and Till Toenshoff.
> 
> 
> Bugs: MESOS-7193
> https://issues.apache.org/jira/browse/MESOS-7193
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixed use of 'GTEST_IS_THREADSAFE'.
> 
> 
> Diffs
> -
> 
>   src/tests/environment.cpp 3d2d8f177b2793f06d6adecfd110819bb7bbb86d 
>   src/tests/main.cpp 5d062c3451bdfb5d5fc459ac7c071ab18e6d8043 
> 
> 
> Diff: https://reviews.apache.org/r/58348/diff/3/
> 
> 
> Testing
> ---
> 
> make check (using automake and CMake with macOS, Linux and Windows)
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 58347: Introduced a filter for test cases that need thread-safety.

[Neil Conway]

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


Ship it!




Ship It!

- Neil Conway


On May 15, 2017, 10:25 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58347/
> ---
> 
> (Updated May 15, 2017, 10:25 a.m.)
> 
> 
> Review request for mesos, Neil Conway and Till Toenshoff.
> 
> 
> Bugs: MESOS-7193
> https://issues.apache.org/jira/browse/MESOS-7193
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Instead of using asserts that would fail a test at runtime when
> thread-safety of the test environment isn't given, these tests are now
> filtered out before running the tests.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/tests/future_tests.cpp 
> f21361ed1e354778bcd0357afb71300f05d3ecfd 
>   3rdparty/libprocess/src/tests/io_tests.cpp 
> 302fadc2a1894d3fd7c4f4975af3f2cc6c3a22de 
>   3rdparty/libprocess/src/tests/limiter_tests.cpp 
> b80b1da214f97b50aa7b61b79bbf683fd01116aa 
>   3rdparty/libprocess/src/tests/main.cpp 
> 23eead809d1f0ddc9a28f492005ff325701bac59 
>   3rdparty/libprocess/src/tests/metrics_tests.cpp 
> d7fdb06060b273e16be27a263b5ee268842aa25c 
>   3rdparty/libprocess/src/tests/process_tests.cpp 
> bf90c7e78fd50ad7e16cc0a69a248ba71e2a7115 
>   3rdparty/libprocess/src/tests/reap_tests.cpp 
> 30518dee6c2fb904a607c7a457a5ec7366aab818 
> 
> 
> Diff: https://reviews.apache.org/r/58347/diff/3/
> 
> 
> Testing
> ---
> 
> tested in https://reviews.apache.org/r/58348/, don't commit without commiting 
> 58348.
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 58350: Updated GMock/GTest in stout.

[Neil Conway]

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


Ship it!




Commit description seems inaccurate: this commit does not remove references to 
`tr1`.

- Neil Conway


On May 15, 2017, 10:28 a.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58350/
> ---
> 
> (Updated May 15, 2017, 10:28 a.m.)
> 
> 
> Review request for mesos, Neil Conway and Till Toenshoff.
> 
> 
> Bugs: MESOS-7364
> https://issues.apache.org/jira/browse/MESOS-7364
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will fix various issues that occured with version 1.7.0.
> As GTest is now using 'std::tuple' instead of 'std::tr1::tuple', some
> test cases have been updated accordingly.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/3rdparty/Makefile.am 
> 73f6e058eff6ed8b08d119fcb61bc28c8d30c346 
>   3rdparty/stout/Makefile.am ebf1069eb1b787f063a2066a4db0b3f5de4a56da 
>   3rdparty/stout/cmake/StoutTestsConfigure.cmake 
> 685f1c840e1bdad0dcc4c6b0629a944cffbb309f 
>   3rdparty/stout/configure.ac 50c1f6f0b1935a3770724c10f651190dcf002633 
> 
> 
> Diff: https://reviews.apache.org/r/58350/diff/2/
> 
> 
> Testing
> ---
> 
> Tested in https://reviews.apache.org/r/58351/
> 
> Don't commit without 58351.
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Re: Review Request 58349: Updated GMock/GTest to version 1.8.0.

[Neil Conway]

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


Ship it!




There are more `tr1` references in `sorter_tests.cpp` -- can you fix these?

See also earlier comments on updating gmock references in `LICENSE` and 
`entrypoint.sh`.

- Neil Conway


On May 15, 2017, 12:05 p.m., Jan Schlicht wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58349/
> ---
> 
> (Updated May 15, 2017, 12:05 p.m.)
> 
> 
> Review request for mesos, Neil Conway and Till Toenshoff.
> 
> 
> Bugs: MESOS-7364
> https://issues.apache.org/jira/browse/MESOS-7364
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This will fix various issues that occured with version 1.7.0.
> As GTest is now using 'std::tuple' instead of 'std::tr1::tuple', some
> test cases have been updated accordingly.
> 
> 
> Diffs
> -
> 
>   3rdparty/CMakeLists.txt cb118f6c454c3bb36990e292a31703e4a3f99483 
>   3rdparty/Makefile.am 61d832b2d83cdeaf95341c062e8493ab72d0724e 
>   3rdparty/cmake/Versions.cmake 728f88fe57aef4dd7be1d40c7c0227ad2db5015e 
>   3rdparty/gmock-1.7.0.tar.gz 09f5ea3ce95fab34c505367ae04965e01c8bb30d 
>   3rdparty/googletest-release-1.8.0.tar.gz PRE-CREATION 
>   3rdparty/versions.am b8144702adca9dd41199f69cb6fb0c0c4b490dbd 
>   configure.ac fe5b20b53ea007dd8f2e3349e9904f6c4f43a14b 
>   src/Makefile.am 6bb81fd49b4564a0afa993b2cef6baa9d370ee7a 
>   src/tests/cmake/MesosTestsConfigure.cmake 
> 62e274114e7ab062e037ef194781828146e15fed 
>   src/tests/fetcher_cache_tests.cpp 3bd63ed0a66493829a82c542ad05ebe0f7828d1a 
>   src/tests/hierarchical_allocator_tests.cpp 
> 08180b9975869de328f0c095dd3cddf0c84fbecf 
>   src/tests/scheduler_tests.cpp 0f5d9ada6eb880379baf5f106fd2d5b12e9738db 
>   src/tests/sorter_tests.cpp 8e7ff79a0401ed721429752be4b010727b875e11 
> 
> 
> Diff: https://reviews.apache.org/r/58349/diff/4/
> 
> 
> Testing
> ---
> 
> Tested in https://reviews.apache.org/r/58351/
> 
> Don't commit without 58350 and 58351.
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

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

[Chun-Hung Hsiao]

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


Ship it!




Ship It!

- Chun-Hung Hsiao


On April 19, 2017, 4:23 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58361/
> ---
> 
> (Updated April 19, 2017, 4: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
> ---
> 
> Content copied from https://github.com/google/protobuf/blob/v3.2.0/LICENSE.
> 
> 
> Diffs
> -
> 
>   LICENSE 512b089c5dd2540f9f83afb559e5d0998d97ea22 
> 
> 
> Diff: https://reviews.apache.org/r/58361/diff/7/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

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

[Chun-Hung Hsiao]

> On April 19, 2017, 5:07 p.m., Anand Mazumdar wrote:
> > src/tests/protobuf_utils_tests.cpp
> > Lines 289 (patched)
> > 
> >
> > Can you add a comment around why we added this test?

It seems to me that if we don't want to enforce a hard dependency on 
protobuf-3.x, we should not add it.


- Chun-Hung


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


On April 19, 2017, 4:23 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58360/
> ---
> 
> (Updated April 19, 2017, 4:23 p.m.)
> 
> 
> Review request for mesos and Anand Mazumdar.
> 
> 
> 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
> -
> 
>   src/tests/protobuf_utils_tests.cpp 5239182812835b93a28e85146b2df2b20ae77328 
> 
> 
> Diff: https://reviews.apache.org/r/58360/diff/3/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 56681: Use glog to log EXIT() messages.

[James Peach]

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

(Updated May 16, 2017, 11:19 p.m.)


Review request for mesos, haosdent huang and Jiang Yan Xu.


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


Repository: mesos


Description
---

Use glog to log EXIT() messages.


Diffs
-

  3rdparty/stout/include/stout/exit.hpp 
762864afd8f86f7e1f439ef6ea7e3965a5f147d5 


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


Testing (updated)
---

make check (Fedora 25)

**Example output after this change:**
```
jpeach@jpeach src]$ sudo ./mesos-agent --work_dir=/nothing/here 
--master=phoney:5050
WARNING: Logging before InitGoogleLogging() is written to STDERR
I0516 16:31:34.630625  4987 main.cpp:322] Build: 2017-05-11 11:40:39 by jpeach
I0516 16:31:34.630748  4987 main.cpp:323] Version: 1.4.0
I0516 16:31:34.630760  4987 main.cpp:330] Git SHA: 
62e0f726198e90d63e9e88156432d833ca88476c
I0516 16:31:34.647601  4987 systemd.cpp:238] systemd version `231` detected
I0516 16:31:34.647650  4987 main.cpp:432] Inializing systemd state
I0516 16:31:34.656100  4987 systemd.cpp:326] Started systemd slice 
`mesos_executors.slice`
I0516 16:31:34.658421  4987 containerizer.cpp:221] Using isolation: 
posix/cpu,posix/mem,filesystem/posix,network/cni
I0516 16:31:34.668910  4987 linux_launcher.cpp:150] Using 
/sys/fs/cgroup/freezer as the freezer hierarchy for the Linux launcher
I0516 16:31:34.670874  4987 provisioner.cpp:249] Using default backend 'overlay'
E0516 16:31:34.684639  4987 main.cpp:461] EXIT with status 1: Failed to create 
a master detector: Failed to parse 'phoney:5050'
```

**Example output before this change:**
```
[jpeach@jpeach build]$ sudo /opt/mesos/sbin//mesos-agent 
--work_dir=/nothing/here --master=phoney:5050
WARNING: Logging before InitGoogleLogging() is written to STDERR
I0320 11:03:43.868015  1188 main.cpp:243] Build: 2017-03-10 10:49:55 by jpeach
I0320 11:03:43.868135  1188 main.cpp:244] Version: 1.3.0
I0320 11:03:43.868151  1188 main.cpp:251] Git SHA: 
487c667260ec89127e18c77b9e8c8c243cf6ec57
I0320 11:03:43.886561  1188 systemd.cpp:238] systemd version `231` detected
I0320 11:03:43.886631  1188 main.cpp:342] Inializing systemd state
I0320 11:03:43.896606  1188 systemd.cpp:326] Started systemd slice 
`mesos_executors.slice`
I0320 11:03:43.899202  1188 containerizer.cpp:221] Using isolation: 
posix/cpu,posix/mem,filesystem/posix,network/cni
I0320 11:03:43.911183  1188 linux_launcher.cpp:150] Using 
/sys/fs/cgroup/freezer as the freezer hierarchy for the Linux launcher
I0320 11:03:43.913216  1188 provisioner.cpp:249] Using default backend 'overlay'
Failed to create a master detector: Failed to parse 'phoney:5050'
```


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 May 16, 2017, 11:17 p.m.)


Review request for mesos, Neil Conway, Vinod Kone, 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 14de72fa4a8746504a251d735bf56041b934df40 
  src/slave/state.hpp a497ce1f58fb8dc7718ee5bb10bc62dd7479efa5 
  src/slave/state.cpp 33dcc7a148f9a6b1a3216cce45710da8fd819ba6 
  src/tests/reservation_tests.cpp 4504831d77c1bfcf5f2ddf6d28cd45dea2c421ad 
  src/tests/slave_recovery_tests.cpp 52e78b6b6280a159233b402ce2849448204d4f11 


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

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


Testing
---

make check


Thanks,

Megha Sharma

Re: Review Request 55895: Extract a BasicBlocks class for disk block arithmetic.

[James Peach]

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

(Updated May 16, 2017, 11:09 p.m.)


Review request for mesos, Jie Yu and Jiang Yan Xu.


Changes
---

Updated with review feedback.


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


Repository: mesos


Description
---

Extract a BasicBlocks class to handle disk blocks to clarify block-based
arithmetic in the XFS disk isolator.


Diffs (updated)
-

  src/slave/containerizer/mesos/isolators/xfs/utils.hpp 
eddd4c37fb42339ca21ecb392dea47acf6b277bb 
  src/slave/containerizer/mesos/isolators/xfs/utils.cpp 
8018ad348d26bd962357543a5fb9f6cb43ff13b1 
  src/tests/containerizer/xfs_quota_tests.cpp 
7beb60b059910a0d4451b1ace895a35dc974a043 


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

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


Testing
---

sudo make check (Fedora 25)


Thanks,

James Peach

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

[Chun-Hung Hsiao]

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



I personally don't like using an RC version. We should update protobuf to an 
official release, say 3.3.1?

- Chun-Hung Hsiao


On April 25, 2017, 10:51 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58358/
> ---
> 
> (Updated April 25, 2017, 10:51 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`.
> 
> Note that reviewboard seems to have problem for uploading/dowloading binary 
> patches, so this particular commit is also available at 
> https://github.com/apache/mesos/pull/208
> 
> 
> 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
> ---
> 
> 1. make check;
> 2. used the modified `test-upgrade.py` in r/58357 to test upgrade/downgrade 
> between this chain and Mesos 1.2;
> 3. build this branch in Apache Aurora's vagrant based box, and verifies that 
> Aurora scheduler (using java) and executor (using python) still works with 
> `libmesos.so` built from this branch.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

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

[Chun-Hung Hsiao]

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


Fix it, then Ship it!




Please rebase.


3rdparty/CMakeLists.txt
Line 349 (original), 348 (patched)


This line can be removed as well.


- Chun-Hung Hsiao


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 58515: Update 3rdparty build to support protobuf 3.2.0.

[Chun-Hung Hsiao]

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




3rdparty/cmake/Versions.cmake
Line 14 (original), 14 (patched)


Please rebase.


- Chun-Hung Hsiao


On April 19, 2017, 10:25 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58515/
> ---
> 
> (Updated April 19, 2017, 10: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
> ---
> 
> Update 3rdparty build to support protobuf 3.2.0.
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake 912726351ff744dd839b8d1c8d64dcc373d879be 
>   3rdparty/versions.am b8144702adca9dd41199f69cb6fb0c0c4b490dbd 
> 
> 
> Diff: https://reviews.apache.org/r/58515/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

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

[Chun-Hung Hsiao]

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


Ship it!




Please rebase.

- Chun-Hung Hsiao


On April 19, 2017, 10:25 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58515/
> ---
> 
> (Updated April 19, 2017, 10: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
> ---
> 
> Update 3rdparty build to support protobuf 3.2.0.
> 
> 
> Diffs
> -
> 
>   3rdparty/cmake/Versions.cmake 912726351ff744dd839b8d1c8d64dcc373d879be 
>   3rdparty/versions.am b8144702adca9dd41199f69cb6fb0c0c4b490dbd 
> 
> 
> Diff: https://reviews.apache.org/r/58515/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

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

[Chun-Hung Hsiao]

> On April 19, 2017, 6:28 p.m., Zhitao Li wrote:
> > src/java/mesos.pom.in
> > Line 86 (original), 86 (patched)
> > 
> >
> > This is simply done by a sweeping grep and replace in the code base.
> > 
> > I don't know whether we want to force framework to move to protobuf 
> > 3.2.0. I guess if the framework author wants to use proto2, they should 
> > still be able to do it with the same `syntax=proto2` annotation as what 
> > Mesos does?
> > 
> > I'm fine to keep this out of the current changeset for now, although I 
> > don't know how compatible protobuf-java at lower version can handle newer 
> > java classes.
> > 
> > Do we have formal rules about compatibility on released libraries?

This needs to be set to the same version, otherwise maven won't be able to find 
the generated `protobuf-java-3.3.1.jar` and will produce `package 
com.google.protobuf.GeneratedMessageV3 does not exist` errors. I'm not familiar 
with maven so not sure if this can be resolved without restricting the 
dependency.


> On April 19, 2017, 6:28 p.m., Zhitao Li wrote:
> > src/python/interface/setup.py.in
> > Line 29 (original), 29 (patched)
> > 
> >
> > Similar to the comment above on java.

The Python unit test works fine without changing the protobuf version 
dependency FYI.


- Chun-Hung


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


On April 18, 2017, 6:25 p.m., Zhitao Li wrote:
> 
> ---
> 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
> ---
> 
> Update Mesos build library to use protobuf 3.2.0.
> 
> 
> Diffs
> -
> 
>   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/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

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 May 16, 2017, 10:12 p.m.)


Review request for mesos, Neil Conway, Vinod Kone, 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 14de72fa4a8746504a251d735bf56041b934df40 
  src/slave/state.hpp a497ce1f58fb8dc7718ee5bb10bc62dd7479efa5 
  src/slave/state.cpp 33dcc7a148f9a6b1a3216cce45710da8fd819ba6 
  src/tests/slave_recovery_tests.cpp 52e78b6b6280a159233b402ce2849448204d4f11 


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

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


Testing
---

make check


Thanks,

Megha Sharma

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

[Megha Sharma]

> On May 3, 2017, 12:04 a.m., Vinod Kone wrote:
> > src/slave/slave.cpp
> > Lines 5957 (patched)
> > 
> >
> > s/info/info after a reboot/
> > 
> > also please log the `future.failure()` here? are we sure that a failed 
> > future here can only happen due to incompatibile agent info?

Indeed there are more reasons for the future to fail in addition to the 
incompatibile agent info so I agree the message needs to be more generic.


- Megha


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


On April 26, 2017, 6:16 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56895/
> ---
> 
> (Updated April 26, 2017, 6:16 p.m.)
> 
> 
> Review request for mesos, Neil Conway, Vinod Kone, 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
> -
> 
>   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/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Megha Sharma
> 
>

Re: Review Request 59219: Removed logic for handling missing FrameworkID in ExecutorInfo.

[Neil Conway]

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

(Updated May 16, 2017, 8:36 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Update description.


Summary (updated)
-

Removed logic for handling missing FrameworkID in ExecutorInfo.


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


Repository: mesos


Description (updated)
---

The master will inject missing `FrameworkID`s into `ExecutorInfo` since
Mesos 0.23 (see MESOS-2290), so it should be safe to assume it is always
set.


Diffs (updated)
-

  src/master/master.cpp 4e7a161f431624bd78d3e9032eb8587687149cad 


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

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


Testing
---

make check


Thanks,

Neil Conway

Re: Review Request 59219: Removed code for old agents in agent re-registration logic.

[Neil Conway]

> On May 12, 2017, 9:40 p.m., Benjamin Mahler wrote:
> > src/master/master.cpp
> > Lines 7564-7573 (original), 7564 (patched)
> > 
> >
> > Hm.. this seems to refer to the scheduler driver rather than a slave 
> > version?

Ah, true -- the comment's wording confused me. This validation was moved to the 
master 2 years ago: https://reviews.apache.org/r/30952/

Since the master has added missing `FrameworkID`s to `Launch` messages since 
Mesos 0.23, it should be safe to assume the `FrameworkID` is set in 
`ExecutorInfo`.


- Neil


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


On May 11, 2017, 11:32 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59219/
> ---
> 
> (Updated May 11, 2017, 11:32 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-6977
> https://issues.apache.org/jira/browse/MESOS-6977
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> 0.x agents are no longer supported.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 4e7a161f431624bd78d3e9032eb8587687149cad 
> 
> 
> Diff: https://reviews.apache.org/r/59219/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 59286: Updated Mesos 1.3.0 CHANGELOG for removal of hierarchical roles.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On May 15, 2017, 12:13 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59286/
> ---
> 
> (Updated May 15, 2017, 12:13 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated Mesos 1.3.0 CHANGELOG for removal of hierarchical roles.
> 
> 
> Diffs
> -
> 
>   CHANGELOG d95a5ceb817ff42c57f2b1978c39d6d03fe2f35c 
> 
> 
> Diff: https://reviews.apache.org/r/59286/diff/1/
> 
> 
> Testing
> ---
> 
> Visual inspection.
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 59285: Disabled support for hierarchical roles.

[Michael Park]

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


Ship it!




Ship It!

- Michael Park


On May 15, 2017, 12:13 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59285/
> ---
> 
> (Updated May 15, 2017, 12:13 p.m.)
> 
> 
> Review request for mesos and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The previous plan was to include support for hierarchical roles in Mesos
> 1.3.0 but to mark it as experimental. Upon reflection, it seems safer to
> disable hierarchical roles in Mesos 1.3.0, and address the open issues
> during the 1.4 development process.
> 
> 
> Diffs
> -
> 
>   src/common/roles.cpp 123e99073f8acbf4eb50e257e349e5485fba1cc6 
>   src/tests/master_allocator_tests.cpp 
> 3b072b251275e0cdfee3c40feebf78c6ad3a79d7 
>   src/tests/role_tests.cpp 56422b507a0863ac1d9395eee63820291c2a4df5 
> 
> 
> Diff: https://reviews.apache.org/r/59285/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Re: Review Request 59320: Added test case for agent re-registration.

[Neil Conway]

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

(Updated May 16, 2017, 8:19 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Add querying of state endpoint to test case.


Repository: mesos


Description
---

Add an explanatory comment and a test case for a particular case in the
master's logic for handling agent re-registration.


Diffs (updated)
-

  src/master/master.cpp 4e7a161f431624bd78d3e9032eb8587687149cad 
  src/tests/partition_tests.cpp 4ff428564d1fa6cb96e6f8ec8edc331da88a3eb6 


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

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


Testing
---

`make check`


Thanks,

Neil Conway

Re: Review Request 59214: Removed code for handling missing FrameworkInfo of a running task.

[Neil Conway]

> On May 12, 2017, 9:36 p.m., Benjamin Mahler wrote:
> > src/master/master.cpp
> > Lines 6083-6084 (original), 6060-6061 (patched)
> > 
> >
> > What does this null pointer case correspond to? Is it a framework that 
> > will be "recovered" later in the re-registration? A comment might help 
> > clarify this for the reader.

I added a comment and a test case to cover this scenario in a separate RR: 
https://reviews.apache.org/r/59320/


> On May 12, 2017, 9:36 p.m., Benjamin Mahler wrote:
> > src/master/master.cpp
> > Lines 6180-6229 (original), 6157-6197 (patched)
> > 
> >
> > Hm.. do we still need `ids`? It seems like we could simplify this for 
> > the reader a bit by removing the `ids` and by consolidating the two cases 
> > into a single loop:
> > 
> > ```
> > foreach (const FrameworkInfo& frameworkInfo, frameworks) {
> > Framework* framework = getFramework(frameworkId);
> > 
> > if (isCompletedFramework(frameworkId)) {
> >   // do nothing, __reregister already sent the shutdown message
> > } else if (framework != nullptr)) {
> >   // send update framework message
> > } else {
> >   // recover the framework
> > }
> > }
> > ```
> > 
> > With the framework cases together I found it easier to read.
> > 
> > Looking at this code, I found the breakdown of responsibility a little 
> > confusing, in that it wasn't clear to me why `__reregisterSlave` sends the 
> > shutdown but `___reregisterSlave` deals with the update and recovery of the 
> > framework (I assume it's due to the call-site requirements). Something to 
> > think about separately from this change.

I implemented this cleanup as a separate RR: https://reviews.apache.org/r/59302/

Your point about the division of logic between `__reregisterSlave` and 
`___reregisterSlave` is a good one, but seems like we can defer it for later.


> On May 12, 2017, 9:36 p.m., Benjamin Mahler wrote:
> > src/master/master.cpp
> > Lines 6779-6780 (original), 6735-6736 (patched)
> > 
> >
> > If you want to send a separate cleanup patch, looks like this case and 
> > other PARTITION_AWARE cases were missed in the sweep to use 
> > `framework->capabilities`:
> > 
> > ```
> > if (!framework->capabilities.partitionAware) {
> >   newTaskState = TASK_LOST;
> > }
> > ```

I did this cleanup in a separate RR: https://reviews.apache.org/r/59259/


- Neil


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


On May 15, 2017, 11:13 p.m., Neil Conway wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59214/
> ---
> 
> (Updated May 15, 2017, 11:13 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-6977
> https://issues.apache.org/jira/browse/MESOS-6977
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In previous versions of Mesos, the master might know about a task
> running on an agent, but might not have a `FrameworkInfo` for that
> task's framework. This might occur if the master fails over, the agent
> re-registers, but the framework has not (yet) re-registered.
> 
> In Mesos 1.0, the agent will now supply the FrameworkInfo for all tasks
> it is running when it re-registers with the master. Since we no longer
> support 0.x agents, we can remove the old backward compatibility code
> for handling a running task with unknown FrameworkInfo.
> 
> Note that this means that "orphan tasks", "orphan executors", and
> "unregistered frameworks" are no longer possible.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 7060b8fa53e0682681c50e051908ffbbf50fb7da 
>   src/master/master.cpp 4e7a161f431624bd78d3e9032eb8587687149cad 
> 
> 
> Diff: https://reviews.apache.org/r/59214/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Neil Conway
> 
>

Review Request 59320: Added test case for agent re-registration.

[Neil Conway]

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

Review request for mesos and Benjamin Mahler.


Repository: mesos


Description
---

Add an explanatory comment and a test case for a particular case in the
master's logic for handling agent re-registration.


Diffs
-

  src/master/master.cpp 4e7a161f431624bd78d3e9032eb8587687149cad 
  src/tests/partition_tests.cpp 4ff428564d1fa6cb96e6f8ec8edc331da88a3eb6 


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


Testing
---

`make check`


Thanks,

Neil Conway

Review Request 59318: Replace in EXPECT with an ASSERT in master_tests to avoid segfaults.

[Kapil Arya]

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

Review request for mesos and Gilbert Song.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/tests/master_tests.cpp 37a54138b03229778bc62501532e698e87760218 


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


Testing
---

Make check with an without the patch.


Thanks,

Kapil Arya

Re: Review Request 59000: Added environment secret isolator.

[Kapil Arya]

> On May 15, 2017, 9:42 a.m., Gilbert Song wrote:
> > src/slave/containerizer/mesos/isolators/environment_secret.cpp
> > Lines 137-138 (patched)
> > 
> >
> > How about custom executor/default executor? `task_environment` is for 
> > command task specific.

We already set the `launchInfo.environment`; are you suggesting to not set 
task_environment?


- Kapil


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


On May 16, 2017, 3:22 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59000/
> ---
> 
> (Updated May 16, 2017, 3:22 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added environment secret isolator.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt eef718d95b5d8e051a5094369dc9b4532bc307ff 
>   src/Makefile.am 6bb81fd49b4564a0afa993b2cef6baa9d370ee7a 
>   src/launcher/executor.cpp b05f73e539c22d4d40f07df76168a06373b818d4 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 97837c83cc223950750e4cd088f4da067023c96c 
>   src/slave/containerizer/mesos/isolators/environment_secret.hpp PRE-CREATION 
>   src/slave/containerizer/mesos/isolators/environment_secret.cpp PRE-CREATION 
>   src/tests/CMakeLists.txt 9f2af9cdd1cf50485f4cd84ce67bcceba64b9328 
>   src/tests/containerizer/environment_secret_isolator_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/59000/diff/7/
> 
> 
> Testing
> ---
> 
> Added a new test and ran `make check`.
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 58999: Added --secret_resolver flag to agent.

[Kapil Arya]

> On May 15, 2017, 7:57 a.m., Gilbert Song wrote:
> > src/slave/flags.cpp
> > Lines 961-964 (patched)
> > 
> >
> > Should we consider make the `default secret resolver` as the default 
> > value for this flag? So that we can get rid of the logic in 
> > `SecretResolver::create()`.

I am not sure about it because the create logic will only change from `if 
(moduleName.isNone()) {...}` to `if (moduleName == DEFAULT_SECRET_RESOLVER) 
{...}`.


- Kapil


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


On May 16, 2017, 3:22 p.m., Kapil Arya wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58999/
> ---
> 
> (Updated May 16, 2017, 3:22 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated Containerizer to accept SecretResolver.
> 
> 
> Diffs
> -
> 
>   src/local/local.cpp e47980929db2da1f31cf899a0e1fc452070e11f3 
>   src/slave/containerizer/containerizer.hpp 
> 4c31a1f5c853c1dc66480c7b4c867a87a1bb5c41 
>   src/slave/containerizer/containerizer.cpp 
> 9024371b6c4228f0903cfeef3bbec736e1a425f8 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 04ab997454534b8e5e821b53b83e166e5018e11c 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 97837c83cc223950750e4cd088f4da067023c96c 
>   src/slave/flags.hpp e5784ef81ad0720c7ec061ee0b28b8fadae77afd 
>   src/slave/flags.cpp bc63a6a4cb6115b4b4d592e67e34045f52b50d4c 
>   src/slave/main.cpp 507d59996a90f51c7cd1e0f836a34c2e9c484791 
> 
> 
> Diff: https://reviews.apache.org/r/58999/diff/7/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kapil Arya
> 
>

Re: Review Request 59001: Added volume secret isolator.

[Kapil Arya]

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

(Updated May 16, 2017, 3:22 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.


Changes
---

Addressed Gilbert's comments.


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


Repository: mesos


Description
---

Added volume secret isolator.


Diffs (updated)
-

  src/CMakeLists.txt eef718d95b5d8e051a5094369dc9b4532bc307ff 
  src/Makefile.am 6bb81fd49b4564a0afa993b2cef6baa9d370ee7a 
  src/slave/containerizer/mesos/containerizer.cpp 
97837c83cc223950750e4cd088f4da067023c96c 
  src/slave/containerizer/mesos/isolators/volume/secret.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/volume/secret.cpp PRE-CREATION 
  src/tests/CMakeLists.txt 9f2af9cdd1cf50485f4cd84ce67bcceba64b9328 
  src/tests/containerizer/environment_secret_isolator_tests.cpp PRE-CREATION 
  src/tests/containerizer/rootfs.cpp fdfecc65a3fcd19d6a4dfa574320f4d1f2755322 
  src/tests/containerizer/volume_secret_isolator_tests.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/59001/diff/9/

Changes: https://reviews.apache.org/r/59001/diff/8-9/


Testing
---

Added new tests an ran `make check`.


Thanks,

Kapil Arya

Re: Review Request 58999: Added --secret_resolver flag to agent.

[Kapil Arya]

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

(Updated May 16, 2017, 3:22 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.


Changes
---

Addressed Gilbert's comments.


Repository: mesos


Description
---

Updated Containerizer to accept SecretResolver.


Diffs (updated)
-

  src/local/local.cpp e47980929db2da1f31cf899a0e1fc452070e11f3 
  src/slave/containerizer/containerizer.hpp 
4c31a1f5c853c1dc66480c7b4c867a87a1bb5c41 
  src/slave/containerizer/containerizer.cpp 
9024371b6c4228f0903cfeef3bbec736e1a425f8 
  src/slave/containerizer/mesos/containerizer.hpp 
04ab997454534b8e5e821b53b83e166e5018e11c 
  src/slave/containerizer/mesos/containerizer.cpp 
97837c83cc223950750e4cd088f4da067023c96c 
  src/slave/flags.hpp e5784ef81ad0720c7ec061ee0b28b8fadae77afd 
  src/slave/flags.cpp bc63a6a4cb6115b4b4d592e67e34045f52b50d4c 
  src/slave/main.cpp 507d59996a90f51c7cd1e0f836a34c2e9c484791 


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

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


Testing
---


Thanks,

Kapil Arya

Re: Review Request 59000: Added environment secret isolator.

[Kapil Arya]

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

(Updated May 16, 2017, 3:22 p.m.)


Review request for mesos, Gilbert Song, Greg Mann, Jie Yu, and Vinod Kone.


Changes
---

Addressed Gilbert's comments.


Repository: mesos


Description
---

Added environment secret isolator.


Diffs (updated)
-

  src/CMakeLists.txt eef718d95b5d8e051a5094369dc9b4532bc307ff 
  src/Makefile.am 6bb81fd49b4564a0afa993b2cef6baa9d370ee7a 
  src/launcher/executor.cpp b05f73e539c22d4d40f07df76168a06373b818d4 
  src/slave/containerizer/mesos/containerizer.cpp 
97837c83cc223950750e4cd088f4da067023c96c 
  src/slave/containerizer/mesos/isolators/environment_secret.hpp PRE-CREATION 
  src/slave/containerizer/mesos/isolators/environment_secret.cpp PRE-CREATION 
  src/tests/CMakeLists.txt 9f2af9cdd1cf50485f4cd84ce67bcceba64b9328 
  src/tests/containerizer/environment_secret_isolator_tests.cpp PRE-CREATION 


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

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


Testing
---

Added a new test and ran `make check`.


Thanks,

Kapil Arya

Re: Review Request 58760: Added default secret resolver module.

[Kapil Arya]

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

(Updated May 16, 2017, 3:22 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.


Changes
---

Addressed Gilbert's comments.


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


Repository: mesos


Description
---

Added default secret resolver module.


Diffs (updated)
-

  src/CMakeLists.txt eef718d95b5d8e051a5094369dc9b4532bc307ff 
  src/Makefile.am 6bb81fd49b4564a0afa993b2cef6baa9d370ee7a 
  src/secret/resolver.cpp PRE-CREATION 


Diff: https://reviews.apache.org/r/58760/diff/9/

Changes: https://reviews.apache.org/r/58760/diff/8-9/


Testing
---


Thanks,

Kapil Arya

Re: Review Request 58759: Introduced SecretResolver module interface.

[Kapil Arya]

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

(Updated May 16, 2017, 3:22 p.m.)


Review request for mesos, Gilbert Song, Jie Yu, and Vinod Kone.


Changes
---

Addressed Gilbert's comments.


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


Repository: mesos


Description
---

Introduced SecretResolver module interface.


Diffs (updated)
-

  include/mesos/module.hpp c28d01df54906b2b8ebfe7ac9e719a0d1be45614 
  include/mesos/module/secret_resolver.hpp PRE-CREATION 
  include/mesos/secret/resolver.hpp PRE-CREATION 
  src/Makefile.am 6bb81fd49b4564a0afa993b2cef6baa9d370ee7a 
  src/module/manager.cpp 7d875fcb7fcec0d57274e644b0a3b67b333ac193 


Diff: https://reviews.apache.org/r/58759/diff/9/

Changes: https://reviews.apache.org/r/58759/diff/8-9/


Testing
---


Thanks,

Kapil Arya

Re: Review Request 59300: Updated documentation for `registrar/log/network_size`.

[Anindya Sinha]

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


Ship it!




Ship It!

- Anindya Sinha


On May 15, 2017, 11:08 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59300/
> ---
> 
> (Updated May 15, 2017, 11:08 p.m.)
> 
> 
> Review request for mesos, Anindya Sinha and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7507
> https://issues.apache.org/jira/browse/MESOS-7507
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated documentation for `registrar/log/network_size`.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md c42afce0a26dc15e033ee1375b7cb23d55be40ab 
> 
> 
> Diff: https://reviews.apache.org/r/59300/diff/1/
> 
> 
> Testing
> ---
> 
> Markdown viewer.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 59290: Added '/log/network_size' metric.

[Anindya Sinha]

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


Ship it!




Ship It!

- Anindya Sinha


On May 15, 2017, 10:47 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59290/
> ---
> 
> (Updated May 15, 2017, 10:47 p.m.)
> 
> 
> Review request for mesos, Anindya Sinha and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7507
> https://issues.apache.org/jira/browse/MESOS-7507
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added '/log/network_size' metric.
> 
> Will update monitoring.md next.
> 
> 
> Diffs
> -
> 
>   src/log/log.hpp a600025f4ca38e3fad0f64f48b007138da8e22d4 
>   src/log/metrics.hpp PRE-CREATION 
>   src/log/metrics.cpp PRE-CREATION 
>   src/tests/log_tests.cpp 52b1bfedd8d3f8f2ab3308c4be9f167126ee694b 
> 
> 
> Diff: https://reviews.apache.org/r/59290/diff/1/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 59300: Updated documentation for `registrar/log/network_size`.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [59289, 59290, 59300]

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 May 15, 2017, 11:08 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59300/
> ---
> 
> (Updated May 15, 2017, 11:08 p.m.)
> 
> 
> Review request for mesos, Anindya Sinha and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7507
> https://issues.apache.org/jira/browse/MESOS-7507
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Updated documentation for `registrar/log/network_size`.
> 
> 
> Diffs
> -
> 
>   docs/monitoring.md c42afce0a26dc15e033ee1375b7cb23d55be40ab 
> 
> 
> Diff: https://reviews.apache.org/r/59300/diff/1/
> 
> 
> Testing
> ---
> 
> Markdown viewer.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 59289: Refactored log Metrics into separate files.

[Anindya Sinha]

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


Ship it!




Ship It!

- Anindya Sinha


On May 15, 2017, 10:46 p.m., Jiang Yan Xu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59289/
> ---
> 
> (Updated May 15, 2017, 10:46 p.m.)
> 
> 
> Review request for mesos, Anindya Sinha and Jie Yu.
> 
> 
> Bugs: MESOS-7507
> https://issues.apache.org/jira/browse/MESOS-7507
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> A separate cpp file is more appropriate as we add more metrics and 
> make the struct more complex.
> 
> No functional change.
> 
> 
> Diffs
> -
> 
>   src/CMakeLists.txt eef718d95b5d8e051a5094369dc9b4532bc307ff 
>   src/Makefile.am 6bb81fd49b4564a0afa993b2cef6baa9d370ee7a 
>   src/log/log.hpp a600025f4ca38e3fad0f64f48b007138da8e22d4 
>   src/log/log.cpp 2301eef564f2a42958c6c2c8eef0cc4b2fd76353 
>   src/log/metrics.hpp PRE-CREATION 
>   src/log/metrics.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/59289/diff/1/
> 
> 
> Testing
> ---
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Re: Review Request 53842: Add specific metrics for sorting runs across frameworks of a role.

[Anindya Sinha]

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

(Updated May 16, 2017, 6:27 p.m.)


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


Changes
---

Fixed the view aspect of docs/monitoring.md.


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


Repository: mesos


Description
---

Added the following 2 metrics which is maintained on a role level
(as long as there is at least one framework of that role):
a) allocator/mesos/frameworks/role//sort_runs: Number of framework
   level sorts (based on role) in DRF Sorter.
b) allocator/mesos/frameworks/role//sort_run: Latency in framework
   level sorts (based on role) in DRF Sorter.


Diffs (updated)
-

  docs/monitoring.md c42afce0a26dc15e033ee1375b7cb23d55be40ab 
  src/master/allocator/mesos/hierarchical.hpp 
123f97cf495bff0f822838e09df0d88818f04da6 
  src/master/allocator/mesos/hierarchical.cpp 
b75ed9a20a9a42f958cebbacd91e5e15b0d21394 
  src/tests/hierarchical_allocator_tests.cpp 
08180b9975869de328f0c095dd3cddf0c84fbecf 


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

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


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 59203: Enable NetTest.LinkDevice and NetTest.Mac tests on Windows platform.

[Andrew Schwartzmeyer]

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


Ship it!




Ship It!

- Andrew Schwartzmeyer


On May 11, 2017, 8:09 p.m., Jeff Coffler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59203/
> ---
> 
> (Updated May 11, 2017, 8:09 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and 
> Li Li.
> 
> 
> Bugs: MESOS-5938
> https://issues.apache.org/jira/browse/MESOS-5938
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enable NetTest.LinkDevice and NetTest.Mac tests on Windows platform.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/tests/ip_tests.cpp 4c6f81bd53413360182b447ddb149a18e406870e 
>   3rdparty/stout/tests/mac_tests.cpp d4560e75ae4e03d1765c599298e88842381690b9 
> 
> 
> Diff: https://reviews.apache.org/r/59203/diff/1/
> 
> 
> Testing
> ---
> 
> On Linux, did a 'make check' and insured that all tests (particularly stout 
> tests, where the work was performed) passed.
> 
> On Windows, did the rough equivalent. Here's relevant output from the Windows 
> stout-tests.exe program:
> 
> [--] 8 tests from NetTest
> **[ RUN  ] NetTest.LinkDevice
> [   OK ] NetTest.LinkDevice (4 ms)**
> [ RUN  ] NetTest.ConstructIPv4
> [   OK ] NetTest.ConstructIPv4 (1 ms)
> [ RUN  ] NetTest.ConstructIPv6
> [   OK ] NetTest.ConstructIPv6 (0 ms)
> [ RUN  ] NetTest.ConstructIPv4Network
> [   OK ] NetTest.ConstructIPv4Network (0 ms)
> [ RUN  ] NetTest.ConstructIPv6Network
> [   OK ] NetTest.ConstructIPv6Network (0 ms)
> **[ RUN  ] NetTest.Mac
> [   OK ] NetTest.Mac (3 ms)**
> [ RUN  ] NetTest.ConstructMAC
> [   OK ] NetTest.ConstructMAC (1 ms)
> [ RUN  ] NetTest.ParseMAC
> [   OK ] NetTest.ParseMAC (1 ms)
> [--] 8 tests from NetTest (19 ms total)
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>

Re: Review Request 59297: Fix autotools to include ip.hpp & mac.hpp in stout/{posix|windows}.

[Andrew Schwartzmeyer]

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


Ship it!




Ship It!

- Andrew Schwartzmeyer


On May 15, 2017, 10:43 p.m., Jeff Coffler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59297/
> ---
> 
> (Updated May 15, 2017, 10:43 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and 
> Li Li.
> 
> 
> Bugs: MESOS-5938
> https://issues.apache.org/jira/browse/MESOS-5938
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Required since autotools includes .hpp files.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/Makefile.am ea4341b2dccc3d73b9d347d29486ab4c8ff42217 
> 
> 
> Diff: https://reviews.apache.org/r/59297/diff/1/
> 
> 
> Testing
> ---
> 
> This was an autotools-specific problem, I didn't catch this right away since 
> my tests didn't build with autotools.
> 
> Now they do:
> 
> 1. Autotools built successfully (cmake -j6 distcheck), and
> 2. Cmake built successfully (cmake -j6 check).
> 
> I'll be careful to check autotools in the future, particularly when adding 
> files.
> 
> Note that a few lines had line continution not lining up. I resolved those 
> for consistency with the rest of the file.
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>

Re: Review Request 59279: Uses temp dir for docker and appc stores in test.

[Zhitao Li]

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

(Updated May 16, 2017, 4:39 p.m.)


Review request for mesos, Anand Mazumdar, Gilbert Song, and Jie Yu.


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


Repository: mesos


Description (updated)
---

Uses temp dir for docker and appc stores in test.


Diffs (updated)
-

  src/tests/mesos.cpp a79ec62d6b8354a97a50533dc66e04c1afc4bef6 


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

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


Testing
---

Ran through all tests.


Thanks,

Zhitao Li

Re: Review Request 59297: Fix autotools to include ip.hpp & mac.hpp in stout/{posix|windows}.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [59200, 59201, 59202, 59203, 59297]

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 May 15, 2017, 10:43 p.m., Jeff Coffler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59297/
> ---
> 
> (Updated May 15, 2017, 10:43 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, John Kordich, Joseph Wu, and 
> Li Li.
> 
> 
> Bugs: MESOS-5938
> https://issues.apache.org/jira/browse/MESOS-5938
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Required since autotools includes .hpp files.
> 
> 
> Diffs
> -
> 
>   3rdparty/stout/include/Makefile.am ea4341b2dccc3d73b9d347d29486ab4c8ff42217 
> 
> 
> Diff: https://reviews.apache.org/r/59297/diff/1/
> 
> 
> Testing
> ---
> 
> This was an autotools-specific problem, I didn't catch this right away since 
> my tests didn't build with autotools.
> 
> Now they do:
> 
> 1. Autotools built successfully (cmake -j6 distcheck), and
> 2. Cmake built successfully (cmake -j6 check).
> 
> I'll be careful to check autotools in the future, particularly when adding 
> files.
> 
> Note that a few lines had line continution not lining up. I resolved those 
> for consistency with the rest of the file.
> 
> 
> Thanks,
> 
> Jeff Coffler
> 
>

Review Request 59314: Fixed the fd type in tests for consistency.

[Alexander Rukletsov]

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

Review request for mesos, Gastón Kleiman and Vinod Kone.


Repository: mesos


Description
---

See summary.


Diffs
-

  src/tests/containerizer/io_switchboard_tests.cpp 
b19961ee91af3c174d4377c2514465d6c99cf331 
  src/tests/containerizer/nested_mesos_containerizer_tests.cpp 
f8c1511d1a85c0e3bf985404da2abff9039e3e27 


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


Testing
---

`make check` on various Linux distros


Thanks,

Alexander Rukletsov

Re: Review Request 59294: Optionally scale egress bandwidth with CPU.

[Ilya Pronin]

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



Looks good overall. Just a few comments.


src/slave/containerizer/mesos/isolators/network/port_mapping.hpp
Lines 323 (patched)


Make it a `const` method?



src/slave/containerizer/mesos/isolators/network/port_mapping.cpp
Lines 696-697 (patched)


Maybe add a note here that the logic below is guarded by this check, so 
someone won't accidently "simplify" it?



src/slave/containerizer/mesos/isolators/network/port_mapping.cpp
Line 1589 (original), 1715-1716 (patched)


Should we also add a check that `minimum_egress_rate_limit <= 
maximum_egress_rate_limit`? If that's not true egress rate limit will always be 
set to `maximum_egress_rate_limit`, even if originally it was smaller.

What if `speed < maximum_egress_rate_limit`? Should we provide a warning?



src/slave/containerizer/mesos/isolators/network/port_mapping.cpp
Lines 2176 (patched)


`strings::remove(handle, "0", strings::SUFFIX)`? We use "1:0" handle, but 
just in case.



src/slave/containerizer/mesos/isolators/network/port_mapping.cpp
Lines 2679-2681 (patched)


Should we also log this when no ephemeral ports were found for the 
container?



src/slave/containerizer/mesos/isolators/network/port_mapping.cpp
Lines 2680-2681 (patched)


`Try::operator->` can be used instead of `Try::get()`.



src/slave/containerizer/mesos/isolators/network/port_mapping.cpp
Lines 4177-4178 (patched)


What if a container uses `cpus:0.5` and `minimum_egress_rate_limit` is 
unset? Shouldn't `limit` have some minimum resonable value instead of 0?



src/slave/containerizer/mesos/isolators/network/port_mapping.cpp
Lines 4184 (patched)


There's no need to use `Bytes()` constructor here. The result of `max()` 
here is already `Bytes`.



src/slave/containerizer/mesos/isolators/network/port_mapping.cpp
Lines 4188 (patched)


Ditto.


- Ilya Pronin


On May 15, 2017, 9:56 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59294/
> ---
> 
> (Updated May 15, 2017, 9:56 p.m.)
> 
> 
> Review request for mesos, Dmitry Zhuk, Ilya Pronin, Jie Yu, and Santhosh 
> Kumar Shanmugham.
> 
> 
> Bugs: MESOS-7508
> https://issues.apache.org/jira/browse/MESOS-7508
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add support to isolators/port_mapping for optionally scaling egress bandwidth 
> with CPU and with minimum and maximum limits.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
> 9d38289c7161d5e931053b587d115684ccc44c94 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> cd008aaebcd42554a9a81d2b059269546f59c966 
>   src/slave/flags.hpp e5784ef81ad0720c7ec061ee0b28b8fadae77afd 
>   src/slave/flags.cpp bc63a6a4cb6115b4b4d592e67e34045f52b50d4c 
>   src/tests/containerizer/port_mapping_tests.cpp 
> a528382e8b4831b9c7e8dcc877a5e242909f0cd5 
> 
> 
> Diff: https://reviews.apache.org/r/59294/diff/1/
> 
> 
> Testing
> ---
> 
> # added a new test 
> $ make check
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 59279: Uses temp dir for docker and appc stores in test.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [59279]

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 May 15, 2017, 6:02 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59279/
> ---
> 
> (Updated May 15, 2017, 6:02 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Gilbert Song, and Jie Yu.
> 
> 
> Bugs: MESOS-7381
> https://issues.apache.org/jira/browse/MESOS-7381
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Certain tests which relies on docker/appc stores to store images could be 
> flaky when certain changes to the store code is chaging.
> 
> This patch moves the usage of these flags to the temporary directory created 
> for each test case, so they do not affect each other.
> 
> 
> Diffs
> -
> 
>   src/tests/mesos.cpp a79ec62d6b8354a97a50533dc66e04c1afc4bef6 
> 
> 
> Diff: https://reviews.apache.org/r/59279/diff/1/
> 
> 
> Testing
> ---
> 
> Ran through all tests.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 58898: Send task kill for non-Partition Aware frameworks.

[Megha Sharma]

> On May 10, 2017, 11:24 p.m., Neil Conway wrote:
> > My apologies for the delay in reviewing this.
> > 
> > High-level comments:
> > 
> > (a) Can we improve the description of the problem in the commit summary? It 
> > took me quite a while to figure out what is actually going on here. My 
> > understanding is:
> > 
> > (1) Agent re-registers
> > (2) Master sends `ShutdownFrameworkMessage` for non-PA frameworks on the 
> > agent
> > (3) Master offers agent resources to framework
> > (4) Framework launches new task on agent _before the agent has finished 
> > shutting down the framework_
> > (5) Agent ignores launch task because it believes the framework is still 
> > terminating.
> > 
> > This is a race condition, because if the agent finished shutting down the 
> > framework (in #4) before the launch task was received, there would not be a 
> > problem. Is my understanding correct?
> > 
> > (2) I'd prefer a new unit test that constructs exactly this scenario, 
> > rather than changing existing unit tests.
> > 
> > (3) The new behavior is that the framework will receive `TASK_KILLED` for 
> > any non-PA tasks running on a partitioned agent that re-registers. This 
> > does not seem ideal, because `TASK_KILLED` _normally_ corresponds to a 
> > framework-initiated `killTask` operation.
> > 
> > (4) Thinking out loud, what if a custom executor ignores the `killTask` 
> > request? When shutting down a framework, it seems we forcibly destroy the 
> > container (via `containerizer->destroy()`), if the executor doesn't exit 
> > promptly upon receiving the framework shutdown request. AFAIK we don't have 
> > similar logic for `killTask` requests.
> > 
> > 3 + 4 suggests that maybe we want a different way to terminate the task on 
> > the agent -- let's discuss.

Summarizing the 2 approaches we talked about.

Approach 1: ShutdownFrameworkMessage

1. Upon agent re-registration, master will add tasks even for non-PA frameworks 
on this agent. This is needed by the master to do correct resource accounting 
and not offer resources already in use on this agent. We need to mutate the 
TaskState on the Task before adding them to the master's data structures since 
the TaskState might be non-terminal when the agent sends these tasks with 
ReregisterSlaveMessage. And the master has already sent TASK_LOST for these 
tasks to the frameworks so we need to set the TaskState to TASK_LOST so that 
any future reconciliations with the framework doesn't have this task 
transitioning from TASK_LOST to TASK_RUNNNG/TASK_STARTING. This is to avoid 
unnecessary confusion about task state as observed by the framework but indeed 
this could have happened with non-strict registry as well where the framework 
can actually receive a non terminal task state update after receiving a 
TASK_LOST for the same task in the past.

2. When the agent re-registers, the master will continue to send a 
ShutdownFrameworkMessage to the agent to kill the tasks pertaining to non-PA 
frameworks on the agent as it does today. An additional optional field will be 
added to the ShutdownFrameworkMessage to indicate whether or not the shutdown 
was initiated internally.

3. During framework shutdown the state of the framework is set to 
Framework::TERMINATING which prevents it from launching new tasks. Here, since 
the framework is not really terminating so in order to allow it to launch new 
tasks, the agent will not set the state to terminating if the 
ShutdownFrameworkMessage is generated internally.

4. The framework shutdown today doesn't generate any status updates which needs 
to change. The status updates will be sent if the framework shutdown is 
triggered internally, this is needed to remove the tasks of non-PA frameworks 
that got added when the agent re-registered (1).

Approach 2: Do not shutdown non-PA framework when agent re-registers and let 
the frameworks make the decision on what needs to be done when they receive 
non-terminal status updates for tasks for which they have already received a 
TASK_LOST. This hopefully won't break any frameworks since it could have 
happened in the past with non-strict registry as well and frameworks should be 
resilient enough to handle this scenario.

Let me know if I have missed anything. Personally, I am in favor of approach 1 
as it seems less catastrophic for the frameworks. What do you think?


- Megha


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


On May 1, 2017, 11:58 p.m., Megha Sharma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58898/
> ---
> 
> (Updated May 1, 2017, 11:58 p.m.)
> 
> 
> Review request for mesos, Neil Conway and Jiang Yan Xu.
> 

Re: Review Request 59294: Optionally scale egress bandwidth with CPU.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [59294]

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 May 15, 2017, 8:56 p.m., Ian Downes wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59294/
> ---
> 
> (Updated May 15, 2017, 8:56 p.m.)
> 
> 
> Review request for mesos, Dmitry Zhuk, Ilya Pronin, Jie Yu, and Santhosh 
> Kumar Shanmugham.
> 
> 
> Bugs: MESOS-7508
> https://issues.apache.org/jira/browse/MESOS-7508
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Add support to isolators/port_mapping for optionally scaling egress bandwidth 
> with CPU and with minimum and maximum limits.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
> 9d38289c7161d5e931053b587d115684ccc44c94 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> cd008aaebcd42554a9a81d2b059269546f59c966 
>   src/slave/flags.hpp e5784ef81ad0720c7ec061ee0b28b8fadae77afd 
>   src/slave/flags.cpp bc63a6a4cb6115b4b4d592e67e34045f52b50d4c 
>   src/tests/containerizer/port_mapping_tests.cpp 
> a528382e8b4831b9c7e8dcc877a5e242909f0cd5 
> 
> 
> Diff: https://reviews.apache.org/r/59294/diff/1/
> 
> 
> Testing
> ---
> 
> # added a new test 
> $ make check
> 
> 
> Thanks,
> 
> Ian Downes
> 
>

Re: Review Request 53842: Add specific metrics for sorting runs across frameworks of a role.

[Anindya Sinha]

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

(Updated May 16, 2017, 7:22 a.m.)


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


Changes
---

Updated docs.


Summary (updated)
-

Add specific metrics for sorting runs across frameworks of a role.


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


Repository: mesos


Description
---

Added the following 2 metrics which is maintained on a role level
(as long as there is at least one framework of that role):
a) allocator/mesos/frameworks/role//sort_runs: Number of framework
   level sorts (based on role) in DRF Sorter.
b) allocator/mesos/frameworks/role//sort_run: Latency in framework
   level sorts (based on role) in DRF Sorter.


Diffs (updated)
-

  docs/monitoring.md c42afce0a26dc15e033ee1375b7cb23d55be40ab 
  src/master/allocator/mesos/hierarchical.hpp 
123f97cf495bff0f822838e09df0d88818f04da6 
  src/master/allocator/mesos/hierarchical.cpp 
b75ed9a20a9a42f958cebbacd91e5e15b0d21394 
  src/tests/hierarchical_allocator_tests.cpp 
08180b9975869de328f0c095dd3cddf0c84fbecf 


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

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


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 53841: Added metrics for sorting in the role and quota sorters.

[Anindya Sinha]

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

(Updated May 16, 2017, 7:22 a.m.)


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


Changes
---

Updated docs.


Summary (updated)
-

Added metrics for sorting in the role and quota sorters.


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


Repository: mesos


Description
---

Following metrics have been added:
a) allocator/mesos/roles/sort_runs: Number of role level sorts.
b) allocator/mesos/roles/sort_run: Latency in role level sorts.
c) allocator/mesos/quotas/sort_runs: Number of quota level sorts.
d) allocator/mesos/quotas/sort_run: Latency in quota level sorts.


Diffs (updated)
-

  docs/monitoring.md c42afce0a26dc15e033ee1375b7cb23d55be40ab 
  src/master/allocator/mesos/hierarchical.hpp 
123f97cf495bff0f822838e09df0d88818f04da6 
  src/master/allocator/sorter/drf/metrics.hpp 
61568cb520826ab59d675824b212e0d3deb63764 
  src/master/allocator/sorter/drf/metrics.cpp 
ff63fbac5bbcf54e1ae39c3b650c0dafe7ea46d4 
  src/master/allocator/sorter/drf/sorter.hpp 
fee58d6d1f08163e2a06a4a20c891fe535c3dcff 
  src/master/allocator/sorter/drf/sorter.cpp 
26b77f578f3235a8792c72d4575d607cdb2c7de7 
  src/tests/hierarchical_allocator_tests.cpp 
08180b9975869de328f0c095dd3cddf0c84fbecf 


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

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


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 53840: Metric in the allocator to track latency in running allocations.

[Anindya Sinha]

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

(Updated May 16, 2017, 7:22 a.m.)


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


Changes
---

Updated docs.


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


Repository: mesos


Description
---

Added a metric 'allocator/mesos/allocation_run_interval_ms' which
tracks the latency between scheduling an allocation to an actual
allocation run.


Diffs (updated)
-

  docs/monitoring.md c42afce0a26dc15e033ee1375b7cb23d55be40ab 
  src/master/allocator/mesos/hierarchical.cpp 
b75ed9a20a9a42f958cebbacd91e5e15b0d21394 
  src/master/allocator/mesos/metrics.hpp 
ce486eb079dc44bf0bbfad8668426c7ae87c97b3 
  src/master/allocator/mesos/metrics.cpp 
ec987fe0ce8605bf9e7c0ccbe3ac384deaf53be1 
  src/tests/hierarchical_allocator_tests.cpp 
08180b9975869de328f0c095dd3cddf0c84fbecf 


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

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


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 59293: Windows: Fixed apply-reviews.py to retain line feeds.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [59293]

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 May 15, 2017, 8:45 p.m., Andrew Schwartzmeyer wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59293/
> ---
> 
> (Updated May 15, 2017, 8:45 p.m.)
> 
> 
> Review request for mesos and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Write using 'wb' instead of 'w' for binary mode. This writes the
> downloaded patch file exactly as it came, instead of treating as text
> and changing line endings. This resolves a bug where `git apply` doesn't
> always work with CRLF endings in patch files.
> 
> 
> Diffs
> -
> 
>   support/apply-reviews.py b495675de80fd995f208f1b9ca8c88a717cf2850 
> 
> 
> Diff: https://reviews.apache.org/r/59293/diff/1/
> 
> 
> Testing
> ---
> 
> Applied patch 59177 successfully after failing to apply it without change due 
> to:
> 
> ```
> 59177.patch:9: trailing whitespace.
> 
> 59177.patch:18: trailing whitespace.
> def autocomplete(cmds, plugins, config, argv):
> 59177.patch:26: trailing whitespace.
> current_word = argv[0]
> 59177.patch:27: trailing whitespace.
> argv = argv[1:]
> 59177.patch:36: trailing whitespace.
> return plugin_class(settings, config).__autocomplete_base__(
> error: git apply: bad git-diff - expected /dev/null on line 167
> ```
> 
> 
> Thanks,
> 
> Andrew Schwartzmeyer
> 
>

Re: Review Request 53842: Add role specific metrics for sorting runs.

[Anindya Sinha]

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

(Updated May 16, 2017, 6:24 a.m.)


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


Changes
---

Rebased.


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


Repository: mesos


Description
---

Added the following 2 metrics which is maintained on a role level
(as long as there is at least one framework of that role):
a) allocator/mesos/frameworks/role//sort_runs: Number of framework
   level sorts (based on role) in DRF Sorter.
b) allocator/mesos/frameworks/role//sort_run: Latency in framework
   level sorts (based on role) in DRF Sorter.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
123f97cf495bff0f822838e09df0d88818f04da6 
  src/master/allocator/mesos/hierarchical.cpp 
b75ed9a20a9a42f958cebbacd91e5e15b0d21394 
  src/tests/hierarchical_allocator_tests.cpp 
08180b9975869de328f0c095dd3cddf0c84fbecf 


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

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


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 53841: Added metrics for sorting of the sorters in the allocator.

[Anindya Sinha]

> On April 20, 2017, 9:15 p.m., James Peach wrote:
> > src/master/allocator/sorter/drf/metrics.hpp
> > Lines 59 (patched)
> > 
> >
> > Why is this necessary? Is there a reason that publishing the dominant 
> > resources and shares for quotas doesn't make sense?

The reason publishing the dominant resources and shares for quotas is disabled 
because that is what was originally. The flag `includeClientMetrics` is most 
useful for framework sorters since we keep it disabled because of the 
possibility of high number of framework sorters 
(https://reviews.apache.org/r/53842/). However, I think it makes sense to 
enable this for quotas though.


- Anindya


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


On May 16, 2017, 6:24 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53841/
> ---
> 
> (Updated May 16, 2017, 6:24 a.m.)
> 
> 
> Review request for mesos, James Peach and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6579
> https://issues.apache.org/jira/browse/MESOS-6579
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Following metrics have been added:
> a) allocator/mesos/roles/sort_runs: Number of role level sorts.
> b) allocator/mesos/roles/sort_run: Latency in role level sorts.
> c) allocator/mesos/quotas/sort_runs: Number of quota level sorts.
> d) allocator/mesos/quotas/sort_run: Latency in quota level sorts.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 123f97cf495bff0f822838e09df0d88818f04da6 
>   src/master/allocator/sorter/drf/metrics.hpp 
> 61568cb520826ab59d675824b212e0d3deb63764 
>   src/master/allocator/sorter/drf/metrics.cpp 
> ff63fbac5bbcf54e1ae39c3b650c0dafe7ea46d4 
>   src/master/allocator/sorter/drf/sorter.hpp 
> fee58d6d1f08163e2a06a4a20c891fe535c3dcff 
>   src/master/allocator/sorter/drf/sorter.cpp 
> 26b77f578f3235a8792c72d4575d607cdb2c7de7 
>   src/tests/hierarchical_allocator_tests.cpp 
> 08180b9975869de328f0c095dd3cddf0c84fbecf 
> 
> 
> Diff: https://reviews.apache.org/r/53841/diff/4/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 53841: Added metrics for sorting of the sorters in the allocator.

[Anindya Sinha]

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

(Updated May 16, 2017, 6:24 a.m.)


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


Changes
---

Rebased and minor edits.


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


Repository: mesos


Description
---

Following metrics have been added:
a) allocator/mesos/roles/sort_runs: Number of role level sorts.
b) allocator/mesos/roles/sort_run: Latency in role level sorts.
c) allocator/mesos/quotas/sort_runs: Number of quota level sorts.
d) allocator/mesos/quotas/sort_run: Latency in quota level sorts.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
123f97cf495bff0f822838e09df0d88818f04da6 
  src/master/allocator/sorter/drf/metrics.hpp 
61568cb520826ab59d675824b212e0d3deb63764 
  src/master/allocator/sorter/drf/metrics.cpp 
ff63fbac5bbcf54e1ae39c3b650c0dafe7ea46d4 
  src/master/allocator/sorter/drf/sorter.hpp 
fee58d6d1f08163e2a06a4a20c891fe535c3dcff 
  src/master/allocator/sorter/drf/sorter.cpp 
26b77f578f3235a8792c72d4575d607cdb2c7de7 
  src/tests/hierarchical_allocator_tests.cpp 
08180b9975869de328f0c095dd3cddf0c84fbecf 


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

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


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 53840: Metric in the allocator to track latency in running allocations.

[Anindya Sinha]

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

(Updated May 16, 2017, 6:23 a.m.)


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


Changes
---

Addressed review comments.


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


Repository: mesos


Description
---

Added a metric 'allocator/mesos/allocation_run_interval_ms' which
tracks the latency between scheduling an allocation to an actual
allocation run.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.cpp 
b75ed9a20a9a42f958cebbacd91e5e15b0d21394 
  src/master/allocator/mesos/metrics.hpp 
ce486eb079dc44bf0bbfad8668426c7ae87c97b3 
  src/master/allocator/mesos/metrics.cpp 
ec987fe0ce8605bf9e7c0ccbe3ac384deaf53be1 
  src/tests/hierarchical_allocator_tests.cpp 
08180b9975869de328f0c095dd3cddf0c84fbecf 


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

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


Testing
---

All tests passed.


Thanks,

Anindya Sinha

Re: Review Request 53840: Metric in the allocator to track latency in running allocations.

[Anindya Sinha]

> On May 15, 2017, 5:25 a.m., Jiang Yan Xu wrote:
> > src/tests/hierarchical_allocator_tests.cpp
> > Lines 3459-3463 (original), 3459-3462 (patched)
> > 
> >
> > Why this change? Seems like the two ways of expressing the expectation 
> > are equivalent.

Reinstated the original.


> On May 15, 2017, 5:25 a.m., Jiang Yan Xu wrote:
> > src/tests/hierarchical_allocator_tests.cpp
> > Lines 3537 (patched)
> > 
> >
> > Seems inelegant to reuse the test for `allocation_run`, for 
> > `allocation_run` we are storing a full list of statistics in `statistics` 
> > but we are not doing the same for the new metric. Can we create an 
> > independent test?

Created a separate test for `allocation_run_latency`.


- Anindya


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


On May 16, 2017, 6:23 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53840/
> ---
> 
> (Updated May 16, 2017, 6:23 a.m.)
> 
> 
> Review request for mesos, James Peach and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6579
> https://issues.apache.org/jira/browse/MESOS-6579
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a metric 'allocator/mesos/allocation_run_interval_ms' which
> tracks the latency between scheduling an allocation to an actual
> allocation run.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> b75ed9a20a9a42f958cebbacd91e5e15b0d21394 
>   src/master/allocator/mesos/metrics.hpp 
> ce486eb079dc44bf0bbfad8668426c7ae87c97b3 
>   src/master/allocator/mesos/metrics.cpp 
> ec987fe0ce8605bf9e7c0ccbe3ac384deaf53be1 
>   src/tests/hierarchical_allocator_tests.cpp 
> 08180b9975869de328f0c095dd3cddf0c84fbecf 
> 
> 
> Diff: https://reviews.apache.org/r/53840/diff/5/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>

Re: Review Request 53840: Metric in the allocator to track latency in running allocations.

[Anindya Sinha]

> On April 25, 2017, 4:41 p.m., James Peach wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Line 1427 (original), 1428 (patched)
> > 
> >
> > You also need to stop the latency metric here, otherwise you will 
> > capture multiple allocator dispatches.

As Yan mentioned, I stop the `allocation_run_latency` as the first thing done 
in this function.


- Anindya


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


On May 16, 2017, 6:23 a.m., Anindya Sinha wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53840/
> ---
> 
> (Updated May 16, 2017, 6:23 a.m.)
> 
> 
> Review request for mesos, James Peach and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6579
> https://issues.apache.org/jira/browse/MESOS-6579
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a metric 'allocator/mesos/allocation_run_interval_ms' which
> tracks the latency between scheduling an allocation to an actual
> allocation run.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> b75ed9a20a9a42f958cebbacd91e5e15b0d21394 
>   src/master/allocator/mesos/metrics.hpp 
> ce486eb079dc44bf0bbfad8668426c7ae87c97b3 
>   src/master/allocator/mesos/metrics.cpp 
> ec987fe0ce8605bf9e7c0ccbe3ac384deaf53be1 
>   src/tests/hierarchical_allocator_tests.cpp 
> 08180b9975869de328f0c095dd3cddf0c84fbecf 
> 
> 
> Diff: https://reviews.apache.org/r/53840/diff/5/
> 
> 
> Testing
> ---
> 
> All tests passed.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>