Hrmm. There's an implicit assumption that a thread won't span environments. In what circumstances is that assumption violated?
On Thu, Jul 22, 2010 at 8:47 PM, Markus Roberts <[email protected]>wrote: > -1 unfortunately. > > This makes known resources per-thread instead of per-environment, > which means that we'll loose all environment specificity--the resource > types would then be the same for all environments. :( > > Making it per thread x environment doesn't quite work either, at least > in the most obvious (too me) extrapolation. > > I'll try to come up with a counter-patch this evening. > > > On Thu, Jul 22, 2010 at 11:37 AM, Brice Figureau > <[email protected]> wrote: > > In 2.6 we moved access to loaded type to the environment. > > Each time the compiler was accessing the loaded types, we were checking > > if the manifests did change. > > This incurred a large performance decrease compared to 0.25. > > This also introduced a race condition if manifests changed while a thread > > was in the middle of a compilation operation. > > > > This tentative fix makes sure each thread will get access to the same > > loaded types collection for his live, even if the manifests change. > > Only new thread will notice the change and reload the manifests. > > But as long as the manifest don't change, the thread will share the > > same collection, so there is no duplicate in memory. > > > > Signed-off-by: Brice Figureau <[email protected]> > > --- > > lib/puppet/node/environment.rb | 6 +++++- > > spec/unit/node/environment_spec.rb | 35 > ++++++++++++++++++++++++++++++++--- > > 2 files changed, 37 insertions(+), 4 deletions(-) > > > > diff --git a/lib/puppet/node/environment.rb > b/lib/puppet/node/environment.rb > > index 762599c..c2b8d28 100644 > > --- a/lib/puppet/node/environment.rb > > +++ b/lib/puppet/node/environment.rb > > @@ -70,11 +70,15 @@ class Puppet::Node::Environment > > end > > > > def known_resource_types > > + return Thread.current[:known_resource_types] if > Thread.current[:known_resource_types] > > + > > if @known_resource_types.nil? or @known_resource_types.stale? > > @known_resource_types = Puppet::Resource::TypeCollection.new(self) > > @known_resource_types.perform_initial_import > > end > > - @known_resource_types > > + > > + Thread.current[:known_resource_types] = @known_resource_types > > + Thread.current[:known_resource_types] > > end > > > > def module(name) > > diff --git a/spec/unit/node/environment_spec.rb > b/spec/unit/node/environment_spec.rb > > index b400865..6edcce5 100755 > > --- a/spec/unit/node/environment_spec.rb > > +++ b/spec/unit/node/environment_spec.rb > > @@ -53,6 +53,7 @@ describe Puppet::Node::Environment do > > @env = Puppet::Node::Environment.new("dev") > > @collection = Puppet::Resource::TypeCollection.new(@env) > > @collection.stubs(:perform_initial_import) > > + Thread.current[:known_resource_types] = nil > > end > > > > it "should create a resource type collection if none exists" do > > @@ -71,13 +72,41 @@ describe Puppet::Node::Environment do > > @env.known_resource_types > > end > > > > - it "should create and return a new collection rather than returning > a stale collection" do > > - @env.known_resource_types.expects(:stale?).returns true > > + it "should return the same collection even if stale if it's the same > thread" do > > + Puppet::Resource::TypeCollection.stubs(:new).returns @collection > > + @env.known_resource_types.stubs(:stale?).returns true > > > > - Puppet::Resource::TypeCollection.expects(:new).returns @collection > > + @env.known_resource_types.should equal(@collection) > > + end > > + > > + it "should return the current thread associated collection if there > is one" do > > + Thread.current[:known_resource_types] = @collection > > > > @env.known_resource_types.should equal(@collection) > > end > > + > > + it "should give to all threads the same collection if it didn't > change" do > > + Puppet::Resource::TypeCollection.expects(:new).with(@env).returns > @collection > > + @env.known_resource_types > > + > > + t = Thread.new { > > + @env.known_resource_types.should equal(@collection) > > + } > > + t.join > > + end > > + > > + it "should give to new threads a new collection if it isn't stale" > do > > + Puppet::Resource::TypeCollection.expects(:new).with(@env).returns > @collection > > + @env.known_resource_types.expects(:stale?).returns(true) > > + > > + Puppet::Resource::TypeCollection.expects(:new).returns @collection > > + > > + t = Thread.new { > > + @env.known_resource_types.should equal(@collection) > > + } > > + t.join > > + end > > + > > end > > > > [:modulepath, :manifestdir].each do |setting| > > -- > > 1.6.6.1 > > > > -- > > 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]<puppet-dev%[email protected]> > . > > For more options, visit this group at > http://groups.google.com/group/puppet-dev?hl=en. > > > > > > > > -- > ----------------------------------------------------------- > The power of accurate observation is > commonly called cynicism by those > who have not got it. ~George Bernard Shaw > ------------------------------------------------------------ > > -- > 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]<puppet-dev%[email protected]> > . > For more options, visit this group at > http://groups.google.com/group/puppet-dev?hl=en. > > -- 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.
