[Home]

Summary:ASTERISK-03434: [patch] IAX and SIP Realtime Improvement
Reporter:Anthony Minessale (anthm)Labels:
Date Opened:2005-02-04 16:32:27.000-0600Date Closed:2008-01-15 15:24:59.000-0600
Priority:MajorRegression?No
Status:Closed/CompleteComponents:Channels/chan_iax2
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) sip_iax_real_rev0.diff
( 1) sip_iax_real_rev1.diff
( 2) sip_iax_real_rev2.diff
( 3) sip_iax_real_rev3.diff
Description:This patch adds new params to iax.conf and sip.conf here is an excerpt from the doc.

;rtcachefriends=yes             ; Cache realtime friends by adding them to the internal list
                               ; just like friends added from the config file only on a
                               ; as-needed basis.
;rtnoupdate=yes                 ; do not send the update request over realtime.
;rtautoclear=yes                ; Auto-Expire friends created on the fly on the same schedule
                               ; as if it had just registered when the registration expires
                               ; the friend will vanish from the configuration until requested
                               ; again.


It allows you to tweak the realtime lookup so once it finds a peer it will be added to the live config this has many benifits in certian situations and may not always be desired which is why it is off by default.

Benifits:

With rtcachefriends one can say "iax2 show peer fred" and it will do a realtime lookup right then and there and add it to your config allowing you to perform a "show" on any realtime peer.  

If you have the rtautoclear set then the on the fly peer will disappear by itself when the timer is up.  The lifespan of the peer will be extended if a registration is performed.

If you set rtnoupdate then the realtime peer will not send the update call back to your config engine.  This is for cases where you are using the on the fly technique above and have no use for that information.  I left this optional in case you still wanted to recieve the update req as a sort of log.






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

Disclaimer On File
anthmct@yahoo.com
Comments:By: () 2005-02-04 17:01:22.000-0600

This is a long awaited feature! Thank You!
By: raiden (raiden) 2005-02-04 17:13:49.000-0600

Tested and worked flawless so far. (still need more testing) This patch is *VERY* much needed and I hope it gets included into CVS ASAP. Excellent work Anthm!
By: Mark Spencer (markster) 2005-02-06 20:40:35.000-0600

Fantastic work.  Could we get a CLI option that would permit us to force expiration of one / all such peers?  That way if someone updates the database, they can issue a command to force asterisk to dump that particular user.
By: Brian West (bkw918) 2005-02-06 20:57:33.000-0600

reload would do that :P

bkw
By: Anthony Minessale (anthm) 2005-02-07 08:46:30.000-0600

reload "module" will already clear *all* the realtimes.

so on a per peer basis perhaps something like
"[sip|iax2] prune peer <peername>" ?

or is clearing then all via "reload chan_[sip|iax2].so" enough?
By: Mark Spencer (markster) 2005-02-07 09:01:35.000-0600

prune peer would be good, otherwise this makes realtime essentially work the same as non-realtime, that is requiring a reload for changes to take effect.
By: Anthony Minessale (anthm) 2005-02-07 09:46:58.000-0600

right, except don't forget you can set them to expire after however long the registration timeout happens to be too if you set rtautoclear=true
but I'll make prune peer possibler in a few min since that would be useful too if you wanted to clear them over manager or something.
By: Anthony Minessale (anthm) 2005-02-07 13:46:14.000-0600

rev1

sip  prune realtime [<peer>|all]
iax2 prune realtime [<peer>|all]


sip show peer <peername> [load]
iax2 show peer <peername> [load]

the optional keyword "load" is now required to automagicly load any realtime peers.
By: Kevin P. Fleming (kpfleming) 2005-02-07 16:22:02.000-0600

Some thoughts:

- is there a particular reason the new SIP_ flags were not added using the existing flag variable (that still has 4 bits left)? (actually, this is a prime example of where using bitfields instead of flag macros would be a tremendous improvement)

- somehow we need to document for people (maybe just in the response message, maybe somewhere else) that "prune peer" will not actually make the peer disappear if that peer is involved in a call

- 'iax2 prune realtime all' does a reload_config, which seems pretty heavy-handed for something that was just supposed to clear out cache entries

- the docs for 'iax2 prune realtime' say that it takes a username, but it only handles peers (and the completion function will only return peer names)

- i don't see any usage of the global IAX_RTAUTOCLEAR flag at all; it gets set, but never used

