Re: Review Request 66308: Delayed construction of the agent's resource provider manager.

2018-04-20 Thread Benjamin Bannier

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

(Updated April 20, 2018, 12:23 p.m.)


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


Changes
---

Added a comment as suggested by Chun.


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


Repository: mesos


Description
---

By delaying the construction of the agent's resource provider manager
we prepare for a following patch introducing a dependency on the
resource provider manager on the agent's ID.

Depending on whether the agent was able to recover an agent ID from
its log or still needs to obtain on in a first registration with the
master, we can only construct the resource provider manager after
different points in the initialization of the agent. To capture the
common code we introduce a helper function performing the necessary
steps.


Diffs (updated)
-

  src/slave/slave.hpp d00c7b2e2466fd1de71e9e55343a27d3808bde3e 
  src/slave/slave.cpp e5d6c3fac5054a6b0a0b5b77abd850a35be6ccc5 
  src/tests/resource_provider_manager_tests.cpp 
c52541bf130ccf4795b989b53331176a64a097ea 


Diff: https://reviews.apache.org/r/66308/diff/6/

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


Testing
---

`make check`


Thanks,

Benjamin Bannier



Re: Review Request 66308: Delayed construction of the agent's resource provider manager.

2018-04-19 Thread Chun-Hung Hsiao

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




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


How about adding a comment to explain why this might be called twice and we 
should ignore the subsequent call?


- Chun-Hung Hsiao


