On Wed, 2010-09-15 at 21:48 -0700, Markus Roberts wrote: > PuppetLabs Request For Comments: 1 > Authors: Paul & Markus > Title: Semantics of autoloaded classes [snip] > Proposed Solution I: rebuild known_resource_types on each compile > ----------------------------------------------------------------- > > Each time an agent contacts the master, rebuild known_resource_types > from scratch (using cached copies of the ASTs for the relevant files). > > Pros: > > - This eliminates inconsistencies caused by servicing previous nodes. > > - Minimal user impact. Manifests that worked consistantly in 0.25 > and > 2.6 will continue to work without modification. > > - It will cause manifests which depended on the node-order issues (and > thus failed occasionaly) to fail consistently, making them much > easier > to diagnose.
This is good, failing early is better. > Cons: > > - This does nothing to address order dependencies or the existing > performance issues with autoloading. > > - It introduces a slight additional performance hit, since > known_resource_types must be rebuilt from scratch with each compile. > However, we estimate this performance hit to be very small in > practice (<1% of total compilation time spent on the master). There's another con, you'll keep in memory n copies of the AST where n is your max concurrency (thanks to the way the GC works). I don't really know how much memory we're talking about, maybe it's something we should not care about. > > Proposed Solution II: restrict the contents of autoloaded files > --------------------------------------------------------------- > > When autoloading a file, check that it only declares things > within a namespace corresponding to its filename. For example, > the file $module_path/foo/manifests/init.pp may declare a > class (or definition or node) foo, any class within the namespace > foo (e.g. foo::bar), and any resources within those classes. It > may not declare any toplevel resources or any other toplevel > classes. These same restrictions apply to a file ./foo.pp > autoloaded from the current working directory. > > In a similar vein, a file $module_path/foo/manifests/bar.pp (or > ./foo/bar.pp) may only declare things within the namespace > foo::bar. Exception: it may also declare the class foo, but it > may not declare anything inside it other than bar. These rules > are extended in the obvious way for more deeply nested > files (e.g. ./foo/bar/baz.pp). > > Also, modify the search order of autoloading so that it looks in > the order of general to specific. For example, when trying to > autoload foo::bar::baz, if foo is a module, it looks in the order > > $module_path/foo/manifests/init.pp, then > $module_path/foo/manifests/bar.pp, then > $module_path/foo/manifests/bar/baz.pp. > > If foo is not a module, then it looks in the order > > ./foo.pp, then > ./foo/bar.pp, then > ./foo/bar/baz.pp. > > In either case, it stops as soon as it finds > the thing being autoloaded. > > Pros: > > - This eliminates most sources of inconsistencies and order > dependencies by formalizing a relationship between files and > classes that most users are probably following anyway. > > - It eliminates the remaining sources of inconsistencies and > order dependencies by making a small change to search order > that is unlikely to affect most users. > > - It forces users to follow a naming convention that will help > them to organize their manifests well. I like this. Enforcing this can be a pain, but in the end the manifest architecture would be way better (ie cleaner code). I think lot of people will not like this, though :( > Cons: > > - This does not address any performance issues with autoloading. Which is not good. I do agree that we should aim for the correct result before talking about performance, but if in Statler we lose 50% performance in compilation a lot of people will get angry :) > - Potentially large user impact. Unconventionally structured > manifests that worked in 0.25 and 2.6 may require substantial > renaming / relocation of classes in order to meet the new file > organization requirements. (However, users can work around > this using explicit imports.) If there is a (well) documented work-around, it should be better. > Proposed Solution III: eliminate the autoloading feature > -------------------------------------------------------- > > Do not allow autoloading. Require the user to explicitly import all > necessary manifest files either directly or indirectly from site.pp. > > Pros: > > - This eliminates inconsistencies and order dependencies because all > files are imported before evaluation of any nodes begins. > > - It improves performance, because it eliminates the need to go to the > filesystem when resolving names. > > - It improves predictability, because it always makes it explicitly > clear which manifest files can contribute to a catalog. > > - Minimal user impact. Manifests that worked in 0.25 and 2.6 may > require explicit imports in order to work properly without > autoloading, > however it will be easy to tell from the error messages what > imports > to add. > > Cons: > > - It eliminates a feature which users may perceive to be of high > value. Even though I use a mix of manual import and auto-loading, I still think auto-loading is really nice. In a few words, I'd really like to have both the performance and the nice features. For this reason, I'd vote if feasible, for #1 with the strict checking of #2 if possible :) -- Brice Figureau Follow the latest Puppet Community evolutions on www.planetpuppet.org! -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
