Feature #7285
Use Augeas NO_LOAD/incl to optimise loading
| Status: | Closed | Start date: | 04/29/2011 | |
|---|---|---|---|---|
| Priority: | Normal | Due date: | ||
| Assignee: |  Jacob Helwig | % Done: | 0% | |
| Category: | augeas | |||
| Target version: | 3.0.0 | |||
| Affected Puppet version: | Branch: | https://github.com/domcleal/puppet/tree/tickets/master/7285c | ||
| Keywords: | augeas performance | |||
Description
Augeas will load all lenses and then read and parse all files listed in the default “incl” lists for all lenses when opened. A large speed increase could be gained by only loading the file(s) implied by the “context” property given in the augeas type.
The NO_LOAD flag can be given to Augeas which will populate the /augeas/load//incl nodes, but it won’t call aug_load to read the files. The incl nodes that aren’t relevant can be deleted, then aug_load called to parse only the necessary files.
This differs from the optimisation available when the lens and incl properties are given, as this wouldn’t require the lens to be known or specified, nor would it require specifying both the context and incl properties (with near-identical values).
Related issues
History
#1
Updated by Dominic Cleal about 2 years ago
- Branch set to https://github.com/domcleal/puppet/tree/tickets/master/7285
This is a bit of an experiment, but it produces at least a 50% speedup when running a catalog with five Augeas resources for me (YMMV!).
The implementation’s a little inelegant, so I’m open to other ideas. The code also isn’t being tested directly as it seems to be very difficult to stub the open_augeas method, so I’ve only unit tested the glob method.
Branch set above and sent to -dev, comments welcome.
#2
Updated by Nigel Kersten about 2 years ago
- Status changed from Unreviewed to Accepted
#3
Updated by Dominic Cleal about 2 years ago
- Branch changed from https://github.com/domcleal/puppet/tree/tickets/master/7285 to https://github.com/domcleal/puppet/tree/tickets/master/7285-augeas-198
Thanks to Patrick (kc7zzv), the glob part has been incorporated upstream into a new glob() function to be included with Augeas 0.8.2.
I’ve reimplemented the patch to the provider, which uses this feature if available and makes it more robust. The only unfortunate thing is that we can’t optimise if the context given is less specific than a file (e.g. context => /etc/sysconfig), but I don’t think this is a great loss.
#4
Updated by Jacob Helwig over 1 year ago
- Status changed from Accepted to Tests Insufficient
Dominic,
In going to review your branch for this I noticed that there weren’t any tests. Before I dig in to add some (since I know next to nothing about Augeas), is there any chance I could convince you to add some for this change?
#5
Updated by Dominic Cleal over 1 year ago
Jacob Helwig wrote:
In going to review your branch for this I noticed that there weren’t any tests. Before I dig in to add some (since I know next to nothing about Augeas), is there any chance I could convince you to add some for this change?
I can have a look. This method relies on the Augeas library itself, so hasn’t ever had a unit test written for it – the only tests that exist now are ones for methods that can be run without Augeas (e.g. parsing methods).
It should be possible to lock down the spec to only run when the Augeas feature is present, I guess?
#6
Updated by Jacob Helwig over 1 year ago
Limiting the spec to run only when Augeas is available sounds reasonable for this thing, and should be pretty easy to do. Something like the following (or s/describe/it/, depending on how the tests are setup):
describe 'foo', :if => Puppet.features.augeas? do
...
end
#7
Updated by Dominic Cleal over 1 year ago
- Status changed from Tests Insufficient to In Topic Branch Pending Review
- Assignee set to Jacob Helwig
- Branch changed from https://github.com/domcleal/puppet/tree/tickets/master/7285-augeas-198 to https://github.com/domcleal/puppet/tree/tickets/master/7285c
Thanks for the code sample, that helped. I’ve added unit tests both before and after the optimisation now, see pull request #239.
#8
Updated by Jacob Helwig over 1 year ago
Left a comment inline in the commit that causes this, but the test suite won’t work as-is unless Augeas is installed.
#9
Updated by Dominic Cleal over 1 year ago
Jacob Helwig wrote:
Left a comment inline in the commit that causes this, but the test suite won’t work as-is unless Augeas is installed.
Cheers, I’ve changed the latest commit on the branch with the fix.
#10
Updated by Jacob Helwig over 1 year ago
- Status changed from In Topic Branch Pending Review to Merged - Pending Release
- Target version set to 3.x
Merged into master in commit:2d52b1941c648443bd4d82473c374ca4e80e247e
(#7285) Use Augeas NO_LOAD/incl to optimise loading based on context
When the context property is given, use the NO_LOAD flag to stop Augeas loading
all files for all lenses. Removes all /augeas/load//incl nodes where the glob
doesn't match the context given before loading.
This speeds up loading when the context is provided, as Augeas only reads and
parses relevant files, instead of every known file for each resource.
Requires Augeas 0.8.2 with the glob() path function call.
and
(#7285) Add spec for Augeas initialisation and file loading
Simple initialisaton tests with combinations of lens include paths, explicit
file/lens loading against a few example files.
#11
Updated by Daniel Pittman about 1 year ago
- Target version changed from 3.x to 3.0.0
#12
Updated by Matthaus Owens 10 months ago
- Status changed from Merged - Pending Release to Closed
Released in Puppet 3.0.0-rc4