Re: Review Request 55972: Updated master to handle non-MULTI_ROLE agents.

[Benjamin Mahler]

> On Jan. 30, 2017, 1:46 a.m., Guangya Liu wrote:
> > src/master/master.cpp, lines 5620-5628
> > 
> >
> > Same comments as /r/55971/ here

Dropped per previous comments, would like to follow up and figure out what to 
do about this! :)


- Benjamin


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


On Jan. 27, 2017, 12:29 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55972/
> ---
> 
> (Updated Jan. 27, 2017, 12:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-6968
> https://issues.apache.org/jira/browse/MESOS-6968
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With the addition of MULTI_ROLE framweork support, allocated
> resources have a `Resource.AllocationInfo` set. While the new
> MULTI_ROLE capable agents will be sending allocated resources,
> the old agents may not be since they may not preserve the
> unknown fields.
> 
> To cope with this, the master ensures the `Resource.AllocationInfo`
> is set for non-MULTI_ROLE capable agents, by injecting it. Note
> that we can only do this so long as it is unambiguous! This will
> be prevented by not allowing frameworks with multiple to use
> agents without the MULTI_ROLE support, see: MESOS-6940.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 7e38af41ca16241dbbe3bc2e80c0848e82762a45 
>   src/master/master.cpp 0f2c7cd32161576fad742ef350dff64874b80854 
> 
> Diff: https://reviews.apache.org/r/55972/diff/
> 
> 
> Testing
> ---
> 
> The tests pass at the end of this review chain.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 55972: Updated master to handle non-MULTI_ROLE agents.

[Michael Park]

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


Fix it, then Ship it!





src/master/master.cpp (line 5617)


Let's use `foreach` here.



src/master/master.cpp (lines 5620 - 5628)


The state of `AllocationInfo` is fully determined by `MULTI_ROLE` 
capability of the agent, right? Even in the downgrade scenario, the agent would 
ignore the `AllocationInfo` during recover and send over `Resource`s without 
`AllocationInfo` set?



src/master/master.cpp (line 5633)


not used. as you mentioned.



src/master/master.cpp (lines 5646 - 5647)


Let's use `foreach` here.


- Michael Park


On Jan. 26, 2017, 4:29 p.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55972/
> ---
> 
> (Updated Jan. 26, 2017, 4:29 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-6968
> https://issues.apache.org/jira/browse/MESOS-6968
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With the addition of MULTI_ROLE framweork support, allocated
> resources have a `Resource.AllocationInfo` set. While the new
> MULTI_ROLE capable agents will be sending allocated resources,
> the old agents may not be since they may not preserve the
> unknown fields.
> 
> To cope with this, the master ensures the `Resource.AllocationInfo`
> is set for non-MULTI_ROLE capable agents, by injecting it. Note
> that we can only do this so long as it is unambiguous! This will
> be prevented by not allowing frameworks with multiple to use
> agents without the MULTI_ROLE support, see: MESOS-6940.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 7e38af41ca16241dbbe3bc2e80c0848e82762a45 
>   src/master/master.cpp 0f2c7cd32161576fad742ef350dff64874b80854 
> 
> Diff: https://reviews.apache.org/r/55972/diff/
> 
> 
> Testing
> ---
> 
> The tests pass at the end of this review chain.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 55972: Updated master to handle non-MULTI_ROLE agents.

[Benjamin Mahler]

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




src/master/master.cpp (lines 5663 - 5667)


The adjusted boolean isn't being set, can we just use the multi role 
boolean instead of needing an extra adjusted boolean?


- Benjamin Mahler


On Jan. 27, 2017, 12:29 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55972/
> ---
> 
> (Updated Jan. 27, 2017, 12:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-6968
> https://issues.apache.org/jira/browse/MESOS-6968
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With the addition of MULTI_ROLE framweork support, allocated
> resources have a `Resource.AllocationInfo` set. While the new
> MULTI_ROLE capable agents will be sending allocated resources,
> the old agents may not be since they may not preserve the
> unknown fields.
> 
> To cope with this, the master ensures the `Resource.AllocationInfo`
> is set for non-MULTI_ROLE capable agents, by injecting it. Note
> that we can only do this so long as it is unambiguous! This will
> be prevented by not allowing frameworks with multiple to use
> agents without the MULTI_ROLE support, see: MESOS-6940.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 7e38af41ca16241dbbe3bc2e80c0848e82762a45 
>   src/master/master.cpp 0f2c7cd32161576fad742ef350dff64874b80854 
> 
> Diff: https://reviews.apache.org/r/55972/diff/
> 
> 
> Testing
> ---
> 
> The tests pass at the end of this review chain.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Re: Review Request 55972: Updated master to handle non-MULTI_ROLE agents.

[Guangya Liu]

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




src/master/master.cpp (lines 5620 - 5628)


Same comments as /r/55971/ here


- Guangya Liu


On 一月 27, 2017, 12:29 a.m., Benjamin Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55972/
> ---
> 
> (Updated 一月 27, 2017, 12:29 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-6968
> https://issues.apache.org/jira/browse/MESOS-6968
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With the addition of MULTI_ROLE framweork support, allocated
> resources have a `Resource.AllocationInfo` set. While the new
> MULTI_ROLE capable agents will be sending allocated resources,
> the old agents may not be since they may not preserve the
> unknown fields.
> 
> To cope with this, the master ensures the `Resource.AllocationInfo`
> is set for non-MULTI_ROLE capable agents, by injecting it. Note
> that we can only do this so long as it is unambiguous! This will
> be prevented by not allowing frameworks with multiple to use
> agents without the MULTI_ROLE support, see: MESOS-6940.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 7e38af41ca16241dbbe3bc2e80c0848e82762a45 
>   src/master/master.cpp 0f2c7cd32161576fad742ef350dff64874b80854 
> 
> Diff: https://reviews.apache.org/r/55972/diff/
> 
> 
> Testing
> ---
> 
> The tests pass at the end of this review chain.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Review Request 55972: Updated master to handle non-MULTI_ROLE agents.

[Benjamin Mahler]

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

Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael 
Park.


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


Repository: mesos


Description
---

With the addition of MULTI_ROLE framweork support, allocated
resources have a `Resource.AllocationInfo` set. While the new
MULTI_ROLE capable agents will be sending allocated resources,
the old agents may not be since they may not preserve the
unknown fields.

To cope with this, the master ensures the `Resource.AllocationInfo`
is set for non-MULTI_ROLE capable agents, by injecting it. Note
that we can only do this so long as it is unambiguous! This will
be prevented by not allowing frameworks with multiple to use
agents without the MULTI_ROLE support, see: MESOS-6940.


Diffs
-

  src/master/master.hpp 7e38af41ca16241dbbe3bc2e80c0848e82762a45 
  src/master/master.cpp 0f2c7cd32161576fad742ef350dff64874b80854 

Diff: https://reviews.apache.org/r/55972/diff/


Testing
---

The tests pass at the end of this review chain.


Thanks,

Benjamin Mahler