Skip to content

Commit

Permalink
Use cached get() results to limit calls to get()
Browse files Browse the repository at this point in the history
My understanding... there may be cases I haven't considered.

In my testing, with simple_get_filter not implemented, removing the
names.nil? checks stopped get(context) being called again for every resource
which had pending changes. All resources of the given type were
collected from the system with one get() call and cached. Subsequent
calls to rsapi_provider_get returned the cached information.

When simple_get_filter is not implemented:
1. The cached data is returned, if the cache has been marked as complete and names is nil.
2. Therefore, if names is NOT nil or if the cache hasn't been marked complete, information about all resources of the given type is fetched by calling the provider's get() function.
3. The fetched information for all relevant resources is added to the cache.
4. The cache is marked complete if names is nil and simple_get_feature is not implemented.
5. Subsequent calls to rsapi_provider_get (e.g. to retrieve the current state of a resource which has pending changes) pass in a value for 'names', and therefore names.nil? is false, and the cache in point 1 above isn't returned or used. get() is therefore called again per resource to retrieve all resource information again.

Removing the names.nil? checks on lines 255 and 268 allows the cache to be populated with information about all of the resources of the given type with one get() call, mark the cache as complete, and therefore allow the cache to be used in subsequent calls to rsapi_provider_get for each resource.

Simple_get_filter behaviour wouldn't change, as when simple_get_filter is implemented the cache would never be marked as complete or returned: my_provider.get(context, names) would still be called every time.
  • Loading branch information
Amanda Harth committed Apr 12, 2024
1 parent e4a8fc8 commit 59772ca
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions lib/puppet/resource_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ def to_resource_shim(resource)
def self.rsapi_provider_get(names = nil)
# If the cache has been marked as having all instances, then just return the
# full contents:
return rsapi_provider_get_cache.all if rsapi_provider_get_cache.cached_all? && names.nil?
return rsapi_provider_get_cache.all if rsapi_provider_get_cache.cached_all?

fetched = if type_definition.feature?('simple_get_filter')
my_provider.get(context, names)
Expand All @@ -265,7 +265,7 @@ def self.rsapi_provider_get(names = nil)
rsapi_provider_get_cache.add(build_title(type_definition, resource_hash), resource_hash)
end

if names.nil? && !type_definition.feature?('simple_get_filter')
if !type_definition.feature?('simple_get_filter')
# Mark the cache as having all possible instances:
rsapi_provider_get_cache.cached_all
end
Expand Down

0 comments on commit 59772ca

Please sign in to comment.