mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joseph Wu <jos...@mesosphere.io>
Subject Re: Review Request 64612: Added flag to pass VolumeProfileAdaptor module to SLRP.
Date Tue, 19 Dec 2017 02:37:32 GMT


> On Dec. 15, 2017, 3:26 p.m., Jie Yu wrote:
> > src/resource_provider/local.cpp
> > Lines 39 (patched)
> > <https://reviews.apache.org/r/64612/diff/1/?file=1916278#file1916278line39>
> >
> >     It's weird that LocalResourceProvider needs to be aware `volumeProfileAdaptor`
which is very StorageLocalResourceProvider specific.
> >     
> >     It's more appropriate to instantiate that in StorageLocalResourceProvider and
make it a singleton.
> >     
> >     probably passing the agent flags all the way to this method (or use `Parameters`
to pass in provider specifc parameters).

I've changed the injection method.  I did it the way I did ('create', 'get', and 'set' methods)
because tests may instantiate different varieties of the module in tests, so just having a
`once` is not sufficient to allow this.


- Joseph


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64612/#review193997
-----------------------------------------------------------


On Dec. 18, 2017, 6:35 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64612/
> -----------------------------------------------------------
> 
> (Updated Dec. 18, 2017, 6:35 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Jie Yu.
> 
> 
> Bugs: MESOS-8251
>     https://issues.apache.org/jira/browse/MESOS-8251
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This adds an agent flag to choose the VolumeProfileAdaptor module
> to load and passes this information down to the storage resource
> providers. A single module can potentially be used by multiple
> storage resource providers.
> 
> We choose to pass the module along as a global variable because the
> module is not needed by many of the components in the injection path.
> Specifically, the Storage Local Resource Provider will need the module,
> but its parents, the Local Resource Provider and Resource Provider
> Daemons will not.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resource_provider/volume_profile.hpp PRE-CREATION 
>   src/resource_provider/storage/provider.cpp 158b6b408002209aa9a79a6772da30c984aad61a

>   src/resource_provider/volume_profile.cpp PRE-CREATION 
>   src/slave/flags.hpp 5918484825c3c813883c42465f3e5a5693d6a2cd 
>   src/slave/flags.cpp d4b95b7542733b22299bc19235cf99220534c022 
>   src/slave/slave.hpp 75cc5836e39b2413eaefc5a5c9e905134db83743 
>   src/slave/slave.cpp e69d42a2bfae09e2defcd8333c0434a9fd8bac4c 
> 
> 
> Diff: https://reviews.apache.org/r/64612/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> Basically just a plumbing change to pass the module into the SLRP.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message