Re: Review Request 71341: Validated provider ID use in some resource provider calls.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [71339, 71340, 71341]

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

- Mesos Reviewbot


On Aug. 22, 2019, 6:04 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71341/
> ---
> 
> (Updated Aug. 22, 2019, 6:04 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.
> 
> 
> Bugs: MESOS-9482
> https://issues.apache.org/jira/browse/MESOS-9482
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For some calls we expect resource providers to set provider IDs with the
> calls. While the resource provider manager has always asserted that the
> calls were correct we never validated this.
> 
> With this patch we perform additional validation for calls taking a
> `ResourceProviderInfo` into account. We add both unit tests for the
> validation code and an integration test confirming that the validation
> is actually triggered.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/manager.cpp ceed1225b37d23998f523afcc2184dfaaad60636 
>   src/resource_provider/validation.cpp 
> df55b5efe3543c1dfd8441997302ab76fdd4bcc1 
>   src/tests/resource_provider_manager_tests.cpp 
> bcf6a03aa5d4931feff0299c811faa216efd95b6 
>   src/tests/resource_provider_validation_tests.cpp 
> a9989412ae30bd8244be808fc88fbe70f47d6ad9 
> 
> 
> Diff: https://reviews.apache.org/r/71341/diff/4/
> 
> 
> Testing
> ---
> 
> `ninja check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 71343: Fixed out-of-order processing of terminal status updates in agent.

[Gilbert Song]

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


Fix it, then Ship it!





src/slave/slave.cpp
Lines 10775-10776 (patched)


It does not seem likely to crash. But to be safe, could we consider to 
relax these CHECKs? E.g., just log a warning and return?


- Gilbert Song


On Aug. 21, 2019, 10:53 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71343/
> ---
> 
> (Updated Aug. 21, 2019, 10:53 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, and Qian Zhang.
> 
> 
> Bugs: MESOS-9887
> https://issues.apache.org/jira/browse/MESOS-9887
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, Mesos agent could send TASK_FAILED status update on
> executor termination while processing of TASK_FINISHED status update
> was in progress. Processing of task status updates involves sending
> requests to the containerizer, which might finish processing of these
> requests out-of-order, e.g. `MesosContainerizer::status`. Also,
> the agent does not overwrite status of the terminal status update once
> it's stored in the `terminatedTasks`. Hence, there was a race condition
> between two terminal status updates.
> 
> Note that V1 Executors are not affected by this problem because they
> wait for an acknowledgement of the terminal status update by the agent
> before terminating.
> 
> This patch introduces a new data structure `pendingStatusUpdates`,
> which holds a list of status updates that are being processed. This
> data structure allows validating the order of processing of status
> updates by the agent.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp a17bbee13cb8291ad694f1520b613764b57b046b 
>   src/slave/slave.cpp 1d0ec9d2428c3ffa28ad3e960b74f171013cf0c2 
> 
> 
> Diff: https://reviews.apache.org/r/71343/diff/2/
> 
> 
> Testing
> ---
> 
> 1. manual testing described in MESOS-9887
> 2. internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 71317: Added draining test for momentarily disconnected agents.

[Greg Mann]

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


Fix it, then Ship it!




Looks great! Just one small comment below.


src/tests/master_draining_tests.cpp
Lines 757 (patched)


The agent isn't unreachable here


- Greg Mann


On Aug. 23, 2019, 7:22 p.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71317/
> ---
> 
> (Updated Aug. 23, 2019, 7:22 p.m.)
> 
> 
> Review request for mesos, Benno Evers and Greg Mann.
> 
> 
> Bugs: MESOS-9892
> https://issues.apache.org/jira/browse/MESOS-9892
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This exercises the agent draining code when the agent is disconnected
> from the master at the time of starting draining.  Draining is expected
> to proceed once the agent reregisters.
> 
> 
> Diffs
> -
> 
>   src/tests/master_draining_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71317/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 71318: Added agent reactivations to the existing agent draining tests.

[Joseph Wu]

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

(Updated Aug. 23, 2019, 2:01 p.m.)


Review request for mesos, Benno Evers and Greg Mann.


Changes
---

Rebase!


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


Repository: mesos


Description
---

This adds an extra step to a couple of the agent draining tests,
which calls REACTIVATE_AGENT at the end.


Diffs (updated)
-

  src/tests/master_draining_tests.cpp PRE-CREATION 


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

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


Testing
---

make check


Thanks,

Joseph Wu

Re: Review Request 71318: Added agent reactivations to the existing agent draining tests.

[Mesos Reviewbot]

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



Bad review!

Reviews applied: [71318, 71317, 71316, 71315, 71314]

Error:
2019-08-23 20:18:10 URL:https://reviews.apache.org/r/71318/diff/raw/ 
[5411/5411] -> "71318.patch" [1]
error: patch failed: src/tests/master_draining_tests.cpp:825
error: src/tests/master_draining_tests.cpp: patch does not apply

- Mesos Reviewbot


On Aug. 20, 2019, 10:28 a.m., Joseph Wu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71318/
> ---
> 
> (Updated Aug. 20, 2019, 10:28 a.m.)
> 
> 
> Review request for mesos, Benno Evers and Greg Mann.
> 
> 
> Bugs: MESOS-9892
> https://issues.apache.org/jira/browse/MESOS-9892
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This adds an extra step to a couple of the agent draining tests,
> which calls REACTIVATE_AGENT at the end.
> 
> 
> Diffs
> -
> 
>   src/tests/master_draining_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71318/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Re: Review Request 71357: Used boost `small_vector` in `Resources`.

[Benjamin Mahler]

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


Fix it, then Ship it!





include/mesos/resources.hpp
Lines 636-647 (original), 637-650 (patched)


Ditto here from last review, seems like there's probably a cleaner way to 
get the const-ness?



include/mesos/v1/resources.hpp
Lines 756 (patched)


Can we document how we came up with 15 or what 15 can handle?

E.g. up to unreserved of 5 first class resources + 2 reservations of 5 
first class resources


- Benjamin Mahler


On Aug. 23, 2019, 7:44 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71357/
> ---
> 
> (Updated Aug. 23, 2019, 7:44 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9806
> https://issues.apache.org/jira/browse/MESOS-9806
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master + previous patch:
> *HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 16.307044003secs
> Made 0 allocation in 14.948262599secs
> 
> Master + previous patch + this patch:
> *HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 15.385276405secs
> Made 0 allocation in 13.718502414secs
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp e5e87a04b42ebd277d02100dd49b8e2ce98fdc04 
>   include/mesos/v1/resources.hpp 6a9751af967ed29ae77dc1c3e34453a306eae9d2 
> 
> 
> Diff: https://reviews.apache.org/r/71357/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> Benchmark result in the description
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71355: Used boost `small_vector` in Resource Quantities and Limits.

[Benjamin Mahler]

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


Fix it, then Ship it!





src/common/resource_quantities.cpp
Lines 155-157 (original), 150-153 (patched)


Is there a cleaner way to add const-ness?

E.g.

```
const auto& self = *this;
return self.begin();
```


- Benjamin Mahler


On Aug. 23, 2019, 7:44 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71355/
> ---
> 
> (Updated Aug. 23, 2019, 7:44 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9806
> https://issues.apache.org/jira/browse/MESOS-9806
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master + previous patch
> *HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 16.831380548secs
> Made 0 allocation in 15.102885644secs
> 
> Master + previous patch + this patch:
> *HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 16.307044003secs
> Made 0 allocation in 14.948262599secs
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_quantities.hpp 
> cdb34271868ab5931d7e35273af1219824d4d5b9 
>   src/common/resource_quantities.cpp 7c7ede32fd6e5aa4a960a8ca030b5aba115200f6 
> 
> 
> Diff: https://reviews.apache.org/r/71355/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> benchmark result in description
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71355: Used boost `small_vector` in Resource Quantities and Limits.

[Meng Zhu]

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

(Updated Aug. 23, 2019, 12:44 p.m.)


Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description (updated)
---

Master + previous patch
*HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
Made 3500 allocations in 16.831380548secs
Made 0 allocation in 15.102885644secs

Master + previous patch + this patch:
*HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
Made 3500 allocations in 16.307044003secs
Made 0 allocation in 14.948262599secs


Diffs (updated)
-

  include/mesos/resource_quantities.hpp 
cdb34271868ab5931d7e35273af1219824d4d5b9 
  src/common/resource_quantities.cpp 7c7ede32fd6e5aa4a960a8ca030b5aba115200f6 


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

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


Testing
---

make check
benchmark result in description


Thanks,

Meng Zhu

Re: Review Request 71357: Used boost `small_vector` in `Resources`.

[Meng Zhu]

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

(Updated Aug. 23, 2019, 12:44 p.m.)


Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description (updated)
---

Master + previous patch:
*HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
Made 3500 allocations in 16.307044003secs
Made 0 allocation in 14.948262599secs

Master + previous patch + this patch:
*HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
Made 3500 allocations in 15.385276405secs
Made 0 allocation in 13.718502414secs


Diffs (updated)
-

  include/mesos/resources.hpp e5e87a04b42ebd277d02100dd49b8e2ce98fdc04 
  include/mesos/v1/resources.hpp 6a9751af967ed29ae77dc1c3e34453a306eae9d2 


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

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


Testing
---

make check
Benchmark result in the description


Thanks,

Meng Zhu

Re: Review Request 71355: Used boost `small_vector` in Resource Quantities and Limits.

[Meng Zhu]

> On Aug. 23, 2019, 11:38 a.m., Benjamin Mahler wrote:
> > include/mesos/resource_quantities.hpp
> > Lines 168-169 (patched)
> > 
> >
> > How will we know to update this if we have more first class resources?
> > 
> > 5 seems ok if we think we'll to update this when we add another 
> > resource name

Made it 7 and add a comment. Since we don't a native concept of first class 
resources, I don't see a way to automate this or even raise a warning.


- Meng


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


On Aug. 23, 2019, 12:44 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71355/
> ---
> 
> (Updated Aug. 23, 2019, 12:44 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9806
> https://issues.apache.org/jira/browse/MESOS-9806
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master + previous patch
> *HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 16.831380548secs
> Made 0 allocation in 15.102885644secs
> 
> Master + previous patch + this patch:
> *HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 16.307044003secs
> Made 0 allocation in 14.948262599secs
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_quantities.hpp 
> cdb34271868ab5931d7e35273af1219824d4d5b9 
>   src/common/resource_quantities.cpp 7c7ede32fd6e5aa4a960a8ca030b5aba115200f6 
> 
> 
> Diff: https://reviews.apache.org/r/71355/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> benchmark result in description
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71360: Optimized shrinkResources() by filtering before copying.

[Benjamin Mahler]

> On Aug. 23, 2019, 7:25 p.m., Benjamin Mahler wrote:
> > src/common/resources_utils.cpp
> > Line 915 (original), 915-917 (patched)
> > 
> >
> > It's not clear to me that this is less expensive, if every name is in 
> > the target it's strictly more expensive?
> > 
> > How about adding an overload of the `RepeatedPtrField` 
> > operator that works off of an rvalue of the object? That way we can 
> > actually move out rather than copy out?

Oh.. I guess it won't help since every Resource will have a shared count > 1, 
so can't move it out.


- Benjamin


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


On Aug. 23, 2019, 4:56 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71360/
> ---
> 
> (Updated Aug. 23, 2019, 4:56 a.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9806
> https://issues.apache.org/jira/browse/MESOS-9806
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master + previous patch:
> 
> HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 15.635489759secs
> Made 0 allocation in 14.291803907secs
> 
> Master + previous patch + this patch:
> 
> HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 15.361303511secs
> Made 0 allocation in 14.323725559secs
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_quantities.hpp 
> cdb34271868ab5931d7e35273af1219824d4d5b9 
>   src/common/resource_quantities.cpp 7c7ede32fd6e5aa4a960a8ca030b5aba115200f6 
>   src/common/resources_utils.cpp cbdad4b045ad957b2ab1fd09a6b9fb12bb4fb523 
> 
> 
> Diff: https://reviews.apache.org/r/71360/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71360: Optimized shrinkResources() by filtering before copying.

[Benjamin Mahler]

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




src/common/resources_utils.cpp
Line 915 (original), 915-917 (patched)


It's not clear to me that this is less expensive, if every name is in the 
target it's strictly more expensive?

How about adding an overload of the `RepeatedPtrField` operator 
that works off of an rvalue of the object? That way we can actually move out 
rather than copy out?


- Benjamin Mahler


On Aug. 23, 2019, 4:56 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71360/
> ---
> 
> (Updated Aug. 23, 2019, 4:56 a.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9806
> https://issues.apache.org/jira/browse/MESOS-9806
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master + previous patch:
> 
> HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 15.635489759secs
> Made 0 allocation in 14.291803907secs
> 
> Master + previous patch + this patch:
> 
> HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 15.361303511secs
> Made 0 allocation in 14.323725559secs
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_quantities.hpp 
> cdb34271868ab5931d7e35273af1219824d4d5b9 
>   src/common/resource_quantities.cpp 7c7ede32fd6e5aa4a960a8ca030b5aba115200f6 
>   src/common/resources_utils.cpp cbdad4b045ad957b2ab1fd09a6b9fb12bb4fb523 
> 
> 
> Diff: https://reviews.apache.org/r/71360/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71316: Added draining tests for empty agents.

[Joseph Wu]

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

(Updated Aug. 23, 2019, 12:22 p.m.)


Review request for mesos, Benno Evers and Greg Mann.


Changes
---

Flipped the order of some `EXPECT_EQ` statements since the type inference 
causes a warning.


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


Repository: mesos


Description
---

This splits the existing agent draining tests into two variants:
1) where the agent has nothing running, and
2) where the agent has one task running.


Diffs (updated)
-

  src/tests/master_draining_tests.cpp PRE-CREATION 


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

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


Testing
---

make check


Thanks,

Joseph Wu

Re: Review Request 71317: Added draining test for momentarily disconnected agents.

[Joseph Wu]

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

(Updated Aug. 23, 2019, 12:22 p.m.)


Review request for mesos, Benno Evers and Greg Mann.


Changes
---

Flipped the order of some `EXPECT_EQ` statements since the type inference 
causes a warning.


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


Repository: mesos


Description
---

This exercises the agent draining code when the agent is disconnected
from the master at the time of starting draining.  Draining is expected
to proceed once the agent reregisters.


Diffs (updated)
-

  src/tests/master_draining_tests.cpp PRE-CREATION 


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

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


Testing
---

make check


Thanks,

Joseph Wu

Re: Review Request 71360: Optimized shrinkResources() by filtering before copying.

[Benjamin Mahler]

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




src/common/resource_quantities.cpp
Lines 183-197 (patched)


Do we need the new function? Can't we just:

```
  Value::Scalar zero;
 
  google::protobuf::RepeatedPtrField resourceVector =
resources.filter(
[](const Resource& r) { return target.get(r.name()) != zero; 
});
```


- Benjamin Mahler


On Aug. 23, 2019, 4:56 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71360/
> ---
> 
> (Updated Aug. 23, 2019, 4:56 a.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9806
> https://issues.apache.org/jira/browse/MESOS-9806
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master + previous patch:
> 
> HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 15.635489759secs
> Made 0 allocation in 14.291803907secs
> 
> Master + previous patch + this patch:
> 
> HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 15.361303511secs
> Made 0 allocation in 14.323725559secs
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_quantities.hpp 
> cdb34271868ab5931d7e35273af1219824d4d5b9 
>   src/common/resource_quantities.cpp 7c7ede32fd6e5aa4a960a8ca030b5aba115200f6 
>   src/common/resources_utils.cpp cbdad4b045ad957b2ab1fd09a6b9fb12bb4fb523 
> 
> 
> Diff: https://reviews.apache.org/r/71360/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71179: WIP: Added a flag for running example framework with a list of roles.

[Mesos Reviewbot]

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



Bad review!

Reviews applied: []

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

- Mesos Reviewbot


On July 29, 2019, 4:44 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71179/
> ---
> 
> (Updated July 29, 2019, 4:44 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Bugs: MESOS-8503
> https://issues.apache.org/jira/browse/MESOS-8503
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> WIP: Added a flag for running example framework with a list of roles.
> 
> 
> Diffs
> -
> 
>   src/examples/long_lived_framework.cpp 
> 621a5ca973640b810d429827dacf4ab72cfebbb8 
> 
> 
> Diff: https://reviews.apache.org/r/71179/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>

Re: Review Request 71355: Used boost `small_vector` in Resource Quantities and Limits.

[Benjamin Mahler]

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




include/mesos/resource_quantities.hpp
Lines 168-169 (patched)


How will we know to update this if we have more first class resources?

5 seems ok if we think we'll to update this when we add another resource 
name


- Benjamin Mahler


On Aug. 22, 2019, 11:58 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71355/
> ---
> 
> (Updated Aug. 22, 2019, 11:58 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9806
> https://issues.apache.org/jira/browse/MESOS-9806
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master
> *HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 23.37 secs
> Made 0 allocation in 19.72 secs
> 
> Master + this patch:
> *HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 22.065721878secs
> Made 0 allocation in 19.467366742secs
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_quantities.hpp 
> cdb34271868ab5931d7e35273af1219824d4d5b9 
>   src/common/resource_quantities.cpp 7c7ede32fd6e5aa4a960a8ca030b5aba115200f6 
> 
> 
> Diff: https://reviews.apache.org/r/71355/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> benchmark result in description
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71355: Used boost `small_vector` in Resource Quantities and Limits.

[Benjamin Mahler]

> On Aug. 23, 2019, 6:24 p.m., Benjamin Mahler wrote:
> > include/mesos/resource_quantities.hpp
> > Lines 121-124 (original), 122-127 (patched)
> > 
> >
> > "small_vector is convertible to small_vector_base > Allocator> that is independent from the preallocated element capacity, so 
> > client code does not need to be templated on that N argument."
> > 
> > Is it possible to expose the non-sized version to the clients? i.e. 
> > 
> > ```
> > boost::container::small_vector > Value::Scalar>>::const_iterator
> > ```

Whoops, small_vector -> small_vector_base


- Benjamin


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


On Aug. 22, 2019, 11:58 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71355/
> ---
> 
> (Updated Aug. 22, 2019, 11:58 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9806
> https://issues.apache.org/jira/browse/MESOS-9806
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master
> *HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 23.37 secs
> Made 0 allocation in 19.72 secs
> 
> Master + this patch:
> *HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 22.065721878secs
> Made 0 allocation in 19.467366742secs
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_quantities.hpp 
> cdb34271868ab5931d7e35273af1219824d4d5b9 
>   src/common/resource_quantities.cpp 7c7ede32fd6e5aa4a960a8ca030b5aba115200f6 
> 
> 
> Diff: https://reviews.apache.org/r/71355/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> benchmark result in description
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71355: Used boost `small_vector` in Resource Quantities and Limits.

[Benjamin Mahler]

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




include/mesos/resource_quantities.hpp
Lines 121-124 (original), 122-127 (patched)


"small_vector is convertible to small_vector_base that is independent from the preallocated element capacity, so 
client code does not need to be templated on that N argument."

Is it possible to expose the non-sized version to the clients? i.e. 

```
boost::container::small_vector>::const_iterator
```


- Benjamin Mahler


On Aug. 22, 2019, 11:58 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71355/
> ---
> 
> (Updated Aug. 22, 2019, 11:58 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9806
> https://issues.apache.org/jira/browse/MESOS-9806
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master
> *HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 23.37 secs
> Made 0 allocation in 19.72 secs
> 
> Master + this patch:
> *HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 22.065721878secs
> Made 0 allocation in 19.467366742secs
> 
> 
> Diffs
> -
> 
>   include/mesos/resource_quantities.hpp 
> cdb34271868ab5931d7e35273af1219824d4d5b9 
>   src/common/resource_quantities.cpp 7c7ede32fd6e5aa4a960a8ca030b5aba115200f6 
> 
> 
> Diff: https://reviews.apache.org/r/71355/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> benchmark result in description
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71356: Updated the boost library.

[Benjamin Mahler]

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


Ship it!




Ship It!

- Benjamin Mahler


On Aug. 22, 2019, 11:58 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71356/
> ---
> 
> (Updated Aug. 22, 2019, 11:58 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This update includes adding `container/small_vector.hpp`.
> 
> 
> Diffs
> -
> 
>   3rdparty/boost-1.65.0.tar.gz e9f02d71e85d04b9ab020a413d05c5e2522a0fc5 
>   3rdparty/boost.md e6373f295bac270cb164ee73db125967dadc3446 
>   3rdparty/cmake/Versions.cmake 0788e613251092a422e9a2e9f57ae9a59c793c7a 
> 
> 
> Diff: https://reviews.apache.org/r/71356/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71359: Optimized the allocation loop.

[Benjamin Mahler]

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


Ship it!




Nice one!


src/master/allocator/mesos/hierarchical.cpp
Line 2084 (original), 2102-2106 (patched)


now that it's written this way, maybe put `guaranteesAllocation + ...` at 
the front?



src/master/allocator/mesos/hierarchical.cpp
Lines 2086-2113 (original), 2108-2135 (patched)


Not for this review, just noticing that it might read easier if we have 
additionalScalarAllocation more contained (e.g. in a lambda)



src/master/allocator/mesos/hierarchical.cpp
Lines 2225-2226 (original), 2241-2242 (patched)


maybe:

Reservations (including the role's ancestors' reservations), non-scalar ...


- Benjamin Mahler


On Aug. 23, 2019, 4:55 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71359/
> ---
> 
> (Updated Aug. 23, 2019, 4:55 a.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9806
> https://issues.apache.org/jira/browse/MESOS-9806
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Master + previous patch:
> 
> HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 20.58648254secs
> Made 0 allocation in 17.705055744secs
> 
> Master + previous patch + this patch:
> 
> HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 15.635489759secs
> Made 0 allocation in 14.291803907secs
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 649de3b635a1ab95554406b0f7a061095f484ca4 
> 
> 
> Diff: https://reviews.apache.org/r/71359/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71347: Optimized shrinkResources.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [71353, 71354, 71347]

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

- Mesos Reviewbot


On Aug. 22, 2019, 3:34 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71347/
> ---
> 
> (Updated Aug. 22, 2019, 3:34 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Meng Zhu.
> 
> 
> Bugs: MESOS-9806
> https://issues.apache.org/jira/browse/MESOS-9806
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The shrinkResources function now is a friend of Resources,
> in order to perform an in-place shuffle of the vector.
> 
> Master + previous patches:
> *HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 23.37 secs
> Made 0 allocation in 19.72 secs
> 
> Master + previous patches + this patch:
> *HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/2
> Made 3500 allocations in 21.71 secs
> Made 0 allocation in 18.49 secs
> 
> 
> Diffs
> -
> 
>   include/mesos/resources.hpp a6a052ba51e13e2804eca846f08605e7f0714cfd 
>   src/common/resources_utils.cpp 720b954b74a5db72438b1846d7df837d6a1fa4a4 
> 
> 
> Diff: https://reviews.apache.org/r/71347/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 71178: Implemented displaying roles of multi-role frameworks as a tree.

[Andrei Sekretenko]

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

(Updated Aug. 23, 2019, 5:34 p.m.)


Review request for mesos, Benjamin Mahler and Meng Zhu.


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


Repository: mesos


Description
---

This patch makes the UI pages `frameworks` and `framework` display
roles of multi-role frameworks as collapsible tree (instead of a list).


Diffs (updated)
-

  src/webui/app/app.js f6f11384f35b260dcaff432c884dc063ea5e0f0e 
  src/webui/app/frameworks/framework.html 
82f6b279a9416d147fcfab094a326d67d0951dcc 
  src/webui/app/frameworks/frameworks.html 
d37c6137b638a27e5bd0f70f08733d81550b3ace 
  src/webui/app/frameworks/roles-tree-root.html PRE-CREATION 
  src/webui/app/frameworks/roles-tree.html PRE-CREATION 
  src/webui/app/frameworks/roles.html PRE-CREATION 
  src/webui/assets/css/mesos.css 0ff47cda36cc897f2e8f43804d38046f3e27e575 


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

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


Testing
---

Tested manually with conbination of frameworks with thousands of roles and 
frameworks with 1-2 roles.

Performance: re-rendering time of `frameworks` page with 50 frameworks with 
~4000 roles each is around 500 ms on my hardware. 
With all subtrees collapsed, more than half of this time is spent inside 
`intermediateRoleTree()` and `aggregateRoleTree()` (roughly equal amounts of 
time).
If/when the master starts to store the roles as a tree, it might make sense to 
convert this to getting roles in the tree form.


Thanks,

Andrei Sekretenko

Re: Review Request 71361: Added missing `return` statement in `Slave::statusUpdate`.

[Qian Zhang]

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


Ship it!




Ship It!

- Qian Zhang


On Aug. 23, 2019, 8:57 p.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71361/
> ---
> 
> (Updated Aug. 23, 2019, 8:57 p.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, and Qian Zhang.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, if `statusUpdate` was called for a pending task, it would
> forward the status update and then continue executing `statusUpdate`,
> which then checks if there is an executor that is aware of this task.
> Given that a pending task is not known to any executor, it would always
> handle it by forwarding status update one more time. This patch adds
> missing `return` statement, which fixes the issue.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 1d0ec9d2428c3ffa28ad3e960b74f171013cf0c2 
> 
> 
> Diff: https://reviews.apache.org/r/71361/diff/1/
> 
> 
> Testing
> ---
> 
> `sudo make check`
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Re: Review Request 71343: Fixed out-of-order processing of terminal status updates in agent.

[Qian Zhang]

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




src/slave/slave.cpp
Lines 6137-6138 (patched)


Nit: I'd suggest to swap these two lines.


- Qian Zhang


On Aug. 22, 2019, 1:53 a.m., Andrei Budnik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71343/
> ---
> 
> (Updated Aug. 22, 2019, 1:53 a.m.)
> 
> 
> Review request for mesos, Gilbert Song, Greg Mann, and Qian Zhang.
> 
> 
> Bugs: MESOS-9887
> https://issues.apache.org/jira/browse/MESOS-9887
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, Mesos agent could send TASK_FAILED status update on
> executor termination while processing of TASK_FINISHED status update
> was in progress. Processing of task status updates involves sending
> requests to the containerizer, which might finish processing of these
> requests out-of-order, e.g. `MesosContainerizer::status`. Also,
> the agent does not overwrite status of the terminal status update once
> it's stored in the `terminatedTasks`. Hence, there was a race condition
> between two terminal status updates.
> 
> Note that V1 Executors are not affected by this problem because they
> wait for an acknowledgement of the terminal status update by the agent
> before terminating.
> 
> This patch introduces a new data structure `pendingStatusUpdates`,
> which holds a list of status updates that are being processed. This
> data structure allows validating the order of processing of status
> updates by the agent.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp a17bbee13cb8291ad694f1520b613764b57b046b 
>   src/slave/slave.cpp 1d0ec9d2428c3ffa28ad3e960b74f171013cf0c2 
> 
> 
> Diff: https://reviews.apache.org/r/71343/diff/2/
> 
> 
> Testing
> ---
> 
> 1. manual testing described in MESOS-9887
> 2. internal CI
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>

Review Request 71361: Added missing `return` statement in `Slave::statusUpdate`.

[Andrei Budnik]

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

Review request for mesos, Gilbert Song, Greg Mann, and Qian Zhang.


Repository: mesos


Description
---

Previously, if `statusUpdate` was called for a pending task, it would
forward the status update and then continue executing `statusUpdate`,
which then checks if there is an executor that is aware of this task.
Given that a pending task is not known to any executor, it would always
handle it by forwarding status update one more time. This patch adds
missing `return` statement, which fixes the issue.


Diffs
-

  src/slave/slave.cpp 1d0ec9d2428c3ffa28ad3e960b74f171013cf0c2 


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


Testing
---

`sudo make check`


Thanks,

Andrei Budnik

Re: Review Request 71351: Sped up `ExampleTest.DiskFullFramework` test.

[Benjamin Bannier]

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

(Updated Aug. 23, 2019, 1:08 p.m.)


Review request for mesos and Till Toenshoff.


Changes
---

Rebase


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


Repository: mesos


Description
---

This patch lowers the authentication timeout and increases the disk
watch interval which can speed up the test by about 10s.


Diffs (updated)
-

  src/tests/disk_full_framework_test.sh 
83ae0947d4f5ec0a960e8cefa925cacbe6c6808e 


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

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


Testing
---

`make check`

With this and the preceeding patch the test takes <3s on my setup from >17s 
before.


Thanks,

Benjamin Bannier

Re: Review Request 71350: Used `local` master in `ExampleTest.DiskFullFramework`.

[Benjamin Bannier]

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

(Updated Aug. 23, 2019, 1:08 p.m.)


Review request for mesos and Till Toenshoff.


Changes
---

Address issue raised by Jan


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


Repository: mesos


Description
---

This removes up to 4s runtime from the test as we do not have to wait
for the master or agent to be up and running in the script.


Diffs (updated)
-

  cmake/MesosConfigure.cmake 83d41addcd2c14358fba8bab2ac654475626a3e8 
  src/tests/disk_full_framework_test.sh 
83ae0947d4f5ec0a960e8cefa925cacbe6c6808e 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 71285: Fixed recovery of agent resources and operations after crash.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [71284, 71285]

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

- Mesos Reviewbot


On Aug. 22, 2019, 8:40 p.m., Greg Mann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71285/
> ---
> 
> (Updated Aug. 22, 2019, 8:40 p.m.)
> 
> 
> Review request for mesos, Gastón Kleiman, James Peach, and Joseph Wu.
> 
> 
> Bugs: MESOS-9875
> https://issues.apache.org/jira/browse/MESOS-9875
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Fixes an issue where the agent may incorrectly send an
> OPERATION_FINISHED update for a failed offer operation
> following agent failover and recovery.
> 
> The agent previously relied on the difference between the
> set of checkpointed operations and the set of operation
> IDs recovered from the operation status update manager to
> apply any operations which had not been applied due to an
> ill-timed agent failover.
> 
> However, this approach did not work in the case where a
> persistent volume failed to be successfully created by
> syncCheckpointedResources(). In order to handle this
> case, this patch changes the agent code to continue with
> the old approach of a two-phase-commit of persistent
> volumes to disk, where the agent will fail to complete
> recovery if syncCheckpointedResources() cannot be
> executed successfully after failover.
> 
> 
> Diffs
> -
> 
>   src/slave/paths.hpp e077587fd02bd8e35fee7ce12ae436e3dca25e47 
>   src/slave/paths.cpp 28a7cf9f9c70fb31eeefe2e823cd7e19ffcf126a 
>   src/slave/slave.cpp 1d0ec9d2428c3ffa28ad3e960b74f171013cf0c2 
>   src/slave/state.cpp cd3fac72dd57da21ed5ac46b17066531af26d42a 
> 
> 
> Diff: https://reviews.apache.org/r/71285/diff/3/
> 
> 
> Testing
> ---
> 
> Tested manually by doing the following:
> 
> 1) Run a master
> 2) Run an agent with statically reserved resources for use in the following 
> step
> 3) Run the persistent volume example framework to create a volume on the 
> agent, which leads to checkpointing of resources
> 4) Use `chattr -R +i /agent/volumes/dir` to make the agent's persistent 
> volume directory immutable
> 5) Run the persistent volume example framework again; it will fail and the 
> agent will crash
> 6) Restart the agent; confirm that it continues to crash
> 7) Use `chattr -R -i /agent/volumes/dir` to remove the immutable attribute
> 8) Restart the agent; confirm that it recovers successfully, with the 
> persistent volume created on disk
> 9) Run the persistent volume example framework again; it should succeed
> 
> 
> Thanks,
> 
> Greg Mann
> 
>

Re: Review Request 71351: Sped up `ExampleTest.DiskFullFramework` test.

[Jan Schlicht]

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


Ship it!




Ship It!

- Jan Schlicht


On Aug. 22, 2019, 3:17 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71351/
> ---
> 
> (Updated Aug. 22, 2019, 3:17 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-9952
> https://issues.apache.org/jira/browse/MESOS-9952
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch lowers the authentication timeout and increases the disk
> watch interval which can speed up the test by about 10s.
> 
> 
> Diffs
> -
> 
>   src/tests/disk_full_framework_test.sh 
> 83ae0947d4f5ec0a960e8cefa925cacbe6c6808e 
> 
> 
> Diff: https://reviews.apache.org/r/71351/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> With this and the preceeding patch the test takes <3s on my setup from >17s 
> before.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 71350: Used `local` master in `ExampleTest.DiskFullFramework`.

[Jan Schlicht]

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


Fix it, then Ship it!





src/tests/disk_full_framework_test.sh
Line 15 (original), 13 (patched)


s/MESOS_RUNTIME_DIR/MESOS_WORK_DIR/


- Jan Schlicht


On Aug. 22, 2019, 3:17 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71350/
> ---
> 
> (Updated Aug. 22, 2019, 3:17 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-9952
> https://issues.apache.org/jira/browse/MESOS-9952
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This removes up to 4s runtime from the test as we do not have to wait
> for the master or agent to be up and running in the script.
> 
> 
> Diffs
> -
> 
>   src/tests/disk_full_framework_test.sh 
> 83ae0947d4f5ec0a960e8cefa925cacbe6c6808e 
> 
> 
> Diff: https://reviews.apache.org/r/71350/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 71341: Validated provider ID use in some resource provider calls.

[Jan Schlicht]

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


Ship it!




Ship It!

- Jan Schlicht


On Aug. 22, 2019, 3:04 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71341/
> ---
> 
> (Updated Aug. 22, 2019, 3:04 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Jan Schlicht.
> 
> 
> Bugs: MESOS-9482
> https://issues.apache.org/jira/browse/MESOS-9482
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> For some calls we expect resource providers to set provider IDs with the
> calls. While the resource provider manager has always asserted that the
> calls were correct we never validated this.
> 
> With this patch we perform additional validation for calls taking a
> `ResourceProviderInfo` into account. We add both unit tests for the
> validation code and an integration test confirming that the validation
> is actually triggered.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/manager.cpp ceed1225b37d23998f523afcc2184dfaaad60636 
>   src/resource_provider/validation.cpp 
> df55b5efe3543c1dfd8441997302ab76fdd4bcc1 
>   src/tests/resource_provider_manager_tests.cpp 
> bcf6a03aa5d4931feff0299c811faa216efd95b6 
>   src/tests/resource_provider_validation_tests.cpp 
> a9989412ae30bd8244be808fc88fbe70f47d6ad9 
> 
> 
> Diff: https://reviews.apache.org/r/71341/diff/4/
> 
> 
> Testing
> ---
> 
> `ninja check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 71335: Used cached cgroups for updating resources in Docker containerizer.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Aug. 21, 2019, 5:24 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71335/
> ---
> 
> (Updated Aug. 21, 2019, 5:24 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Gilbert Song.
> 
> 
> Bugs: MESOS-9836
> https://issues.apache.org/jira/browse/MESOS-9836
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Used cached cgroups for updating resources in Docker containerizer.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/docker.hpp ca41f3b6f48f583b0aa1eb07df01d3872b80fa6c 
>   src/slave/containerizer/docker.cpp e4ad94572d34d49d599197afc08fd937253ecb0f 
>   src/tests/containerizer/docker_containerizer_tests.cpp 
> e25acfda336f10b2e3d6a79d1a336a290dc5b407 
> 
> 
> Diff: https://reviews.apache.org/r/71335/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 71318: Added agent reactivations to the existing agent draining tests.

[Mesos Reviewbot]

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



Bad patch!

Reviews applied: [71314, 71315, 71316, 71317, 71318]

Failed command: ['bash', '-c', "set -o pipefail; export OS='ubuntu:14.04' 
BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose 
--disable-libtool-wrappers --disable-parallel-test-execution' 
ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh 2>&1 | tee 
build_71318"]

Error:
..
/\" -DHAVE_CXX11=1 -DHAVE_PTHREAD_PRIO_INHERIT=1 -DHAVE_PTHREAD=1 
-DHAVE_OPENSSL_SSL_H=1 -DHAVE_FTS_H=1 -DHAVE_APR_POOLS_H=1 -DHAVE_LIBAPR_1=1 
-DHAVE_LIBCURL=1 -DMESOS_HAS_JAVA=1 -DHAVE_LIBSASL2=1 -DHAVE_SVN_VERSION_H=1 
-DHAVE_LIBSVN_SUBR_1=1 -DHAVE_SVN_DELTA_H=1 -DHAVE_LIBSVN_DELTA_1=1 
-DHAVE_ZLIB_H=1 -DHAVE_LIBZ=1 -DHAVE_PYTHON=\"2.7\" -DMESOS_HAS_PYTHON=1 -I. 
-I../../src   -Werror -DLIBDIR=\"/mesos/mesos-1.9.0/_inst/lib\" 
-DPKGLIBEXECDIR=\"/mesos/mesos-1.9.0/_inst/libexec/mesos\" 
-DPKGDATADIR=\"/mesos/mesos-1.9.0/_inst/share/mesos\" 
-DPKGMODULEDIR=\"/mesos/mesos-1.9.0/_inst/lib/mesos/modules\" -I../../include 
-I../include -I../include/mesos -D__STDC_FORMAT_MACROS 
-I../3rdparty/boost-1.65.0 -I../3rdparty/concurrentqueue-7b69a8f 
-I../3rdparty/elfio-3.2 -I../3rdparty/glog-0.4.0/src 
-I../3rdparty/grpc-1.10.0/include -I../3rdparty/leveldb-1.19/include 
-I../3rdparty/libarchive-3.3.2/libarchive/ -I../../3rdparty/libprocess/include  
-I../3rdparty/nvml-352.79 -I../3rdparty/picojson-1.3.0 -
 I../3rdparty/protobuf-3.5.0/src -I../3rdparty/rapidjson-1.1.0/include 
-I../../3rdparty/stout/include -I../3rdparty/zookeeper-3.4.8/src/c/include 
-I../3rdparty/zookeeper-3.4.8/src/c/generated 
-DSOURCE_DIR=\"/mesos/mesos-1.9.0/_build/..\" 
-DBUILD_DIR=\"/mesos/mesos-1.9.0/_build\" 
-I../3rdparty/googletest-release-1.8.0/googletest/include 
-I../3rdparty/googletest-release-1.8.0/googlemock/include 
-DTESTLIBEXECDIR=\"/mesos/mesos-1.9.0/_inst/libexec/mesos/tests\" 
-DSBINDIR=\"/mesos/mesos-1.9.0/_inst/sbin\" 
-I/usr/lib/jvm/java-7-openjdk-amd64/include 
-I/usr/lib/jvm/java-7-openjdk-amd64/include/linux -DZOOKEEPER_VERSION=\"3.4.8\" 
-I/usr/include/subversion-1 -I/usr/include/apr-1 -I/usr/include/apr-1.0 
 -pthread -Wall -Wsign-compare -Wformat-security -fstack-protector -fPIC -fPIE 
-g1 -O0 -Wno-unused-local-typedefs -std=c++11 -c -o 
tests/mesos_tests-cram_md5_authentication_tests.o `test -f 
'tests/cram_md5_authentication_tests.cpp' || echo 
'../../src/'`tests/cram_md5_authentication_tests.c
 pp
g++ -DPACKAGE_NAME=\"mesos\" -DPACKAGE_TARNAME=\"mesos\" 
-DPACKAGE_VERSION=\"1.9.0\" -DPACKAGE_STRING=\"mesos\ 1.9.0\" 
-DPACKAGE_BUGREPORT=\"\" -DPACKAGE_URL=\"\" -DPACKAGE=\"mesos\" 
-DVERSION=\"1.9.0\" -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 
-DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 
-DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DHAVE_DLFCN_H=1 
-DLT_OBJDIR=\".libs/\" -DHAVE_CXX11=1 -DHAVE_PTHREAD_PRIO_INHERIT=1 
-DHAVE_PTHREAD=1 -DHAVE_OPENSSL_SSL_H=1 -DHAVE_FTS_H=1 -DHAVE_APR_POOLS_H=1 
-DHAVE_LIBAPR_1=1 -DHAVE_LIBCURL=1 -DMESOS_HAS_JAVA=1 -DHAVE_LIBSASL2=1 
-DHAVE_SVN_VERSION_H=1 -DHAVE_LIBSVN_SUBR_1=1 -DHAVE_SVN_DELTA_H=1 
-DHAVE_LIBSVN_DELTA_1=1 -DHAVE_ZLIB_H=1 -DHAVE_LIBZ=1 -DHAVE_PYTHON=\"2.7\" 
-DMESOS_HAS_PYTHON=1 -I. -I../../src   -Werror 
-DLIBDIR=\"/mesos/mesos-1.9.0/_inst/lib\" 
-DPKGLIBEXECDIR=\"/mesos/mesos-1.9.0/_inst/libexec/mesos\" 
-DPKGDATADIR=\"/mesos/mesos-1.9.0/_inst/share/mesos\" 
-DPKGMODULEDIR=\"/mesos/mesos-
 1.9.0/_inst/lib/mesos/modules\" -I../../include -I../include 
-I../include/mesos -D__STDC_FORMAT_MACROS -I../3rdparty/boost-1.65.0 
-I../3rdparty/concurrentqueue-7b69a8f -I../3rdparty/elfio-3.2 
-I../3rdparty/glog-0.4.0/src -I../3rdparty/grpc-1.10.0/include 
-I../3rdparty/leveldb-1.19/include -I../3rdparty/libarchive-3.3.2/libarchive/ 
-I../../3rdparty/libprocess/include  -I../3rdparty/nvml-352.79 
-I../3rdparty/picojson-1.3.0 -I../3rdparty/protobuf-3.5.0/src 
-I../3rdparty/rapidjson-1.1.0/include -I../../3rdparty/stout/include 
-I../3rdparty/zookeeper-3.4.8/src/c/include 
-I../3rdparty/zookeeper-3.4.8/src/c/generated 
-DSOURCE_DIR=\"/mesos/mesos-1.9.0/_build/..\" 
-DBUILD_DIR=\"/mesos/mesos-1.9.0/_build\" 
-I../3rdparty/googletest-release-1.8.0/googletest/include 
-I../3rdparty/googletest-release-1.8.0/googlemock/include 
-DTESTLIBEXECDIR=\"/mesos/mesos-1.9.0/_inst/libexec/mesos/tests\" 
-DSBINDIR=\"/mesos/mesos-1.9.0/_inst/sbin\" 
-I/usr/lib/jvm/java-7-openjdk-amd64/include -I/usr/lib/jvm/java-7-
 openjdk-amd64/include/linux -DZOOKEEPER_VERSION=\"3.4.8\" 
-I/usr/include/subversion-1 -I/usr/include/apr-1 -I/usr/include/apr-1.0 
-pthread -Wall -Wsign-compare -Wformat-security -fstack-protector -fPIC 
-fPIE -g1 -O0 -Wno-unused-local-typedefs -std=c++11 -c -o 
tests/mesos_tests-credentials_tests.o `test -f