[asterisk-dev] [Code Review] 4193: Stasis: allow for subscriptions to dictate message delivery on a threadpool for certain situations

[Matt Jordan]

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

Review request for Asterisk Developers.


Bugs: ASTERISK-24533
    https://issues.asterisk.org/jira/browse/ASTERISK-24533


Repository: Asterisk


Description
-------

Note: don't be fooled by the size of this review. Most of the changes are small and are in stasis.c/stasis_message_router.c.

Rob (and CDR on the asterisk-users list, although he never followed up when we asked for more information) discovered that we were creating two additional threads per SIP peer in chan_sip. This actually would occur for a large variety of 'endpoints' in Asterisk, regardless of the channel driver. The two threads were stasis subscriptions, specifically for MWI and for endpoints detecting the destruction of related channels.

Stasis subscriptions currently get a dedicated thread. In contrast, prior to r400178 (see review https://reviewboard.asterisk.org/r/2881/), the subscriptions shared a thread pool. It was discovered that for a low subscription count with high message throughput, the threadpool was not as performant as simply having a dedicated thread per subscriber.

Since then, our subscription pattern has changed slightly. While we still have plenty of subscriptions that would follow the model just described (AMI, CDRs, CEL, etc.) - there are plenty that also fall into the following two categories:
* Large number of subscriptions, specifically those tied to endpoints/peers.
* Low number of messages. Some subscriptions exist specifically to coordinate a single message - the subscription is created, a message is published, the delivery is synchronized, and the subscription is destroyed.
In both of the latter two cases, creating a dedicated thread is wasteful (and in the case of a large number of peers/endpoints, harmful).

This patch adds the ability of a subscriber to Stasis to choose whether or not their messages are dispatched on a dedicated thread or on a threadpool. The threadpool is currently hard coded in this patch; if configuration is necessary, we could re-add the configuration options in r2881. Pre-mature optimization and all that led me to simply go with a simpler model for now.

Note that the approach taken here was to add additional API calls rather than modify existing ones. If we'd rather the thread dispatch model be dictated by a parameter, the new API calls can be removed in trunk and the old API calls modified appropriately.


Diffs
-----

  /team/mjordan/12-threadpool/tests/test_stasis.c 428167 
  /team/mjordan/12-threadpool/res/res_xmpp.c 428167 
  /team/mjordan/12-threadpool/res/res_stasis_device_state.c 428167 
  /team/mjordan/12-threadpool/res/res_pjsip_refer.c 428167 
  /team/mjordan/12-threadpool/res/res_pjsip_pubsub.c 428167 
  /team/mjordan/12-threadpool/res/res_pjsip_mwi.c 428167 
  /team/mjordan/12-threadpool/res/res_jabber.c 428167 
  /team/mjordan/12-threadpool/res/parking/parking_bridge_features.c 428167 
  /team/mjordan/12-threadpool/res/parking/parking_applications.c 428167 
  /team/mjordan/12-threadpool/main/stasis_message_router.c 428167 
  /team/mjordan/12-threadpool/main/stasis_channels.c 428167 
  /team/mjordan/12-threadpool/main/stasis_cache.c 428167 
  /team/mjordan/12-threadpool/main/stasis.c 428167 
  /team/mjordan/12-threadpool/main/endpoints.c 428167 
  /team/mjordan/12-threadpool/include/asterisk/stasis_message_router.h 428167 
  /team/mjordan/12-threadpool/include/asterisk/stasis_internal.h 428167 
  /team/mjordan/12-threadpool/include/asterisk/stasis.h 428167 
  /team/mjordan/12-threadpool/channels/sig_pri.c 428167 
  /team/mjordan/12-threadpool/channels/chan_skinny.c 428167 
  /team/mjordan/12-threadpool/channels/chan_sip.c 428167 
  /team/mjordan/12-threadpool/channels/chan_mgcp.c 428167 
  /team/mjordan/12-threadpool/channels/chan_iax2.c 428167 
  /team/mjordan/12-threadpool/channels/chan_dahdi.c 428167 

Diff: https://reviewboard.asterisk.org/r/4193/diff/


Testing
-------

New unit tests were written to cover the new subscription types. This tests receiving messages sent using the threadpool.

The MWI tests and channel driver tests in the Test Suite were run and passed.


Thanks,

Matt Jordan

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