Bug #507

Random behavior with cyclical dependancies

Added by Jordan Curzon almost 5 years ago. Updated over 4 years ago.

Status:Closed Start date:
Priority:High Due date:
Assignee:Luke Kanies % Done:

0%
Category:-
Target version:0.25.0
Affected Puppet version:0.25.4 Branch:
Keywords:
Votes: 0

Description

The following site.pp content:

file { "/tmp/test1": ensure => present, require => File[[tmptest2]] }
file { "/tmp/test2": ensure => present, require => File[[tmptest1]] }

Causes the following stack trace some random amount of the time in puppet 0.22.1:

notice: Ignoring --listen on onetime run
debug: Loaded state in 0.00 seconds
debug: Calling puppetmaster.freshness
info: Config is up to date
notice: Starting configuration run
debug: //File[/tmp/test2]/require: requires File[/tmp/test1]
debug: //File[/tmp/test1]/require: requires File[/tmp/test2]
err: Found a bug: too few arguments
/usr/lib/ruby/gems/1.8/gems/puppet-0.22.1/lib/puppet/pgraph.rb:70:in @%'
/usr/lib/ruby/gems/1.8/gems/puppet-0.22.1/lib/puppet/pgraph.rb:70:in @check_cycle'
/usr/lib/ruby/gems/1.8/gems/puppet-0.22.1/lib/puppet/transaction.rb:508:in @relationship_graph'
/usr/lib/ruby/gems/1.8/gems/puppet-0.22.1/lib/puppet/transaction.rb:472:in @prepare'
/usr/lib/ruby/gems/1.8/gems/puppet-0.22.1/lib/puppet/transaction.rb:295:in @evaluate'
/usr/lib/ruby/gems/1.8/gems/puppet-0.22.1/lib/puppet/client/master.rb:130:in @apply'
/usr/lib/ruby/gems/1.8/gems/puppet-0.22.1/lib/puppet/client/master.rb:359:in @run'
/usr/lib/ruby/gems/1.8/gems/puppet-0.22.1/lib/puppet/util.rb:212:in @benchmark'
/usr/lib/ruby/1.8/benchmark.rb:293:in @measure'
/usr/lib/ruby/1.8/benchmark.rb:307:in @realtime'
/usr/lib/ruby/gems/1.8/gems/puppet-0.22.1/lib/puppet/util.rb:211:in @benchmark'
/usr/lib/ruby/gems/1.8/gems/puppet-0.22.1/lib/puppet/client/master.rb:358:in @run'
/usr/lib/ruby/1.8/sync.rb:229:in @synchronize'
/usr/lib/ruby/gems/1.8/gems/puppet-0.22.1/lib/puppet/client/master.rb:344:in @run'
/usr/lib/ruby/gems/1.8/gems/puppet-0.22.1/bin/puppetd:424
/usr/bin/puppetd:18
debug: Storing state
debug: Stored state in 0.02 seconds
notice: Finished configuration run in 0.07 seconds

After working through the code I found it happens because on random occasions, sorted on line 55 of pgraph.rb contains N+1 elements from the pgraph causing bad to be empty. I looked for information about the top sort algorithm and on http://www.sciface.com/support/doc/40/en/Graph/topSort.html it says: “If G contains any cycle then a topological sorting does not exist and the call of Graph::topSort results in an error.” This is talking about another library but it indicates to me that the contents of the topsort algorithm are undefined in the case of a cyclical graph and that is what is causing the random exception. The only other state I found the topsort list in was empty which also provides no information on where the cyclical dependency is.

History

#1
Updated by Luke Kanies almost 5 years ago

  • Status changed from 1 to 2

#2
Updated by Luke Kanies almost 5 years ago

I can’t reproduce this, which makes it essentially impossible to test. There seems to be a better search algorithm here: http://www.bitformation.com/art/python_toposort.html. I’ll implement it when I get the chance, but in the meantime, I’ve patched the code in r2308 so it correctly notices when the ‘bad’ array is empty. Puppet was correctly catching the cycle here, it just wasn’t logging correctly.

At least here you’ll get a slightly more informative error.

#3
Updated by Luke Kanies almost 5 years ago

  • Status changed from 2 to 1

Allen Ballman says we can use back edges to correctly check when cycles appear, which should make this go away entirely. Hopefully he’ll be able to fix this.

#4
Updated by Luke Kanies almost 5 years ago

  • Status changed from 1 to 2

In looking at the code for topsort in GRATR:

      def topsort(start = nil, &block)
        result  = []
        go      = true 
        back    = Proc.new {|e| go = false } 
        push    = Proc.new {|v| result.unshift(v) if go}
        start   ||= verticesr0
        dfs({:exit_vertex => push, :back_edge => back, :start => start})        result.each {|v| block.call(v)} if block; result
      end

We should be able to just reimplement this method, throwing an exception if we encounter any back edges, which should (assuming the graphs actually work) completely prevent a circular graph.

#5
Updated by Luke Kanies over 4 years ago

  • Status changed from 2 to Closed
  • 7 set to fixed

Fixed in r2521.

Also available in: Atom PDF