On Sat, Feb 8, 2014 at 7:01 AM, Brice Figureau < brice-pup...@daysofwonder.com> wrote:
> On 08/02/14 00:37, Peter Meier wrote: > > On 02/07/2014 08:26 PM, John Bollinger wrote: > > > > > >> On Friday, February 7, 2014 3:57:00 AM UTC-6, Brice Figureau > >> wrote: > > > >> In this specific bug, I'm wondering if we really need to have > >> those defined type as resources in the RAL catalog (I believe it > >> might be used for relationship or event propagation). > > > > > > > >> I suspect that defined type instances could be removed from the > >> catalog by replacing them with the collections of resources the > >> represent (recursively). However, I would expect that to cause an > >> explosion in the number of relationships (both signaling and > >> non-signaling) that must appear in the catalog. > > > > > > "Flattening" the catalog is something that we have discussed before > > [1] and which would give imho quite some improvements. > > > > However, we would likely loose important context information, that > > should then be preserved differently. > > > > Coming back to the "Bug": I think swapping the statement would give > > already some improvement and is a fix that could go into the next > > (bugfix)-release. Given that there are no concerns regarding possible > > side effects. > > Unfortunately there's a side-effect. The problem is when you start the > master, it creates a settings catalog with file resources in it. Those > will be then subject to the to_hash method, which calls parse_title > which calls in turn resource_type. With the swapped statements, this > will call known_resource_types. Since it's the first time, it will > initialize itself and will trigger a full parse of the manifests. > Beforehand, it would end-up in the Puppet::Type.type which would return > true and completely skip this initial parsing until a catalog gets > compiled. > > That is a similar problem that pops up all over the place. Joshua Partlow and I struggled with it a lot while we were working on PUP-1118. The real solution to this is to get the code to stop assuming what the environment is and either have it passed in to it via a parameter, or use the new current environment. Right now there are places throughout the code that assumes all sorts of things. During PUP-1118 we tried to change some of these, but in a lot of places we were a bit more conservative. Maybe we shouldn't have been. When the settings catalog is being applied, there should be a specific environment, not tied to any of the real environments, that has no manifests to parse. In the system that we have on master, this could be done by having all of the relevant code paths use Puppet.lookup(:current_environment) and use Puppet.override(:current_environment => fake_settings_environment) when creating and applying the settings catalog. > I have an interim patch that instead mark defined type resources as > defined types, I'll update the bug report with my findings. > The drawback of course is that it adds a property that gets serialized > in the catalog. If the property is not present (like an older master to > a newer agent), the agent resorts to the old behavior of testing first > for a builtin type then a definition. > > I'll polish the patch, and make sure there are tests for it and will > push a PR as soon as I can for review. > > I've also been whittling away at some of this. I haven't tried to tackle the agent side, but on the master side I've got a couple patches that show some good speedups in my benchmarks. https://github.com/puppetlabs/puppet/pull/2341 https://github.com/puppetlabs/puppet/pull/2340 2341 also shows another way of approaching the settings catalog problem. I added a setter for the resource_type to the Puppet::Resource class, which lets us short circuit the type lookups. This could be used as a hack in the settings catalog. > Thanks! > -- > Brice Figureau > My Blog: http://www.masterzen.fr/ > > -- > You received this message because you are subscribed to the Google Groups > "Puppet Developers" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to puppet-dev+unsubscr...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/puppet-dev/52F646AE.302%40daysofwonder.com > . > For more options, visit https://groups.google.com/groups/opt_out. > -- Andrew Parker a...@puppetlabs.com Freenode: zaphod42 Twitter: @aparker42 Software Developer *Join us at PuppetConf 2014, September 23-24 in San Francisco - * http://bit.ly/pupconf14 -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To unsubscribe from this group and stop receiving emails from it, send an email to puppet-dev+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-dev/CANhgQXvm7B_Gdnbn9ULQUtG87tPWtuPKktpw_UAAwAn9m_DMRA%40mail.gmail.com. For more options, visit https://groups.google.com/groups/opt_out.
