[Home]

Summary:ASTERISK-16629: [patch] AST_MAX_EXTENSION limitation on hint string length
Reporter:mdu113 (mdu113)Labels:
Date Opened:2010-08-30 09:42:03Date Closed:2010-11-30 03:49:31.000-0600
Priority:BlockerRegression?No
Status:Closed/CompleteComponents:Channels/chan_sip/Subscriptions
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) 1.6.2.14rc1-backto-2.13.txt
( 1) 1.6.2.14rc1-origversion.txt
( 2) 1.6.2.14rc1-origversion-loss.txt
( 3) 20100831__issue17928.diff.txt
( 4) extensions.conf
( 5) hints.inc.conf
( 6) sip.conf
Description:This is basically a reopening of issue 13945 as suggested by tilghman, inspired by commit in r263637 that was closing issue 17257.
The problem with long hint strings has always existed in asterisk 1.4, but since r263637 says "Remove arbitrary size limitation for hints", I thought I'd bring it back to developers attention.
Here's the problem as it looks now:
Devices that fit into AST_MAX_EXTENSION in hint string work perfectly with immediate reaction to state changes.
Devices that don't fit into AST_MAX_EXTENSION seems to also work. Kind of. There's a long (something like 30-60 seconds) delay between device state changes (I place a call on monitored device) and asterisk actually notices it.
Here's an example.
I create a hint that monitors, let's say, 10 devices with hint string length definitely > 80 characters. If I call device listed first in the hint string (it fully fits into AST_MAX_EXTENSION characters) then asterisk notices it right away and hint state changes immediately.
Contrary, if I call device listed last in the hint strings (doesn't fit into AST_MAX_EXTENSION characters) then there will be a long delay before asterisk will notice it.
It looks like devices that appeared too far in the hints string aren't being watched by the standard mechanism, which is probably events-based, but there's some background process that rechecks all the state of all devices periodically (and not very often) and that one catches it.
It would be very nice if that can be fixed (or at least delay to be decreased) as it's very common for us to monitor multiple devices on a single hint.

****** ADDITIONAL INFORMATION ******

Please let me know if you need any additional info.
Comments:By: Tilghman Lesher (tilghman) 2010-08-31 14:11:16

That was a crash issue.  As a feature, this is only eligible for addition into trunk.
By: mdu113 (mdu113) 2010-08-31 14:34:26

Excellent! It seems to work just perfectly.
It's been a really long-awaited fix.
Thanks a lot
By: mdu113 (mdu113) 2010-09-01 13:47:03

BTW, I'm not sure why you qualify it as a feature. In my opinion it's just long standing oversight. The previous behavior (AST_MAX_EXTENSION limit) is counter-intuitive and I haven't seen it documented
By: Tilghman Lesher (tilghman) 2010-09-01 23:56:06

There are lots of features that qualify as long standing oversights.  They don't qualify for entry into RELEASED versions, because we have a standard policy of no new features in release branches.  The only exceptions for this are where we need to add a feature in order to fix a crash or fix a security hole.  There are other exceptions, too, but we tailor them as narrowly as possible to avoid the potential for causing other issues.

Therefore, this is only eligible for entry into unreleased branches.  You are more than welcome, having the source, to apply the patch to any release you like -- it just will not be in the official release.
By: mdu113 (mdu113) 2010-09-02 09:39:09

Understood. Ok, I'll patch it myself.
Thanks again for fixing it
By: Digium Subversion (svnbot) 2010-09-16 15:04:46

Repository: asterisk
Revision: 287118

U   branches/1.4/main/pbx.c

------------------------------------------------------------------------
r287118 | mnicholson | 2010-09-16 15:04:46 -0500 (Thu, 16 Sep 2010) | 8 lines

Don't limit hint processing in ast_hint_state_changed() to AST_MAX_EXTENSION length strings.

(closes issue ASTERISK-16629)
Reported by: mdu113
Patches:
     20100831__issue17928.diff.txt uploaded by tilghman (license 14)
Tested by: mdu113

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=287118
By: Digium Subversion (svnbot) 2010-09-16 15:06:16

Repository: asterisk
Revision: 287119

_U  branches/1.6.2/
U   branches/1.6.2/main/pbx.c

------------------------------------------------------------------------
r287119 | mnicholson | 2010-09-16 15:06:16 -0500 (Thu, 16 Sep 2010) | 15 lines

Merged revisions 287118 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
 r287118 | mnicholson | 2010-09-16 15:04:46 -0500 (Thu, 16 Sep 2010) | 8 lines
 
 Don't limit hint processing in ast_hint_state_changed() to AST_MAX_EXTENSION length strings.
 
 (closes issue ASTERISK-16629)
 Reported by: mdu113
 Patches:
       20100831__issue17928.diff.txt uploaded by tilghman (license 14)
 Tested by: mdu113
........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=287119
By: Digium Subversion (svnbot) 2010-09-16 15:07:38

Repository: asterisk
Revision: 287120

_U  branches/1.8/
U   branches/1.8/main/pbx.c

------------------------------------------------------------------------
r287120 | mnicholson | 2010-09-16 15:07:38 -0500 (Thu, 16 Sep 2010) | 22 lines

Merged revisions 287119 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.6.2

................
 r287119 | mnicholson | 2010-09-16 15:06:16 -0500 (Thu, 16 Sep 2010) | 15 lines
 
 Merged revisions 287118 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r287118 | mnicholson | 2010-09-16 15:04:46 -0500 (Thu, 16 Sep 2010) | 8 lines
   
   Don't limit hint processing in ast_hint_state_changed() to AST_MAX_EXTENSION length strings.
   
   (closes issue ASTERISK-16629)
   Reported by: mdu113
   Patches:
         20100831__issue17928.diff.txt uploaded by tilghman (license 14)
   Tested by: mdu113
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=287120
By: Digium Subversion (svnbot) 2010-09-16 15:08:52

Repository: asterisk
Revision: 287121

_U  trunk/
U   trunk/main/pbx.c

------------------------------------------------------------------------
r287121 | mnicholson | 2010-09-16 15:08:51 -0500 (Thu, 16 Sep 2010) | 29 lines

Merged revisions 287120 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.8

................
 r287120 | mnicholson | 2010-09-16 15:07:38 -0500 (Thu, 16 Sep 2010) | 22 lines
 
 Merged revisions 287119 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.6.2
 
 ................
   r287119 | mnicholson | 2010-09-16 15:06:16 -0500 (Thu, 16 Sep 2010) | 15 lines
   
   Merged revisions 287118 via svnmerge from
   https://origsvn.digium.com/svn/asterisk/branches/1.4
   
   ........
     r287118 | mnicholson | 2010-09-16 15:04:46 -0500 (Thu, 16 Sep 2010) | 8 lines
     
     Don't limit hint processing in ast_hint_state_changed() to AST_MAX_EXTENSION length strings.
     
     (closes issue ASTERISK-16629)
     Reported by: mdu113
     Patches:
           20100831__issue17928.diff.txt uploaded by tilghman (license 14)
     Tested by: mdu113
   ........
 ................
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=287121
By: Digium Subversion (svnbot) 2010-09-17 08:34:37

Repository: asterisk
Revision: 287307

U   branches/1.4/main/pbx.c

------------------------------------------------------------------------
r287307 | mnicholson | 2010-09-17 08:34:36 -0500 (Fri, 17 Sep 2010) | 5 lines

Use ast_strdup() instead of ast_strdupa() while processing in ast_hint_state_changed().

(related to issue ASTERISK-16629)
Reported by: mdu113

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=287307
By: Digium Subversion (svnbot) 2010-09-17 08:36:08

Repository: asterisk
Revision: 287308

_U  branches/1.6.2/
U   branches/1.6.2/main/pbx.c

------------------------------------------------------------------------
r287308 | mnicholson | 2010-09-17 08:36:07 -0500 (Fri, 17 Sep 2010) | 12 lines

Merged revisions 287307 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
 r287307 | mnicholson | 2010-09-17 08:34:34 -0500 (Fri, 17 Sep 2010) | 5 lines
 
 Use ast_strdup() instead of ast_strdupa() while processing in ast_hint_state_changed().
 
 (related to issue ASTERISK-16629)
 Reported by: mdu113
........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=287308
By: Digium Subversion (svnbot) 2010-09-17 08:37:10

Repository: asterisk
Revision: 287309

_U  branches/1.8/
U   branches/1.8/main/pbx.c

------------------------------------------------------------------------
r287309 | mnicholson | 2010-09-17 08:37:10 -0500 (Fri, 17 Sep 2010) | 19 lines

Merged revisions 287308 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.6.2

................
 r287308 | mnicholson | 2010-09-17 08:36:07 -0500 (Fri, 17 Sep 2010) | 12 lines
 
 Merged revisions 287307 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r287307 | mnicholson | 2010-09-17 08:34:34 -0500 (Fri, 17 Sep 2010) | 5 lines
   
   Use ast_strdup() instead of ast_strdupa() while processing in ast_hint_state_changed().
   
   (related to issue ASTERISK-16629)
   Reported by: mdu113
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=287309
By: Digium Subversion (svnbot) 2010-09-17 08:38:23

Repository: asterisk
Revision: 287310

_U  trunk/
U   trunk/main/pbx.c

------------------------------------------------------------------------
r287310 | mnicholson | 2010-09-17 08:38:22 -0500 (Fri, 17 Sep 2010) | 26 lines

Merged revisions 287309 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.8

................
 r287309 | mnicholson | 2010-09-17 08:37:10 -0500 (Fri, 17 Sep 2010) | 19 lines
 
 Merged revisions 287308 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.6.2
 
 ................
   r287308 | mnicholson | 2010-09-17 08:36:07 -0500 (Fri, 17 Sep 2010) | 12 lines
   
   Merged revisions 287307 via svnmerge from
   https://origsvn.digium.com/svn/asterisk/branches/1.4
   
   ........
     r287307 | mnicholson | 2010-09-17 08:34:34 -0500 (Fri, 17 Sep 2010) | 5 lines
     
     Use ast_strdup() instead of ast_strdupa() while processing in ast_hint_state_changed().
     
     (related to issue ASTERISK-16629)
     Reported by: mdu113
   ........
 ................
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=287310
By: Digium Subversion (svnbot) 2010-09-20 10:48:15

Repository: asterisk
Revision: 287555

U   branches/1.4/main/pbx.c

------------------------------------------------------------------------
r287555 | mnicholson | 2010-09-20 10:48:15 -0500 (Mon, 20 Sep 2010) | 5 lines

Use ast_dynamic_str when processing hint state changes

(related to issue ASTERISK-16629)
Reported by: mdu113

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=287555
By: Digium Subversion (svnbot) 2010-09-20 10:51:47

Repository: asterisk
Revision: 287556

_U  branches/1.6.2/

------------------------------------------------------------------------
r287556 | mnicholson | 2010-09-20 10:51:47 -0500 (Mon, 20 Sep 2010) | 14 lines

Use ast_str when processing hint state changes

Merged revisions 287555 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
 r287555 | mnicholson | 2010-09-20 10:48:14 -0500 (Mon, 20 Sep 2010) | 5 lines
 
 Use ast_dynamic_str when processing hint state changes
 
 (related to issue ASTERISK-16629)
 Reported by: mdu113
........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=287556
By: Digium Subversion (svnbot) 2010-09-20 10:56:22

Repository: asterisk
Revision: 287558

_U  branches/1.6.2/
U   branches/1.6.2/main/pbx.c

------------------------------------------------------------------------
r287558 | mnicholson | 2010-09-20 10:56:21 -0500 (Mon, 20 Sep 2010) | 14 lines

Use ast_str when processing hint state changes

Merged revisions 287555 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
 r287555 | mnicholson | 2010-09-20 10:48:14 -0500 (Mon, 20 Sep 2010) | 5 lines
 
 Use ast_dynamic_str when processing hint state changes
 
 (related to issue ASTERISK-16629)
 Reported by: mdu113
........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=287558
By: Digium Subversion (svnbot) 2010-09-20 10:57:14

Repository: asterisk
Revision: 287559

_U  branches/1.8/
U   branches/1.8/main/pbx.c

------------------------------------------------------------------------
r287559 | mnicholson | 2010-09-20 10:57:14 -0500 (Mon, 20 Sep 2010) | 21 lines

Merged revisions 287558 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.6.2

................
 r287558 | mnicholson | 2010-09-20 10:56:21 -0500 (Mon, 20 Sep 2010) | 14 lines
 
 Use ast_str when processing hint state changes
 
 Merged revisions 287555 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r287555 | mnicholson | 2010-09-20 10:48:14 -0500 (Mon, 20 Sep 2010) | 5 lines
   
   Use ast_dynamic_str when processing hint state changes
   
   (related to issue ASTERISK-16629)
   Reported by: mdu113
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=287559
By: Digium Subversion (svnbot) 2010-09-20 10:57:53

Repository: asterisk
Revision: 287560

_U  trunk/
U   trunk/main/pbx.c

------------------------------------------------------------------------
r287560 | mnicholson | 2010-09-20 10:57:53 -0500 (Mon, 20 Sep 2010) | 28 lines

Merged revisions 287559 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.8

................
 r287559 | mnicholson | 2010-09-20 10:57:14 -0500 (Mon, 20 Sep 2010) | 21 lines
 
 Merged revisions 287558 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.6.2
 
 ................
   r287558 | mnicholson | 2010-09-20 10:56:21 -0500 (Mon, 20 Sep 2010) | 14 lines
   
   Use ast_str when processing hint state changes
   
   Merged revisions 287555 via svnmerge from
   https://origsvn.digium.com/svn/asterisk/branches/1.4
   
   ........
     r287555 | mnicholson | 2010-09-20 10:48:14 -0500 (Mon, 20 Sep 2010) | 5 lines
     
     Use ast_dynamic_str when processing hint state changes
     
     (related to issue ASTERISK-16629)
     Reported by: mdu113
   ........
 ................
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=287560
By: Leif Madsen (lmadsen) 2010-10-19 07:50:06

Reopening per schmidts which states this commit causes the numbers of calls and registrations he can handle to be significantly diminished.
By: Stefan Schmidt (schmidts) 2010-10-19 08:10:30

i have noticed some problems with these changes after doing some load tests.
please have a look at the attached sipp logs.
1.6.2.14rc1-origversion shows the max amount of calls per seconds 1.6.2.14 can handle
1.6.2.14rc1-backto-2.13 shows the max amount of cps 1.6.2.13 can handle
1.6.2.14rc1-origversion-loss shows what happens if i try the same value which works with 1.6.2.13 to 2.14

this means we have a loos of around 40% in call handling out of this change.

on suggest of russell bryant i have tried replacing the ast_str_create with ast_str_thread_get but that doesnt improve it much.
By: Stefan Schmidt (schmidts) 2010-10-19 08:36:01

gosh i forget the hints ;)
By: Stefan Schmidt (schmidts) 2010-10-21 15:57:02

leif this change is allready in 1.8.0 look at http://www.asterisk.org/doxygen/asterisk1.8/pbx_8c.html#3abd8b8b4dd20de2436440a2b1993dc0
so this would be for 1.8.1-rc1 ;)
By: Digium Subversion (svnbot) 2010-11-30 03:49:27.000-0600

Repository: asterisk
Revision: 296752

U   trunk/include/asterisk.h
U   trunk/main/asterisk.c
U   trunk/main/pbx.c

------------------------------------------------------------------------
r296752 | schmidts | 2010-11-30 03:49:26 -0600 (Tue, 30 Nov 2010) | 17 lines

move devices from hints into an ao2_container

by splitting up devices from hints into an own ao2_container the callback to
get these devices for statechange handling is faster.
with this changes the length of a device used in a hint isnt longer restricted
to 80 characters.

Tests showed that calling handle_statechange is 40 times faster if no hints
are used and 25 times faster if there are any hints.

(closes issue ASTERISK-16629)
Reported by: mdu113
Tested by: schmidts

Review: https://reviewboard.asterisk.org/r/1003/


------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=296752