> On Oct. 17, 2017, 10:48 p.m., Jie Yu wrote: > > include/mesos/resource_provider/resource_provider.proto > > Lines 38 (patched) > > <https://reviews.apache.org/r/62502/diff/2/?file=1854486#file1854486line38> > > > > No need for `UNPUBLISH` for now. Let's introduce it once we actually > > need it. > > Chun-Hung Hsiao wrote: > We may still want `UNPUBLISH`, depending on what we want to implement in > the following case. > A storage local resource provider needs to call > `csi::Node::NodeUnpublishVolume` before `csi::Controller::DeleteVolume`. > We can either: > 1. Ask the storage local resource provider to checkpoint what's being > published, and do the proper unpublish call before deleting. This requires no > `UNPUBLISH`. > 2. Ask the resource provider manager to checkpoint what's being > published, and it is responsible to issue an `UNPUBLISH` before sending out > the `DestroyVolume` offer operation. > Which one sounds better? Also if we consider general resource providers, > should the manager or the provider checkpoint the published resources? > > Chun-Hung Hsiao wrote: > Now I think since the RP need to remember what is published, because we > don't do `UBPUBLISH` for local resources for now, I'll just make the RP do > it's own unpublish. > > Chun-Hung Hsiao wrote: > If we want to keep the concept of "resource usage" and that of "resource > publish" decoupled and maintain the same "ensure total" semantics in the > future, we probably need `PUBLISH_AT_LEAST` and `PUBLISH_NO_MORE_THAN` > instead of `PUBLISH`/`UNPUBLISH`. These operations provide flexibility for us > to decide how to do reaource unpublishing (eager/periodic GC/lazy) later. Are > there better names for these operations?
I am not sure decoupling these on this level is useful, and for simplicity I'd much prefer callers to only have to use a single pair of messages to signal that resources are needed or not needed anymore. Currently it looks like these messages would be used to inform the resource provider about tasks which are being launched and have ended, and we don't currently have or plan to add knobs to parameterize this behavior along the axes you outline. Not sure what scenarios you have in mind for `PUBLISH_NO_MORE_THAN` (better: use case for isolation? not sure), but as a user I would already expect operations on the plugin to be idempotent enough so that `PUBLISH_AT_LEAST` wouldn't be needed. I also not sure callers at this level would want to parameterize unpublishing like you hinted; this instead sounds like a use case for plugin configuration to me. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62502/#review188386 ----------------------------------------------------------- On Oct. 13, 2017, 2:01 p.m., Jan Schlicht wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62502/ > ----------------------------------------------------------- > > (Updated Oct. 13, 2017, 2:01 p.m.) > > > Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Jie Yu. > > > Bugs: MESOS-8089 > https://issues.apache.org/jira/browse/MESOS-8089 > > > Repository: mesos > > > Description > ------- > > Before launching a task that use resource provider resources, the Mesos > master will instruct the resource provider to make these resources > available for the task. External resource providers will checkpoint > informations to be able to reconcile resource usage in case of a > failover. Resource providers will acknowledge the 'PUBLISH'/'UNPUBLISH' > operations by sending back 'PUBLISHED'/'UNPUBLISHED' together with > the UUID of the operation. > > > Diffs > ----- > > include/mesos/resource_provider/resource_provider.proto > f5a9073075327019fd133bd51265f695ef464845 > include/mesos/v1/resource_provider/resource_provider.proto > e5cbede5b6e57a8641fca1ebfee5454f292cc24c > src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 > src/resource_provider/storage/provider.cpp > 46224997430ac0c568904d80014166a6f059907f > src/resource_provider/validation.cpp > d2927227f60ab0d4ae2481ad73a31ee444b48ee0 > src/tests/mesos.hpp 24d220e292bc1e137992e8f81484477b62bd0896 > > > Diff: https://reviews.apache.org/r/62502/diff/2/ > > > Testing > ------- > > make check > > > Thanks, > > Jan Schlicht > >
