[Asterisk-code-review] res sorcery memory cache: Add support for a full backend cache. (asterisk[13])

[Joshua Colp]

Joshua Colp has posted comments on this change.

Change subject: res_sorcery_memory_cache: Add support for a full backend cache.
......................................................................


Patch Set 4:

(2 comments)

https://gerrit.asterisk.org/#/c/1808/4/res/res_sorcery_memory_cache.c
File res/res_sorcery_memory_cache.c:

Line 936: 		remove_all_from_cache(task_data->cache);
> Should this be locked as well when removing?
The lock is still held during this time, it's unlocked below.


Line 1027: 	struct ao2_container *backend_objects;
         : 
         : 	start_passthru_update();
         : 	backend_objects = ast_sorcery_retrieve_by_fields(sorcery, type, AST_RETRIEVE_FLAG_MULTIPLE | AST_RETRIEVE_FLAG_ALL, NULL);
         : 	end_passthru_update();
         : 
         : 	if (!backend_objects) {
         : 		/* This will occur in off-nominal memory allocation failure scenarios */
         : 		return;
         : 	}
         : 
         : 	if (cache->maximum_objects && ao2_container_count(backend_objects) >= cache->maximum_objects) {
         : 		ast_log(LOG_ERROR, "The backend contains %d objects while the sorcery memory cache '%s' is explicitly configured to only allow %d\n",
         : 			ao2_container_count(backend_objects), cache->name, cache->maximum_objects);
         : 		return;
         : 	}
         : 
         : 	ao2_callback_data(backend_objects, OBJ_NOLOCK | OBJ_NODATA | OBJ_MULTIPLE, object_add_to_cache_callback,
         : 		(struct ast_sorcery*)sorcery, cache);
         : 
         : 	/* If the number of cached objects does not match the number of backend objects we encountered a memory allocation
         : 	 * failure and the cache is incomplete, so drop everything and fall back to querying the backend directly
         : 	 * as it may be able to provide what is wanted.
         : 	 */
         : 	if (ao2_container_count(cache->objects) != ao2_container_count(backend_objects)) {
         : 		ast_log(LOG_WARNING, "The backend contains %d objects while only %d could be added to sorcery memory cache '%s'\n",
         : 			ao2_container_count(backend_objects), ao2_container_count(cache->objects), cache->name);
         : 		remove_all_from_cache(cache);
         : 		abort();
         : 	}
         : 
         : 	ao2_ref(backend_objects, -1);
> Some duped/similar code to the stale_cache_update function. Think it would 
I wrestled with it myself and ended up coming to the conclusion that it's *just* different enough that trying to make some common stuff would just make it harder to follow.


-- 
To view, visit https://gerrit.asterisk.org/1808
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie2993487e9c19de563413ad5561c7403b48caab5
Gerrit-PatchSet: 4
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-HasComments: Yes