search

puppetlabs / puppet-resource_api

This library provides a simple way to write new native resources for https://puppet.com.
Apache License 2.0
27 stars 41 forks source link

Implement caching of provider.get #220

Open DavidS opened 4 years ago

DavidS commented 4 years ago

When puppet processes a catalog for resources that are not marked for purging, the instances method is not called, and instead puppet calls retrieve for every resource individually:

Warning: panos_virtual_router: Fetching panos resource: Puppet::Provider::PanosVirtualRouter::PanosVirtualRouter
/home/david/git/puppet-resource_api/lib/puppet/resource_api.rb:363:in `block (2 levels) in register_type'
/home/david/git/puppet-resource_api/lib/puppet/resource_api.rb:382:in `block (2 levels) in register_type'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/type.rb:1098:in `retrieve_resource'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/transaction/resource_harness.rb:302:in `from_resource'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/transaction/resource_harness.rb:20:in `evaluate'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/transaction.rb:257:in `apply'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/transaction.rb:277:in `eval_resource'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/transaction.rb:181:in `call'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/transaction.rb:181:in `block (2 levels) in evaluate'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/util.rb:519:in `block in thinmark'
/usr/lib/ruby/2.5.0/benchmark.rb:308:in `realtime'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/util.rb:518:in `thinmark'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/transaction.rb:181:in `block in evaluate'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/graph/relationship_graph.rb:121:in `traverse'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/transaction.rb:171:in `evaluate'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/resource/catalog.rb:239:in `block (2 levels) in apply'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/util.rb:519:in `block in thinmark'
/usr/lib/ruby/2.5.0/benchmark.rb:308:in `realtime'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/util.rb:518:in `thinmark'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/resource/catalog.rb:238:in `block in apply'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/util/log.rb:156:in `with_destination'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/transaction/report.rb:146:in `as_logging_destination'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/resource/catalog.rb:237:in `apply'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/configurer.rb:186:in `block (2 levels) in apply_catalog'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/util.rb:519:in `block in thinmark'
/usr/lib/ruby/2.5.0/benchmark.rb:308:in `realtime'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/util.rb:518:in `thinmark'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/configurer.rb:185:in `block in apply_catalog'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/util.rb:232:in `block in benchmark'
/usr/lib/ruby/2.5.0/benchmark.rb:308:in `realtime'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/util.rb:231:in `benchmark'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/configurer.rb:184:in `apply_catalog'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/configurer.rb:369:in `run_internal'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/configurer.rb:237:in `block in run'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/context.rb:65:in `override'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet.rb:260:in `override'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/configurer.rb:211:in `run'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/application/apply.rb:355:in `apply_catalog'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/application/apply.rb:280:in `block (2 levels) in main'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/context.rb:65:in `override'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet.rb:260:in `override'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/application/apply.rb:280:in `block in main'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/context.rb:65:in `override'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet.rb:260:in `override'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/application/apply.rb:233:in `main'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/application/apply.rb:174:in `run_command'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/application/device.rb:299:in `block (3 levels) in main'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/context.rb:65:in `override'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet.rb:260:in `override'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/application/device.rb:298:in `block (2 levels) in main'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/application/device.rb:245:in `each'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/application/device.rb:245:in `collect'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/application/device.rb:245:in `block in main'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/context.rb:65:in `override'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet.rb:260:in `override'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/application/device.rb:230:in `main'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/application.rb:383:in `run_command'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/application.rb:375:in `block in run'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/util.rb:667:in `exit_on_fail'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/application.rb:375:in `run'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/util/command_line.rb:136:in `run'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/lib/puppet/util/command_line.rb:73:in `execute'
/home/david/gems/ruby/2.5.0/gems/puppet-5.5.8/bin/puppet:5:in `<top (required)>'
/home/david/gems/ruby/2.5.0/bin/puppet:23:in `load'
/home/david/gems/ruby/2.5.0/bin/puppet:23:in `<main>'

this causes excessive calls to the underlying provider and API, and slows down operations significantly.

The result from the get calls in resource_api.rb#L342-L344 need to be cached and only be called if there is a cache miss. In the case of non-simple_get_filter, cache misses are authoritative after the first get call.

Special care must be taken that the cache has the right scope (provider), as cursory testing seemed to indicate that regular class-level variables (@@rsapi_cache) are shared across all types. See https://github.com/puppetlabs/puppet-resource_api/blob/6f5fc1ab065ca0ef2ee80cf1c50eab93e92f78fb/lib/puppet/resource_api.rb#L59 for a possible solution approach.

Additional Context

Originally filed as https://tickets.puppetlabs.com/browse/PDK-1234

seanmil commented 4 years ago

In order to allow the cache to be accessed from the provider itself (for example, how SimpleProvider does it) I think that the cache has to also be exposed at least enough so that the provider can retrieve a resource state via the cache.

As part of this, I propose exposing a get() style method which can access the cache. Given the RSAPI design it feels like it maybe should (needs?) to live in context. Thoughts?

fraenki commented 1 year ago

I've just experienced a performance issue and noticed the lack of caching... can we please get this soonish? :)