- the build_user call in chan_sip that checks SIP_PAGE2_RTCACHEFRIENDS always sends '1' to build_user; there is no need for a ?: operator here at all, just invert the result of ast_test_flag and pass it to build_user (same goes for the other places that ast_test_flag's result is used with the ?: operator)

- same comments for 'sip prune realtime' as 'iax2 prune realtime' above
By: Anthony Minessale (anthm) 2005-02-07 17:17:07.000-0600

Going back and doing the same thing 2 similar ways in 2 similar files then being asked to go add the same thing again to the 2 similar files can lead to these silly screw-ups thanks for spotting some.  I had done some of that whilst debugging and forgot to rectify, good eye!

That's why I keep saying that this whole user/peer thing be yanked into a new channel-independant module so you only have to do the work in 1 place. users are nearly obsolete anyway and peers are so close to the same in iax and sip it's not funny I think there should be a user account config and you can specify some sort of per channel type options and prefix the options with some
keyword save these values in the obj the same way chan vars are with a varshead pointer or somehting I'm not gonna brainstorm too hard cos nobody will listen anyway..

; users.conf
[fred]
permit=sip,iax,h323,zap
password=1234
iax_context=context_a
sip_context=context_b









I didnt want to be the one to use up the last bits in the flags so I started a new one it looks like all the rest are reserved for 3 bit flags.


prune all essentially *is* asking for a reload so that was what I opted for the alternative is dont handle "all" at all and make em reload but it's all mutex so..
By: Kevin P. Fleming (kpfleming) 2005-02-07 17:33:59.000-0600

My concern with using "reload" for "prune all" is because someone may have made configuration changes that they are not yet ready to "implement" (they don't want to reload yet) but they decide to do a "prune all" for some entirely different reason. Worse yet, it could be a different person who does the "prune all" without realizing it will reload the entire config.

I agree (and oej has talked to me about this before) that there is starting to be little need for users/peers to be separate entities in IAX2 and SIP. I never used them in my conf files anyway (pretty much everyone is a "friend"), and it's getting harder and harder to deal with the small differences and keep the common behavior the same, plus it causes a lot of code duplication.

There is nothing "reserved" that I am aware of in the flags structure, so they are safe to use.

If it was up to me (which it's not <G>), I'd like to get all the interested parties together on a conference call to talk about this user/peer issue _before_ this bug or the work in ASTERISK-3323355 get merged. I think it would be worth the time to figure out how to simplify all this code before we go adding more complexity back into the mix.

Mark, Olle, you interested?
By: adomjan (adomjan) 2005-02-08 08:05:16.000-0600

Thanks this patch! The regexten works now with realtime!
By: Anthony Minessale (anthm) 2005-02-09 15:45:22.000-0600

rev3 lets rtautoclear be either true/false or an int
if the int is positive it sets the expire time (in seconds)
if its true/false it accepts the default expire of 120

(not tested) please check the code.
By: Mark Spencer (markster) 2005-02-10 01:52:52.000-0600

Functionally I'm happy with this patch, but I'd like to see it tested and also get some of kpflemings comments with respect to the flags and the clearing of all the realtimes addressed.
By: Anthony Minessale (anthm) 2005-02-10 04:44:55.000-0600

What do you want then?
Do you want me to go back and use up the last flags instead? you are still gonna have to address the issue in 2 days when you need more.
So opening up a new int is not acceptable, is it wasting too much memory?

I will yank the "all" if you want cos clear all and reload are
the same thing to me and I don't know why so much attention is being focused on this command to flush I don't really even think it's necessary if you want to flush it, reload, before this patch you had to look them all up every time.
and I do not anticipate people sitting at thier consoles hand clearing cached entries 1 by one.

Please just tell me exactly what I should do to put this patch to bed cos I'm starting to regret writing it.  How did chan_sip get so knarley to begin with if we're being so picky about the code??? Frankly this patch just fixes useability issues that plague people everyday and is only a temporary fix until the entire module can be rewritten from scratch along with dozens of other modules that are way out of hand from feature-ization.

I'm sincerly willing to see it through I just need it to happen on a faster timeline over fewer revisions, I was already back in this code today an you could have told me all the issues you want addressed instead of 1 at a time.  This kind of thing makes me reluctant to submit patches and I don't want that to be the case.  I feel like the cellphone guy "can you merge it now???" "good" "good" Don't mistake this complaint for not wanting feedback, I just want more condensed feedback.  kpflemmings comments were there since day 2 and I saw no more references to them in your correspondance until now.

Mark?? Please find more time for your _voulenteer_ developers they all agree you don't pay enough attention to them that is why I am sponsoring these weekly meetings on Thursdays, I don't want to see asterisk fall into the hands of the dark side.
By: Kevin P. Fleming (kpfleming) 2005-02-10 09:29:46.000-0600

Which weekly meetings on Thursdays would these be? I certainly have never heard about them.

In my opinion, go ahead and use up the remaining flags you need. Once that is merged, I will post a patch to convert chan_sip over to using bitfields for flags, so that we can stop having to worry about this stuff and simplify the code as well.

I am concerned about the feature being "rtcachefriends", and it does cache both users and peers, but there is no way to forcibly prune a user, only a peer. These types of inconsistencies are what cause increased support traffic and future work for people who get frustrated by them (not accusing you of anything here Tony, you are already well aware of the quality and code consistency issues of which I speak <G>).

Here's a real minor nit:

+ if (!ast_test_flag((&globalflags), IAX_RTNOUPDATE) && (ast_test_flag(p, IAX_TEMPONLY) || ast_test_flag(p, IAX_RTCACHEFRIENDS)))

These last two flags can be combined here, the result will be identical.

Otherwise I see nothing else glaring that I would suggest changing, it looks OK to me.
By: Anthony Minessale (anthm) 2005-02-10 10:04:44.000-0600

kp, find me on IRC I'll tell you about it.
By: Mark Spencer (markster) 2005-02-10 14:05:12.000-0600

Added to CVS, thanks.
By: Digium Subversion (svnbot) 2008-01-15 15:24:59.000-0600

Repository: asterisk
Revision: 5003

U   trunk/channels/chan_iax2.c
U   trunk/channels/chan_sip.c
U   trunk/configs/iax.conf.sample
U   trunk/configs/sip.conf.sample

------------------------------------------------------------------------
r5003 | markster | 2008-01-15 15:24:59 -0600 (Tue, 15 Jan 2008) | 2 lines

Merge tony's IAX/SIP realtime cache (bug ASTERISK-3434)

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

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