On 26/03/10 19:52, Luke Kanies wrote: > On Mar 26, 2010, at 11:48 AM, Brice Figureau wrote: > >> On 26/03/10 19:45, Luke Kanies wrote: >>> On Mar 26, 2010, at 9:30 AM, Markus Roberts wrote: >>> >>>>>> brice:tickets/0.25.x/3396 (Faster event propagation) succeeded: >>>>> * 7c3cd8c Fix inefficient SimpleGraph#matching_edge >>>>> 7771 examples, 28 failures, 41 pending >>>>> 4 more in unit/simple_graph.rb (4 total) >>>> >>>> This is not good. Looks like we are missing Luke's glue event patch, or >>>> there is something at play here. >>>> >>>> The event-->events patch appears to be there; the problem appears to >>>> be with the event.source patch (all of the errors look like this): >>>> >>>> 1) >>>> NoMethodError in 'Puppet::SimpleGraph when matching edges should match >>>> edges whose source matches the source of the event' >>>> undefined method `inject' for #<Puppet::Transaction::Event:0x250b610> >>>> /Users/markus/projects/puppet/lib/puppet/simple_graph.rb:164:in >>>> `matching_edges' >>>> spec/unit/simple_graph.rb:361: >>>> /Library/Ruby/Gems/1.8/gems/rspec-1.2.9/lib/spec/example/example_methods.rb:40:in >>>> >>>> `instance_eval' >>>> /Library/Ruby/Gems/1.8/gems/rspec-1.2.9/lib/spec/example/example_methods.rb:40:in >>>> >>>> `execute' >>>> /System/Library/Frameworks/Ruby.framework/Versions/1.8/usr/lib/ruby/1.8/timeout.rb:53:in >>>> >>>> `timeout' >>>> /Library/Ruby/Gems/1.8/gems/rspec-1.2.9/lib/spec/example/example_methods.rb:37:in >>>> >>>> `execute' >>>> /Library/Ruby/Gems/1.8/gems/rspec-1.2.9/lib/spec/example/example_group_methods.rb:214:in >>>> >>>> `run_examples' >>>> /Library/Ruby/Gems/1.8/gems/rspec-1.2.9/lib/spec/example/example_group_methods.rb:212:in >>>> >>>> `each' >>>> /Library/Ruby/Gems/1.8/gems/rspec-1.2.9/lib/spec/example/example_group_methods.rb:212:in >>>> >>>> `run_examples' >>>> /Library/Ruby/Gems/1.8/gems/rspec-1.2.9/lib/spec/example/example_group_methods.rb:103:in >>>> >>>> `run' >>>> /Users/markus/projects/puppet/spec/monkey_patches/add_confine_and_runnable_to_rspec_dsl.rb:22:in >>>> >>>> `run' >>>> /Users/markus/projects/puppet/spec/monkey_patches/add_confine_and_runnable_to_rspec_dsl.rb:17:in >>>> >>>> `each' >>>> /Users/markus/projects/puppet/spec/monkey_patches/add_confine_and_runnable_to_rspec_dsl.rb:17:in >>>> >>>> `run' >>>> /Library/Ruby/Gems/1.8/gems/rspec-1.2.9/lib/spec/runner/options.rb:151:in >>>> >>>> `run_examples' >>>> /Library/Ruby/Gems/1.8/gems/rspec-1.2.9/lib/spec/runner.rb:61:in `run' >>>> /Library/Ruby/Gems/1.8/gems/rspec-1.2.9/lib/spec/runner.rb:45:in >>>> `autorun' >>>> spec/unit/simple_graph.rb:9: >>> >>> I've confirmed that my code still just calls matching_edges with one >>> event but the SimpleGraph class expects multiple of them, thus the >>> inject is failing (because it's being called on one event instead of >>> many). >>> >>> I think the problem is that I refactored queue_event after Brice >>> refactored matching_edges, or something similar. >>> >>> Brice, can you, in your patch, remove the optimizations in >>> EventManager#queue_event and change it so it passes the event list to >>> matching_edges, instead of one event at a time? I think that's the best >>> move, because otherwise we're going to keep dancing around each other in >>> our refactors. >> >> Isn't what the last bit of event patches you sent was doing? >> I mean the one labelled "Changing the method profile of >> EventManager#queue_event". >> (sorry I didn't really read all the events refactorings you did) > > Kind of - it made that one method accept a list of events, but then it > completely duplicates the optimizations you have in > SimpleGraph#matching_edges - it does all of the deduplication and only > calls matching_edges when absolutely necessary. It was a bit silly on > my part, I think. > > The best approach now, I think, is to trim down my method to just pass > the whole event list to matching_edges, and leave the optimizations and > such in matching_edges.
OK, I read your code and tested it. It doesn't behave like 0.25.x behaved for generated resources, so I don't even think my patch for #3396 is needed at all. One of the reason behind my changes in #3396 was that we triggered an event per generated resource that was back propagated to those resources themselves producing a combinatory explosion in edge matching. I wasn't able to reproduce this issue with luke:tickets/master/2759 (it doesn't seem to generate those many events). So, based on this, I think we should completely drop brice:tickets/0.25.x/3396 from the testing branch. -- Brice Figureau My Blog: http://www.masterzen.fr/ -- 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.
