[sr-dev] AVPs in replies?

[Andrei Pelinescu-Onciul]

On Dec 10, 2009 at 18:23, Daniel-Constantin Mierla <miconda at gmail.com> wrote:
> 
> 
> On 12/10/09 5:02 PM, Andrei Pelinescu-Onciul wrote:
> >On Dec 10, 2009 at 12:34, Daniel-Constantin Mierla<miconda at gmail.com>  
> >wrote:
> >   
> >>
> >>On 12/10/09 12:32 PM, Daniel-Constantin Mierla wrote:
> >>     
> >>>Hello,
> >>>
> >>>Andrei, instead of having K compat mode, as global parameter, I would
> >>>prefer to have per transaction function:
> >>>
> >>>t_lock_onreply()
> >>>       
> >>or have t_on_reply(...) accepting a second parameter that indicates the
> >>locking status for onreply_route execution.
> >>     
> >I see 3 possibilities of fixing it:
> >
> >1. make parallel avp operation safe using no locks
> >
> >This would require changes to the way avps are deleted: instead
> >of immediately unlinking them from the avp list and freeing their memory
> >they would have to be only marked as deleted. The memory
> >reclaiming/real delete will happen only when the transaction is
> >destroyed, or for normal route avps, at the end of the route.
> >
> >Just to clarify things $a=something involves 2 avp operations:
> >avp_destroy($a) ; add_new_avp($a, something). So each assignment
> >involves and avp delete/destroy.
> >An onreply_route containing something like $avp="foo" will eat memory
> >for each reply. However this shouldn't really be a problem unless
> >someone uses really huge avps.
> >
> >Another disadvantage is that add_avp() will become slightly slower. OTOH
> >hand this should be offset by the faster destroy_avp.
> >
> >This change will make the avp safe to use from routes executed in
> >parallel, however there is no guarantee that they will have the expected
> >value (it would be like changing unprotected variables from parallel
> >threads).
> >E.g.: $a=$a+1 in an onreply route will be undefined if the route is
> >executed in parallel (e.g. after 2 parallel replies $a could be 1 or 2).
> >To offset that we could introduce what Daniel proposed
> >(t_lock_onreply(), t_unlock_onreply()).
> >
> >
> >2. start with an empty avp list and load the avps and lock the
> >onreply_route only if certain commands are used (e.g.
> >t_onreply_load_avps(), t_onreply_unload_avps()) or a global parameter is
> >set (like k but with the addition of the t_onreply*).
> >
> >3. based on a config param, either allow only read-only access to avps
> >  (and no locking) or read-write access (and locking).
> >   
> I think 3. is much easier to implement comparing with 1. and less cfg 
> complex than 2. I would go for it for 3.0 releases, then look for 
> something better for future.

Actually after re-thinking (and speaking with Miklos), I don't think
that always executing the onreply_route under lock would affect
performance. We do take the reply lock anyway and so the only advantage
of executing the reply route out of the lock would be the possibility to
run it in parallel for the same transaction, or in parallel with the
failure route. OTOH the probability of having 2 replies for the same
transaction processed in parallel is very low.


Andrei