[asterisk-dev] [Code Review] 4215: sorcery: Add additional observer capabilities.

[Kevin Harwell]

> On Dec. 4, 2014, 5:08 p.m., Kevin Harwell wrote:
> > branches/12/main/sorcery.c, lines 1268-1272
> > <https://reviewboard.asterisk.org/r/4215/diff/7/?file=69340#file69340line1268>
> >
> >     This particular observer's invocation was pushed to the taskprocessor.  Do the replacement(s)/new observers need to be as well?  What are the implications of not doing so? For instance, what if one is long running or expected to be ran in another thread. But if you schedule all on the task processor then that might be a problem for the "loading" types.  Thoughts?
> 
> George Joseph wrote:
>     The existing observers remain intact - as pushes to the taskprocessor.  The new observers don't replace any existing observer are purposefully invoked synchronously.
>     
>     The existing observers are more fine grained to include object level create, update and delete and it makes sense (maybe) for them to be run asynchronously.  I'm not sure whether there's any benefit to them being asynchronous but there's no functionality that needs them to be synchronous so there's no point in messing with them.
>     
>     The new observers are more coarse grained (no lower than object type as opposed to object) and are generally not called again once asterisk has finished loading or re-loading.  The first use case for them though is the new pjsip config wizard which does require the observers to be synchronous.  For instance, the pattern for sorcery use is to call ast_sorcery_object_register, register a bunch of fields, then call ast_sorcery_load_object.  If the registered observer is still running in the background when the main thread does the load on that same object type, things are going to get ugly because the registered observer is doing things that will affect the load.
>     
>     
>     
>     
>     
>     
>     
>

Maybe I am reading that section of the code wrong, but it looked like the new notification was replacing the old (with the taskprocessor), but this seems to be the only case and based on your explanation it makes sense. Feel free to drop this issue.


- Kevin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4215/#review13896
-----------------------------------------------------------


On Dec. 3, 2014, 11:36 a.m., George Joseph wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4215/
> -----------------------------------------------------------
> 
> (Updated Dec. 3, 2014, 11:36 a.m.)
> 
> 
> Review request for Asterisk Developers and Joshua Colp.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Interface for the global sorcery observer
> 
> /*! \brief Callback after an instance is created */
> void (*instance_created)(const char *name, struct ast_sorcery *sorcery);
> 
> /*! \brief Callback after an wizard is registered */
> void (*wizard_registered)(const char *name, const struct ast_sorcery_wizard *wizard);
> 
> /*! \brief Callback before a wizard is unregistered */
> void (*wizard_unregistered)(const char *name, const struct ast_sorcery_wizard *wizard);
> 
> /*! \brief Callback before an instance is destroyed */
> void (*instance_destroyed)(const char *name, struct ast_sorcery *sorcery);
> 
> 
> Interface for the sorcery instance observer
> 
> /*! \brief Callback before instance is loaded/reloaded */
> void (*instance_loading)(const char *name, const struct ast_sorcery *sorcery,
>    int reloaded);
> 
> /*! \brief Callback after instance is loaded/reloaded */
> void (*instance_loaded)(const char *name, const struct ast_sorcery *sorcery,
>    int reloaded);
> 
> /*! \brief Callback after a wizard is mapped to an object_type */
> void (*wizard_mapped)(const char *name, struct ast_sorcery *sorcery,
>    const char *object_type, struct ast_sorcery_wizard *wizard, const char *wizard_args,
>    void *wizard_data);
> 
> /*! \brief Callback after any obect_type is registered */
> void (*object_type_registered)(const char *name, struct ast_sorcery *sorcery,
>    const char *object_type);
> 
> /*! \brief Callback before any obect_type is loaded/reloaded */
> void (*object_type_loading)(const char *name, const struct ast_sorcery *sorcery,
>    const char *object_type, int reloaded);
> 
> /*! \brief Callback after any obect_type is loaded/reloaded */
> void (*object_type_loaded)(const char *name, const struct ast_sorcery *sorcery,
>    const char *object_type, int reloaded);
> 
> 
> Interface for the sorcery wizard observer
> 
> /*! \brief Callback before a wizard is loaded/reloaded for any type */
> void (*wizard_loading)(const char *name, const struct ast_sorcery_wizard *wizard,
>    const char *object_type, int reloaded);
> 
> /*! \brief Callback after a wizard is loaded/reloaded for any type */
> void (*wizard_loaded)(const char *name, const struct ast_sorcery_wizard *wizard,
>    const char *object_type, int reloaded);
> 
> 
> Add the following observer add/remove APIs
> 
> int ast_sorcery_global_observer_add(const struct ast_sorcery_global_observer *callbacks);
> void ast_sorcery_global_observer_remove(const struct ast_sorcery_global_observer *callbacks);
> int ast_sorcery_instance_observer_add(struct ast_sorcery *sorcery,
> 	const struct ast_sorcery_instance_observer *callbacks);
> void ast_sorcery_instance_observer_remove(struct ast_sorcery *sorcery,
> 	const struct ast_sorcery_instance_observer *callbacks);
> int ast_sorcery_wizard_observer_add(struct ast_sorcery_wizard *wizard,
> 	const struct ast_sorcery_wizard_observer *callbacks);
> void ast_sorcery_wizard_observer_remove(struct ast_sorcery_wizard *wizard,
> 	const struct ast_sorcery_wizard_observer *callbacks);
> 
> 
> Expose the following apply mapping APIs for future use...
> 
> enum ast_sorcery_apply_result __ast_sorcery_apply_wizard_mapping(struct ast_sorcery *sorcery,
> 		const char *type, const char *module, const char *name, const char *data, unsigned int caching);
> #define ast_sorcery_apply_wizard_mapping(sorcery, type, name, data, caching) \
> 	__ast_sorcery_apply_wizard_mapping((sorcery), (type), AST_MODULE, (name), (data), (caching));
> 
> 
> Diffs
> -----
> 
>   branches/12/tests/test_sorcery.c 428890 
>   branches/12/main/sorcery.c 428890 
>   branches/12/include/asterisk/test.h 428890 
>   branches/12/include/asterisk/sorcery.h 428890 
> 
> Diff: https://reviewboard.asterisk.org/r/4215/diff/
> 
> 
> Testing
> -------
> 
> All existing unit tests pass.
> Additional sorcery units tests were added for the new observers.
> Testsuite testing resulted in no new failures.
> 
> 
> Thanks,
> 
> George Joseph
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20141205/09556588/attachment.html>