[OpenSIPS-Devel] presence non-unique totag and more

Anca Vamanu anca at opensips.org
Wed Sep 29 10:55:39 CEST 2010


Hi Kennard,

I agree, the best solution is to generate a unique to tag in the 
presence module - it is easy to do in tm module ( the b2b_entities 
module already does this, it uses t_reply_with_body function), but in sl 
there is no support to inject the to tag. I will speak with Bogdan to 
see what solution we can find for this.

On 09/28/2010 09:47 PM, Kennard_White at logitech.com wrote:
>
> Hi Anca,
>
> There are some "big" issues that I don't think I can solve. I'll try 
> to describe:
>
> 1. Need to decide if indexing is based upon totag (if globally unique) 
> or the 5-tuple of from_uri,from_tag,to_uri,to_tag,callid. Whichever is 
> decided, should be used informally for hash & db, for all ops (insert, 
> search and delete). Current code has mixture. My own "vote" is to make 
> totag unique (within the sever), much the way PUBLISH's etag is 
> unique. Then a subscription can be identified by a single key (the totag).
>
> 2. The unique totag needs to be generated *before* sending the 
> response. E.g., all the hash&db operations need to complete 
> successfully before sending reply message. With current code where the 
> totag is generated as side-effect of sending 2XX, the client thinks it 
> has a good subscription when in fact it has failed due to DB issue.
>
> 3. Consequence of above, my suggestion is generate totag within 
> presence module, complete DB ops, then send 2XX. Need new API so that 
> the totag can be passed into tm layer, instead of tm layer generating. 
> This is bigger change than I want to do (and even if I did, don't know 
> if patch would be accepted).
>
> 4. The totag generated by tm has a very long prefix that is GUID for 
> the opensips instance, and there is comment in code about this being 
> used to detect spirals. Is this used? If not, perhaps consider 
> dropping this prefix portion and making the dialog-specific portion 
> longer (and unique).
>
> 5. I don't understand why the totag is generated the way it is. The 
> method used makes sense for the sl module: the to tag (because hash of 
> prior hop state) will be the same for replies generated for 
> retransmits. But why do this in tm module? The whole "point" of tm is 
> that it keeps state to handle retransmits. And the whole "point" of 
> the totag is to provide a convenient handle to look up dialog state. 
> Clearly, I'm missing something here.
>
> 6. I only use the "presence_xml" module. The other presence modules 
> (e.g., pua) use the low-level APIs of the presence module. I'm just 
> not familiar with how these other modules work, and cannot assess 
> impact of changes to core presence module.
>
There is no other module to generate a to tag - only presence module 
does this.
>
>
> 7. The startup order is messy. Currently the db-restore is done from 
> mod_init(). This has two problems: (a) the other modules may or may 
> not have been initialized, which leads to non-determinism in how the 
> get_auth_status() is handled, and (b) the statistics variables are not 
> ready. Suggest moving db-restore into the "first" child_init, assuming 
> stats are ready then. But really need a thread-join for all the 
> child-inits prior to handling traffic. E.g., need to block all PUB/SUB 
> traffic until restore is complete. (Ideally generating 5XX with 
> retry-after header). Don't really know how to implement this 
> join/blocking.
>
Indeed, get_auth_status might not be available in mod_init if the 
modules that add events are not loaded before. We can move the reload 
from db in the first child as you suggested.

I will let you know when I finish the  unique to tag implementation.

Thanks and regards,

-- 
Anca Vamanu
www.voice-system.ro


