mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Chun-Hung Hsiao <chhs...@apache.org>
Subject Re: Review Request 66308: Delayed construction of the agent's resource provider manager.
Date Wed, 18 Apr 2018 18:40:46 GMT


> On April 18, 2018, 3:03 a.m., Chun-Hung Hsiao wrote:
> > src/slave/slave.cpp
> > Lines 8833-8840 (patched)
> > <https://reviews.apache.org/r/66308/diff/3/?file=1994904#file1994904line8844>
> >
> >     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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message