[asterisk-dev] [Code Review] 3099: CDRs and Stasis take 2: Synchronize publication of application CDR messages to the CDR engine subscription

[opticron]

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



/branches/12/main/stasis.c
<https://reviewboard.asterisk.org/r/3099/#comment19970>

    When synchronous dispatch is required, why not just dispatch directly as if there were no mailbox?



/branches/12/main/stasis.c
<https://reviewboard.asterisk.org/r/3099/#comment19969>

    This should not have the stasis_ prefix if it isn't available outside this translation unit.


- opticron


On Dec. 31, 2013, 1:24 p.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3099/
> -----------------------------------------------------------
> 
> (Updated Dec. 31, 2013, 1:24 p.m.)
> 
> 
> Review request for Asterisk Developers and David Lee.
> 
> 
> Bugs: ASTERISK-22884
>     https://issues.asterisk.org/jira/browse/ASTERISK-22884
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> In https://reviewboard.asterisk.org/r/3057/, applications and functions that manipulate CDRs were made to interact over Stasis. This was done to synchronize manipulations of CDRs from the dialplan with the updates the engine itself receives over the message bus.
> 
> This change rested on a faulty premise: that messages published to the CDR topic or to a topic that forwards to the CDR topic are synchronized with the messages handled by the CDR topic subscription in the CDR engine. This is not the case. There is no ordering guaranteed for two messages published to the same topic; ordering is only guaranteed if a message is published to the same subscriber.
> 
> Note that the patch actually still fixed a bunch of test failures by virtue of publishing the messages in the first place to the channel topic; however, the hangup handler tests - which use the CDR function to read a value from the engine - did still fail after this patch. Reading data requires an explicit synchronization with the engine.
> 
> In order to fix this problem, this patch does the following:
> (1) It provides a new way to publish a message. It is now possible to publish a message and specify which stasis_subscription you want to synchronize on. A message published in such a fashion is still published to all subscribers of the topic (which is needed; you don't know who all wants your message), but the call will block until the specified subscriber handles the message.
> (2) It updates the CDR engine to exposed its message router (which contains the subscription). CDR related applications now synchronize on the message router when publishing information. As a result, the CDR topic/router now persist across CDR engine disablings; the only thing that is removed when the engine is disabled are the forwarding subscriptions. This keeps a lot of quirky lifetime issues from becoming serious problems, while still maintaining performance if the CDR engine is disabled.
> 
> 
> Diffs
> -----
> 
>   /branches/12/tests/test_stasis.c 404591 
>   /branches/12/main/stasis_message_router.c 404591 
>   /branches/12/main/stasis.c 404591 
>   /branches/12/main/cdr.c 404591 
>   /branches/12/include/asterisk/stasis_message_router.h 404591 
>   /branches/12/include/asterisk/stasis.h 404591 
>   /branches/12/include/asterisk/cdr.h 404591 
>   /branches/12/funcs/func_cdr.c 404591 
>   /branches/12/apps/app_forkcdr.c 404591 
>   /branches/12/apps/app_cdr.c 404591 
> 
> Diff: https://reviewboard.asterisk.org/r/3099/diff/
> 
> 
> Testing
> -------
> 
> Unit test for publishing synchronously was added. It passes.
> 
> All stasis and CDR unit tests pass.
> 
> All CDR and hangup handler test in the test suite now pass. Previously, the hangup handler tests would fail due to getting wrong data back from the CDR engine when reading a value using the CDR function.
> 
> 
> Thanks,
> 
> Matt Jordan
> 
>

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