Re: Review Request 71303: Tracked frameworks in the role sorter.

2019-08-16 Thread Mesos Reviewbot

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



Bad review!

Reviews applied: [71303, 71301, 71269, 71258, 71257, 71255, 71254]

Error:
2019-08-17 03:08:34 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. 17, 2019, 2 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71303/
> ---
> 
> (Updated Aug. 17, 2019, 2 a.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9917
> https://issues.apache.org/jira/browse/MESOS-9917
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This paves the way for removing the framework sorters.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 580d35a3b71c1f7e851fa0504c6b9f037c05c378 
>   src/master/allocator/mesos/sorter/drf/sorter.hpp 
> 537086039fd804453ea8c682cda775d8fdff038f 
>   src/master/allocator/mesos/sorter/drf/sorter.cpp 
> 09889cdf3dc8c0e773b8e2e24154fb0edd2cc254 
>   src/master/allocator/mesos/sorter/random/sorter.hpp 
> f18b014ed15ff8906fbdbd3248becefde896651c 
>   src/master/allocator/mesos/sorter/random/sorter.cpp 
> 60a5797472460a8d3d9be938af9f6711ea51d484 
>   src/master/allocator/mesos/sorter/sorter.hpp 
> 52b8a7b57bf17759311b32aa56c26e614119b773 
> 
> 
> Diff: https://reviews.apache.org/r/71303/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>



Re: Review Request 71302: Marked /quota endpoint as deprecated.

2019-08-16 Thread Mesos Reviewbot

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



Bad patch!

Reviews applied: [71302]

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_71302"]

Error:
..
red disk(allocated: storage/default-role)(reservations: 
[(DYNAMIC,storage),(DYNAMIC,storage/default-role,test-principal)])[MOUNT(org.apache.mesos.csi.test.local,/tmp/CSIVersion_StorageLocalResourceProviderTest_OperatorOperationsWithResourceProviderResources_v0_sJbyTV/2GB-271ab96b-b358-417b-a2e2-d63c8da133bf,test),34b4d8b9-6cff-4741-89a3-40e1dc6b7f00:volume]:2048;
 cpus(allocated: storage/default-role):2; mem(allocated: 
storage/default-role):1024; disk(allocated: storage/default-role):1024; 
ports(allocated: storage/default-role):[31000-32000] (total: cpus:2; mem:1024; 
disk:1024; ports:[31000-32000]; disk(reservations: 
[(DYNAMIC,storage),(DYNAMIC,storage/default-role,test-principal)])[MOUNT(org.apache.mesos.csi.test.local,/tmp/CSIVersion_StorageLocalResourceProviderTest_OperatorOperationsWithResourceProviderResources_v0_sJbyTV/2GB-271ab96b-b358-417b-a2e2-d63c8da133bf,test),34b4d8b9-6cff-4741-89a3-40e1dc6b7f00:volume]:2048,
 allocated: {}) on agent 0e44d8e8-aa0c-466c-aab9-36a25cbdabc4-S0 
 from framework 0e44d8e8-aa0c-466c-aab9-36a25cbdabc4-
I0817 02:23:24.089661 18756 hierarchical.cpp:1268] Framework 
0e44d8e8-aa0c-466c-aab9-36a25cbdabc4- filtered agent 
0e44d8e8-aa0c-466c-aab9-36a25cbdabc4-S0 for 5secs
I0817 02:23:24.093324 18759 master.cpp:12615] Sending operation '' (uuid: 
aa128e0b-7efb-4ae7-a759-b21d4ea40ee5) to agent 
0e44d8e8-aa0c-466c-aab9-36a25cbdabc4-S0 at slave(1224)@172.17.0.2:46053 
(a1a8a771d01f)
I0817 02:23:24.093925 18760 slave.cpp:4352] Ignoring new checkpointed resources 
and operations identical to the current version
I0817 02:23:24.097290 18749 provider.cpp:481] Received APPLY_OPERATION event
I0817 02:23:24.097327 18749 provider.cpp:1295] Received DESTROY operation '' 
(uuid: aa128e0b-7efb-4ae7-a759-b21d4ea40ee5)
I0817 02:23:24.101243 18759 http.cpp:1115] HTTP POST for 
/slave(1224)/api/v1/resource_provider from 172.17.0.2:33404
I0817 02:23:24.102300 18757 slave.cpp:8423] Handling resource provider message 
'UPDATE_OPERATION_STATUS: (uuid: 7c98d58c-8a3f-427c-94b6-0170fc950978) for 
framework  (latest state: OPERATION_FINISHED, status update state: 
OPERATION_FINISHED)'
I0817 02:23:24.102572 18757 slave.cpp:8876] Updating the state of operation 
with no ID (uuid: 7c98d58c-8a3f-427c-94b6-0170fc950978) for an operation API 
call (latest state: OPERATION_FINISHED, status update state: OPERATION_FINISHED)
I0817 02:23:24.102648 18757 slave.cpp:8630] Forwarding status update of 
operation with no ID (operation_uuid: 7c98d58c-8a3f-427c-94b6-0170fc950978) for 
an operator API call
I0817 02:23:24.103116 18752 master.cpp:12271] Updating the state of operation 
'' (uuid: 7c98d58c-8a3f-427c-94b6-0170fc950978) for an operator API call 
(latest state: OPERATION_PENDING, status update state: OPERATION_FINISHED)
I0817 02:23:24.103746 18753 slave.cpp:4352] Ignoring new checkpointed resources 
and operations identical to the current version
I0817 02:23:24.103826 18760 hierarchical.cpp:1510] Performed allocation for 1 
agents in 1.443724ms
I0817 02:23:24.104676 18748 master.cpp:10432] Sending offers [ 
0e44d8e8-aa0c-466c-aab9-36a25cbdabc4-O5 ] to framework 
0e44d8e8-aa0c-466c-aab9-36a25cbdabc4- (default) at 
scheduler-e9821285-3f93-46bc-a771-07ed7646716a@172.17.0.2:46053
I0817 02:23:24.105330 18759 sched.cpp:934] Scheduler::resourceOffers took 
79462ns
I0817 02:23:24.108573 18752 status_update_manager_process.hpp:152] Received 
operation status update OPERATION_FINISHED (Status UUID: 
34b90631-d8ee-400e-bf33-28ba6833aeea) for operation UUID 
aa128e0b-7efb-4ae7-a759-b21d4ea40ee5 on agent 
0e44d8e8-aa0c-466c-aab9-36a25cbdabc4-S0
I0817 02:23:24.108635 18752 status_update_manager_process.hpp:414] Creating 
operation status update stream aa128e0b-7efb-4ae7-a759-b21d4ea40ee5 
checkpoint=true
I0817 02:23:24.108711 18749 provider.cpp:481] Received 
ACKNOWLEDGE_OPERATION_STATUS event
I0817 02:23:24.108919 18752 status_update_manager_process.hpp:929] 
Checkpointing UPDATE for operation status update OPERATION_FINISHED (Status 
UUID: 34b90631-d8ee-400e-bf33-28ba6833aeea) for operation UUID 
aa128e0b-7efb-4ae7-a759-b21d4ea40ee5 on agent 
0e44d8e8-aa0c-466c-aab9-36a25cbdabc4-S0
I0817 02:23:24.125324 18752 status_update_manager_process.hpp:528] Forwarding 
operation status update OPERATION_FINISHED (Status UUID: 
34b90631-d8ee-400e-bf33-28ba6833aeea) for operation UUID 
aa128e0b-7efb-4ae7-a759-b21d4ea40ee5 on agent 
0e44d8e8-aa0c-466c-aa