On April 19, 2018, 9:19 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66308/
> ---
> 
> (Updated April 19, 2018, 9:19 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
> ---
> 
> By delaying the construction of the agent's resource provider manager
> we prepare for a following patch introducing a dependency on the
> resource provider manager on the agent's ID.
> 
> Depending on whether the agent was able to recover an agent ID from
> its log or still needs to obtain on in a first registration with the
> master, we can only construct the resource provider manager after
> different points in the initialization of the agent. To capture the
> common code we introduce a helper function performing the necessary
> steps.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp d00c7b2e2466fd1de71e9e55343a27d3808bde3e 
>   src/slave/slave.cpp e5d6c3fac5054a6b0a0b5b77abd850a35be6ccc5 
>   src/tests/resource_provider_manager_tests.cpp 
> c52541bf130ccf4795b989b53331176a64a097ea 
> 
> 
> Diff: https://reviews.apache.org/r/66308/diff/5/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 66308: Delayed construction of the agent's resource provider manager.

2018-04-19 Thread Benjamin Bannier

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

(Updated April 19, 2018, 11:19 a.m.)


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


Changes
---

Addressed more issues raised by Chun.


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


Repository: mesos


Description
---

By delaying the construction of the agent's resource provider manager
we prepare for a following patch introducing a dependency on the
resource provider manager on the agent's ID.

Depending on whether the agent was able to recover an agent ID from
its log or still needs to obtain on in a first registration with the
master, we can only construct the resource provider manager after
different points in the initialization of the agent. To capture the
common code we introduce a helper function performing the necessary
steps.


Diffs (updated)
-

  src/slave/slave.hpp d00c7b2e2466fd1de71e9e55343a27d3808bde3e 
  src/slave/slave.cpp e5d6c3fac5054a6b0a0b5b77abd850a35be6ccc5 
  src/tests/resource_provider_manager_tests.cpp 
c52541bf130ccf4795b989b53331176a64a097ea 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier



Re: Review Request 66308: Delayed construction of the agent's resource provider manager.

2018-04-19 Thread Benjamin Bannier


> On April 18, 2018, 8:58 p.m., Chun-Hung Hsiao wrote:
> > src/slave/slave.cpp
> > Lines 839 (patched)
> > 
> >
> > One line apart. Actually, let's move this back to it's original place 
> > so all `/api/v1/*` endpoints are declared together.

I moved this up to be grouped with the routes of `/api/v1` and 
`/api/v1/executor`.


> On April 18, 2018, 8:58 p.m., Chun-Hung Hsiao wrote:
> > src/slave/slave.cpp
> > Lines 8189 (patched)
> > 
> >
> > Maybe add a comment saying that checking `additionalResources` is 
> > enough because all resources needs to be published must have been checked 
> > before?

I am not sure that is what is happening here. The assertion here just validates 
that no new additional resources from RPs enter while we haven't even 
initialized the RP manager. This should hold as RP resources can only appear 
after a RP has subscribed which requires the manager. I don't believe that this 
performs a very stringent check, and we just perform the easy check here.

I added a comment to that effect, PTAL.


- Benjamin


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


On April 18, 2018, 4:28 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66308/
> ---
> 
> (Updated April 18, 2018, 4: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
> ---
> 
> By delaying the construction of the agent's resource provider manager
> we prepare for a following patch introducing a dependency on the
> resource provider manager on the agent's ID.
> 
> Depending on whether the agent was able to recover an agent ID from
> its log or still needs to obtain on in a first registration with the
> master, we can only construct the resource provider manager after
> different points in the initialization of the agent. To capture the
> common code we introduce a helper function performing the necessary
> steps.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp d00c7b2e2466fd1de71e9e55343a27d3808bde3e 
>   src/slave/slave.cpp e5d6c3fac5054a6b0a0b5b77abd850a35be6ccc5 
>   src/tests/resource_provider_manager_tests.cpp 
> c52541bf130ccf4795b989b53331176a64a097ea 
> 
> 
> Diff: https://reviews.apache.org/r/66308/diff/4/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 66308: Delayed construction of the agent's resource provider manager.

2018-04-18 Thread Chun-Hung Hsiao

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


Fix it, then Ship it!





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


One line apart. Actually, let's move this back to it's original place so 
all `/api/v1/*` endpoints are declared together.



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


Maybe add a comment saying that checking `additionalResources` is enough 
because all resources needs to be published must have been checked before?


- Chun-Hung Hsiao


On April 18, 2018, 2:28 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66308/
> ---
> 
> (Updated April 18, 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
> ---
> 
> By delaying the construction of the agent's resource provider manager
> we prepare for a following patch introducing a dependency on the
> resource provider manager on the agent's ID.
> 
> Depending on whether the agent was able to recover an agent ID from
> its log or still needs to obtain on in a first registration with the
> master, we can only construct the resource provider manager after
> different points in the initialization of the agent. To capture the
> common code we introduce a helper function performing the necessary
> steps.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp d00c7b2e2466fd1de71e9e55343a27d3808bde3e 
>   src/slave/slave.cpp e5d6c3fac5054a6b0a0b5b77abd850a35be6ccc5 
>   src/tests/resource_provider_manager_tests.cpp 
> c52541bf130ccf4795b989b53331176a64a097ea 
> 
> 
> Diff: https://reviews.apache.org/r/66308/diff/4/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 66308: Delayed construction of the agent's resource provider manager.

2018-04-18 Thread Chun-Hung Hsiao


> On April 18, 2018, 3:03 a.m., Chun-Hung Hsiao wrote:
> > src/slave/slave.cpp
> > Lines 8833-8840 (patched)
> > 
> >
> > There was a recent discussion in the API WG about adding routes 
> > dynamically (after initialization). The discussion started with this 
> > ticket: https://issues.apache.org/jira/browse/MESOS-7697
> > 
> > In summary, libprocess would return a 404 if a route has not been added 
> > yet, but for certain endpoints (e.g., the `/api/v1/scheduler`, or the agent 
> > resource provider config API calls), 404 might also be used by the API 
> > handler to represent missing resources. A client would have no way to 
> > distinguish what's the meaning of a 404, and if it should retry.
> > 
> > Several ideas had been proposed, sech as:
> > (1) Establishing a convention that a Mesos API handler never uses 404, 
> > but use 410 instead. But the semantics of "GONE" dose not quite match "NOT 
> > FOUND".
> > 
> > (2) Make libprocess returns 503 in the beginning, and then at certain 
> > point of time, mark libprocess as "initialized", meaning that no more 
> > routes will be added, and after that libprocess could return 404 if a route 
> > is added.
> > 
> > Along with the discussion of (2), people seems to agree that in most 
> > cases dynamic addition of routes is not very useful and (2) seems a viable 
> > solution. There's no conclusion yet, but if we're going to follow (2), I'd 
> > avoid adding `/api/v1/resource_provider` here, but just registering this in 
> > `Slave::initialize()`, and let the resource provider manager rejects 
> > requests until it obtains the slave ID. This is also what I did for 
> > `LocalResourceProviderDaemon`: 
> > https://github.com/apache/mesos/blob/master/src/resource_provider/daemon.cpp#L147.
> > 
> > Before we establish the convention, I'd suggest that we avoid adding 
> > routes after `Slave::initialize()`. Thoughts?
> 
> Chun-Hung Hsiao wrote:
> Some typo:
> "sech as" -> "such as"
> "return 404 if a route is added" -> "return 404 if a route is *not* there"
> 
> Benjamin Bannier wrote:
> I changed the code to add the route statically on agent initialization, 
> but not servicing it until a resource provider manager has be created.
> 
> This seems to make sense as while the route is owned by the agent (and 
> should have a corresponding lifetime), the agent's ability to serve it 
> depends on whether the rp manager is available. Does that capture what you 
> had in mind?

Yeah this change looks good to me. In the case of 
`LocalResourceProviderDaemon`, it still serves requests (modifying configs) 
even if the agent is not resistered yet. In the case of 
`ResourceProviderManager` it cannot serve anything.


- Chun-Hung


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


On April 18, 2018, 2:28 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66308/
> ---
> 
> (Updated April 18, 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
> ---
> 
> By delaying the construction of the agent's resource provider manager
> we prepare for a following patch introducing a dependency on the
> resource provider manager on the agent's ID.
> 
> Depending on whether the agent was able to recover an agent ID from
> its log or still needs to obtain on in a first registration with the
> master, we can only construct the resource provider manager after
> different points in the initialization of the agent. To capture the
> common code we introduce a helper function performing the necessary
> steps.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp d00c7b2e2466fd1de71e9e55343a27d3808bde3e 
>   src/slave/slave.cpp e5d6c3fac5054a6b0a0b5b77abd850a35be6ccc5 
>   src/tests/resource_provider_manager_tests.cpp 
> c52541bf130ccf4795b989b53331176a64a097ea 
> 
> 
> Diff: https://reviews.apache.org/r/66308/diff/4/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>



Re: Review Request 66308: Delayed construction of the agent's resource provider manager.

2018-04-18 Thread Benjamin Bannier


> On April 18, 2018, 5:03 a.m., Chun-Hung Hsiao wrote:
> > src/slave/slave.hpp
> > Line 815 (original), 819 (patched)
> > 
> >
> > This is inconsistent with the existing codebase. Could you justify why 
> > favoring `unique_ptr` over `Owned`?

I went for a `unique_ptr` while developing because it is much easier to use it 
correctly (see e.g., MESOS-5122). I changed the code to used `Owned` now for 
consistency.


> On April 18, 2018, 5:03 a.m., Chun-Hung Hsiao wrote:
> > src/slave/slave.cpp
> > Line 4606 (original), 4601-4616 (patched)
> > 
> >
> > How about the following?
> > ```
> > Result resourceProviderId =
> >   getResourceProviderId(operation->info());
> > 
> > CHECK(!resourceProviderId.isError());
> > 
> > if (CHECKresourceProviderId.isSome()) {
> >   CHECK_NOTNULL(resourceProviderManager.get())
> > ->acknowedgeOperationStatus(acknowledgement);
> > }
> > ```
> > I prefer this way to encapsulate the implementation details about how 
> > to get the resource provider ID from the consumed resources.

I like that suggestion, adopted.


> On April 18, 2018, 5:03 a.m., Chun-Hung Hsiao wrote:
> > src/slave/slave.cpp
> > Lines 8177-8179 (patched)
> > 
> >
> > Let's check that all resources are agent default resources.

I have added an assertion to that effect here.


> On April 18, 2018, 5:03 a.m., Chun-Hung Hsiao wrote:
> > src/slave/slave.cpp
> > Lines 8822 (patched)
> > 
> >
> > `flags` is not used in this function.

It will be used in https://reviews.apache.org/r/66310/ and I added it here 
already.


> On April 18, 2018, 5:03 a.m., Chun-Hung Hsiao wrote:
> > src/slave/slave.cpp
> > Lines 8829 (patched)
> > 
> >
> > Could you elaborate on this? Or make it a TODO if the comment is too 
> > complicated to be added ;)

Ups, I did remove this in https://reviews.apache.org/r/66310/, and hadn't 
realized it was still _here_. Fixed the artifact here now.


> On April 18, 2018, 5:03 a.m., Chun-Hung Hsiao wrote:
> > src/slave/slave.cpp
> > Lines 8833-8840 (patched)
> > 
> >
> > There was a recent discussion in the API WG about adding routes 
> > dynamically (after initialization). The discussion started with this 
> > ticket: https://issues.apache.org/jira/browse/MESOS-7697
> > 
> > In summary, libprocess would return a 404 if a route has not been added 
> > yet, but for certain endpoints (e.g., the `/api/v1/scheduler`, or the agent 
> > resource provider config API calls), 404 might also be used by the API 
> > handler to represent missing resources. A client would have no way to 
> > distinguish what's the meaning of a 404, and if it should retry.
> > 
> > Several ideas had been proposed, sech as:
> > (1) Establishing a convention that a Mesos API handler never uses 404, 
> > but use 410 instead. But the semantics of "GONE" dose not quite match "NOT 
> > FOUND".
> > 
> > (2) Make libprocess returns 503 in the beginning, and then at certain 
> > point of time, mark libprocess as "initialized", meaning that no more 
> > routes will be added, and after that libprocess could return 404 if a route 
> > is added.
> > 
> > Along with the discussion of (2), people seems to agree that in most 
> > cases dynamic addition of routes is not very useful and (2) seems a viable 
> > solution. There's no conclusion yet, but if we're going to follow (2), I'd 
> > avoid adding `/api/v1/resource_provider` here, but just registering this in 
> > `Slave::initialize()`, and let the resource provider manager rejects 
> > requests until it obtains the slave ID. This is also what I did for 
> > `LocalResourceProviderDaemon`: 
> > https://github.com/apache/mesos/blob/master/src/resource_provider/daemon.cpp#L147.
> > 
> > Before we establish the convention, I'd suggest that we avoid adding 
> > routes after `Slave::initialize()`. Thoughts?
> 
> Chun-Hung Hsiao wrote:
> Some typo:
> "sech as" -> "such as"
> "return 404 if a route is added" -> "return 404 if a route is *not* there"

I changed the code to add the route statically on agent initialization, but not 
servicing it until a resource provider manager has be created.

This seems to make sense as while the route is owned by the agent (and should 
have a corresponding lifetime), the agent's ability to serve it depends on 
whether the rp manager is available. Does that capture what you had in mind?


- Benjamin


---
This is an automatically 

Re: Review Request 66308: Delayed construction of the agent's resource provider manager.

2018-04-18 Thread Benjamin Bannier

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

(Updated April 18, 2018, 4:28 p.m.)


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


Changes
---

Addressed comments from Chun.


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


Repository: mesos


Description
---

By delaying the construction of the agent's resource provider manager
we prepare for a following patch introducing a dependency on the
resource provider manager on the agent's ID.

Depending on whether the agent was able to recover an agent ID from
its log or still needs to obtain on in a first registration with the
master, we can only construct the resource provider manager after
different points in the initialization of the agent. To capture the
common code we introduce a helper function performing the necessary
steps.


Diffs (updated)
-

  src/slave/slave.hpp d00c7b2e2466fd1de71e9e55343a27d3808bde3e 
  src/slave/slave.cpp e5d6c3fac5054a6b0a0b5b77abd850a35be6ccc5 
  src/tests/resource_provider_manager_tests.cpp 
c52541bf130ccf4795b989b53331176a64a097ea 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier



Re: Review Request 66308: Delayed construction of the agent's resource provider manager.

2018-04-17 Thread Chun-Hung Hsiao

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




src/slave/slave.hpp
Line 815 (original), 819 (patched)


This is inconsistent with the existing codebase. Could you justify why 
favoring `unique_ptr` over `Owned`?



src/slave/slave.cpp
Line 4606 (original), 4601-4616 (patched)


How about the following?
```
Result resourceProviderId =
  getResourceProviderId(operation->info());

CHECK(!resourceProviderId.isError());

if (CHECKresourceProviderId.isSome()) {
  CHECK_NOTNULL(resourceProviderManager.get())
->acknowedgeOperationStatus(acknowledgement);
}
```
I prefer this way to encapsulate the implementation details about how to 
get the resource provider ID from the consumed resources.



src/slave/slave.cpp
Lines 8177-8179 (patched)


Let's check that all resources are agent default resources.



src/slave/slave.cpp
Line 8187 (original), 8201 (patched)


```
CHECK_NOTNULL(resourceProviderManager.get())
  ->publishResources(resources);
```



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


`flags` is not used in this function.



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


Could you elaborate on this? Or make it a TODO if the comment is too 
complicated to be added ;)



src/slave/slave.cpp
Lines 8833-8840 (patched)


There was a recent discussion in the API WG about adding routes dynamically 
(after initialization). The discussion started with this ticket: 
https://issues.apache.org/jira/browse/MESOS-7697

In summary, libprocess would return a 404 if a route has not been added 
yet, but for certain endpoints (e.g., the `/api/v1/scheduler`, or the agent 
resource provider config API calls), 404 might also be used by the API handler 
to represent missing resources. A client would have no way to distinguish 
what's the meaning of a 404, and if it should retry.

Several ideas had been proposed, sech as:
(1) Establishing a convention that a Mesos API handler never uses 404, but 
use 410 instead. But the semantics of "GONE" dose not quite match "NOT FOUND".

(2) Make libprocess returns 503 in the beginning, and then at certain point 
of time, mark libprocess as "initialized", meaning that no more routes will be 
added, and after that libprocess could return 404 if a route is added.

Along with the discussion of (2), people seems to agree that in most cases 
dynamic addition of routes is not very useful and (2) seems a viable solution. 
There's no conclusion yet, but if we're going to follow (2), I'd avoid adding 
`/api/v1/resource_provider` here, but just registering this in 
`Slave::initialize()`, and let the resource provider manager rejects requests 
until it obtains the slave ID. This is also what I did for 
`LocalResourceProviderDaemon`: 
https://github.com/apache/mesos/blob/master/src/resource_provider/daemon.cpp#L147.

Before we establish the convention, I'd suggest that we avoid adding routes 
after `Slave::initialize()`. Thoughts?


- 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/66308/
> ---
> 
> (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
> ---
> 
> By delaying the construction of the agent's resource provider manager
> we prepare for a following patch introducing a dependency on the
> resource provider manager on the agent's ID.
> 
> Depending on whether the agent was able to recover an agent ID from
> its log or still needs to obtain on in a first registration with the
> master, we can only construct the resource provider manager after
> different points in the initialization of the agent. To capture the
> common code we introduce a helper function performing the necessary
> steps.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.hpp d00c7b2e2466fd1de71e9e55343a27d3808bde3e 
>   src/slave/slave.cpp e5d6c3fac5054a6b0a0b5b77abd850a35be6ccc5 
>   src/tests/resource_provider_manager_tests.cpp 
> c52541bf130ccf4795b989b53331176a64a097ea 
> 
> 
> Diff: https://reviews.apache.org/r/66308/diff/3/
> 
> 
> Testing
> ---
> 
> `make 

Re: Review Request 66308: Delayed construction of the agent's resource provider manager.

2018-04-10 Thread Benjamin Bannier

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

(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
---

By delaying the construction of the agent's resource provider manager
we prepare for a following patch introducing a dependency on the
resource provider manager on the agent's ID.

Depending on whether the agent was able to recover an agent ID from
its log or still needs to obtain on in a first registration with the
master, we can only construct the resource provider manager after
different points in the initialization of the agent. To capture the
common code we introduce a helper function performing the necessary
steps.


Diffs (updated)
-

  src/slave/slave.hpp d00c7b2e2466fd1de71e9e55343a27d3808bde3e 
  src/slave/slave.cpp e5d6c3fac5054a6b0a0b5b77abd850a35be6ccc5 
  src/tests/resource_provider_manager_tests.cpp 
c52541bf130ccf4795b989b53331176a64a097ea 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier



Re: Review Request 66308: Delayed construction of the agent's resource provider manager.

2018-03-28 Thread Benjamin Bannier

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

(Updated March 28, 2018, 4:28 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
---

By delaying the construction of the agent's resource provider manager
we prepare for a following patch introducing a dependency on the
resource provider manager on the agent's ID.

Depending on whether the agent was able to recover an agent ID from
its log or still needs to obtain on in a first registration with the
master, we can only construct the resource provider manager after
different points in the initialization of the agent. To capture the
common code we introduce a helper function performing the necessary
steps.


Diffs (updated)
-

  src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb 
  src/slave/slave.cpp e76daabad0d2d68aa42d1da809d4a23459eaaacb 
  src/tests/resource_provider_manager_tests.cpp 
d947bd037190e6b7ea7b2277b5fbe47816878de4 


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

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


Testing
---

`make check`


Thanks,

Benjamin Bannier