[sr-dev] git:master: pv: use msg context id when caching the value for $TV

[Daniel-Constantin Mierla]

Hello,

On 4/16/12 7:45 PM, Klaus Darilion wrote:
> Hi Daniel!
>
> Am 15.04.2012 23:02, schrieb Daniel-Constantin Mierla:
>> Hello,
>>
>> On 4/15/12 10:02 PM, Klaus Darilion wrote:
>>> Hi Daniel!
>>>
>>> I reviewed the patch and it still does not work reliable. The new 
>>> approach uses msg-id and pid to find out if $TV(s) still works on 
>>> the same message or if a new message is processed and time stamp 
>>> needs to be fetched.
>>>
>>> Consider the following scenario:
>>>
>>> worker process with pid 100 receives msg with msg-id 7:
>>> --> msg context id = 100,7
>>> --> $TV(s) is calculated on first usage
>>>
>>> forwarding times out, timer with pid 99 processes message with 
>>> msg-id 7 in failure route:
>>> --> msg context id = 99,7
>>> --> $TV(s) is calculated on first usage
>>>
>>> second forwarding times out, timer with pid 99 processes message 
>>> with msg-id 7 in failure route:
>>> --> msg context id = 99,7
>>> --> $TV(s) is not calculated context ids are the same
>>>
>>>
>>> Thus, $TV(s) is different in request route and failure route, but 
>>> identically in both failure routes, if the failure route does not 
>>> process another message. Thus, current implementation behaves 
>>> different, depending on if other requests are forwarded/failed too.
>>>
>>> So, we need a different implementation. Before implementing, we 
>>> should define the expected behavior. There are 2 possible (useful) 
>>> implementations.
>>> a) $TV(s) in failure route has always the same value as in request 
>>> route
>>> b) $TV(s) in failure route has always the value when the requests 
>>> entered the respective failure route
>>>
>>> I would vote for implementation 2.
>>>
>>
>> these are cached values per (message,process) to skip execution of 
>> system calls many times, but also to have more or less same value for 
>> several actions in a raw. So such values are valid as long as a pid 
>> is processing same message one time or many times without handling 
>> another one in between.
>
> Yes, that's the problem. A PV should always return the same value, 
> regardless if there was a another message processed inbetween or not.
>
> IMO, it is a bug. If not, we need a new PVs (see below).
>
> Maybe we should store a timestamp in sip_msg and set it automatically 
> when the request was received or processing recovered in failure route.
>
>> There are other PVs giving the current time attributes always, 
>> without any caching -- one can store the result in other type of 
>> variable then and reuse that.
>
> $TV(sn) and $TV(un) are useless, as they need to be queried one after 
> the other, this getting different points in time.
>
> $TV(Sn) needs string processing afterwards to split it into seconds 
> and useconds, otherwise time calculations are not possible.
>
> I would be more usefull, e.g. if TV(un) reports the cached time, set 
> when querying for TV(sn).
>
>>
>> IMO, the caching system should be coherent across all PVs relying on 
>> it, for other kind of needs there should be different variables.
>>
> User should also rely that PVs return the same result for the same 
> scenario, regardless if other messages are received or not.
>
at some point, iirc, there was a discussion to add the time in the 
sip_msg_t structure, perhaps someone was against it to avoid a system 
call every time a message is received and some memory. I don't have 
anything against.

An alternative will be to replace time_t field in xavp structure with 
timeval and have the time value attached to sip message via xavp- so 
caching will be done in xavp

Cheers,
Daniel

-- 
Daniel-Constantin Mierla
Kamailio Advanced Training, April 23-26, 2012, Berlin, Germany
http://www.asipto.com/index.php/kamailio-advanced-training/