-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4193/
-----------------------------------------------------------
(Updated Dec. 1, 2014, 3:53 p.m.)
Status
------
This change has been marked as submitted.
Review request for Asterisk Developers.
Changes
-------
Committed in revision 428681
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 428600
/team/mjordan/12-threadpool/res/res_xmpp.c 428600
/team/mjordan/12-threadpool/res/res_stasis_device_state.c 428600
/team/mjordan/12-threadpool/res/res_pjsip_refer.c 428600
/team/mjordan/12-threadpool/res/res_pjsip_pubsub.c 428600
/team/mjordan/12-threadpool/res/res_pjsip_mwi.c 428600
/team/mjordan/12-threadpool/res/res_jabber.c 428600
/team/mjordan/12-threadpool/res/parking/parking_bridge_features.c 428600
/team/mjordan/12-threadpool/res/parking/parking_applications.c 428600
/team/mjordan/12-threadpool/main/stasis_message_router.c 428600
/team/mjordan/12-threadpool/main/stasis_channels.c 428600
/team/mjordan/12-threadpool/main/stasis_cache.c 428600
/team/mjordan/12-threadpool/main/stasis.c 428600
/team/mjordan/12-threadpool/main/endpoints.c 428600
/team/mjordan/12-threadpool/include/asterisk/stasis_message_router.h 428600
/team/mjordan/12-threadpool/include/asterisk/stasis_internal.h 428600
/team/mjordan/12-threadpool/include/asterisk/stasis.h 428600
/team/mjordan/12-threadpool/configs/stasis.conf.sample PRE-CREATION
/team/mjordan/12-threadpool/channels/sig_pri.c 428600
/team/mjordan/12-threadpool/channels/chan_skinny.c 428600
/team/mjordan/12-threadpool/channels/chan_sip.c 428600
/team/mjordan/12-threadpool/channels/chan_mgcp.c 428600
/team/mjordan/12-threadpool/channels/chan_iax2.c 428600
/team/mjordan/12-threadpool/channels/chan_dahdi.c 428600
/team/mjordan/12-threadpool/apps/app_queue.c 428600
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.
For the new configuration options:
* Tested under valgrind; confirmed no memory leaks with/without the config file
* Tested that the lack of stasis.conf did not preclude loading and that the
defaults were applied
* Tested with an invalid config file; defaults still applied
* Tested with a valid config file; custom values applied
Thanks,
Matt Jordan
--
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --
asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
http://lists.digium.com/mailman/listinfo/asterisk-dev
