Re: Review Request 70628: Return 409 if `UPDATE_RESOURCE_PROVIDER_CONFIG` names a missing config.

2019-05-10 Thread Mesos Reviewbot 

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



Patch looks great!

Reviews applied: [70628]

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 May 10, 2019, 11:07 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70628/
> ---
> 
> (Updated May 10, 2019, 11:07 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Greg Mann, and James DeFelice.
> 
> 
> Bugs: MESOS-9779
> https://issues.apache.org/jira/browse/MESOS-9779
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Since 404 is returned if the API endpoint route is not set yet, this
> error code becomes ambiguous and makes clients hard to programmatically
> handle it. Therefore, the error code for specifying a missing config
> in this API call is changed to 409 Conflict.
> 
> 
> Diffs
> -
> 
>   include/mesos/agent/agent.proto ff408a4a1f144237e7425961aa3d11cba163e7c0 
>   include/mesos/v1/agent/agent.proto 19d6c424365f8fd6e6d4a5bb7a61fbe5a66f031d 
>   src/slave/http.cpp 2c4e792d16ad4fd3303760a9db3cba4269152e7d 
>   src/tests/agent_resource_provider_config_api_tests.cpp 
> 7185ac00b5137c7ab208b623e36d03ffd43aab6e 
> 
> 
> Diff: https://reviews.apache.org/r/70628/diff/1/
> 
> 
> Testing
> ---
> 
> sudo make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 70627: Explicitly marked agent resource provider config calls as experimental.

2019-05-10 Thread Mesos Reviewbot 

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



Patch looks great!

Reviews applied: [70627]

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 May 10, 2019, 9:17 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70627/
> ---
> 
> (Updated May 10, 2019, 9:17 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and James DeFelice.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The resource provider feature has been marked as experimental, so we
> should also call out the related config API calls are experimental.
> 
> 
> Diffs
> -
> 
>   include/mesos/agent/agent.proto ff408a4a1f144237e7425961aa3d11cba163e7c0 
>   include/mesos/v1/agent/agent.proto 19d6c424365f8fd6e6d4a5bb7a61fbe5a66f031d 
> 
> 
> Diff: https://reviews.apache.org/r/70627/diff/1/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Review Request 70628: Return 409 if `UPDATE_RESOURCE_PROVIDER_CONFIG` names a missing config.

2019-05-10 Thread Chun-Hung Hsiao 

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

Review request for mesos, Benjamin Bannier, Greg Mann, and James DeFelice.


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


Repository: mesos


Description
---

Since 404 is returned if the API endpoint route is not set yet, this
error code becomes ambiguous and makes clients hard to programmatically
handle it. Therefore, the error code for specifying a missing config
in this API call is changed to 409 Conflict.


Diffs
-

  include/mesos/agent/agent.proto ff408a4a1f144237e7425961aa3d11cba163e7c0 
  include/mesos/v1/agent/agent.proto 19d6c424365f8fd6e6d4a5bb7a61fbe5a66f031d 
  src/slave/http.cpp 2c4e792d16ad4fd3303760a9db3cba4269152e7d 
  src/tests/agent_resource_provider_config_api_tests.cpp 
7185ac00b5137c7ab208b623e36d03ffd43aab6e 


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


Testing
---

sudo make check


Thanks,

Chun-Hung Hsiao

Re: Review Request 70627: Explicitly marked agent resource provider config calls as experimental.

2019-05-10 Thread James DeFelice 

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


Ship it!




Ship It!

- James DeFelice


On May 10, 2019, 9:17 p.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70627/
> ---
> 
> (Updated May 10, 2019, 9:17 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and James DeFelice.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The resource provider feature has been marked as experimental, so we
> should also call out the related config API calls are experimental.
> 
> 
> Diffs
> -
> 
>   include/mesos/agent/agent.proto ff408a4a1f144237e7425961aa3d11cba163e7c0 
>   include/mesos/v1/agent/agent.proto 19d6c424365f8fd6e6d4a5bb7a61fbe5a66f031d 
> 
> 
> Diff: https://reviews.apache.org/r/70627/diff/1/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Review Request 70627: Explicitly marked agent resource provider config calls as experimental.

2019-05-10 Thread Chun-Hung Hsiao 

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

Review request for mesos, Benjamin Bannier and James DeFelice.


Repository: mesos


Description
---

The resource provider feature has been marked as experimental, so we
should also call out the related config API calls are experimental.


Diffs
-

  include/mesos/agent/agent.proto ff408a4a1f144237e7425961aa3d11cba163e7c0 
  include/mesos/v1/agent/agent.proto 19d6c424365f8fd6e6d4a5bb7a61fbe5a66f031d 


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


Testing
---

N/A


Thanks,

Chun-Hung Hsiao

Re: Review Request 70618: Encapsulate a framework sorter inside a RoleInfo.

2019-05-10 Thread Mesos Reviewbot 

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



Patch looks great!

Reviews applied: [70626, 70591, 70618]

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 May 10, 2019, 4:32 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70618/
> ---
> 
> (Updated May 10, 2019, 4:32 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9701
> https://issues.apache.org/jira/browse/MESOS-9701
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch consolidates the logic of tracking frameworks tied to
> a specific role in the `RoleInfo`.
> 
> To do this properly, `RoleInfo` is turned into a class which wraps
> access to the framework sorter so that no other entity is capable
> of adding/removing clients (the frameworks) to/from the sorter.
> 
> The patch introduces almost no performance impact.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> c2058baca5159da4cdcab77afd5de3c0d5ae6c48 
>   src/master/allocator/mesos/hierarchical.cpp 
> 64a076ddd29711437d539a06bb0470755828cc87 
> 
> 
> Diff: https://reviews.apache.org/r/70618/diff/2/
> 
> 
> Testing
> ---
> 
> make check
> 
> Benchmarking: 5 runs of 
> BENCHMARK_HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/5 with the 
> optimized build.
> 
> BEFORE (master):
> 
> Added 3000 agents in 52.729305ms
> Added 3000 frameworks in 13.799200231secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 12.560201008secs
> Made 0 allocation in 12.143722849secs
> 
> Added 3000 agents in 64.789364ms
> Added 3000 frameworks in 14.175436362secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 12.505877513secs
> Made 0 allocation in 12.424587206secs
> 
> Added 3000 agents in 50.942631ms
> Added 3000 frameworks in 14.26206239secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 12.220950624secs
> Made 0 allocation in 12.495832704secs
> 
> Added 3000 agents in 50.660372ms
> Added 3000 frameworks in 14.246438788secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 12.505903032secs
> Made 0 allocation in 12.334731087secs
> 
> Added 3000 agents in 50.292757ms
> Added 3000 frameworks in 14.236187327secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 12.531212952secs
> Made 0 allocation in 12.282740343secs
> 
> ---
> AFTER (master + previous patch https://reviews.apache.org/r/70591/ + this 
> patch):
> 
> Added 3000 agents in 51.465368ms
> Added 3000 frameworks in 13.599017611secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 12.272279231secs
> Made 0 allocation in 12.026509432secs
> 
> Added 3000 agents in 52.5567ms
> Added 3000 frameworks in 13.67345101secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 12.447551363secs
> Made 0 allocation in 12.045692187secs
> 
> Added 3000 agents in 52.455703ms
> Added 3000 frameworks in 13.344338641secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 11.988632162secs
> Made 0 allocation in 11.558150131secs
> 
> Added 3000 agents in 53.579201ms
> Added 3000 frameworks in 13.681435728secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 11.966754231secs
> Made 0 allocation in 12.720889223secs
> 
> Added 3000 agents in 52.08713ms
> Added 3000 frameworks in 13.562008608secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 12.19201724secs
> Made 0 allocation in 11.727037034secs
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>

Re: Review Request 70579: Added a Task Scheduler to simplify testing.

2019-05-10 Thread Chun-Hung Hsiao 


> On May 10, 2019, 3:31 a.m., Chun-Hung Hsiao wrote:
> > src/tests/utils/task_scheduler.cpp
> > Lines 289 (patched)
> > 
> >
> > ```
> > offers_.erase(std::remove_if(...), offers_.end());
> > ```
> > Although the effect is the same since there will be only one element 
> > removed, pairing `std::remove_if` and `end` will be more idomatic.
> 
> Benjamin Mahler wrote:
> Hm.. why is this an "issue" and why is that more idiomatic..? Erase has 
> two versions, one take a position and one takes a range.
> 
> Looking at this code again, it's rather unreadable, and I probably will 
> just write the version with a loop to find the one to erase.

The reason it's more idiomatic is because `it = std::remove_if(...)` returns an 
iterator that points to the new end of valid items, which means that, `[it, 
offers_.end())` should be removed. Since internally `std::remove_if` moves 
items past `it` to their "correct" positions before `it`, erasing only `it` but 
not things after it seems very strange at a first glance.

Calling `erase` with one iterator works here because we're relying on the fact 
that `std::remove_if` will only remove one item in this particular case, which 
is not trivial to me (before looking into what `offers_` contains and when the 
lambda returns true.

If you prefer to use `erase` with one position, how about the following:
```
offers_.erase(std::find_if(
offers_.begin(),
offers_.end(),
[&](const v1::Offer& offer) {
...
}));
```

Also, I just noticed that both the snippet you have and the one I wrote above 
relies on the invariant where `event.rescind().offer_id()` exists in `offers_`. 
`vector::erase(pos)` requires that the iterator passed in should be both valid 
and deferenceable. If there are more than one agents, then this invariant will 
be violated.


- Chun-Hung


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


On May 1, 2019, 9:17 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70579/
> ---
> 
> (Updated May 1, 2019, 9:17 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-8511
> https://issues.apache.org/jira/browse/MESOS-8511
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> The caller can submit tasks to the scheduler and get back statuses.
> This reduces the boilerplate of using the low level scheduler
> library.
> 
> 
> Diffs
> -
> 
>   include/mesos/v1/scheduler.hpp 2f72129ba48943c891d7020bfd2cad3066355026 
>   src/Makefile.am 5f97523fbe2d80733fbdcc7706f2761f5a071f9f 
>   src/scheduler/scheduler.cpp 674483aa80bc74b654343c97892a96f49d5c7ed4 
>   src/tests/CMakeLists.txt e6b1d8a097246f0921467d7f778034d6754fcff6 
>   src/tests/utils/task_scheduler.hpp PRE-CREATION 
>   src/tests/utils/task_scheduler.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/70579/diff/1/
> 
> 
> Testing
> ---
> 
> Tested it via the subsequent patch.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 70534: Added tests for the UPDATE_FRAMEWORK call.

2019-05-10 Thread Mesos Reviewbot 

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



Patch looks great!

Reviews applied: [70583, 70531, 70530, 70532, 70533, 70534]

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 May 7, 2019, 12:13 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70534/
> ---
> 
> (Updated May 7, 2019, 12:13 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7258
> https://issues.apache.org/jira/browse/MESOS-7258
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added tests for the UPDATE_FRAMEWORK call.
> 
> 
> Diffs
> -
> 
>   src/Makefile.am 5f97523fbe2d80733fbdcc7706f2761f5a071f9f 
>   src/tests/CMakeLists.txt e6b1d8a097246f0921467d7f778034d6754fcff6 
>   src/tests/update_framework_tests.cpp PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/70534/diff/4/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>

Re: Review Request 70579: Added a Task Scheduler to simplify testing.

2019-05-10 Thread Benjamin Mahler 


> On May 10, 2019, 3:31 a.m., Chun-Hung Hsiao wrote:
> > src/scheduler/scheduler.cpp
> > Lines 1010 (patched)
> > 
> >
> > Instead of introducing this, we could change the `TestMesos` template 
> > in `src/tests/mesos.hpp` to the following:
> > ```
> > namespace scheduler {
> > 
> > template
> > class TestMesos : public Mesos
> > {
> >...
> > };
> > 
> > } // namespace scheduler
> > 
> > namespace v1 {
> > namespace scheduler {
> > 
> > using TestMesos = tests::scheduler::TestMesos<
> > mesos::v1::scheduler::Mesos,
> > MockHTTPSchedluer<
> > mesos::v1::scheduler::Mesos,
> > mesos::v1::scheduler::Event>>;
> > 
> > } // namespace scheduler
> > } // namespace v1
> > ```
> > 
> > Then we can use `tests::scheduler::TestMesos` in `TaskScheduler`.

Hm.. I'm lost. What does that accomplish?

This patch is de-coupling construction from initialization, which IMO should 
have been done in the first place. Currently, you have to pass the callbacks 
when contructing and initialization of the Mesos class will happen immediately. 
The callbacks might not be ready to be invoked! Also, the caller may want to 
control when to "start" rather than it starting implicitly during construction.

Is this what you understood from the change?


> On May 10, 2019, 3:31 a.m., Chun-Hung Hsiao wrote:
> > src/tests/utils/task_scheduler.hpp
> > Lines 17 (patched)
> > 
> >
> > Would it be more consistent if we put this file at 
> > `src/tests/scheduler.hpp`, and call the scheduler `TestScheduler`, just 
> > like what we have in `src/tests/allocator.hpp`?
> > 
> > BTW we already have `src/tests/utils.hpp`, which has the same prefix 
> > `src/tests/utils`, not sure if we want to avoid this.

The gigantic tests/ folder is a disaster. I don't know about you, but it seems 
to me it sure could benefit from some more organization.

For example, I wish all the mocks were clearly organized into a mocks/ folder.

> BTW we already have src/tests/utils.hpp, which has the same prefix 
> src/tests/utils, not sure if we want to avoid this.

Yes I was aware of this, I don't think utils.hpp is a good file. For example, 
most of the functions in there are related to "paths" and could be organized 
accordingly. The metrics and port/ip helpers could also be put into a more 
appropriately named file?


> On May 10, 2019, 3:31 a.m., Chun-Hung Hsiao wrote:
> > src/tests/utils/task_scheduler.cpp
> > Lines 70 (patched)
> > 
> >
> > How about the following:
> > 
> > struct Launchable
> > {
> >   virtual v1::Resources resources() const = 0;
> >   virtual v1::Offer::Operation operation(...) = 0;
> > };
> > 
> > struct Task : public Launchable
> > {
> >   ...
> > };
> > 
> > struct TaskGroup : public Launchable
> > {
> >   ...
> > };
> > 
> > std::queue launchQueue;

That won't work since the objects are getting sliced in this example. You'd 
have to do something like `queue>`.

Personally, I would rather use a variant here than inheritance to keep value 
semantics. I forgot we already have variant, so I'll use that instead of the 
two options!


> On May 10, 2019, 3:31 a.m., Chun-Hung Hsiao wrote:
> > src/tests/utils/task_scheduler.cpp
> > Lines 265-268 (patched)
> > 
> >
> > ```
> > case ...::HEARTBEAT:
> > case ...::UPDATE_OPERATION_STATUS:
> > case ...::INVERSE_OFFERS:
> > case ...::RESCIND_INVERSE_OFFERS:
> >   break;
> > ```
> > Also let's move this to the end and merge with the other two events.
> > 
> > If you prefer to keep the order and use multiple `break`s, let's just 
> > leave one space between the colon and `break`.

I'll move them down

> If you prefer to keep the order and use multiple breaks, let's just leave one 
> space between the colon and break.

We often use whitespace alignment to improve readability, I think that's a good 
thing since readability is a top priority, some examples:

https://github.com/apache/mesos/blob/004fb5fa27c2992b11a2fa51a8ec5a3f3de404db/src/linux/capabilities.cpp#L153-L212
https://github.com/apache/mesos/blob/004fb5fa27c2992b11a2fa51a8ec5a3f3de404db/src/v1/attributes.cpp#L40-L43
https://github.com/apache/mesos/blob/004fb5fa27c2992b11a2fa51a8ec5a3f3de404db/3rdparty/stout/include/stout/gzip.hpp#L60-L69


> On May 10, 2019, 3:31 a.m., Chun-Hung Hsiao wrote:
> > src/tests/utils/task_scheduler.cpp
> > Lines 289 (patched)
> > 
> >
> > ```
> > offers_.erase(std::remove_if(...), offers

Re: Review Request 70591: Added `struct RoleInfo` to track role reservations and framework IDs.

2019-05-10 Thread Andrei Sekretenko 


> On May 9, 2019, 8:55 p.m., Meng Zhu wrote:
> > src/master/allocator/mesos/hierarchical.hpp
> > Lines 111-114 (patched)
> > 
> >
> > I find `roleInfo` as a OK name, espcially it is a value indexed by a 
> > role name.
> > 
> > It seems odd to me that we leave `TODO` for such a task here (it's 
> > obvious we can either do it now or, due the lack of good choices, unlikely 
> > to happen in the future).

Removed this TODO. Indeed, we do not have good choices at this point, and they 
will not appear out of nowhere. 

The renaming might become logical after some other refactoring or functional 
change, but this is another story.


> On May 9, 2019, 8:55 p.m., Meng Zhu wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 2609-2623 (original), 2614-2624 (patched)
> > 
> >
> > Thanks for the refactoring! Let's pull this refactor out into a 
> > separate patch in front this one.

Moved this refactoring into https://reviews.apache.org/r/70626/


- Andrei


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


On May 10, 2019, 4:31 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70591/
> ---
> 
> (Updated May 10, 2019, 4:31 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9701
> https://issues.apache.org/jira/browse/MESOS-9701
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch introduces struct RoleInfo which contains the framework IDs
> and reservatons tracked under a role. Two separate hashmaps used for
> this purpose previously are replaced by a single hashmap, thus the need
> to keep the two consistent with each other is removed. This simplifies
> the role tracking logic in the allocator. The patch introduces minimal
> to no performance impact.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> c2058baca5159da4cdcab77afd5de3c0d5ae6c48 
>   src/master/allocator/mesos/hierarchical.cpp 
> 64a076ddd29711437d539a06bb0470755828cc87 
> 
> 
> Diff: https://reviews.apache.org/r/70591/diff/3/
> 
> 
> Testing
> ---
> 
> make check
> 
> Benchmarking: 5 runs of 
> `BENCHMARK_HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/5` with 
> the optimized build.
> 
> Performance impact in this benchmark seems to be negligible.
> 
> BEFORE:
> Added 3000 agents in 51.553929ms
> Added 3000 frameworks in 15.174748344secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 12.400805171secs
> Made 0 allocation in 12.5850238secs
> 
> Added 3000 agents in 55.739336ms
> Added 3000 frameworks in 14.730404769secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 12.563439682secs
> Made 0 allocation in 13.063555055secs
> 
> Added 3000 agents in 54.414733ms
> Added 3000 frameworks in 15.10136842secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 13.501664915secs
> Made 0 allocation in 12.89034382secs
> 
> Added 3000 agents in 52.58252ms
> Added 3000 frameworks in 14.048350298secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 12.299952145secs
> Made 0 allocation in 11.888248811secs
> 
> Added 3000 agents in 52.821439ms
> Added 3000 frameworks in 15.344450583secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 12.63425secs
> Made 0 allocation in 12.427171541secs
> 
> 
> AFTER:
> 
> Added 3000 agents in 69.716648ms
> Added 3000 frameworks in 15.249001979secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 12.860494226secs
> Made 0 allocation in 12.228866329secs
> 
> Added 3000 agents in 52.639388ms
> Added 3000 frameworks in 15.207895482secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 12.504777266secs
> Made 0 allocation in 12.70388062secs
> 
> Added 3000 agents in 56.865794ms
> Added 3000 frameworks in 15.284003915secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 13.86859815secs
> Made 0 allocation in 12.538958231secs
> 
> Added 3000 agents in 56.028013ms
> Added 3000 frameworks in 13.892577869secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 12.341724418secs
> Made 0

Re: Review Request 70618: Encapsulate a framework sorter inside a RoleInfo.

2019-05-10 Thread Andrei Sekretenko 

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

(Updated May 10, 2019, 4:32 p.m.)


Review request for mesos, Benjamin Mahler and Meng Zhu.


Summary (updated)
-

Encapsulate a framework sorter inside a RoleInfo.


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


Repository: mesos


Description (updated)
---

This patch consolidates the logic of tracking frameworks tied to
a specific role in the `RoleInfo`.

To do this properly, `RoleInfo` is turned into a class which wraps
access to the framework sorter so that no other entity is capable
of adding/removing clients (the frameworks) to/from the sorter.

The patch introduces almost no performance impact.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
c2058baca5159da4cdcab77afd5de3c0d5ae6c48 
  src/master/allocator/mesos/hierarchical.cpp 
64a076ddd29711437d539a06bb0470755828cc87 


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

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


Testing
---

make check

Benchmarking: 5 runs of 
BENCHMARK_HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/5 with the 
optimized build.

BEFORE (master):

Added 3000 agents in 52.729305ms
Added 3000 frameworks in 13.799200231secs
Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
Made 3856 allocations in 12.560201008secs
Made 0 allocation in 12.143722849secs

Added 3000 agents in 64.789364ms
Added 3000 frameworks in 14.175436362secs
Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
Made 3856 allocations in 12.505877513secs
Made 0 allocation in 12.424587206secs

Added 3000 agents in 50.942631ms
Added 3000 frameworks in 14.26206239secs
Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
Made 3856 allocations in 12.220950624secs
Made 0 allocation in 12.495832704secs

Added 3000 agents in 50.660372ms
Added 3000 frameworks in 14.246438788secs
Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
Made 3856 allocations in 12.505903032secs
Made 0 allocation in 12.334731087secs

Added 3000 agents in 50.292757ms
Added 3000 frameworks in 14.236187327secs
Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
Made 3856 allocations in 12.531212952secs
Made 0 allocation in 12.282740343secs

---
AFTER (master + previous patch https://reviews.apache.org/r/70591/ + this 
patch):

Added 3000 agents in 51.465368ms
Added 3000 frameworks in 13.599017611secs
Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
Made 3856 allocations in 12.272279231secs
Made 0 allocation in 12.026509432secs

Added 3000 agents in 52.5567ms
Added 3000 frameworks in 13.67345101secs
Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
Made 3856 allocations in 12.447551363secs
Made 0 allocation in 12.045692187secs

Added 3000 agents in 52.455703ms
Added 3000 frameworks in 13.344338641secs
Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
Made 3856 allocations in 11.988632162secs
Made 0 allocation in 11.558150131secs

Added 3000 agents in 53.579201ms
Added 3000 frameworks in 13.681435728secs
Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
Made 3856 allocations in 11.966754231secs
Made 0 allocation in 12.720889223secs

Added 3000 agents in 52.08713ms
Added 3000 frameworks in 13.562008608secs
Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
Made 3856 allocations in 12.19201724secs
Made 0 allocation in 11.727037034secs


Thanks,

Andrei Sekretenko

Re: Review Request 70589: Logged when `/__processes__` returns.

2019-05-10 Thread Benjamin Mahler 

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


Ship it!





3rdparty/libprocess/src/process.cpp
Lines 3412 (patched)


Perhaps a TODO to generalize the response logging in libprocess?


- Benjamin Mahler


On May 9, 2019, 1:09 p.m., Alexander Rukletsov wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70589/
> ---
> 
> (Updated May 9, 2019, 1:09 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Till Toenshoff.
> 
> 
> Bugs: MESOS-9766
> https://issues.apache.org/jira/browse/MESOS-9766
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Adds a log entry when a response with generated by `/__processes__`
> is about to be returned to the client.
> 
> 
> Diffs
> -
> 
>   3rdparty/libprocess/src/process.cpp 
> 124836472313721a5dbfe4b1ca55f0da3cecd66b 
> 
> 
> Diff: https://reviews.apache.org/r/70589/diff/1/
> 
> 
> Testing
> ---
> 
> built & run on Mac OS 10.13.6
> 
> ```
> I0503 13:13:00.885133 59219968 process.cpp:3599] Handling HTTP event for 
> process '__processes__' with path: '/__processes__'
> I0503 13:13:00.886037 59219968 process.cpp:3412] HTTP GET for /__processes__ 
> from 192.168.178.47:59548: '200 OK' after 1.06214ms
> ```
> 
> 
> Thanks,
> 
> Alexander Rukletsov
> 
>

Re: Review Request 70591: Added `struct RoleInfo` to track role reservations and framework IDs.

2019-05-10 Thread Andrei Sekretenko 

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

(Updated May 10, 2019, 4:31 p.m.)


Review request for mesos, Benjamin Mahler and Meng Zhu.


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


Repository: mesos


Description (updated)
---

This patch introduces struct RoleInfo which contains the framework IDs
and reservatons tracked under a role. Two separate hashmaps used for
this purpose previously are replaced by a single hashmap, thus the need
to keep the two consistent with each other is removed. This simplifies
the role tracking logic in the allocator. The patch introduces minimal
to no performance impact.


Diffs (updated)
-

  src/master/allocator/mesos/hierarchical.hpp 
c2058baca5159da4cdcab77afd5de3c0d5ae6c48 
  src/master/allocator/mesos/hierarchical.cpp 
64a076ddd29711437d539a06bb0470755828cc87 


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

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


Testing
---

make check

Benchmarking: 5 runs of 
`BENCHMARK_HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/5` with the 
optimized build.

Performance impact in this benchmark seems to be negligible.

BEFORE:
Added 3000 agents in 51.553929ms
Added 3000 frameworks in 15.174748344secs
Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
Made 3856 allocations in 12.400805171secs
Made 0 allocation in 12.5850238secs

Added 3000 agents in 55.739336ms
Added 3000 frameworks in 14.730404769secs
Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
Made 3856 allocations in 12.563439682secs
Made 0 allocation in 13.063555055secs

Added 3000 agents in 54.414733ms
Added 3000 frameworks in 15.10136842secs
Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
Made 3856 allocations in 13.501664915secs
Made 0 allocation in 12.89034382secs

Added 3000 agents in 52.58252ms
Added 3000 frameworks in 14.048350298secs
Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
Made 3856 allocations in 12.299952145secs
Made 0 allocation in 11.888248811secs

Added 3000 agents in 52.821439ms
Added 3000 frameworks in 15.344450583secs
Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
Made 3856 allocations in 12.63425secs
Made 0 allocation in 12.427171541secs


AFTER:

Added 3000 agents in 69.716648ms
Added 3000 frameworks in 15.249001979secs
Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
Made 3856 allocations in 12.860494226secs
Made 0 allocation in 12.228866329secs

Added 3000 agents in 52.639388ms
Added 3000 frameworks in 15.207895482secs
Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
Made 3856 allocations in 12.504777266secs
Made 0 allocation in 12.70388062secs

Added 3000 agents in 56.865794ms
Added 3000 frameworks in 15.284003915secs
Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
Made 3856 allocations in 13.86859815secs
Made 0 allocation in 12.538958231secs

Added 3000 agents in 56.028013ms
Added 3000 frameworks in 13.892577869secs
Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
Made 3856 allocations in 12.341724418secs
Made 0 allocation in 12.23022189secs

Added 3000 agents in 52.368219ms
Added 3000 frameworks in 13.978581104secs
Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
Made 3856 allocations in 12.701682501secs
Made 0 allocation in 12.141360313secs


Thanks,

Andrei Sekretenko

Review Request 70626: Refactored `untrackReservations()` method.

2019-05-10 Thread Andrei Sekretenko 

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

Review request for mesos and Meng Zhu.


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


Repository: mesos


Description
---

This patch gets rid of insertion into the beginning of the vector
and also of shadowing HierarchicalAllocatorProcess::roles by a local.


Diffs
-

  src/master/allocator/mesos/hierarchical.cpp 
64a076ddd29711437d539a06bb0470755828cc87 


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


Testing
---


Thanks,

Andrei Sekretenko

Re: Review Request 70532: Added an UPDATE_FRAMEWORK scheduler::Call.

2019-05-10 Thread Andrei Sekretenko 

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

(Updated May 10, 2019, 3 p.m.)


Review request for mesos and Benjamin Mahler.


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


Repository: mesos


Description (updated)
---

This patch definies the call to be used by a framework for updating its
FrameworkInfo fields without re-subscribing.

It is based on the previous implementation attempt:
https://reviews.apache.org/r/66228/


Diffs (updated)
-

  include/mesos/scheduler/scheduler.proto 
b6d79b1c433dce77a6ee4278b3ddf6b69868c1d8 
  include/mesos/v1/scheduler/scheduler.proto 
bddd5c449fd4b294d15430746b708731aff18f2a 


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

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


Testing
---


Thanks,

Andrei Sekretenko

Re: Review Request 70530: Refactored Framework updates for the UPDATE_FRAMEWORK call; fixed race.

2019-05-10 Thread Andrei Sekretenko 

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

(Updated May 10, 2019, 2:50 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
---

Fixed formatting of the description.


Summary (updated)
-

Refactored Framework updates for the UPDATE_FRAMEWORK call; fixed race.


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


Repository: mesos


Description (updated)
---

The main concern of this patch is the behaviour of the framework
re-subscription and of the upcoming UPDATE_FRAMEWORK call in cases when
the framework attempts to change the "immutable" fields of
FrameworkInfo, namely: `user`, `checkpoint`, `principal` and `id`.

The changes introduced by this patch:
- The method `Framework::update()` is simplified: the logic of ignoring
  the immutable fields is removed, this method fails the program if the
  new values differ from the old ones. This is needed to simplify
  keeping UPDATE_FRAMEWORK consistent with re-subscription.
- The method `Master::validateFrameworkSubscription()` now returns
  validation errors on attempts to change `user` or `checkpoint` fields.
  This is needed to enable validating them in the UPDATE_FRAMEWORK call.
- A method `Master::sanitizeFrameworkSubscription()` is introduced to
  preserve the legacy behaviour of re-subscription (which ignores the
  updates of `user` and `checkpoint`).
- A second call of `validateFrameworkSubscription()` is performed after
  the authorization. This is a crude fix of the already existing race
  between two re-subscriptions against an empty master (see MESOS-9763).
  It is necessary because otherwise the change in `Framework::update()`
  would exacerbate this race (namely, it would be possible to crash the
  master).


Diffs (updated)
-

  src/master/framework.cpp 05f5514c589b2dba08afe77281e5fbc4e29f232b 
  src/master/master.hpp 7d9732f1e432f6f0290d234242864cbdbf381fa8 
  src/master/master.cpp a8ee6297e1587c160a47b806914d3c3aa4f35cd7 
  src/master/validation.hpp 95638a17052ece6c957aa76e4cead8d7bfe82024 
  src/master/validation.cpp 9fb0850987ce385d345302cac9721adead7181b8 


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

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


Testing
---


Thanks,

Andrei Sekretenko

Re: Review Request 70622: Added a unit test to verify if SLRP allows changes in volume context.

2019-05-10 Thread Mesos Reviewbot 

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



Patch looks great!

Reviews applied: [70620, 70621, 70622]

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 May 10, 2019, 1:20 a.m., Chun-Hung Hsiao wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70622/
> ---
> 
> (Updated May 10, 2019, 1:20 a.m.)
> 
> 
> Review request for mesos and Benjamin Bannier.
> 
> 
> Bugs: MESOS-9395
> https://issues.apache.org/jira/browse/MESOS-9395
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Added a unit test to verify if SLRP allows changes in volume context.
> 
> 
> Diffs
> -
> 
>   src/examples/test_csi_plugin.cpp 03f782ead136a79c4c5b099496072933f6737598 
>   src/tests/storage_local_resource_provider_tests.cpp 
> ecd7aeef1cb3d1a5b4b3419dfd912d41a8c6 
> 
> 
> Diff: https://reviews.apache.org/r/70622/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>

Re: Review Request 70581: Add flag ignoring docker manifest config metadata.

2019-05-10 Thread Mesos Reviewbot 

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



Patch looks great!

Reviews applied: [70581]

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 May 9, 2019, 7 p.m., Jacob Janco wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70581/
> ---
> 
> (Updated May 9, 2019, 7 p.m.)
> 
> 
> Review request for mesos, Gilbert Song and James Peach.
> 
> 
> Bugs: MESOS-9760
> https://issues.apache.org/jira/browse/MESOS-9760
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Docker runtime isolator propagates manifest configuration metadata.
> This is not always desirable, e.g. a customer may want to ignore the
> defined WORKDIR/ENV defined in the manifest.
> 
> https://issues.apache.org/jira/browse/MESOS-9760
> 
> 
> Diffs
> -
> 
>   docs/configuration/agent.md 04d7114e92c189afd04a8889b3ab853ade9deec6 
>   docs/upgrades.md ece8462b9950626166c832d3b554ad51f68f42e1 
>   src/slave/containerizer/mesos/provisioner/store.cpp 
> 11fce0eb47e9e6dfce6289afe04a1d58a0c4461a 
>   src/slave/flags.hpp 09921cb6172202b5c1d2f8d03f9ccaeb3d0e8c94 
>   src/slave/flags.cpp 49a350f9e13409493fa9612c53d9d58b62122371 
> 
> 
> Diff: https://reviews.apache.org/r/70581/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>

Re: Review Request 70570: Logged headroom related info in the allocator.

2019-05-10 Thread Mesos Reviewbot 

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



Bad review!

Reviews applied: [70570]

Error:
2019-05-10 09:12:39 URL:https://reviews.apache.org/r/70570/diff/raw/ 
[2537/2537] -> "70570.patch" [1]
error: patch failed: src/master/allocator/mesos/hierarchical.cpp:1795
error: src/master/allocator/mesos/hierarchical.cpp: patch does not apply

- Mesos Reviewbot


On May 9, 2019, 12:22 a.m., Meng Zhu wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70570/
> ---
> 
> (Updated May 9, 2019, 12:22 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9759
> https://issues.apache.org/jira/browse/MESOS-9759
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch logs `requiredHeadroom` and `availableHeadroom`
> before each allocation cycle and logs `requiredHeadroom`,
> resources and number of agents held back for quota `headroom`
> as well as resources filtered at the end of each allocation
> cycle.
> 
> While we also held resources back for quota headroom in the
> first stage, we do not log it there. This is because in the
> second stage, we try to allocate all resources (including the
> ones held back in the first stage). Thus only resources held
> back in the second stage are truly held back for the whole
> allocation cycle.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.cpp 
> 708c444ea93a63d566a81b31c4c79ed65949e1a4 
> 
> 
> Diff: https://reviews.apache.org/r/70570/diff/6/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Re: Review Request 70618: Encapsulate a framework sorter inside of a RoleInfo.

2019-05-10 Thread Mesos Reviewbot 

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



Patch looks great!

Reviews applied: [70591, 70618]

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 May 9, 2019, 11:14 p.m., Andrei Sekretenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70618/
> ---
> 
> (Updated May 9, 2019, 11:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-9701
> https://issues.apache.org/jira/browse/MESOS-9701
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch consolidates in the `RoleInfo` the logic of tracking frameworks
> tied to a specific role.
> 
> To do this properly, `RoleInfo` is turned into a class which wraps 
> access to the framework sorter so that no other entity is capable
> of adding/removing clients (the frameworks) to/from the framework sorter.
> 
> The patch introduces almost no performance impact.
> 
> 
> Diffs
> -
> 
>   src/master/allocator/mesos/hierarchical.hpp 
> c2058baca5159da4cdcab77afd5de3c0d5ae6c48 
>   src/master/allocator/mesos/hierarchical.cpp 
> 64a076ddd29711437d539a06bb0470755828cc87 
> 
> 
> Diff: https://reviews.apache.org/r/70618/diff/1/
> 
> 
> Testing
> ---
> 
> make check
> 
> Benchmarking: 5 runs of 
> BENCHMARK_HierarchicalAllocator_WithQuotaParam.LargeAndSmallQuota/5 with the 
> optimized build.
> 
> BEFORE (master):
> 
> Added 3000 agents in 52.729305ms
> Added 3000 frameworks in 13.799200231secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 12.560201008secs
> Made 0 allocation in 12.143722849secs
> 
> Added 3000 agents in 64.789364ms
> Added 3000 frameworks in 14.175436362secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 12.505877513secs
> Made 0 allocation in 12.424587206secs
> 
> Added 3000 agents in 50.942631ms
> Added 3000 frameworks in 14.26206239secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 12.220950624secs
> Made 0 allocation in 12.495832704secs
> 
> Added 3000 agents in 50.660372ms
> Added 3000 frameworks in 14.246438788secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 12.505903032secs
> Made 0 allocation in 12.334731087secs
> 
> Added 3000 agents in 50.292757ms
> Added 3000 frameworks in 14.236187327secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 12.531212952secs
> Made 0 allocation in 12.282740343secs
> 
> ---
> AFTER (master + previous patch https://reviews.apache.org/r/70591/ + this 
> patch):
> 
> Added 3000 agents in 51.465368ms
> Added 3000 frameworks in 13.599017611secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 12.272279231secs
> Made 0 allocation in 12.026509432secs
> 
> Added 3000 agents in 52.5567ms
> Added 3000 frameworks in 13.67345101secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 12.447551363secs
> Made 0 allocation in 12.045692187secs
> 
> Added 3000 agents in 52.455703ms
> Added 3000 frameworks in 13.344338641secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 11.988632162secs
> Made 0 allocation in 11.558150131secs
> 
> Added 3000 agents in 53.579201ms
> Added 3000 frameworks in 13.681435728secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 11.966754231secs
> Made 0 allocation in 12.720889223secs
> 
> Added 3000 agents in 52.08713ms
> Added 3000 frameworks in 13.562008608secs
> Benchmark setup: 3000 agents, 3000 roles, 3000 frameworks, with random sorter
> Made 3856 allocations in 12.19201724secs
> Made 0 allocation in 11.727037034secs
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>