On 2015-16-04 4:30, Bostjan Skufca wrote:
Just recently I was looking at the environments part of puppet
implementation, and I have stumbled upon this curious gimmick.
There is Puppet::Environments class, which is a class for general
Environments management (searching, loaders, caching, etc).
Then there is Puppet::Node::Environment which is instantiated for every
environent found. Logical up to this point.
But, contrary to all OOP notions, get_conf() function is not a method of
Puppet::Node::Environment class.
Rather, it is a lookup function in Puppet::Environments class which
takes environment name as an argument and fetches
Puppet::Settings::EnvironmentConf instance.
Puppet::Environments has two function for environment and environment
data retrieval:
- get() fetches Puppet::Node::Environment instance
- get_conf() fetches Puppet::Settings::EnvironmentConf instance
The curious thing is:
- get() method operates with cache
- get_conf() does not include any caching
I tested this on a catalog of 1633 resources, and:
- get() was called around 220 times
- get_conf() was called around 25.000 times
These are the comments above get_conf() in environments.rb:
# This implementation evicts the cache, and always gets the current
# configuration of the environment
#
# TODO: While this is wasteful since it
# needs to go on a search for the conf, it is too disruptive to
optimize
# this.
My questions are:
- why does get_conf() method belong to Environments instead of
Node::Environment class?
- why are get_conf() results not cached? Why is it "too disruptive"?
The path to the new environments implementation (supporting both
directory environments, and the now deprecated (and in 4.0 removed)
dynamic environments) was paved with problems as it affected more or
less the entire code base.
It should be fine to cache the env conf in an instance of an
environment, and it should be possible to obtain the config given
an instantiated environment. This is safe since the conf will then be
evicted when the environment is evicted (and while the environment is
cached it will not reload or change).
I believe that the large number of calls
to get the conf could almost exclusively be for an already loaded
environment (I should be measured though).
All other calls would be for information purposes and those cannot be
cached unless the environment.conf file is watched (which is difficult
since it may not exist, come into existence, be removed etc.). For those
calls, the cost of checking if the file is up to date is almost as
expensive as loading the conf.
Since the 3.x implementation is very complex due to the dynamic
environments, I suggest that this should be fixed for 4.x.
- henrik
Thanks for answers,
b.
--
You received this message because you are subscribed to the Google
Groups "Puppet Developers" group.
To unsubscribe from this group and stop receiving emails from it, send
an email to puppet-dev+unsubscr...@googlegroups.com
<mailto:puppet-dev+unsubscr...@googlegroups.com>.
To view this discussion on the web visit
https://groups.google.com/d/msgid/puppet-dev/53b560d5-1c8f-4b00-944c-3156343a175f%40googlegroups.com
<https://groups.google.com/d/msgid/puppet-dev/53b560d5-1c8f-4b00-944c-3156343a175f%40googlegroups.com?utm_medium=email&utm_source=footer>.
For more options, visit https://groups.google.com/d/optout.
--
Visit my Blog "Puppet on the Edge"
http://puppet-on-the-edge.blogspot.se/
--
You received this message because you are subscribed to the Google Groups "Puppet
Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to puppet-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/puppet-dev/mgo8ac%24hc5%241%40ger.gmane.org.
For more options, visit https://groups.google.com/d/optout.
