This is the core change of the ticket; rather than using a topological sort to
statically
determine the order in which resources should be applied, we use the graph
wrapper
introduced in the prior commit to dynamically determine the order in which to
apply
resources based on 1) the status of the resource (ready, done) 2) the explicit &
implied dependencies, 3) the salted SHA1 of the title (for stability).
Further work is needed:
1) Resolving the handling of failed resources
2) Tests of the new behavior, to the extent posible
3) Newly-dead-code removal in simple_graph & transaction
4) Fix the name-prefix ordering hack in eval_generate by either:
a) Moving the logic into file
b) Refactoring Type#eval_generate to return a tree
c) ....?
5) Rough performace testing to look for hotspots
6) Investigation of possible interaction with #3788, #5351, #5414, #5876,
#6020, #6810,
and #6944 which may simplify or complicate their resolution.
Paired-with: Jesse Wolfe
Signed-off-by: Markus Roberts <[email protected]>
---
Local-branch: feature/next/resource_application_order
lib/puppet/simple_graph.rb | 8 +++++++
lib/puppet/transaction.rb | 46 +++++++++++++++++++++++++----------------
spec/unit/transaction_spec.rb | 4 ++-
3 files changed, 39 insertions(+), 19 deletions(-)
diff --git a/lib/puppet/simple_graph.rb b/lib/puppet/simple_graph.rb
index 27e068e..31858f1 100644
--- a/lib/puppet/simple_graph.rb
+++ b/lib/puppet/simple_graph.rb
@@ -453,6 +453,10 @@ class Puppet::SimpleGraph
result
end
+ def direct_dependents_of(v)
+ @out_from[v].keys
+ end
+
def upstream_from_vertex(v)
return @upstream_from[v] if @upstream_from[v]
result = @upstream_from[v] = {}
@@ -463,6 +467,10 @@ class Puppet::SimpleGraph
result
end
+ def direct_dependencies_of(v)
+ @in_to[v].keys
+ end
+
# LAK:FIXME This is just a paste of the GRATR code with slight modifications.
# Return a DOT::DOTDigraph for directed graphs or a DOT::DOTSubgraph for an
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index 928a1b4..79fdb04 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -4,6 +4,7 @@
require 'puppet'
require 'puppet/util/tagging'
require 'puppet/application'
+require 'sha1'
class Puppet::Transaction
require 'puppet/transaction/event'
@@ -103,7 +104,7 @@ class Puppet::Transaction
Puppet.info "Applying configuration version '#{catalog.version}'" if
catalog.version
begin
- visit_resources do |resource|
+ relationship_graph.traverse do |resource|
next if stop_processing?
if resource.is_a?(Puppet::Type::Component)
Puppet.warning "Somehow left a component in the relationship graph"
@@ -262,20 +263,6 @@ class Puppet::Transaction
prefetch
end
- def visit_resources(&block)
- eval_generated = {}
- while r = relationship_graph.topsort.find { |r|
!applied_resources.include?(r) }
- #p relationship_graph.topsort.collect { |r0| r0.title[/(-0.*)/,1] }
- if !eval_generated[r]
- #p [:generate,r.title]
- eval_generate(r)
- eval_generated[r] = true
- else
- #p [:apply,r.title]
- yield r
- end
- end
- end
# We want to monitor changes in the relationship graph of our
# catalog but this is complicated by the fact that the catalog
@@ -293,18 +280,41 @@ class Puppet::Transaction
# except via the Transaction#relationship_graph
class Relationship_graph_wrapper
+ attr_reader :real_graph,:transaction,:ready,:generated,:done
def initialize(real_graph,transaction)
@real_graph = real_graph
@transaction = transaction
+ @ready = {}
+ @generated = {}
+ @done = {}
+ vertices.each { |v| check_if_now_ready(v) }
end
def method_missing(*args,&block)
- @real_graph.send(*args,&block)
+ real_graph.send(*args,&block)
end
def add_vertex(v)
- @real_graph.add_vertex(v)
+ real_graph.add_vertex(v)
end
def add_edge(f,t)
- @real_graph.add_edge(f,t)
+ ready.delete(t)
+ real_graph.add_edge(f,t)
+ end
+ def check_if_now_ready(r)
+ ready[r] = true if direct_dependencies_of(r).all? { |r2| done[r2] }
+ end
+ def traverse(&block)
+ unguessable_deterministic_key = Hash.new { |h,k| h[k] =
Digest::SHA1.hexdigest("NaCl, MgSO4 (salts) and then #{k.title}") }
+ while r = ready.keys.sort_by { |r0| unguessable_deterministic_key[r0]
}.first
+ if !generated[r]
+ transaction.eval_generate(r)
+ generated[r] = true
+ else
+ ready.delete(r)
+ yield r
+ done[r] = true
+ direct_dependents_of(r).each { |v| check_if_now_ready(v) }
+ end
+ end
end
end
diff --git a/spec/unit/transaction_spec.rb b/spec/unit/transaction_spec.rb
index 57bf800..076b626 100755
--- a/spec/unit/transaction_spec.rb
+++ b/spec/unit/transaction_spec.rb
@@ -390,9 +390,11 @@ describe Puppet::Transaction do
describe 'within an evaluate call' do
before do
@resource = stub 'resource', :ref => 'some_ref'
+ @relationship_graph = stub 'relationship_graph'
@catalog.add_resource @resource
@transaction.stubs(:prepare)
- @transaction.stubs(:visit_resources).yields(@resource)
+ @transaction.stubs(:relationship_graph).returns @relationship_graph
+ @relationship_graph.stubs(:traverse).yields @resource
end
it 'should stop processing if :stop_processing? is true' do
--
1.7.0.4
--
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.
