[
https://issues.apache.org/jira/browse/MESOS-5293?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Benjamin Bannier updated MESOS-5293:
------------------------------------
Description:
The way endpoints are routed is inconsistent between master and agent code
which makes adding or extending handlers error prone.
In the master we route like this:
{code}
// Setup HTTP routes.
route("/api/v1/scheduler",
DEFAULT_HTTP_FRAMEWORK_AUTHENTICATION_REALM,
Http::SCHEDULER_HELP(),
[this](const process::http::Request& request,
const Option<string>& principal) {
Http::log(request);
return http.scheduler(request, principal);
});
{code}
We capture a pointer to the current {{Master}} in the callback which allows us
to access master state from an endpoint handler. The handler is a (probably
non-static) member function of the master's member variable {{http}} which
outlives the callback.
Routing in the agent looks like this:
{code}
// Setup HTTP routes.
Http http = Http(this);
route("/api/v1/executor",
Http::EXECUTOR_HELP(),
[http](const process::http::Request& request) {
Http::log(request);
return http.executor(request);
});
{code}
In contrast to the master code we here copy a {{Http}} into the callback. Since
the callback is currently treated like a value and might e.g., be freely copied
around we are only guaranteed that it lives long enough for the handler (here
{{Http::executor}}) to return. In particular, since endpoint handlers return a
{{Future<Response>}} there is no guarantee that the used {{http}} lives long
enough to be still valid once a conventional (e.g., non-static member)
continuation is executed.
Both models have their merit:
* capturing a pointer to the master simplifies reasoning about lifetimes,
* capturing just a {{Http}} with very short lifetime minimizes interactions
among (e.g., concurrent) invocations of endpoint handlers.
This great inconsistency comes with a cost though, as employing patterns
borrowed from master endpoint handlers in agent code will lead to potentially
subtle bugs where a developer assuming that {{http}} would outlive a handler's
execution might introduce code invoking member functions of already destructed
variables. This is especially likely in code employing multiple layers of
{{delay}} or {{defer}} for which compilers seem unable to detect lifetime
problems and emit diagnostics.
It would be great if we could to use just one of the patterns to minimize
confusion, e.g., the more straight-forward master pattern.
was:
The way endpoints in routed is inconsistent between master and agent code which
makes added new handlers error prone.
In the master we route like this:
{code}
// Setup HTTP routes.
route("/api/v1/scheduler",
DEFAULT_HTTP_FRAMEWORK_AUTHENTICATION_REALM,
Http::SCHEDULER_HELP(),
[this](const process::http::Request& request,
const Option<string>& principal) {
Http::log(request);
return http.scheduler(request, principal);
});
{code}
We capture a pointer to the current {{Master}} in the callback which allows us
to access master state from an endpoint handler. The handler is a (probably
non-static) member function of the master's member variable {{http}} which
outlives the callback.
Routing in the agent looks like this:
{code}
// Setup HTTP routes.
Http http = Http(this);
route("/api/v1/executor",
Http::EXECUTOR_HELP(),
[http](const process::http::Request& request) {
Http::log(request);
return http.executor(request);
});
{code}
In contrast to the master code we here copy a {{Http}} into the callback. Since
the callback is currently treated like a value and might e.g., be freely copied
around we are only guaranteed that it lives long enough for the handler (here
{{Http::executor}}) to return. In particular, since endpoint handlers return a
{{Future<Response>}} there is no guarantee that the used {{http}} lives long
enough to be still valid once a conventional (e.g., non-static member)
continuation is executed.
Both models have their merit:
* capturing a pointer to the master simplifies reasoning about lifetimes,
* capturing just a {{Http}} with very short lifetime minimizes interactions
among (e.g., concurrent) invocations of endpoint handlers.
This great inconsistency comes with a cost though, as employing patterns
borrowed from master endpoint handlers in agent code will lead to potentially
subtle bugs where a developer assuming that {{http}} would outlive a handler's
execution might introduce code invoking member functions of already destructed
variables. This is especially likely in code employing multiple layers of
{{delay}} or {{defer}} for which compilers seem unable to detect lifetime
problems and emit diagnostics.
It would be great if we could to use just one of the patterns to minimize
confusion, e.g., the more straight-forward master pattern.
> Endpoint handlers for master and agent need to be implemented surprisingly
> differently
> --------------------------------------------------------------------------------------
>
> Key: MESOS-5293
> URL: https://issues.apache.org/jira/browse/MESOS-5293
> Project: Mesos
> Issue Type: Bug
> Components: master, slave
> Reporter: Benjamin Bannier
> Labels: tech-debt
>
> The way endpoints are routed is inconsistent between master and agent code
> which makes adding or extending handlers error prone.
> In the master we route like this:
> {code}
> // Setup HTTP routes.
> route("/api/v1/scheduler",
> DEFAULT_HTTP_FRAMEWORK_AUTHENTICATION_REALM,
> Http::SCHEDULER_HELP(),
> [this](const process::http::Request& request,
> const Option<string>& principal) {
> Http::log(request);
> return http.scheduler(request, principal);
> });
> {code}
> We capture a pointer to the current {{Master}} in the callback which allows
> us to access master state from an endpoint handler. The handler is a
> (probably non-static) member function of the master's member variable
> {{http}} which outlives the callback.
> Routing in the agent looks like this:
> {code}
> // Setup HTTP routes.
> Http http = Http(this);
> route("/api/v1/executor",
> Http::EXECUTOR_HELP(),
> [http](const process::http::Request& request) {
> Http::log(request);
> return http.executor(request);
> });
> {code}
> In contrast to the master code we here copy a {{Http}} into the callback.
> Since the callback is currently treated like a value and might e.g., be
> freely copied around we are only guaranteed that it lives long enough for the
> handler (here {{Http::executor}}) to return. In particular, since endpoint
> handlers return a {{Future<Response>}} there is no guarantee that the used
> {{http}} lives long enough to be still valid once a conventional (e.g.,
> non-static member) continuation is executed.
> Both models have their merit:
> * capturing a pointer to the master simplifies reasoning about lifetimes,
> * capturing just a {{Http}} with very short lifetime minimizes interactions
> among (e.g., concurrent) invocations of endpoint handlers.
> This great inconsistency comes with a cost though, as employing patterns
> borrowed from master endpoint handlers in agent code will lead to potentially
> subtle bugs where a developer assuming that {{http}} would outlive a
> handler's execution might introduce code invoking member functions of already
> destructed variables. This is especially likely in code employing multiple
> layers of {{delay}} or {{defer}} for which compilers seem unable to detect
> lifetime problems and emit diagnostics.
> It would be great if we could to use just one of the patterns to minimize
> confusion, e.g., the more straight-forward master pattern.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
