> On May 7, 2015, 12:38 a.m., Jie Yu wrote: > > include/mesos/mesos.proto, lines 454-458 > > <https://reviews.apache.org/r/33865/diff/1/?file=950607#file950607line454> > > > > Chatted with Vinod offline (PS: Vinod is going to send out a summary of > > the discussion). > > > > In short, OVERSUBSCRIBED = (Allocated but unused) + (Unallocated) > > > > So, the type `OVERSUBSCRIBED` here is too general and ambiguate. > > > > You description of this review is interesting. Resources reserved for a > > different role but unused (or unallocated) can be revokable. The question > > is to whom should we send this revokable resources. For instance, if a > > framework under role A reserved some resources on the slave (resources' > > role == A) and not using any of them yet. Should we send revokable offers > > to framework under role A, or all frameworks. It's not clear yet and I > > think that's a policy issue. It would be better to let a modulized > > component (e.g., resources estimator) to control that so that we can > > potentially use different policies. If we decide to send revokable offer > > for those reserved resources to all frameworks, we could strip the role in > > the revokable resources (i.e., make them * resources). > > > > However, I don't know what type should we set for the RevokableInfo for > > the above case? How can we distinguish that with the case where the > > resources are from unreserved resources (i.e., anyone can use that)? > > > > As a result, I think maybe we should punt on the `type` here right now > > and just leave an empty RevokableInfo since it's not strictly needed?
Fair point. I will remove the type for now. > On May 7, 2015, 12:38 a.m., Jie Yu wrote: > > include/mesos/mesos.proto, line 455 > > <https://reviews.apache.org/r/33865/diff/1/?file=950607#file950607line455> > > > > 2 spaces indent please. N/A > On May 7, 2015, 12:38 a.m., Jie Yu wrote: > > src/common/resources.cpp, lines 490-501 > > <https://reviews.apache.org/r/33865/diff/1/?file=950608#file950608line490> > > > > Should we put these checks to master validation? As I discussed with > > Mpark week ago, the validations in Resources object should be minimal > > checks to ensure all methods in Resources are well behaved. > > > > Clearly, the check you have here is a bit high level and probably > > should be put in master validation? Moved to validation.cpp. Also, I think the last check in Resource::validate() ("*" resource cannot be dynamically reserved) should be moved to master validation? - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33865/#review82763 ----------------------------------------------------------- On May 5, 2015, 9:13 p.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33865/ > ----------------------------------------------------------- > > (Updated May 5, 2015, 9:13 p.m.) > > > Review request for mesos, Jie Yu, Joris Van Remoortere, and Niklas Nielsen. > > > Bugs: MESOS-2691 > https://issues.apache.org/jira/browse/MESOS-2691 > > > Repository: mesos > > > Description > ------- > > RevocableInfo currently supports OVERSUBSCRIBED resources. In the future it > can be easily extended to use other types of revocable reosurces (e.g., > resources allocated to other roles). > > Disabled the ability to use revocable resources for reservation or > persistence because the semantics seem weird. We can enable it in the future > if there is a use case for that. > > > Diffs > ----- > > include/mesos/mesos.proto db4fc8c001dd68bc3b9ca83650170c4f26db18c7 > src/common/resources.cpp 235930ff2dbb3ea49a3a0696dc070f2bd56fba4b > src/tests/resources_tests.cpp a7ec59ea217ad71f7d1e93ca6039d5b2491b3237 > > Diff: https://reviews.apache.org/r/33865/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
