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