> On Aug. 28, 2015, 12:59 p.m., Jie Yu wrote:
> > src/slave/containerizer/provisioners/appc.cpp, lines 79-97
> > <https://reviews.apache.org/r/37881/diff/1/?file=1057722#file1057722line79>
> >
> >     Some high level comments.
> >     
> >     I think it will be beneficial if we can move the fetch and discovery 
> > logic to the Store. For the MVP, we don't have to impl. that in the Store.
> >     
> >     The Store only has one interface, which is the 'get' method. 'get' 
> > takes an Image::Appc and returns a vector of paths to layers. THis 
> > semantics is more natural and easy to understand: get layers for my image, 
> > if some layers are already cached, return them directly, otherwise, 
> > discover it and fetch it.
> >     
> >     The job of the provisioner is to get layers from the Store and stack 
> > them into a rootfs using the backend. It also manages the rootfs 
> > directories.
> >     
> >     To me, this is a more natural split of functionalities among 
> > components. In the future, we can also potentially unify the impl. of the 
> > provisioner (i.e., one single provisioner for both Docker and Appc images). 
> > All the image specific details are hiden in the Store interface.
> >     
> >     Thoughts?
> 
> Jiang Yan Xu wrote:
>     I am OK the this direction but a few points:
>     
>     1) I think a "dumb" store that only returns what it has and stores what 
> is given to it is natual too. (So how about stripping the fetching logic from 
> the store and put it in the provisioner? This way the provisioner is the 
> "manager" that controls all the dumb single-purpose helpers.)
>     
>     2) Unifying the provisioner would be nice but one of the remaining 
> obstacles is that we need to make sure we have a common "provisioner 
> direcotry layout", otherwise they need to be further extracted out from the 
> provisioner and the provisioner becomes so thin that there is no point in 
> having a unified provisioner anymore. --> So I am not totally sure if this 
> will happen.
>     
>     3) In the future there may be a lot of more logic required for the new 
> Store if it is responsible for doing all the heavy-lifting: cache eviction, 
> P2P fetching, registering new provision requests with ongoing fetching 
> processes, etc. But I guess this is not an argument for keeping these in the 
> provisioner, we should create other single purpose class to handle them, we 
> just need to design a Store API that's takes these into consideration.
>     
>     For now since none of these aformentioned functionality is necessary for 
> our "read-only, no dependency" provisioner let's find a way to check this as 
> long as it doesn't make it hard to refactor in the future.
> 
> Jie Yu wrote:
>     Those are valid points! Let me express my rationale in some depth:
>     
>     Store manages store_dir and privisioner manages rootfs_dir. A natural 
> split is that: any thing involved in updating 'store_dir' should be handled 
> by Store and anything involved in updating 'rootfs_dir' should be handled by 
> rootfs_dir.
>     
>     In that sense, fetching and caching should be handled by the Store. As 
> you said, you can introduce sub-component in 'Store' to dealing with 
> Discovery, fetching, caching, etc.
>     
>     Another reason I want fetching and caching to be done in Store, 
> independent of provisioner, is that: maybe, in the future, we can support 
> 'pre-fetching' which is not associated with any provisioning at all. Putting 
> all such logics in the provisioner will complicate it a lot. A component with 
> less dependency is definitely more managable.

OK I am going to make the recommended change beause I realized that this is 
basically the two patterns for caching:

1) Cache-aside: the cache returns none if it's not in it and the application 
reads the data and put it in the cache.
2) Read-through: the cache fetches the data transparently if it's not cached.

We cannot do 1) cleanly because given the size of the data on the disk we need 
the data to be downloaded directly into the cache and the application shouldn't 
need to know how the cache manages its data.

So we can just do 2).


- Jiang Yan


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


On Aug. 28, 2015, 2:04 a.m., Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/37881/
> -----------------------------------------------------------
> 
> (Updated Aug. 28, 2015, 2:04 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Timothy Chen.
> 
> 
> Bugs: MESOS-2796
>     https://issues.apache.org/jira/browse/MESOS-2796
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implemented AppcProvisioner.
> 
> 
> Diffs
> -----
> 
>   src/Makefile.am 7b620ff66856b3f0adac121b3297d55ed71a3d99 
>   src/slave/containerizer/provisioner.cpp 
> efc7e6996ff6663bebaf61989a7e040bd2ad7a5e 
>   src/slave/containerizer/provisioners/appc.hpp PRE-CREATION 
>   src/slave/containerizer/provisioners/appc.cpp PRE-CREATION 
>   src/slave/containerizer/provisioners/appc/paths.hpp 
> 41e3bf79da0854406c488855f953111e67353829 
>   src/slave/containerizer/provisioners/appc/paths.cpp 
> 3113c84b9526dd9e9e89fb9aa4ec75ed66a996c7 
>   src/slave/flags.hpp e56738e2dfd6593ef8f093687919da287af78f77 
>   src/slave/flags.cpp b36710d6d7a7250bc071a57310a2d54bfb3bc624 
>   src/tests/containerizer/appc_provisioner_tests.cpp 
> 47b66b9c30cefe8f9a8e2c1c1341776c2d235020 
> 
> Diff: https://reviews.apache.org/r/37881/diff/
> 
> 
> Testing
> -------
> 
> sudo make check.
> 
> More test cases coming.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Reply via email to