Re: Review Request 71291: Refactored (un)trackReservations() in the allocator.

[Mesos Reviewbot]

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



Bad review!

Reviews applied: [71291, 71269, 71258, 71257, 71255, 71254]

Error:
2019-08-16 04:23:48 URL:https://reviews.apache.org/r/71254/diff/raw/ 
[5967/5967] -> "71254.patch" [1]
error: patch failed: src/master/allocator/mesos/sorter/drf/sorter.cpp:17
error: src/master/allocator/mesos/sorter/drf/sorter.cpp: patch does not apply
error: patch failed: src/master/allocator/mesos/sorter/random/sorter.cpp:17
error: src/master/allocator/mesos/sorter/random/sorter.cpp: patch does not apply

- Mesos Reviewbot


On Aug. 14, 2019, 2:46 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71291/
> ---
> 
> (Updated Aug. 14, 2019, 2:46 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored (un)trackReservations() in the allocator.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 8be8dcee04e8fc5b97f730b2f058d14c81678788 
>   src/master/allocator/mesos/hierarchical.cpp 
> 580d35a3b71c1f7e851fa0504c6b9f037c05c378 
> 
> 
> Diff: https://reviews.apache.org/r/71291/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71269: Added a role tree class in the allocator.

[Meng Zhu]

> On Aug. 13, 2019, 7:58 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.hpp
> > Lines 231 (patched)
> > 
> >
> > This can be `hashmap` which would avoid the 
> > potential leak of forgetting `delete`, and avoid the extra memory 
> > allocation.
> 
> Meng Zhu wrote:
> hmm, would prefer keep the map just as a simple lookup table. Feels 
> somewhat convoluted if also using it to manage the life cycle.
> The simple destructors should be enough to guard against any leaks.
> Also, the "get" call would need to return a coy of `Role` instead of a 
> pointer, not quite what we want.
> 
> Benjamin Mahler wrote:
> > hmm, would prefer keep the map just as a simple lookup table. Feels 
> somewhat convoluted if also using it to manage the life cycle.
> 
> The map is already managing lifecycle? No `Role*` should exist outside 
> the map, right? If we remove a `Role*` from the map, it needs to be 
> `delete`ed right?
> 
> > Also, the "get" call would need to return a coy of Role instead of a 
> pointer, not quite what we want.
> 
> Why would you need to copy? You can just return a pointer to the value 
> from the map, that's no problem?

OK, didn't think deep about it. Updated the code to use the map for life cycle 
management.


- Meng


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


On Aug. 15, 2019, 8:36 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71269/
> ---
> 
> (Updated Aug. 15, 2019, 8:36 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9917
> https://issues.apache.org/jira/browse/MESOS-9917
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The role concept in Mesos fits into a tree structure naturally.
> However, the role state in the allocator are currenstored
> in a hashmap. This is less efficient and harder to use and reason.
> 
> This patch introduced a `RoleTree` structure in the allocator
> and organizes all the roles in to a tree. This should simplify
> the code logic and opens further refactor and optimization
> opportunities.
> 
> In addition, the master code also lacks a proper tree structure
> for tracking roles. We should leverage the same role tree code
> here to simplify that as well.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 8be8dcee04e8fc5b97f730b2f058d14c81678788 
>   src/master/allocator/mesos/hierarchical.cpp 
> 580d35a3b71c1f7e851fa0504c6b9f037c05c378 
> 
> 
> Diff: https://reviews.apache.org/r/71269/diff/4/
> 
> 
> Testing
> ---
> 
> make check
> 
> Benchmaring using optimized build shows no noticable performance change.
> 
> Benchmark: `BENCHMARK_HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota`
> 
> ## Before:
> 
> Added 30 agents in 1.252621ms
> Added 30 frameworks in 7.747202ms
> Benchmark setup: 30 agents, 30 roles, 30 frameworks, with drf sorter
> Made 36 allocations in 12.791427ms
> 
> Added 300 agents in 9.765295ms
> Added 300 frameworks in 273.978325ms
> Benchmark setup: 300 agents, 300 roles, 300 frameworks, with drf sorter
> Made 350 allocations in 424.603736ms
> 
> Added 3000 agents in 79.646516ms
> Added 3000 frameworks in 19.17449514secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with drf sorter
> Made 3500 allocations in 31.687207066secs
> 
> ## After:
> 
> Added 30 agents in 1.376619ms
> Added 30 frameworks in 7.647487ms
> Benchmark setup: 30 agents, 30 roles, 30 frameworks, with drf sorter
> Made 36 allocations in 11.638116ms
> 
> Added 300 agents in 9.506101ms
> Added 300 frameworks in 263.008198ms
> Benchmark setup: 300 agents, 300 roles, 300 frameworks, with drf sorter
> Made 350 allocations in 418.962396ms
> 
> Added 3000 agents in 81.553527ms
> Added 3000 frameworks in 19.201708305secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with drf sorter
> Made 3500 allocations in 31.583417136secs
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71269: Added a role tree class in the allocator.

[Meng Zhu]

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

(Updated Aug. 15, 2019, 8:36 p.m.)


Review request for mesos, Andrei Sekretenko and Benjamin Mahler.


Changes
---

Used map to manage the lifecycle of the role tree node.


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


Repository: mesos


Description
---

The role concept in Mesos fits into a tree structure naturally.
However, the role state in the allocator are currenstored
in a hashmap. This is less efficient and harder to use and reason.

This patch introduced a `RoleTree` structure in the allocator
and organizes all the roles in to a tree. This should simplify
the code logic and opens further refactor and optimization
opportunities.

In addition, the master code also lacks a proper tree structure
for tracking roles. We should leverage the same role tree code
here to simplify that as well.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
8be8dcee04e8fc5b97f730b2f058d14c81678788 
  src/master/allocator/mesos/hierarchical.cpp 
580d35a3b71c1f7e851fa0504c6b9f037c05c378 


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

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


Testing
---

make check

Benchmaring using optimized build shows no noticable performance change.

Benchmark: `BENCHMARK_HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota`

## Before:

Added 30 agents in 1.252621ms
Added 30 frameworks in 7.747202ms
Benchmark setup: 30 agents, 30 roles, 30 frameworks, with drf sorter
Made 36 allocations in 12.791427ms

Added 300 agents in 9.765295ms
Added 300 frameworks in 273.978325ms
Benchmark setup: 300 agents, 300 roles, 300 frameworks, with drf sorter
Made 350 allocations in 424.603736ms

Added 3000 agents in 79.646516ms
Added 3000 frameworks in 19.17449514secs
Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with drf sorter
Made 3500 allocations in 31.687207066secs

## After:

Added 30 agents in 1.376619ms
Added 30 frameworks in 7.647487ms
Benchmark setup: 30 agents, 30 roles, 30 frameworks, with drf sorter
Made 36 allocations in 11.638116ms

Added 300 agents in 9.506101ms
Added 300 frameworks in 263.008198ms
Benchmark setup: 300 agents, 300 roles, 300 frameworks, with drf sorter
Made 350 allocations in 418.962396ms

Added 3000 agents in 81.553527ms
Added 3000 frameworks in 19.201708305secs
Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with drf sorter
Made 3500 allocations in 31.583417136secs


Thanks,

Meng Zhu

Re: Review Request 71287: Added a test `SlaveTest.DefaultExecutorResources`.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Aug. 14, 2019, 12:54 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71287/
> ---
> 
> (Updated Aug. 14, 2019, 12:54 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Gilbert Song.
> 
> 
> Bugs: MESOS-9925
> https://issues.apache.org/jira/browse/MESOS-9925
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test `SlaveTest.DefaultExecutorResources`.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp 1a926029a9f81eb0c43e9cddd73c318557760c27 
> 
> 
> Diff: https://reviews.apache.org/r/71287/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 71244: Included task group's resources in the ExecutorInfo.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Aug. 14, 2019, 12:52 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71244/
> ---
> 
> (Updated Aug. 14, 2019, 12:52 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Gilbert Song.
> 
> 
> Bugs: MESOS-9925
> https://issues.apache.org/jira/browse/MESOS-9925
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Included task group's resources in the ExecutorInfo.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 74eb45744d6603b91676e812ed008a7b1ab39a49 
> 
> 
> Diff: https://reviews.apache.org/r/71244/diff/2/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 71174: Recovered network info for nested/standalone containers in CNI isolator.

[Gilbert Song]

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



Is it possible to avoid adding container_info in state?

This is a public facing API change. No longer can be chagned again once public 
module start adopting it

- Gilbert Song


On July 29, 2019, 1:40 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71174/
> ---
> 
> (Updated July 29, 2019, 1:40 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Gilbert Song.
> 
> 
> Bugs: MESOS-9909
> https://issues.apache.org/jira/browse/MESOS-9909
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Recovered network info for nested/standalone containers in CNI isolator.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/containerizer.proto 
> a60c96302a6cec90ecd0a0885b844fff8d37db71 
>   src/common/protobuf_utils.hpp 5d6a35d6e3bae35b83e87827724206f7c5dfb2d8 
>   src/common/protobuf_utils.cpp 7778e7f2475e9d6125d1c599715c91715f3654d3 
>   src/slave/containerizer/mesos/containerizer.cpp 
> a01edc8793a2eaa655f1729a01a01f1f61fbf7cb 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> f2989cf7a2161154bb7d9bf2112bee8dd3cc5cf5 
> 
> 
> Diff: https://reviews.apache.org/r/71174/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 71286: Added container transition times to the logs.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Aug. 13, 2019, 11:12 p.m., James Peach wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71286/
> ---
> 
> (Updated Aug. 13, 2019, 11:12 p.m.)
> 
> 
> Review request for mesos and Gilbert Song.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> It's very useful to know directly how long a container spent in each
> state. Capture a timestamp at each state transition time and log the
> elapsed time in the corresponding state.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/containerizer.hpp 
> 271d6326e9739634b9e2b5edece3ace20f3a150c 
>   src/slave/containerizer/mesos/containerizer.cpp 
> 75e7cdac499692be6838aa122578129d9cfa724c 
> 
> 
> Diff: https://reviews.apache.org/r/71286/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> James Peach
> 
>

Re: Review Request 71222: Added a test `VolumeSecretIsolatorCleanupTest.ROOT_FailInPreparing`.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Aug. 1, 2019, 12:18 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71222/
> ---
> 
> (Updated Aug. 1, 2019, 12:18 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Gilbert Song.
> 
> 
> Bugs: MESOS-9893
> https://issues.apache.org/jira/browse/MESOS-9893
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a test `VolumeSecretIsolatorCleanupTest.ROOT_FailInPreparing`.
> 
> 
> Diffs
> -
> 
>   src/tests/containerizer/volume_secret_isolator_tests.cpp 
> b6c43c3c3fcc61cd1bd03725cde78e58bcff801e 
> 
> 
> Diff: https://reviews.apache.org/r/71222/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 71221: Moved const string `.secret` to paths.hpp.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On Aug. 1, 2019, 12:16 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71221/
> ---
> 
> (Updated Aug. 1, 2019, 12:16 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Gilbert Song.
> 
> 
> Bugs: MESOS-9893
> https://issues.apache.org/jira/browse/MESOS-9893
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Moved const string `.secret` to paths.hpp.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/volume/secret.cpp 
> 4bbcc7af0216a483d71b367c154c24500545a40b 
>   src/slave/containerizer/mesos/paths.hpp 
> c0033354bb07c68e4dda3cd49ca2d23164343e79 
> 
> 
> Diff: https://reviews.apache.org/r/71221/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 71201: Implemented `cleanup` method for `volume/secret` isolator.

[Gilbert Song]

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


Ship it!




Ship It!

- Gilbert Song


On July 31, 2019, 12:04 a.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71201/
> ---
> 
> (Updated July 31, 2019, 12:04 a.m.)
> 
> 
> Review request for mesos, Andrei Budnik and Gilbert Song.
> 
> 
> Bugs: MESOS-9893
> https://issues.apache.org/jira/browse/MESOS-9893
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Previously, after `volume/secret` isolator resolves a secret and write
> it into a path (i.e., /.secret/) on agent host for a
> container, if the container fails to launch somehow (e.g., fails in
> another isolator's `prepare` method), that path on the host will never
> be cleaned up. In this patch, `volume/secret` isolator is improved to
> write all the resolved secrets for a container into a single directory
> (i.e., /.secret/) on agent host, and the
> `cleanup` method of the `volume/secret` isolator is implemented to
> remove that directory when the container is destroyed.
> 
> 
> Diffs
> -
> 
>   src/slave/containerizer/mesos/isolators/volume/secret.hpp 
> a1664915bc6c5ff223fcc9949448408883c3010c 
>   src/slave/containerizer/mesos/isolators/volume/secret.cpp 
> 4bbcc7af0216a483d71b367c154c24500545a40b 
> 
> 
> Diff: https://reviews.apache.org/r/71201/diff/1/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Qian Zhang
> 
>

Re: Review Request 71269: Added a role tree class in the allocator.

[Benjamin Mahler]

> On Aug. 14, 2019, 2:58 a.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.hpp
> > Lines 212-215 (patched)
> > 
> >
> > Ditto here, it seems at the overall explanation of the RoleTree 
> > abstraction we need to explain the "." subtlety, along with an example role 
> > tree to make it clear.
> 
> Meng Zhu wrote:
> I am not sure what you mean by the "." subtlety. Unlike sorter, there is 
> no need for any virtual node ".". I think the tree fits in the normal 
> expectation. But drew an example nevertheless.

Oh, somehow I didn't realize that this tree is not using the "." approach of 
the sorter's tree. My first impression of that is that we'd have a problem with 
role lifecycle, but I guess it's ok because the following invariant holds:

* If we remove a role implicitly, then it's the case that the caller will not 
try to remove it.

And now I'm realizing that we don't need "." because we model that using 
framework IDs (or other non-default configs) as the "leaves" (effectively).


> On Aug. 14, 2019, 2:58 a.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.hpp
> > Lines 231 (patched)
> > 
> >
> > This can be `hashmap` which would avoid the 
> > potential leak of forgetting `delete`, and avoid the extra memory 
> > allocation.
> 
> Meng Zhu wrote:
> hmm, would prefer keep the map just as a simple lookup table. Feels 
> somewhat convoluted if also using it to manage the life cycle.
> The simple destructors should be enough to guard against any leaks.
> Also, the "get" call would need to return a coy of `Role` instead of a 
> pointer, not quite what we want.

> hmm, would prefer keep the map just as a simple lookup table. Feels somewhat 
> convoluted if also using it to manage the life cycle.

The map is already managing lifecycle? No `Role*` should exist outside the map, 
right? If we remove a `Role*` from the map, it needs to be `delete`ed right?

> Also, the "get" call would need to return a coy of Role instead of a pointer, 
> not quite what we want.

Why would you need to copy? You can just return a pointer to the value from the 
map, that's no problem?


- Benjamin


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


On Aug. 14, 2019, 9:45 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71269/
> ---
> 
> (Updated Aug. 14, 2019, 9:45 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9917
> https://issues.apache.org/jira/browse/MESOS-9917
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The role concept in Mesos fits into a tree structure naturally.
> However, the role state in the allocator are currenstored
> in a hashmap. This is less efficient and harder to use and reason.
> 
> This patch introduced a `RoleTree` structure in the allocator
> and organizes all the roles in to a tree. This should simplify
> the code logic and opens further refactor and optimization
> opportunities.
> 
> In addition, the master code also lacks a proper tree structure
> for tracking roles. We should leverage the same role tree code
> here to simplify that as well.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 8be8dcee04e8fc5b97f730b2f058d14c81678788 
>   src/master/allocator/mesos/hierarchical.cpp 
> 580d35a3b71c1f7e851fa0504c6b9f037c05c378 
> 
> 
> Diff: https://reviews.apache.org/r/71269/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> Benchmaring using optimized build shows no noticable performance change.
> 
> Benchmark: `BENCHMARK_HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota`
> 
> ## Before:
> 
> Added 30 agents in 1.252621ms
> Added 30 frameworks in 7.747202ms
> Benchmark setup: 30 agents, 30 roles, 30 frameworks, with drf sorter
> Made 36 allocations in 12.791427ms
> 
> Added 300 agents in 9.765295ms
> Added 300 frameworks in 273.978325ms
> Benchmark setup: 300 agents, 300 roles, 300 frameworks, with drf sorter
> Made 350 allocations in 424.603736ms
> 
> Added 3000 agents in 79.646516ms
> Added 3000 frameworks in 19.17449514secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with drf sorter
> Made 3500 allocations in 31.687207066secs
> 
> ## After:
> 
> Added 30 agents in 1.376619ms
> Added 30 frameworks in 7.647487ms
> Benchmark setup: 30 agents, 30 roles, 30 frameworks, with drf sorter
> Made 36 allocations in 11.638116ms
> 
> Added 300 agents in 9.506101ms
> Added 300 frameworks in 263.008198ms
> Benchmark 

Re: Review Request 71291: Refactored (un)trackReservations() in the allocator.

[Benjamin Mahler]

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


Ship it!




- Benjamin Mahler


On Aug. 14, 2019, 9:46 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71291/
> ---
> 
> (Updated Aug. 14, 2019, 9:46 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Refactored (un)trackReservations() in the allocator.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> 8be8dcee04e8fc5b97f730b2f058d14c81678788 
>   src/master/allocator/mesos/hierarchical.cpp 
> 580d35a3b71c1f7e851fa0504c6b9f037c05c378 
> 
> 
> Diff: https://reviews.apache.org/r/71291/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 71151: Performed periodic storage local provider reconciliations.

[Mesos Reviewbot]

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



Patch looks great!

Reviews applied: [71143, 71144, 71145, 71146, 71147, 71148, 71149, 71150, 71151]

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. 15, 2019, 1:31 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71151/
> ---
> 
> (Updated Aug. 15, 2019, 1:31 p.m.)
> 
> 
> Review request for mesos and Chun-Hung Hsiao.
> 
> 
> Bugs: MESOS-9254
> https://issues.apache.org/jira/browse/MESOS-9254
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Performed periodic storage local provider reconciliations.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/storage/provider.cpp 
> 6d632606f411d3ca99d3573a57c9f68b02ba8072 
>   src/tests/storage_local_resource_provider_tests.cpp 
> 69b59d48ceefebbb7accefe411c54ac5cecff1c3 
> 
> 
> Diff: https://reviews.apache.org/r/71151/diff/5/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 71151: Performed periodic storage local provider reconciliations.

[Benjamin Bannier]

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

(Updated Aug. 15, 2019, 3:31 p.m.)


Review request for mesos and Chun-Hung Hsiao.


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


Repository: mesos


Description
---

Performed periodic storage local provider reconciliations.


Diffs (updated)
-

  src/resource_provider/storage/provider.cpp 
6d632606f411d3ca99d3573a57c9f68b02ba8072 
  src/tests/storage_local_resource_provider_tests.cpp 
69b59d48ceefebbb7accefe411c54ac5cecff1c3 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 71150: Factored out storage provider method to update resources.

[Benjamin Bannier]

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

(Updated Aug. 15, 2019, 3:31 p.m.)


Review request for mesos and Chun-Hung Hsiao.


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


Repository: mesos


Description
---

Factored out storage provider method to update resources.


Diffs (updated)
-

  src/resource_provider/storage/provider.cpp 
6d632606f411d3ca99d3573a57c9f68b02ba8072 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier