Re: Review Request 66310: Passed on registrar when constructing resource provider manager.

[Chun-Hung Hsiao]

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


Ship it!




Ship It!

- Chun-Hung Hsiao


On April 26, 2018, 12:39 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66310/
> ---
> 
> (Updated April 26, 2018, 12:39 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8735
> https://issues.apache.org/jira/browse/MESOS-8735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In order to support recovering resource provider manager information
> in the future, we need to adjust the construction of the manager to be
> able to consume a registrar.
> 
> This patch lays the groundwork by adjusting interfaces and their
> usage; we will make use of the passed on information in a following
> patch.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/manager.hpp bc017fa3998b780cec82718cabe2a8b470a66db2 
>   src/resource_provider/manager.cpp 68e1866986bfb91bf8355099ee1f0a2a86aba39a 
>   src/resource_provider/registrar.cpp 
> 92ef9aecb1e93d10f46e53984391558f9901a60b 
>   src/slave/slave.cpp d0ff5f872433b3891b32e35aea6212bf45693ae5 
>   src/tests/resource_provider_manager_tests.cpp 
> c52541bf130ccf4795b989b53331176a64a097ea 
> 
> 
> Diff: https://reviews.apache.org/r/66310/diff/10/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66310: Passed on registrar when constructing resource provider manager.

[Benjamin Bannier]

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

(Updated April 26, 2018, 2:39 p.m.)


Review request for mesos, Chun-Hung Hsiao, Jie Yu, and Jan Schlicht.


Changes
---

Explicitly tore down resource provider manager with the agent.

This prevents multiple resource provider managers from being active when
an agent is terminated (without being destructed) and reassigned from a
newly constructed one.


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


Repository: mesos


Description
---

In order to support recovering resource provider manager information
in the future, we need to adjust the construction of the manager to be
able to consume a registrar.

This patch lays the groundwork by adjusting interfaces and their
usage; we will make use of the passed on information in a following
patch.


Diffs (updated)
-

  src/resource_provider/manager.hpp bc017fa3998b780cec82718cabe2a8b470a66db2 
  src/resource_provider/manager.cpp 68e1866986bfb91bf8355099ee1f0a2a86aba39a 
  src/resource_provider/registrar.cpp 92ef9aecb1e93d10f46e53984391558f9901a60b 
  src/slave/slave.cpp d0ff5f872433b3891b32e35aea6212bf45693ae5 
  src/tests/resource_provider_manager_tests.cpp 
c52541bf130ccf4795b989b53331176a64a097ea 


Diff: https://reviews.apache.org/r/66310/diff/10/

Changes: https://reviews.apache.org/r/66310/diff/9-10/


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 66310: Passed on registrar when constructing resource provider manager.

[Chun-Hung Hsiao]

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


Ship it!




Ship It!

- Chun-Hung Hsiao


On April 23, 2018, 11:18 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66310/
> ---
> 
> (Updated April 23, 2018, 11:18 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8735
> https://issues.apache.org/jira/browse/MESOS-8735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In order to support recovering resource provider manager information
> in the future, we need to adjust the construction of the manager to be
> able to consume a registrar.
> 
> This patch lays the groundwork by adjusting interfaces and their
> usage; we will make use of the passed on information in a following
> patch.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/manager.hpp bc017fa3998b780cec82718cabe2a8b470a66db2 
>   src/resource_provider/manager.cpp 68e1866986bfb91bf8355099ee1f0a2a86aba39a 
>   src/resource_provider/registrar.cpp 
> 92ef9aecb1e93d10f46e53984391558f9901a60b 
>   src/slave/slave.cpp 2b8c6e0771a58efacf26628abae6cbe46380a369 
>   src/tests/resource_provider_manager_tests.cpp 
> c52541bf130ccf4795b989b53331176a64a097ea 
> 
> 
> Diff: https://reviews.apache.org/r/66310/diff/7/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66310: Passed on registrar when constructing resource provider manager.

[Benjamin Bannier]

> On April 23, 2018, 3:05 a.m., Chun-Hung Hsiao wrote:
> > src/tests/resource_provider_manager_tests.cpp
> > Line 132 (original), 132 (patched)
> > 
> >
> > Do you think it is reasonable to always require a registrar, i.e., 
> > disallow `nullptr` and use `InMemoryStore` here? Please drop this if you 
> > think we should allow `nullptr`.

There's a trade-off between simpler testing code and internal complexity. In 
this case I agree with you that we can simplify the implementation of the 
manager a lot by requiring users to always pass in a registry. Adjusted the 
code for that, also added some assertions.


- Benjamin


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


On April 23, 2018, 1:18 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66310/
> ---
> 
> (Updated April 23, 2018, 1:18 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8735
> https://issues.apache.org/jira/browse/MESOS-8735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In order to support recovering resource provider manager information
> in the future, we need to adjust the construction of the manager to be
> able to consume a registrar.
> 
> This patch lays the groundwork by adjusting interfaces and their
> usage; we will make use of the passed on information in a following
> patch.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/manager.hpp bc017fa3998b780cec82718cabe2a8b470a66db2 
>   src/resource_provider/manager.cpp 68e1866986bfb91bf8355099ee1f0a2a86aba39a 
>   src/resource_provider/registrar.cpp 
> 92ef9aecb1e93d10f46e53984391558f9901a60b 
>   src/slave/slave.cpp 2b8c6e0771a58efacf26628abae6cbe46380a369 
>   src/tests/resource_provider_manager_tests.cpp 
> c52541bf130ccf4795b989b53331176a64a097ea 
> 
> 
> Diff: https://reviews.apache.org/r/66310/diff/7/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66310: Passed on registrar when constructing resource provider manager.

[Benjamin Bannier]

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

(Updated April 23, 2018, 1:18 p.m.)


Review request for mesos, Chun-Hung Hsiao, Jie Yu, and Jan Schlicht.


Changes
---

Addressed comment from Chun.


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


Repository: mesos


Description
---

In order to support recovering resource provider manager information
in the future, we need to adjust the construction of the manager to be
able to consume a registrar.

This patch lays the groundwork by adjusting interfaces and their
usage; we will make use of the passed on information in a following
patch.


Diffs (updated)
-

  src/resource_provider/manager.hpp bc017fa3998b780cec82718cabe2a8b470a66db2 
  src/resource_provider/manager.cpp 68e1866986bfb91bf8355099ee1f0a2a86aba39a 
  src/resource_provider/registrar.cpp 92ef9aecb1e93d10f46e53984391558f9901a60b 
  src/slave/slave.cpp 2b8c6e0771a58efacf26628abae6cbe46380a369 
  src/tests/resource_provider_manager_tests.cpp 
c52541bf130ccf4795b989b53331176a64a097ea 


Diff: https://reviews.apache.org/r/66310/diff/7/

Changes: https://reviews.apache.org/r/66310/diff/6-7/


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 66310: Passed on registrar when constructing resource provider manager.

[Chun-Hung Hsiao]

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




src/tests/resource_provider_manager_tests.cpp
Line 132 (original), 132 (patched)


Do you think it is reasonable to always require a registrar, i.e., disallow 
`nullptr` and use `InMemoryStore` here? Please drop this if you think we should 
allow `nullptr`.


- Chun-Hung Hsiao


On April 19, 2018, 12:28 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66310/
> ---
> 
> (Updated April 19, 2018, 12:28 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8735
> https://issues.apache.org/jira/browse/MESOS-8735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In order to support recovering resource provider manager information
> in the future, we need to adjust the construction of the manager to be
> able to consume a registrar.
> 
> This patch lays the groundwork by adjusting interfaces and their
> usage; we will make use of the passed on information in a following
> patch.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/manager.hpp bc017fa3998b780cec82718cabe2a8b470a66db2 
>   src/resource_provider/manager.cpp 68e1866986bfb91bf8355099ee1f0a2a86aba39a 
>   src/slave/slave.cpp e5d6c3fac5054a6b0a0b5b77abd850a35be6ccc5 
>   src/tests/resource_provider_manager_tests.cpp 
> c52541bf130ccf4795b989b53331176a64a097ea 
> 
> 
> Diff: https://reviews.apache.org/r/66310/diff/6/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66310: Passed on registrar when constructing resource provider manager.

[Chun-Hung Hsiao]

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


Ship it!




Ship It!

- Chun-Hung Hsiao


On April 19, 2018, 12:28 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66310/
> ---
> 
> (Updated April 19, 2018, 12:28 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8735
> https://issues.apache.org/jira/browse/MESOS-8735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In order to support recovering resource provider manager information
> in the future, we need to adjust the construction of the manager to be
> able to consume a registrar.
> 
> This patch lays the groundwork by adjusting interfaces and their
> usage; we will make use of the passed on information in a following
> patch.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/manager.hpp bc017fa3998b780cec82718cabe2a8b470a66db2 
>   src/resource_provider/manager.cpp 68e1866986bfb91bf8355099ee1f0a2a86aba39a 
>   src/slave/slave.cpp e5d6c3fac5054a6b0a0b5b77abd850a35be6ccc5 
>   src/tests/resource_provider_manager_tests.cpp 
> c52541bf130ccf4795b989b53331176a64a097ea 
> 
> 
> Diff: https://reviews.apache.org/r/66310/diff/5/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66310: Passed on registrar when constructing resource provider manager.

[Benjamin Bannier]

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

(Updated April 19, 2018, 2:28 p.m.)


Review request for mesos, Chun-Hung Hsiao, Jie Yu, and Jan Schlicht.


Changes
---

Addressed issue raised by Chun.


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


Repository: mesos


Description
---

In order to support recovering resource provider manager information
in the future, we need to adjust the construction of the manager to be
able to consume a registrar.

This patch lays the groundwork by adjusting interfaces and their
usage; we will make use of the passed on information in a following
patch.


Diffs (updated)
-

  src/resource_provider/manager.hpp bc017fa3998b780cec82718cabe2a8b470a66db2 
  src/resource_provider/manager.cpp 68e1866986bfb91bf8355099ee1f0a2a86aba39a 
  src/slave/slave.cpp e5d6c3fac5054a6b0a0b5b77abd850a35be6ccc5 
  src/tests/resource_provider_manager_tests.cpp 
c52541bf130ccf4795b989b53331176a64a097ea 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 66310: Passed on registrar when constructing resource provider manager.

[Benjamin Bannier]

> On April 19, 2018, 12:25 a.m., Chun-Hung Hsiao wrote:
> > src/resource_provider/manager.cpp
> > Line 160 (original), 161 (patched)
> > 
> >
> > `Owned&& _registrar`?

https://reviews.apache.org/r/66309/#comment_rc282702-201502


> On April 19, 2018, 12:25 a.m., Chun-Hung Hsiao wrote:
> > src/slave/slave.cpp
> > Lines 8869-8871 (patched)
> > 
> >
> > The current implementation makes this assertion never fail ;)

That's exactly why this is an assertion, not a branch :D


- Benjamin


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


On April 19, 2018, 2:28 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66310/
> ---
> 
> (Updated April 19, 2018, 2:28 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Jie Yu, and Jan Schlicht.
> 
> 
> Bugs: MESOS-8735
> https://issues.apache.org/jira/browse/MESOS-8735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In order to support recovering resource provider manager information
> in the future, we need to adjust the construction of the manager to be
> able to consume a registrar.
> 
> This patch lays the groundwork by adjusting interfaces and their
> usage; we will make use of the passed on information in a following
> patch.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/manager.hpp bc017fa3998b780cec82718cabe2a8b470a66db2 
>   src/resource_provider/manager.cpp 68e1866986bfb91bf8355099ee1f0a2a86aba39a 
>   src/slave/slave.cpp e5d6c3fac5054a6b0a0b5b77abd850a35be6ccc5 
>   src/tests/resource_provider_manager_tests.cpp 
> c52541bf130ccf4795b989b53331176a64a097ea 
> 
> 
> Diff: https://reviews.apache.org/r/66310/diff/5/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66310: Passed on registrar when constructing resource provider manager.

[Chun-Hung Hsiao]

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




src/resource_provider/manager.hpp
Lines 42 (patched)


`process::Owned&& registrar`?

Also it seems to me that the `mesos::` prefix can be omitted.



src/resource_provider/manager.cpp
Line 160 (original), 161 (patched)


`Owned&& _registrar`?



src/slave/slave.cpp
Lines 8869-8871 (patched)


The current implementation makes this assertion never fail ;)



src/slave/slave.cpp
Lines 8873 (patched)


This is completely redundent because of line 8848.


- Chun-Hung Hsiao


On April 10, 2018, 12:07 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66310/
> ---
> 
> (Updated April 10, 2018, 12:07 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-8735
> https://issues.apache.org/jira/browse/MESOS-8735
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In order to support recovering resource provider manager information
> in the future, we need to adjust the construction of the manager to be
> able to consume a registrar.
> 
> This patch lays the groundwork by adjusting interfaces and their
> usage; we will make use of the passed on information in a following
> patch.
> 
> 
> Diffs
> -
> 
>   src/resource_provider/manager.hpp bc017fa3998b780cec82718cabe2a8b470a66db2 
>   src/resource_provider/manager.cpp 68e1866986bfb91bf8355099ee1f0a2a86aba39a 
>   src/slave/slave.cpp e5d6c3fac5054a6b0a0b5b77abd850a35be6ccc5 
>   src/tests/resource_provider_manager_tests.cpp 
> c52541bf130ccf4795b989b53331176a64a097ea 
> 
> 
> Diff: https://reviews.apache.org/r/66310/diff/4/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 66310: Passed on registrar when constructing resource provider manager.

[Benjamin Bannier]

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

(Updated April 10, 2018, 2:07 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Rebased.


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


Repository: mesos


Description
---

In order to support recovering resource provider manager information
in the future, we need to adjust the construction of the manager to be
able to consume a registrar.

This patch lays the groundwork by adjusting interfaces and their
usage; we will make use of the passed on information in a following
patch.


Diffs (updated)
-

  src/resource_provider/manager.hpp bc017fa3998b780cec82718cabe2a8b470a66db2 
  src/resource_provider/manager.cpp 2d3ffcdd01671f81729fe538348f2ca4e555eb4f 
  src/slave/slave.cpp e5d6c3fac5054a6b0a0b5b77abd850a35be6ccc5 
  src/tests/resource_provider_manager_tests.cpp 
c52541bf130ccf4795b989b53331176a64a097ea 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier