Please review pull request #39: Implement caching on YAML Backend opened by (glarizza)
Description:
Previously, the YAML backend would read from disk every time it needed
to check a hierarchy. This commit introduces memoization through an
instance variable so a file is read only if the instance variable is
empty, and only if the file has not changed. To check if the file has
changed (or if the cache is stale), we stat the file and compare the
mtime, size, and inode. Tests have been added for this new
functionality.
Fix Bugs and Commit Tests
- Opened: Wed Apr 11 18:06:51 UTC 2012
- Based on: puppetlabs:master (4b2ae058e9a60532886158af1ef8d7ce08118653)
- Requested merge: glarizza:bug/master/13600_hiera_cache (e6dea8ee21eafcd165fb5db21e04992aa6f6f9db)
Diff follows:
diff --git a/lib/hiera/backend/yaml_backend.rb b/lib/hiera/backend/yaml_backend.rb
index 147a76c..bad83c2 100644
--- a/lib/hiera/backend/yaml_backend.rb
+++ b/lib/hiera/backend/yaml_backend.rb
@@ -3,8 +3,9 @@ module Backend
class Yaml_backend
def initialize
require 'yaml'
-
Hiera.debug("Hiera YAML backend starting")
+ @data = ""
+ @cache = Hash.new
end
def lookup(key, scope, order_override, resolution_type)
@@ -14,21 +15,28 @@ def lookup(key, scope, order_override, resolution_type)
Backend.datasources(scope, order_override) do |source|
Hiera.debug("Looking for data source #{source}")
-
yamlfile = Backend.datafile(:yaml, scope, source, "yaml") || next
- data = ""
-
- next if ! data
- next if data.empty?
- next unless data.include?(key)
+ # If you call stale? BEFORE you do encounter the YAML.load_file line
+ # it will populate the @cache variable and return true. The second
+ # time you call it, it will return false because @cache has been
+ # populated. Because of this there are two conditions to check:
+ # is @data[yamlfile] populated AND is the cache stale.
+ if @data[yamlfile]
+ @data[yamlfile] = YAML.load_file(yamlfile) if stale?(yamlfile)
+ else
+ @data[yamlfile] = YAML.load_file(yamlfile)
+ end
+ next if ! @data[yamlfile]
+ next if @data[yamlfile].empty?
+ next unless @data[yamlfile].include?(key)
# for array resolution we just append to the array whatever
# we find, we then goes onto the next file and keep adding to
# the array
#
# for priority searches we break after the first found data item
- new_answer = Backend.parse_answer(data[key], scope)
+ new_answer = Backend.parse_answer(@data[yamlfile][key], scope)
case resolution_type
when :array
raise Exception, "Hiera type mismatch: expected Array and got #{new_answer.class}" unless new_answer.kind_of? Array or new_answer.kind_of? String
@@ -44,6 +52,18 @@ def lookup(key, scope, order_override, resolution_type)
return answer
end
+
+ def stale?(yamlfile)
+ # NOTE: The mtime change in a file MUST be > 1 second before being
+ # recognized as stale. File mtime changes within 1 second will
+ # not be recognized.
+ stat = File.stat(yamlfile)
+ current = { 'inode' => stat.ino, 'mtime' => stat.mtime, 'size' => stat.size }
+ return false if @cache[yamlfile] == current
+
+ @cache[yamlfile] = current
+ return true
+ end
end
end
end
diff --git a/spec/unit/backend/yaml_backend_spec.rb b/spec/unit/backend/yaml_backend_spec.rb
index cfcb644..7514ec9 100644
--- a/spec/unit/backend/yaml_backend_spec.rb
+++ b/spec/unit/backend/yaml_backend_spec.rb
@@ -1,6 +1,7 @@
require 'spec_helper'
-
+require 'tmpdir'
require 'hiera/backend/yaml_backend'
+require 'fileutils'
class Hiera
module Backend
@@ -9,6 +10,7 @@ module Backend
Hiera.stubs(:debug)
Hiera.stubs(:warn)
@backend = Yaml_backend.new
+ @backend.stubs(:stale?).returns(true)
end
describe "#initialize" do
@@ -136,5 +138,60 @@ module Backend
end
end
end
+
+ describe '#stale?' do
+ before do
+ Hiera.stubs(:debug)
+ Hiera.stubs(:warn)
+ @backend = Yaml_backend.new
+ @fakestat = Struct.new(:ino, :mtime, :size)
+ end
+
+ def create_yaml_file(data, path)
+ File.open(path, 'w') do |f|
+ f.write(data)
+ end
+ end
+
+ def update_file(data, path)
+ File.open(path, 'a') do |f|
+ f.write(data)
+ end
+ end
+
+ it 'should report a stale cache if a data lookup has not been performed' do
+ tmp_yamlfile = Pathname(Dir.mktmpdir('yaml')) + 'yamlfile'
+ create_yaml_file({'foo' => 'bar'}.to_yaml, tmp_yamlfile)
+ @backend.stale?(tmp_yamlfile).should == true
+ end
+
+ describe 'lookup tests' do
+ before(:each) do
+ @tmp_yamlfile = Pathname(Dir.mktmpdir('yaml')) + 'yamlfile'
+ create_yaml_file({'foo' => 'bar'}.to_yaml, @tmp_yamlfile)
+ Backend.expects(:datasources).yields("one")
+ Backend.expects(:datafile).with(:yaml, {}, "one", "yaml").returns(@tmp_yamlfile)
+ end
+
+ it 'should not report a stale cache after a data lookup' do
+ @backend.stale?(@tmp_yamlfile).should == true
+ @backend.lookup('foo', {}, nil, :priority).should == 'bar'
+ @backend.stale?(@tmp_yamlfile).should == false
+ end
+
+ [:ino, :mtime, :size].each do |attribute|
+ it "should report a stale cache if a backend file's #{attribute} has changed" do
+ @stat_instance = @fakestat.new(1234, 1234, 1234)
+ File.expects(:stat).with(@tmp_yamlfile).returns(@stat_instance).twice
+ @backend.stale?(@tmp_yamlfile).should == true
+ @backend.lookup('foo', {}, nil, :priority).should == 'bar'
+ @backend.stale?(@tmp_yamlfile).should == false
+ @stat_instance[attribute] += 1
+ File.unstub && File.expects(:stat).with(@tmp_yamlfile).returns(@stat_instance)
+ @backend.stale?(@tmp_yamlfile).should == true
+ end
+ end
+ end
+ end
end
end
-- 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.
