> On April 13, 2017, 12:21 a.m., Vinod Kone wrote: > > src/master/allocator/mesos/allocator.hpp > > Line 70 (original), 70-71 (patched) > > <https://reviews.apache.org/r/57817/diff/3/?file=1684648#file1684648line70> > > > > This is making me think whether we want framework to subscribe with > > "inactive" field instead "suppress". > > > > The reason being, "suppressOffers" was originally added instead of > > "deactivateFramework" because the former was intended to have some filters. > > One such filter has been recently added: you can suppress offers for a > > specific role instead of all roles. Do we want to give the same flexibility > > for suppression during subscription? > > > > For example, here it's not clear to me what is the difference between > > "active" and "offersSuppressed" booleans as far as the allocator is > > concerned. > > > > Thoughts? > > Anindya Sinha wrote: > I agree that the 2 APIs are very similar and I agree we can also achieve > the same by an `inactive` (instead of `suppress_offers`) field of `SUBSCRIBE` > override the definition of an active framework which is currently just based > on framework state. However, it seemed cleaner to me to *not* update the > definition of an active framework based on a field in the `SUBSCRIBE` call. > Basically, `framework->state` is *modifiable* characteristic of a framework, > whereas `suppress_offers` is an initial setting to register that framework. > > I think functionally it would achieve the same so I am fine with using an > `inactive` field in `SUBSCRIBE` redefine the definition of an active > framework. > > Let me know what you think and I can adapt accordingly. > > Vinod Kone wrote: > Thinking a bit more, "inactive" field in the SUBSCRIBE message might not > be intuitive for framework authors to figure out how to make it "active". We > don't have a REACTIVATE call. So maybe "supress_offers" is more intuitive > because they would intuitively know to call REVIVE to un-suppress. > > Given that, how about "Suppress suppress" field inside "Subcribe" instead > of "boolean suppress_offers" to be more future proof? Ideally, we would have > a way to atomically send "SUBSCRIBE" and "SUPPRESS" calls together in one > calls but not sure how to easily achieve that.
That sounds good. Updated `SUBSCRIBE` to contain `Suppress suppress` (for being future proof), and made changes accordingly in all the reviews in this chain. - Anindya ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57817/#review171630 ----------------------------------------------------------- On April 11, 2017, 11:10 p.m., Anindya Sinha wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57817/ > ----------------------------------------------------------- > > (Updated April 11, 2017, 11:10 p.m.) > > > Review request for mesos, James Peach, Vinod Kone, and Jiang Yan Xu. > > > Bugs: MESOS-7015 > https://issues.apache.org/jira/browse/MESOS-7015 > > > Repository: mesos > > > Description > ------- > > If requested in SUBSCRIBE api call, offers are suppressed on > framework registration. > > > Diffs > ----- > > include/mesos/allocator/allocator.hpp > 6eda1b8619269c1501a935045b18b1deaf845b33 > src/master/allocator/mesos/allocator.hpp > 57b54b86c43c7731e64d422d285c4b8ca7e27a60 > src/master/allocator/mesos/hierarchical.hpp > f84b0574ce9a392c9528c87b04b01dbb2053cff7 > src/master/allocator/mesos/hierarchical.cpp > 051f749dd5921a322ca930a042c31814616d38f9 > src/master/master.hpp d537933d0b467a6f9996951c601b31338bb9d034 > src/master/master.cpp 0f4c64c6b102ef201779a331c96b5d78a98281ad > src/tests/allocator.hpp 6b71c574e0e4facd1795ef50ee0869c03b87833d > src/tests/hierarchical_allocator_tests.cpp > 33e7b455f8664858eb4f03727b076a10c80cd6e0 > src/tests/master_allocator_tests.cpp > 119e318f8a01d50e8dae5c30cf5fa6a017c3c625 > src/tests/persistent_volume_endpoints_tests.cpp > 1237d081d781948975f66a8240ae9bdb5e819a3b > src/tests/reservation_tests.cpp 4504831d77c1bfcf5f2ddf6d28cd45dea2c421ad > src/tests/resource_offers_tests.cpp > f0bca1d9e03013ce35215b0ffa6b50b38972dc0c > src/tests/slave_recovery_tests.cpp 53f33a2b0411c8158326074ce043c7b1dbeef5b4 > > > Diff: https://reviews.apache.org/r/57817/diff/4/ > > > Testing > ------- > > All tests passed. > > > Thanks, > > Anindya Sinha > >
