You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Jie Yu <yu...@gmail.com> on 2014/12/04 23:56:40 UTC
Review Request 28723: Added protobuf protocol for communicating
persisted resources between master and slave.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28723/
-----------------------------------------------------------
Review request for mesos and Ben Mahler.
Bugs: MESOS-2100
https://issues.apache.org/jira/browse/MESOS-2100
Repository: mesos-git
Description
-------
Added protobuf protocol for communicating persisted resources between master and slave.
Diffs
-----
src/messages/messages.proto 28e593f338a154892b1cdf398710bb44e3f9f119
Diff: https://reviews.apache.org/r/28723/diff/
Testing
-------
make check
Thanks,
Jie Yu
Re: Review Request 28723: Added protobuf protocol for communicating
persisted resources between master and slave.
Posted by Jie Yu <yu...@gmail.com>.
> On Dec. 5, 2014, 7:44 p.m., Michael Park wrote:
> > Perhaps we consider naming the fields "checkpointedResources" in order to stay clear of confusing it with persistent resources.
Ditto BenM's comments. I think dynamic reservation is like role-persisted resources, and persistent volumes is like diskinfo-persisted resources. So I don't think it's too confusing here.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28723/#review64063
-----------------------------------------------------------
On Dec. 4, 2014, 10:56 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28723/
> -----------------------------------------------------------
>
> (Updated Dec. 4, 2014, 10:56 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-2100
> https://issues.apache.org/jira/browse/MESOS-2100
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added protobuf protocol for communicating persisted resources between master and slave.
>
>
> Diffs
> -----
>
> src/messages/messages.proto 28e593f338a154892b1cdf398710bb44e3f9f119
>
> Diff: https://reviews.apache.org/r/28723/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 28723: Added protobuf protocol for communicating
persisted resources between master and slave.
Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28723/#review64063
-----------------------------------------------------------
Perhaps we consider naming the fields "checkpointedResources" in order to stay clear of confusing it with persistent resources.
- Michael Park
On Dec. 4, 2014, 10:56 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28723/
> -----------------------------------------------------------
>
> (Updated Dec. 4, 2014, 10:56 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-2100
> https://issues.apache.org/jira/browse/MESOS-2100
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added protobuf protocol for communicating persisted resources between master and slave.
>
>
> Diffs
> -----
>
> src/messages/messages.proto 28e593f338a154892b1cdf398710bb44e3f9f119
>
> Diff: https://reviews.apache.org/r/28723/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 28723: Added protobuf protocol for communicating
persisted resources between master and slave.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28723/#review65559
-----------------------------------------------------------
src/messages/messages.proto
<https://reviews.apache.org/r/28723/#comment108784>
What if we call all of these 'resource_transformations'?
Does that better capture the fact that each one of these is a dynamic "transformations" that gets applied to the statically configured slave.resources()?
It might also make the code a bit clearer, for example in the master:
```
// Compute transformations.
CompositeTransformation composite;
foreach (const Resource& r, resourceTransformations) {
if (r.has_disk()) {
composite.add(PersistentDisk(r));
} else if (r.has_dynamic_role()) {
composite.add(DynamicReservation(r));
} else {
// Error.
}
}
```
- Ben Mahler
On Dec. 4, 2014, 10:56 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28723/
> -----------------------------------------------------------
>
> (Updated Dec. 4, 2014, 10:56 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-2100
> https://issues.apache.org/jira/browse/MESOS-2100
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added protobuf protocol for communicating persisted resources between master and slave.
>
>
> Diffs
> -----
>
> src/messages/messages.proto 28e593f338a154892b1cdf398710bb44e3f9f119
>
> Diff: https://reviews.apache.org/r/28723/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 28723: Added protobuf protocol for communicating
persisted resources between master and slave.
Posted by Jie Yu <yu...@gmail.com>.
> On Dec. 19, 2014, 1:42 a.m., Ben Mahler wrote:
> > src/messages/messages.proto, lines 248-251
> > <https://reviews.apache.org/r/28723/diff/1/?file=782815#file782815line248>
> >
> > Per our discussion, we should probably call out that these are resources that must be explicitly released by the framework.
Added.
> On Dec. 19, 2014, 1:42 a.m., Ben Mahler wrote:
> > src/messages/messages.proto, line 328
> > <https://reviews.apache.org/r/28723/diff/1/?file=782815#file782815line328>
> >
> > Let's add a comment that this is sent to the slave whenever there is an acquisition or release of a persistent resource (and we probably need to piggy-back it on (re-)registration too? Maybe a TODO in this patch?)
Done.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28723/#review65577
-----------------------------------------------------------
On Dec. 19, 2014, 6:39 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28723/
> -----------------------------------------------------------
>
> (Updated Dec. 19, 2014, 6:39 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-2100
> https://issues.apache.org/jira/browse/MESOS-2100
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added protobuf protocol for communicating persisted resources between master and slave.
>
> The reason for introducing the protocol is because the master needs to tell the slave to update it's persisted resources (for either persitent disk or dynamic reservation) in the case of acquiring/releasing persistent disk or dynamic reservations. Also, the slave needs to tell the master what resources it has persisted during registration and re-registration.
>
> We've been thought about by sending diffs (i.e., newly acquired/released persisted resources). However, due to various race conditions between master and slave, we decided to instead send totals (i.e., master sends what it thinks is the total persisted resources for that slave to the slave).
>
>
> Diffs
> -----
>
> src/messages/messages.proto 28e593f338a154892b1cdf398710bb44e3f9f119
>
> Diff: https://reviews.apache.org/r/28723/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 28723: Added protobuf protocol for communicating
persisted resources between master and slave.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28723/#review65577
-----------------------------------------------------------
Ship it!
I'm ok with 'persisted_resources' as a name, considering that both "persistent disk" and "persistent role" (a.k.a. dynamic reservation) resources all fall under the category of resources that are persistent/durable/sticky/etc.
If it turns out that we find a better name when dynamic reservations enter the picture, we can safely change the name as long as the tag number remains the same.
src/messages/messages.proto
<https://reviews.apache.org/r/28723/#comment108823>
Per our discussion, we should probably call out that these are resources that must be explicitly released by the framework.
src/messages/messages.proto
<https://reviews.apache.org/r/28723/#comment108825>
Let's add a comment that this is sent to the slave whenever there is an acquisition or release of a persistent resource (and we probably need to piggy-back it on (re-)registration too? Maybe a TODO in this patch?)
- Ben Mahler
On Dec. 4, 2014, 10:56 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28723/
> -----------------------------------------------------------
>
> (Updated Dec. 4, 2014, 10:56 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-2100
> https://issues.apache.org/jira/browse/MESOS-2100
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added protobuf protocol for communicating persisted resources between master and slave.
>
>
> Diffs
> -----
>
> src/messages/messages.proto 28e593f338a154892b1cdf398710bb44e3f9f119
>
> Diff: https://reviews.apache.org/r/28723/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 28723: Added protobuf protocol for communicating
persisted resources between master and slave.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28723/
-----------------------------------------------------------
(Updated Dec. 19, 2014, 6:39 p.m.)
Review request for mesos and Ben Mahler.
Bugs: MESOS-2100
https://issues.apache.org/jira/browse/MESOS-2100
Repository: mesos-git
Description (updated)
-------
Added protobuf protocol for communicating persisted resources between master and slave.
The reason for introducing the protocol is because the master needs to tell the slave to update it's persisted resources (for either persitent disk or dynamic reservation) in the case of acquiring/releasing persistent disk or dynamic reservations. Also, the slave needs to tell the master what resources it has persisted during registration and re-registration.
We've been thought about by sending diffs (i.e., newly acquired/released persisted resources). However, due to various race conditions between master and slave, we decided to instead send totals (i.e., master sends what it thinks is the total persisted resources for that slave to the slave).
Diffs
-----
src/messages/messages.proto 28e593f338a154892b1cdf398710bb44e3f9f119
Diff: https://reviews.apache.org/r/28723/diff/
Testing
-------
make check
Thanks,
Jie Yu
Re: Review Request 28723: Added protobuf protocol for communicating
persisted resources between master and slave.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28723/#review63929
-----------------------------------------------------------
Patch looks great!
Reviews applied: [28711, 28720, 28723]
All tests passed.
- Mesos ReviewBot
On Dec. 4, 2014, 10:56 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28723/
> -----------------------------------------------------------
>
> (Updated Dec. 4, 2014, 10:56 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-2100
> https://issues.apache.org/jira/browse/MESOS-2100
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added protobuf protocol for communicating persisted resources between master and slave.
>
>
> Diffs
> -----
>
> src/messages/messages.proto 28e593f338a154892b1cdf398710bb44e3f9f119
>
> Diff: https://reviews.apache.org/r/28723/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 28723: Added protobuf protocol for communicating
persisted resources between master and slave.
Posted by Jie Yu <yu...@gmail.com>.
> On Dec. 18, 2014, 2:36 a.m., Ben Mahler wrote:
> > src/messages/messages.proto, lines 270-273
> > <https://reviews.apache.org/r/28723/diff/1/?file=782815#file782815line270>
> >
> > Hm.. could you add some design description to this review or the ticket?
> >
> > I can't remember why we'd want to send it during re-registration, would be great to document the "protocol" for others as well!
Added some description in this review.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28723/#review65443
-----------------------------------------------------------
On Dec. 19, 2014, 6:39 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28723/
> -----------------------------------------------------------
>
> (Updated Dec. 19, 2014, 6:39 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-2100
> https://issues.apache.org/jira/browse/MESOS-2100
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added protobuf protocol for communicating persisted resources between master and slave.
>
> The reason for introducing the protocol is because the master needs to tell the slave to update it's persisted resources (for either persitent disk or dynamic reservation) in the case of acquiring/releasing persistent disk or dynamic reservations. Also, the slave needs to tell the master what resources it has persisted during registration and re-registration.
>
> We've been thought about by sending diffs (i.e., newly acquired/released persisted resources). However, due to various race conditions between master and slave, we decided to instead send totals (i.e., master sends what it thinks is the total persisted resources for that slave to the slave).
>
>
> Diffs
> -----
>
> src/messages/messages.proto 28e593f338a154892b1cdf398710bb44e3f9f119
>
> Diff: https://reviews.apache.org/r/28723/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 28723: Added protobuf protocol for communicating
persisted resources between master and slave.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28723/#review65443
-----------------------------------------------------------
src/messages/messages.proto
<https://reviews.apache.org/r/28723/#comment108587>
Hm.. could you add some design description to this review or the ticket?
I can't remember why we'd want to send it during re-registration, would be great to document the "protocol" for others as well!
- Ben Mahler
On Dec. 4, 2014, 10:56 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28723/
> -----------------------------------------------------------
>
> (Updated Dec. 4, 2014, 10:56 p.m.)
>
>
> Review request for mesos and Ben Mahler.
>
>
> Bugs: MESOS-2100
> https://issues.apache.org/jira/browse/MESOS-2100
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Added protobuf protocol for communicating persisted resources between master and slave.
>
>
> Diffs
> -----
>
> src/messages/messages.proto 28e593f338a154892b1cdf398710bb44e3f9f119
>
> Diff: https://reviews.apache.org/r/28723/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>