mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Mann <g...@mesosphere.io>
Subject Re: Review Request 69162: Added agent and resource provider IDs to operation status messages.
Date Wed, 05 Dec 2018 19:40:40 GMT


> On Dec. 4, 2018, 11:20 p.m., James DeFelice wrote:
> > include/mesos/v1/mesos.proto
> > Lines 2434 (patched)
> > <https://reviews.apache.org/r/69162/diff/7/?file=2111243#file2111243line2434>
> >
> >     I'd like this part to be a bit more fleshed out:
> >     
> >     When `uuid` is set then it MUST be possible to acknowledge the status update
by using the specified `agent_id` and `resource_provider_id` (for local providers); and `resource_provider_id`
(for external providers).
> 
> Benjamin Bannier wrote:
>     I'd argue that this is already implied by the documentation of `uuid`,
>     ````
>     // Statuses that are delivered reliably to the scheduler will
>     // include a `uuid`. The status is considered delivered once
>     // it is acknowledged by the scheduler.
>     optional UUID uuid = 5;
>     ````
>     
>     Dropping for now.
> 
> James DeFelice wrote:
>     I see what you mean by it being implied. However, I'd really like for it to be more
explicitly documented so that it's not accidentally overlooked later.
> 
> Greg Mann wrote:
>     I'm not sure that I understand the suggestion. Does the statement "When uuid is set
then it MUST be possible to acknowledge the status update by using the specified agent_id
and resource_provider_id" imply that when `uuid` is set, `agent_id` and `resource_provider_id`
must best set (or just RP ID in the case of ext. providers)? That is not the case, as we will
support both operations on agent default resources (no RP ID) and operations by operators
(no agent ID) in the future.
>     
>     James, is the following an accurate representation of your intended meaning? -
>     
>     "When `uuid` is set then it MUST be possible to acknowledge the status update by
using the specified `agent_id` and/or `resource_provider_id`, when present."
>     
>     
>     
>     I'm also not sure why this comment would mention acknowledgement - isn't it true
that the desired constraint here is that it should be possible for any update which contains
a `uuid` to be _reconciled_ by the framework using the supplied agent/RP ID, when present?
> 
> James DeFelice wrote:
>     The goal is to document that Mesos will never provide a framework a UUID that cannot
be acknowledged due to informtion that's missing in the status update proto.

I'm not sure that a comment here is necessary to accomplish that. I might update the comments
in the `AcknowledgeOperationStatus` message in the scheduler API to say:

```
  message AcknowledgeOperationStatus {
    // If either `agent_id` or `resource_provider_id` are set in the
    // operation status received by the scheduler, they should be set
    // here as well.
    optional AgentID agent_id = 1;
    optional ResourceProviderID resource_provider_id = 2;

    required bytes uuid = 3;
    required OperationID operation_id = 4;
  }
```

This would provide explicit instructions to framework devs to include agent and/or RP IDs
in the acknowledgement any time they're set in the status.

WDYT?


- Greg


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


On Dec. 5, 2018, noon, Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69162/
> -----------------------------------------------------------
> 
> (Updated Dec. 5, 2018, noon)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gastón Kleiman, and James DeFelice.
> 
> 
> Bugs: MESOS-9293
>     https://issues.apache.org/jira/browse/MESOS-9293
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch adds agent and resource provider IDs to
> `UpdateOperationStatus` and `UpdateOperationStatusMessage`. With that
> frameworks are able to reconcile enough information after failover to
> construct operation acknowledgements.
> 
> We will add code to populate these fields in a follow-up patch.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto c822cc747cf153435b6c3ae1004168d5a289c97b 
>   include/mesos/v1/mesos.proto 51c1bfdbb360b3554eba2229ed386d6271e0315b 
>   src/common/type_utils.cpp ef13eae47b88efc15f1b2d00852b6387c2fffcbc 
>   src/internal/devolve.cpp 491ed2aa131a92e958bfa71cccfc5f257cd4b3f9 
>   src/internal/evolve.cpp aa60efed5a28ed4d847a4a27aa8e994cda82175d 
> 
> 
> Diff: https://reviews.apache.org/r/69162/diff/8/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


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