On Dec 10, 2009 at 18:23, Daniel-Constantin Mierla miconda@gmail.com wrote:
On 12/10/09 5:02 PM, Andrei Pelinescu-Onciul wrote:
On Dec 10, 2009 at 12:34, Daniel-Constantin Mierlamiconda@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:
- 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()).
- 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*).
- 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