[asterisk-dev] [Code Review]: Initial stasis-core message bus

[David Lee]

> On Feb. 21, 2013, 4:32 p.m., Matt Jordan wrote:
> > /trunk/main/stasis_cache.c, lines 335-352
> > <https://reviewboard.asterisk.org/r/2339/diff/1/?file=33518#file33518line335>
> >
> >     I might be incorrect in this, but there's a subtle race condition here that may confuse subscribers.
> >     
> >     Say we have the following:
> >      channel-update-1 (updates cache)
> >      some-other-event
> >      channel-update-2 (updates cache)
> >     
> >     Before channel-update-1 is published, it updates the cache, then is published. The act of publishing does not mean it is immediately serviced by the various entities; rather, it is queued up for delivery to their individual task processors. Immediately behind it, some-other-event is published, then a second channel update that also updates the cache.
> >     
> >     A subscriber receives notification of a cache update (1), then gets the some-other-event message. As part of that handling, it goes and retrieves the channel state from the cache.
> >     
> >     Unfortunately, channel-update-2 already updated the cache with its state, even though we haven't seen the event yet. And so we get future state with a prior event.
> >     
> >     This is especially dangerous for events that remove items from the cache - by the time something services the request, the item is gone from the cache and they can no longer request it.
> >     
> >     An alternative to this would be to update the cache after all subscribers have received the update; I'm not sure how feasible this is however.

Delaying the cache update would be even more confusing. Querying the cache would return old data until some slow subscriber somewhere is finished reading it? Doesn't sound pleasant.

I'd have to think about it more, but I believe that reading objects from the cache and receiving the events from the cache topic should be an either-or proposition. That would be the only way to make sure that you have a consistent view of the channel.

I'll have to give this more thought.


- David


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


On Feb. 18, 2013, 4:32 p.m., David Lee wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2339/
> -----------------------------------------------------------
> 
> (Updated Feb. 18, 2013, 4:32 p.m.)
> 
> 
> Review request for Asterisk Developers, Matt Jordan and kmoore.
> 
> 
> Summary
> -------
> 
> This patch adds a new message bus API to Asterisk.
> 
> For the initial use of this bus, I took some work kmoore did creating
> channel snapshots. So rather than create AMI events directly in the
> channel code, this patch generates Stasis events, which manager.c uses
> to then publish the AMI event.
> 
> This message bus provides a generic publish/subscribe mechanism within
> Asterisk. This message bus is:
> 
>  - Loosely coupled; new message types can be added in seperate modules.
>  - Easy to use; publishing and subscribing are straightforward
>    operations.
>  - Consistent memory management; all message bus objects are AO2
>    managed objects, using ao2_ref() and ao2_cleanup() to manage the
>    reference counting.
> 
> In addition to basic publish/subscribe, the patch also provides a
> mechanisms for message forwarding, and for message caching.
> 
> 
> This addresses bug ASTERISK-20959.
>     https://issues.asterisk.org/jira/browse/ASTERISK-20959
> 
> 
> Diffs
> -----
> 
>   /trunk/include/asterisk/channel.h 381699 
>   /trunk/include/asterisk/channel_internal.h 381699 
>   /trunk/include/asterisk/stasis.h PRE-CREATION 
>   /trunk/main/asterisk.c 381699 
>   /trunk/main/channel.c 381699 
>   /trunk/main/channel_internal_api.c 381699 
>   /trunk/main/manager.c 381699 
>   /trunk/main/pbx.c 381699 
>   /trunk/main/stasis.c PRE-CREATION 
>   /trunk/main/stasis_cache.c PRE-CREATION 
>   /trunk/main/stasis_message.c PRE-CREATION 
>   /trunk/tests/test_stasis.c PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/2339/diff
> 
> 
> Testing
> -------
> 
> Unit tests; manually verified AMI events for a few calls.
> 
> 
> Thanks,
> 
> David
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130228/33e5d563/attachment.htm>