[asterisk-dev] [Code Review] 2419: stasis: Fixed message ordering issues when forwarding

[opticron]

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

Ship it!


Guaranteed ordering is definitely a good thing.

- opticron


On March 29, 2013, 11:40 p.m., David Lee wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2419/
> -----------------------------------------------------------
> 
> (Updated March 29, 2013, 11:40 p.m.)
> 
> 
> Review request for Asterisk Developers, Matt Jordan and opticron.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This patch fixes an issue of message ordering that occurs when
> multiple topics are forwarded to an aggregator topic (such as
> ast_channel_topic_all()).
> 
> It is (very reasonably) expected that the rules governing message
> dispatch order still apply, so long as the messages start from the
> same thread, and are received by the same subscription. Because the
> existing code had an additional layer of dispatching via the Stasis
> thread pool for forwards, those promises couldn't be kept.
> 
> Forwarding subscriptions no longer have their own mailbox, and now
> dispatch directly from the forwarding topic's stasis_publish()
> call. This means that the topic's lock is held for the duration of not
> only a message's dispatch, but the dispatch of all the forwards. This
> shouldn't be a problem right now, but if an aggregator topic had many
> subscribers, it could become a problem. But I figure we can write more
> clever code when the time comes, if necessary.
> 
> 
> Diffs
> -----
> 
>   /team/dlee/stasis-interleaving/tests/test_stasis.c 384385 
>   /team/dlee/stasis-interleaving/main/stasis.c 384385 
> 
> Diff: https://reviewboard.asterisk.org/r/2419/diff/
> 
> 
> Testing
> -------
> 
> Unit test that demonstrates the expected behavior. It fails without the patch; passes with it.
> 
> 
> Thanks,
> 
> David Lee
> 
>

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