Review Request 71303: Tracked frameworks in the role sorter.

2019-08-16 Thread Meng Zhu

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

Review request for mesos, Andrei Sekretenko and Benjamin Mahler.


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


Repository: mesos


Description
---

This paves the way for removing the framework sorters.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
580d35a3b71c1f7e851fa0504c6b9f037c05c378 
  src/master/allocator/mesos/sorter/drf/sorter.hpp 
537086039fd804453ea8c682cda775d8fdff038f 
  src/master/allocator/mesos/sorter/drf/sorter.cpp 
09889cdf3dc8c0e773b8e2e24154fb0edd2cc254 
  src/master/allocator/mesos/sorter/random/sorter.hpp 
f18b014ed15ff8906fbdbd3248becefde896651c 
  src/master/allocator/mesos/sorter/random/sorter.cpp 
60a5797472460a8d3d9be938af9f6711ea51d484 
  src/master/allocator/mesos/sorter/sorter.hpp 
52b8a7b57bf17759311b32aa56c26e614119b773 


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


Testing
---

make check


Thanks,

Meng Zhu



Re: Review Request 71300: Removed mesos-style transition script.

2019-08-16 Thread Mesos Reviewbot

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



Patch looks great!

Reviews applied: [70096, 71203, 71204, 71299, 71205, 71206, 71207, 71208, 
71209, 71300]

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. 16, 2019, 12:42 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71300/
> ---
> 
> (Updated Aug. 16, 2019, 12:42 p.m.)
> 
> 
> Review request for mesos, Benno Evers and Till Toenshoff.
> 
> 
> Bugs: MESOS-9630
> https://issues.apache.org/jira/browse/MESOS-9630
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Removed mesos-style transition script.
> 
> 
> Diffs
> -
> 
>   support/mesos-style.py cd490bd3deeb8477295bbad010f36d984ce17d27 
> 
> 
> Diff: https://reviews.apache.org/r/71300/diff/2/
> 
> 
> Testing
> ---
> 
> n/a
> 
> THIS PATCH SHOULD ONLY BE COMMITTED AFTER THE PRECEEDING CHAIN HAS BEEN 
> LANDED FOR SOME TIME TO GIVE CONTRIBUTORS A CHANCE TO ADJUST THEIR WORKFLOW.
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 71302: Marked /quota endpoint as deprecated.

