Mailing-List: contact mesos-dev-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: mesos-dev@incubator.apache.org
Content-Type: multipart/alternative;
 boundary="===============8366808899680484429=="
MIME-Version: 1.0
Subject: Re: Review Request: WDRF 2 - Add concept of valid roles
From: "Vinod Kone" <vinodkone@gmail.com>
To: "Benjamin Hindman" <benh@berkeley.edu>
Cc: "Thomas Marshall" <twm378@gmail.com>,
 "mesos" <mesos-dev@incubator.apache.org>, "Vinod Kone" <vinodkone@gmail.com>
Date: Thu, 13 Jun 2013 23:44:28 -0000
Message-ID: <20130613234428.1041.52962@reviews.apache.org>
Auto-Submitted: auto-generated
Sender: "Vinod Kone" <noreply@reviews.apache.org>
References: <20130613192457.3558.28038@reviews.apache.org>
In-Reply-To: <20130613192457.3558.28038@reviews.apache.org>
Reply-To: "Vinod Kone" <vinodkone@gmail.com>

--===============8366808899680484429==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable


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


src/master/master.hpp
<https://reviews.apache.org/r/11206/#comment45171>

    const?


src/master/master.cpp
<https://reviews.apache.org/r/11206/#comment45174>

    Why do we always want to allow a default role?


src/master/master.cpp
<https://reviews.apache.org/r/11206/#comment45175>

    why don't we call the member 'role' instead of 'name'?
    =

    I think it is a bit confusing that we call the flag tokens 'role's, but=
 we have a class called 'Role' and an attribute of RoleInfo called 'name' w=
hich is actually a 'role'.
    =


src/master/master.cpp
<https://reviews.apache.org/r/11206/#comment45176>

    Don't we have to check this in re-register too?


src/tests/allocator_tests.cpp
<https://reviews.apache.org/r/11206/#comment45177>

    Can we add tests for when frameworks connect with a role that is known/=
unknown to the master?


- Vinod Kone


On June 13, 2013, 7:24 p.m., Thomas Marshall wrote:
> =

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

> (Updated June 13, 2013, 7:24 p.m.)
> =

> =

> Review request for mesos and Benjamin Hindman.
> =

> =

> Description
> -------
> =

> Previously when we were doing hierarchical allocation by user, we created=
 and removed user pools for allocation based on what users had frameworks c=
urrently running in the cluster. However, with the role abstraction it make=
s sense to have more persistence than that, especially once we add weights =
- if you set the weight for a role, you want the allocator to remember it e=
ven if there aren't any frameworks for that role currently running.
> =

> So, I decided that it made sense to create a concept of specific roles th=
at are allowable in the cluster. With this patch, its only possible to pass=
 roles in to the master as a command line flag (to ease what I assume will =
be the common case - clusters with relatively static sets of roles), but a =
future wdrf patch will add http endpoints to add, remove, and update roles.
> =

> This patch also enforces that frameworks register with valid role (this w=
on't affect people who don't care about roles, since there's always the "*"=
 role, which is the default for frameworks that don't specify a role).
> =

> =

> This addresses bug MESOS-504.
>     https://issues.apache.org/jira/browse/MESOS-504
> =

> =

> Diffs
> -----
> =

>   src/master/allocator.hpp 78c75bb =

>   src/master/flags.hpp f4ce8c1 =

>   src/master/hierarchical_allocator_process.hpp 1048a28 =

>   src/master/master.hpp 86c5232 =

>   src/master/master.cpp 60c6d4f =

>   src/messages/messages.proto 2c196ee =

>   src/tests/allocator_tests.cpp 32f0a90 =

>   src/tests/allocator_zookeeper_tests.cpp 1daaecd =

>   src/tests/mesos.hpp fca41aa =

>   src/tests/resource_offers_tests.cpp 3d5f02d =

> =

> Diff: https://reviews.apache.org/r/11206/diff/
> =

> =

> Testing
> -------
> =

> make check
> =

> =

> Thanks,
> =

> Thomas Marshall
> =

>


--===============8366808899680484429==--