> As a consequence of all the above, I don't think I can ever give you a 
> final patch, or even any much better than what you have now. I just 
> didn't want you to think my patch was good or ready for production. 
> Need to resolve above big issues. If you decide to go with a global 
> totag, then some of my changes will need to be backed out. I'll 
> happily deal with any version control conflicts if it gets us closer 
> to final solution.
>
> Kennard
>
> Inactive hide details for "SourceForge.net" ---09/28/2010 02:30:29 
> AM---Patches item #3076805, was opened at 2010-09-28 00:00 
> M"SourceForge.net" ---09/28/2010 02:30:29 AM---Patches item #3076805, 
> was opened at 2010-09-28 00:00 Message generated for change (Comment 
> added) m
>
> From: "SourceForge.net" <noreply at sourceforge.net>
> To: noreply at sourceforge.net
> Date: 09/28/2010 02:30 AM
> Subject: [OpenSIPS-Devel] [ opensips-Patches-3076805 ] presence 
> non-unique totag and more
> Sent by: devel-bounces at lists.opensips.org
>
> ------------------------------------------------------------------------
>
>
>
> Patches item #3076805, was opened at 2010-09-28 00:00
> Message generated for change (Comment added) made by anca_vamanu
> You can respond by visiting:
> https://sourceforge.net/tracker/?func=detail&atid=1086412&aid=3076805&group_id=232389 
> <https://sourceforge.net/tracker/?func=detail&atid=1086412&aid=3076805&group_id=232389>
>
> Please note that this message will contain a full copy of the comment 
> thread,
> including the initial issue submission, for this request,
> not just the latest update.
> Category: modules
> Group: trunk
> Status: Open
> Resolution: None
> Priority: 5
> Private: No
> Submitted By: Kennard White (kennardwhite)
> >Assigned to: Anca Vamanu (anca_vamanu)
> Summary: presence non-unique totag and more
>
> Initial Comment:
> Attached is a bundle of changes to presence module. We deployed 
> opensips1.6 and saw a number of different failures under production 
> load. This patch isn't finished quality: it is just snapshot of where 
> I stand now. I thought it worth sharing the issues I've seen so far. A 
> clean solutions really require larger change outside the scope of just 
> the presence module. I'll send email to the list about these ideas.
>
> This includes:
> * Partial bug fix for non-unique totags. The hash-table and DB code 
> for deleting subscriptions assumes that the totag is unique. But the 
> totag has only 16 bits of possible uniqueness per server (it is CRC of 
> branch info). IN practice, easy to get totag collisions after few 
> thousand subscribers. Patch includes fix to the hash-table side but 
> the DB side is still broken. For now, code just does nothing to DB 
> when deleteting a subscription, and just lets it expire out.
> * Bug fix for expiring subscriptions from DB prior to updating. 
> Subscriptions that we renewd (via reSUB) are deleted from DB prior to 
> writing the new (unexpired) time into the DB. Change to do DB delete 
> after sync.
> * Bug fix for "if ( foo & NO_UPDATE_DB) when NO_UPDATE_DB is zero 
> value. Note that at least one other module has same bug and is not 
> patched here.
> * Addition of statistics for presence module. The isn't cleanest of 
> implementations because the DB restore function runs before statistics 
> are available. Would be much cleaner if DB restore could run after 
> stats are available.
> * Addition of new 'activeSubs' mi command to help trace down these 
> problems.
> * More info in many of the log messages to help trace down these errors.
> * Addition of 'override_481_code' as work-around for bad resiprocate 
> clients. We use this option to force a 5xx code instead of standard 
> 481 since some versions of resip get stuck with 481.
> * Change to NOTIFY code to node register a response callback function 
> if subscription is being terminated. No point is looking for 481 
> response if sub is terminated.
> * Fix for expires_offset as discussed previously to prevent 
> subscriptions walking to zero length.
>
> ----------------------------------------------------------------------
>
> >Comment By: Anca Vamanu (anca_vamanu)
> Date: 2010-09-28 12:30
>
> Message:
> Hi Kennard,
>
> Thank you for this complex patch - some issues are really bugs that we
> were not aware of and the additions seem useful. I will review this patch
> and send comments if I have any. I will not apply it until you say 
> it's the
> final version.
>
> Regards,
> Anca
>
> ----------------------------------------------------------------------
>
> You can respond by visiting:
> https://sourceforge.net/tracker/?func=detail&atid=1086412&aid=3076805&group_id=232389 
> <https://sourceforge.net/tracker/?func=detail&atid=1086412&aid=3076805&group_id=232389>
>
> _______________________________________________
> Devel mailing list
> Devel at lists.opensips.org
> http://lists.opensips.org/cgi-bin/mailman/listinfo/devel
>
>
> _______________________________________________
> Devel mailing list
> Devel at lists.opensips.org
> http://lists.opensips.org/cgi-bin/mailman/listinfo/devel
>    
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.opensips.org/pipermail/devel/attachments/20100929/6037be83/attachment-0001.htm 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: image/gif
Size: 105 bytes
Desc: not available
Url : http://lists.opensips.org/pipermail/devel/attachments/20100929/6037be83/attachment-0001.gif 


More information about the Devel mailing list