2019-08-16 Thread Meng Zhu

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


Ship it!




Ship It!

- Meng Zhu


On Aug. 16, 2019, 4:09 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71302/
> ---
> 
> (Updated Aug. 16, 2019, 4:09 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Meng Zhu.
> 
> 
> Bugs: MESOS-9669
> https://issues.apache.org/jira/browse/MESOS-9669
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This endpoint is already hidden from the quota documentation,
> this updates the endpoint to note the deprecation.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 684a0f73f9a598fbc44e64b9a66aadbf91b7e7e6 
>   src/master/master.hpp 783e4a38ad9e36cb38d5e78b6b942033bc806a0a 
>   src/master/master.cpp 599f62d1affe0961bdf01b41c009563b008b8a2a 
> 
> 
> Diff: https://reviews.apache.org/r/71302/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>



Review Request 71302: Marked /quota endpoint as deprecated.

2019-08-16 Thread Benjamin Mahler

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

Review request for mesos, Andrei Sekretenko and Meng Zhu.


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


Repository: mesos


Description
---

This endpoint is already hidden from the quota documentation,
this updates the endpoint to note the deprecation.


Diffs
-

  src/master/http.cpp 684a0f73f9a598fbc44e64b9a66aadbf91b7e7e6 
  src/master/master.hpp 783e4a38ad9e36cb38d5e78b6b942033bc806a0a 
  src/master/master.cpp 599f62d1affe0961bdf01b41c009563b008b8a2a 


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


Testing
---

make check


Thanks,

Benjamin Mahler



Review Request 71301: Added a framework id field to the allocator Framework struct.

2019-08-16 Thread Meng Zhu

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

Review request for mesos, Andrei Sekretenko and Benjamin Mahler.


Repository: mesos


Description
---

This provides easy lookup.

Also refactored related functions to take in `Framework&`
instead of `FrameworkId&`.


Diffs
-

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


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


Testing
---

make check


Thanks,

Meng Zhu



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

2019-08-16 Thread Meng Zhu

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

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


Review request for mesos, Andrei Sekretenko and Benjamin Mahler.


Changes
---

Embedded a metrics handle in the role sorter to publish role related metrics.


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

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


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 71291: Refactored (un)trackReservations() in the allocator.

2019-08-16 Thread Mesos Reviewbot

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



Bad review!

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

Error:
2019-08-16 21:31:33 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.

2019-08-16 Thread Meng Zhu


> On Aug. 16, 2019, 9:41 a.m., Benjamin Mahler wrote:
> > I think the biggest thing to consider here is that the current interface 
> > still allows the caller to easily break the tracking invariants: the caller 
> > has to ensure that it always calls tryRemove whenever making a modification 
> > to Role that could make it "empty". Much like the reservation tracking, we 
> > could resolve this by having functions in the tree class for:
> > 
> > - update quota
> > - update weight
> > - track / untrack framework id
> > 
> > That will make the Role read-only for a caller, and enforce the role 
> > tracking invariants internally without the caller having to care about it. 
> > See comment below.

Yeah, I was thinking of doing these in subsequent patches. Might as well do it 
now. Also sweep over the interface to make everything as tight as possible.


- Meng


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


On Aug. 16, 2019, 1:40 p.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71269/
> ---
> 
> (Updated Aug. 16, 2019, 1:40 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/5/
> 
> 
> 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.

2019-08-16 Thread Meng Zhu

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

(Updated Aug. 16, 2019, 1:40 p.m.)


Review request for mesos, Andrei Sekretenko and Benjamin Mahler.


Changes
---

Addressed Ben's comment. Encapsulated the Role and RoleTree interface as tight 
as possible.


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

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


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 71297: Fixed a flaky operation reconciliation test.

2019-08-16 Thread Mesos Reviewbot

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



Patch looks great!

Reviews applied: [71297]

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. 16, 2019, 1:57 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71297/
> ---
> 
> (Updated Aug. 16, 2019, 1:57 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko, Greg Mann, Joseph Wu, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-9928
> https://issues.apache.org/jira/browse/MESOS-9928
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The FrameworkReconciliationRaceWithUpdateSlave test from the
> operation reconciliation tests was flaky since we did not wait
> for the scheduler to reconnect before advancing the clock to
> trigger reregistration.
> 
> 
> Diffs
> -
> 
>   src/tests/operation_reconciliation_tests.cpp 
> 9d084c027ec2f910515cafebf715f7428c43f1a9 
> 
> 
> Diff: https://reviews.apache.org/r/71297/diff/1/
> 
> 
> Testing
> ---
> 
> `./src/mesos-tests 
> --gtest_filter="*FrameworkReconciliationRaceWithUpdateSlaveMessage*" 
> --gtest_repeat=200` while simultaneously running `stress-ng` in the 
> background.
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Review Request 71299: Added separate script to install developer setup.

2019-08-16 Thread Benjamin Bannier

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

Review request for mesos, Benno Evers and Till Toenshoff.


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


Repository: mesos


Description
---

This patch breaks the installation of developer tools (i.e., linter
configuration files and git hooks) out of `./bootstrap`. This not only
simplifies and streamlines the setup, but will allow us to add
developer-only features without breaking users who are just interested
in building a distribution tarball.


Diffs
-

  bootstrap 7be2cc95c7ace55d710315427f38284cc5b2af4c 
  bootstrap.bat 584b5c3ca228ff485b22473cd167d3f454d6dad4 
  docs/advanced-contribution.md 573138d3642e901f46ee35db58299627f188b94c 
  docs/beginner-contribution.md 471f5dd0da30d921cc3b29484d2b02f164f8ca75 
  docs/clang-format.md 4289813c2ca78e1b963aa22a596abd6e8aa3a28b 
  support/gitignore 7218eda0d78b8e6fc4568f215016961bd7a11a1b 
  support/llvm/README.md 188930224ba8ff53dbef8770bc56625be936bb76 
  support/setup_dev.sh PRE-CREATION 


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


Testing
---


Thanks,

Benjamin Bannier



Review Request 71300: Removed mesos-style transition script.

2019-08-16 Thread Benjamin Bannier

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

Review request for mesos, Benno Evers and Till Toenshoff.


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


Repository: mesos


Description
---

Removed mesos-style transition script.


Diffs
-

  support/mesos-style.py cd490bd3deeb8477295bbad010f36d984ce17d27 


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


Testing
---

n/a

THIS PATCH SHOULD ONLY BE COMMITTED AFTER THE PRECEEDING CHAIN HAS BEEN LANDED 
FOR SOME TIME TO GIVE CONTRIBUTORS A CHANCE TO ADJUST THEIR WORKFLOW.


Thanks,

Benjamin Bannier



Re: Review Request 71205: Switch commit hooks to pre-commit.

2019-08-16 Thread Benjamin Bannier


> On Aug. 14, 2019, 1:04 p.m., Benno Evers wrote:
> > bootstrap
> > Line 55 (original), 55 (patched)
> > 
> >
> > Copying my comment from slack here so the discussion isn't split over 
> > too many places:
> > 
> > As we discussed privately yesterday, I think installing this in 
> > bootstrap is a bit problematic because that is also part of the source 
> > tarball, used by non-developers to build mesos w/o even having a git 
> > repository. Additionally, I think I'd be not particularly amused if I 
> > cloned a random open-source project and the first thing it does is install 
> > a bunch of stuff in my home directory.
> > 
> > I'm not sure if it's possible to implement, but imho the ideal workflow 
> > would be something like this:
> > 
> > ```
> > $ git commit -m "Foo the bar."
> > WARNING: Your commit touched a `.cpp` file, but `cpplint` is not 
> > installed on your system. It is highly recommended to install it to avoid 
> > embarrassing mistakes.
> > 
> > You can also run `pre-commit ` to automatically install a usable 
> > version of `cpplint` in you home directory.
> > ```
> 
> Benjamin Bannier wrote:
> Totally agree on the mix of building and dev setup in `bootstrap` is 
> problematic. Let me break out an `autogen.sh` to be used for setting up a 
> build env, and `boostrap` also setting up a dev env.
> 
> We already download packages from the internet from `mesos-style.py` when 
> e.g., setting up the virtualenv to run node linters. There is however no 
> caching so one needs to potentially download this again and again after 
> clearing the build dir; with `pre-commit` files are cached in `$HOME/.cache` 
> and even for different linter versions or configs with the lifetime managed 
> explicitly with `pre-commit`. I feel this is a superior, more controllable 
> approach than pulling stuff with weird heuristics (`mesos-style.py` hardcodes 
> a few of these and they tend to perform more work than necessary).
> 
> The only dependency of the linters now becomes `pre-commit` (which needs 
> to be clearly documented) and potentially some Python interpreters or sim. 
> Linters like e.g., eslint are automatically set up and do not need to be 
> installed by users (in fact we want to control the exact versions of these 
> tools, so `pre-commit` enforcing that is great). `cpplint` is a linter 
> present in the source tree and does not need to be installed.
> 
> Benjamin Bannier wrote:
> I added a dummy script for `support/mesos-style.py` to ease the 
> transition. In another patch I'll also split installation of dev tools out of 
> `./bootstrap` to reduce dependencies non-contributors need to install, and 
> add additional documentation.

Broke installation of dev tools out into https://reviews.apache.org/r/71299/; 
transitional `./support/mesos-style.py` is removed with 
https://reviews.apache.org/r/71299/.


- Benjamin


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


On July 30, 2019, 11:01 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71205/
> ---
> 
> (Updated July 30, 2019, 11:01 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-9360
> https://issues.apache.org/jira/browse/MESOS-9360
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch switches commit hooks to be orchestrated by the pre-commit
> tool mirroring the previous linters invoked through git commit
> hooks (orchestrated by `support/mesos-style.py` or standalone hooks).
> 
> Using pre-commit removes the burden of maintaining
> `support/mesos-style.py`, making sure that hooks have the expected
> environment (e.g., Python version, Node installed). Additionally,
> upstream provides a number of additional linters which are not hard to
> add to Mesos' hooks.
> 
> 
> Diffs
> -
> 
>   .pre-commit-config.yaml PRE-CREATION 
>   bootstrap 7be2cc95c7ace55d710315427f38284cc5b2af4c 
> 
> 
> Diff: https://reviews.apache.org/r/71205/diff/2/
> 
> 
> Testing
> ---
> 
> * used successfully for a couple of months
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 71206: Removed old mesos-style and references.

2019-08-16 Thread Benjamin Bannier

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

(Updated Aug. 16, 2019, 9:42 p.m.)


Review request for mesos and Till Toenshoff.


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


Repository: mesos


Description
---

This patch removes references to `support/mesos-style.py` which was
replaced with a pre-commit setup in a previous commit. We also remove
the tool itself.


Diffs (updated)
-

  docs/c++-style-guide.md 8a48afe780f23736c9b7abeb7337977521cecfa5 
  support/build-virtualenv 7dc03b054f7663979e4eb4b11ad51d759b7f1ad3 
  support/hooks/commit-msg a0c218deee3fb4b7594fe39b76c1025045ba0725 
  support/hooks/post-rewrite 1ab14abf711d1923a7ae69beb33581317009a94a 
  support/hooks/pre-commit 519567bf5f20a74b273c8d8514577fe4342dc45d 
  support/mesos-split.py 0a77c257386ffe576abd12f59f926640836ad900 
  support/mesos-style.py cd490bd3deeb8477295bbad010f36d984ce17d27 


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

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


Testing
---

n/a


Thanks,

Benjamin Bannier



Re: Review Request 71205: Switch commit hooks to pre-commit.

2019-08-16 Thread Benjamin Bannier


> On Aug. 14, 2019, 1:04 p.m., Benno Evers wrote:
> > bootstrap
> > Line 55 (original), 55 (patched)
> > 
> >
> > Copying my comment from slack here so the discussion isn't split over 
> > too many places:
> > 
> > As we discussed privately yesterday, I think installing this in 
> > bootstrap is a bit problematic because that is also part of the source 
> > tarball, used by non-developers to build mesos w/o even having a git 
> > repository. Additionally, I think I'd be not particularly amused if I 
> > cloned a random open-source project and the first thing it does is install 
> > a bunch of stuff in my home directory.
> > 
> > I'm not sure if it's possible to implement, but imho the ideal workflow 
> > would be something like this:
> > 
> > ```
> > $ git commit -m "Foo the bar."
> > WARNING: Your commit touched a `.cpp` file, but `cpplint` is not 
> > installed on your system. It is highly recommended to install it to avoid 
> > embarrassing mistakes.
> > 
> > You can also run `pre-commit ` to automatically install a usable 
> > version of `cpplint` in you home directory.
> > ```
> 
> Benjamin Bannier wrote:
> Totally agree on the mix of building and dev setup in `bootstrap` is 
> problematic. Let me break out an `autogen.sh` to be used for setting up a 
> build env, and `boostrap` also setting up a dev env.
> 
> We already download packages from the internet from `mesos-style.py` when 
> e.g., setting up the virtualenv to run node linters. There is however no 
> caching so one needs to potentially download this again and again after 
> clearing the build dir; with `pre-commit` files are cached in `$HOME/.cache` 
> and even for different linter versions or configs with the lifetime managed 
> explicitly with `pre-commit`. I feel this is a superior, more controllable 
> approach than pulling stuff with weird heuristics (`mesos-style.py` hardcodes 
> a few of these and they tend to perform more work than necessary).
> 
> The only dependency of the linters now becomes `pre-commit` (which needs 
> to be clearly documented) and potentially some Python interpreters or sim. 
> Linters like e.g., eslint are automatically set up and do not need to be 
> installed by users (in fact we want to control the exact versions of these 
> tools, so `pre-commit` enforcing that is great). `cpplint` is a linter 
> present in the source tree and does not need to be installed.

I added a dummy script for `support/mesos-style.py` to ease the transition. In 
another patch I'll also split installation of dev tools out of `./bootstrap` to 
reduce dependencies non-contributors need to install, and add additional 
documentation.


- Benjamin


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


On July 30, 2019, 11:01 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71205/
> ---
> 
> (Updated July 30, 2019, 11:01 p.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-9360
> https://issues.apache.org/jira/browse/MESOS-9360
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch switches commit hooks to be orchestrated by the pre-commit
> tool mirroring the previous linters invoked through git commit
> hooks (orchestrated by `support/mesos-style.py` or standalone hooks).
> 
> Using pre-commit removes the burden of maintaining
> `support/mesos-style.py`, making sure that hooks have the expected
> environment (e.g., Python version, Node installed). Additionally,
> upstream provides a number of additional linters which are not hard to
> add to Mesos' hooks.
> 
> 
> Diffs
> -
> 
>   .pre-commit-config.yaml PRE-CREATION 
>   bootstrap 7be2cc95c7ace55d710315427f38284cc5b2af4c 
> 
> 
> Diff: https://reviews.apache.org/r/71205/diff/2/
> 
> 
> Testing
> ---
> 
> * used successfully for a couple of months
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



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

2019-08-16 Thread Benjamin Mahler

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



I think the biggest thing to consider here is that the current interface still 
allows the caller to easily break the tracking invariants: the caller has to 
ensure that it always calls tryRemove whenever making a modification to Role 
that could make it "empty". Much like the reservation tracking, we could 
resolve this by having functions in the tree class for:

- update quota
- update weight
- track / untrack framework id

That will make the Role read-only for a caller, and enforce the role tracking 
invariants internally without the caller having to care about it. See comment 
below.


src/master/allocator/mesos/hierarchical.hpp
Line 112 (original), 113-119 (patched)


an extra 'or' and should be 'subscribed'

also, since we have a list, would be clearer to bullet it and indent it:

```
// We track a role when it has:
//
//   * a non-default weight, or
//   * a non-default quota, or
//   * frameworks subscribed to it, or
//   * reservations, or
//   * descendent roles meeting any of the above conditions.
```



src/master/allocator/mesos/hierarchical.hpp
Line 119 (original), 129 (patched)


s/getR/r/



src/master/allocator/mesos/hierarchical.hpp
Lines 145-146 (patched)


Align the comments or don't?

```
  const std::string role; // E.g. "a/b/c"
  const std::string baseName; // E.g. "c"
```

or:

```
  const std::string role; // E.g. "a/b/c"
  const std::string baseName; // E.g. "c"
```



src/master/allocator/mesos/hierarchical.hpp
Lines 183-193 (patched)


Ok, now that I understand that you can reason about this tree as having 
leaves that are {framework id, non default weight, non default quota, 
reservations}, it's clearer to me. This example doesn't really help with that, 
so I think we can do without it.



src/master/allocator/mesos/hierarchical.hpp
Lines 203-204 (patched)


s/getAllRoles/roles/



src/master/allocator/mesos/hierarchical.hpp
Lines 211-214 (patched)


>   // Try to remove the role associated with the role.

whoops?

> The role and its ancestors will be removed if they become "empty".

This seems a little inaccurate given it's only the ancestors that can 
"become" empty during this call, whereas the role must "be" empty to be removed?

This is a significant break in the interface where the caller has to be 
very careful to call tryRemove whenever potentially making an Role empty. It 
exists because we've allowed mutation on Role. We could make it automatic if we 
added additional mutation functions in the tree itself, just like we did for 
trackReservations/untrackReservations:

- track / untrack framework id
- update quota
- update weight

With these, we also won't need the `add(role)` function, the Role tracking 
will be fully managed by the tree interface and the caller can't screw it up by 
adding an empty Role or forgetting to tryRemove an empty Role. Re-iterated this 
in the top-level comment.



src/master/allocator/mesos/hierarchical.hpp
Lines 217-224 (patched)


Just my 2 cents, it would have been better to do the `hashmap -> Resources` change in front of this patch or in this patch if 
that's not possible, so that there's 1 less thing the reviewer has to hold in 
their head.



src/master/allocator/mesos/hierarchical.cpp
Lines 1401-1403 (original), 1567-1577 (patched)


Yikes.. all this logic would become:

```
roleTree.updateQuota(role, quota);
```

If we have the tree take care of the lifecycle management.



src/master/allocator/mesos/hierarchical.cpp
Line 1419 (original), 1595-1606 (patched)


Ditto here.


- Benjamin Mahler


On Aug. 16, 2019, 3:36 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71269/
> ---
> 
> (Updated Aug. 16, 2019, 3:36 a.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 fit

Re: Review Request 71209: Enabled a number of additional pre-commit checks.

2019-08-16 Thread Mesos Reviewbot

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



Patch looks great!

Reviews applied: [70096, 71203, 71204, 71205, 71206, 71207, 71208, 71209]

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 July 31, 2019, 5:01 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71209/
> ---
> 
> (Updated July 31, 2019, 5:01 a.m.)
> 
> 
> Review request for mesos and Till Toenshoff.
> 
> 
> Bugs: MESOS-9360
> https://issues.apache.org/jira/browse/MESOS-9360
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Enabled a number of additional pre-commit checks.
> 
> 
> Diffs
> -
> 
>   .pre-commit-config.yaml PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/71209/diff/1/
> 
> 
> Testing
> ---
> 
> * used for development for a couple of months
> * reports no issues in the current source tree as indentified issues were 
> fixed
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 71297: Fixed a flaky operation reconciliation test.

2019-08-16 Thread Andrei Sekretenko

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




src/tests/operation_reconciliation_tests.cpp
Lines 1842 (patched)


We are restarting the master once, but the scheduler gets 
connected/disconnected event pair twice during the master restart... is this 
guaranteed by TestMesos + StandaloneMasterDetector, a stable coincidence, or 
just the most likely outcome of some other race? 

IMO, this is at least worth a comment - but what about preventing it? (Will 
something like `detector->appoint(None())` + `AWAIT_READY(disconnected)` before 
killing the master help?)


- Andrei Sekretenko


On Aug. 16, 2019, 1:57 p.m., Benno Evers wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71297/
> ---
> 
> (Updated Aug. 16, 2019, 1:57 p.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko, Greg Mann, Joseph Wu, and Till 
> Toenshoff.
> 
> 
> Bugs: MESOS-9928
> https://issues.apache.org/jira/browse/MESOS-9928
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The FrameworkReconciliationRaceWithUpdateSlave test from the
> operation reconciliation tests was flaky since we did not wait
> for the scheduler to reconnect before advancing the clock to
> trigger reregistration.
> 
> 
> Diffs
> -
> 
>   src/tests/operation_reconciliation_tests.cpp 
> 9d084c027ec2f910515cafebf715f7428c43f1a9 
> 
> 
> Diff: https://reviews.apache.org/r/71297/diff/1/
> 
> 
> Testing
> ---
> 
> `./src/mesos-tests 
> --gtest_filter="*FrameworkReconciliationRaceWithUpdateSlaveMessage*" 
> --gtest_repeat=200` while simultaneously running `stress-ng` in the 
> background.
> 
> 
> Thanks,
> 
> Benno Evers
> 
>



Review Request 71297: Fixed a flaky operation reconciliation test.

2019-08-16 Thread Benno Evers

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

Review request for mesos, Andrei Sekretenko, Greg Mann, Joseph Wu, and Till 
Toenshoff.


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


Repository: mesos


Description
---

The FrameworkReconciliationRaceWithUpdateSlave test from the
operation reconciliation tests was flaky since we did not wait
for the scheduler to reconnect before advancing the clock to
trigger reregistration.


Diffs
-

  src/tests/operation_reconciliation_tests.cpp 
9d084c027ec2f910515cafebf715f7428c43f1a9 


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


Testing
---

`./src/mesos-tests 
--gtest_filter="*FrameworkReconciliationRaceWithUpdateSlaveMessage*" 
--gtest_repeat=200` while simultaneously running `stress-ng` in the background.


Thanks,

Benno Evers



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

2019-08-16 Thread Qian Zhang


> On Aug. 16, 2019, 4:45 a.m., Gilbert Song wrote:
> > 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

Unless we let CNI isolator itself persist container_info, I am afraid that 
adding container_info in state is the only way that we can get container_info 
during recovery because currently containerizer is the only one who perisists 
container_info (as part of `ContainerConfig`). Before this patch, in 
`ContainerState` we only have container_info for top-level containers but not 
for any nested containers, I think container_info is a useful info for 
recovering nested containers too, and there may be other isolators needs such 
info to do recovery as well in future, so I think it should not be harmful.


- Qian


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


On July 29, 2019, 4:40 p.m., Qian Zhang wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71174/
> ---
> 
> (Updated July 29, 2019, 4:40 p.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
> 
>