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

Daniel-Constantin Mierla miconda at gmail.com
Sun Apr 15 23:02:11 CEST 2012


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.

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.

IMO, the caching system should be coherent across all PVs relying on it, 
for other kind of needs there should be different variables.

Cheers,
Daniel
>
>
>
> On 23.03.2012 12:54, Daniel-Constantin Mierla wrote:
>> Module: sip-router
>> Branch: master
>> Commit: e4972ebd926b1fea6af795f58785a2b766439e5d
>> URL:    
>> http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=e4972ebd926b1fea6af795f58785a2b766439e5d
>>
>> Author: Daniel-Constantin Mierla<miconda at gmail.com>
>> Committer: Daniel-Constantin Mierla<miconda at gmail.com>
>> Date:   Fri Mar 23 12:53:31 2012 +0100
>>
>> pv: use msg context id when caching the value for $TV
>>
>> - reported by Klaus Darilion
>>
>> ---
>>
>>   modules_k/pv/pv_time.c |   30 ++++++++++++++----------------
>>   1 files changed, 14 insertions(+), 16 deletions(-)
>>
>> diff --git a/modules_k/pv/pv_time.c b/modules_k/pv/pv_time.c
>> index ad7dd2c..9cbc375 100644
>> --- a/modules_k/pv/pv_time.c
>> +++ b/modules_k/pv/pv_time.c
>> @@ -35,17 +35,15 @@
>>
>>   #include "pv_time.h"
>>
>> -static unsigned int _pv_msg_id = 0;
>> -static int _pv_msg_pid = 0;
>> +static msg_ctx_id_t _pv_time_msgid = { 0 };
>>   static time_t _pv_msg_tm = 0;
>>
>> -static int pv_update_time(struct sip_msg *msg, time_t *t)
>> +static int pv_update_time(sip_msg_t *msg, time_t *t)
>>   {
>> -    if(_pv_msg_id != msg->id || _pv_msg_pid != msg->pid || 
>> _pv_msg_tm==0)
>> +    if(msg_ctx_id_match(msg,&_pv_time_msgid)!=1 || _pv_msg_tm==0)
>>       {
>>           _pv_msg_tm = time(NULL);
>> -        _pv_msg_id = msg->id;
>> -        _pv_msg_pid = msg->pid;
>> +        msg_ctx_id_set(msg,&_pv_time_msgid);
>>
>>           if(t!=NULL)
>>               *t=_pv_msg_tm;
>> @@ -124,7 +122,7 @@ int pv_parse_strftime_name(pv_spec_p sp, str *in)
>>   }
>>
>>   static struct tm _cfgutils_ts;
>> -static unsigned int _cfgutils_msg_id = 0;
>> +static msg_ctx_id_t _cfgutils_msgid = { 0 };
>>
>>   /**
>>    * return broken-down time attributes
>> @@ -137,10 +135,10 @@ int pv_get_time(struct sip_msg *msg, pv_param_t 
>> *param,
>>       if(msg==NULL || param==NULL)
>>           return -1;
>>
>> -    if(_cfgutils_msg_id != msg->id)
>> +    if(msg_ctx_id_match(msg,&_cfgutils_msgid)!=1)
>>       {
>>           pv_update_time(msg,&t);
>> -        _cfgutils_msg_id = msg->id;
>> +        msg_ctx_id_set(msg,&_cfgutils_msgid);
>>           if(localtime_r(&t,&_cfgutils_ts) == NULL)
>>           {
>>               LM_ERR("unable to break time to attributes\n");
>> @@ -189,10 +187,10 @@ int pv_get_strftime(struct sip_msg *msg, 
>> pv_param_t *param,
>>       if(msg==NULL || param==NULL)
>>           return -1;
>>
>> -    if(_cfgutils_msg_id != msg->id)
>> +    if(msg_ctx_id_match(msg,&_cfgutils_msgid)!=1)
>>       {
>>           pv_update_time(msg,&t);
>> -        _cfgutils_msg_id = msg->id;
>> +        msg_ctx_id_set(msg,&_cfgutils_msgid);
>>           if(localtime_r(&t,&_cfgutils_ts) == NULL)
>>           {
>>               LM_ERR("unable to break time to attributes\n");
>> @@ -267,7 +265,7 @@ int pv_get_timeb(struct sip_msg *msg, pv_param_t 
>> *param,
>>   }
>>
>>   static struct timeval _timeval_ts;
>> -static unsigned int _timeval_msg_id = 0;
>> +static msg_ctx_id_t _timeval_msgid = { 0 };
>>   static char _timeval_ts_buf[32];
>>
>>   int pv_get_timeval(struct sip_msg *msg, pv_param_t *param,
>> @@ -282,14 +280,14 @@ int pv_get_timeval(struct sip_msg *msg, 
>> pv_param_t *param,
>>       switch(param->pvn.u.isname.name.n)
>>       {
>>           case 1:
>> -            if(_timeval_msg_id != msg->id)
>> +            if(msg_ctx_id_match(msg,&_timeval_msgid)!=1)
>>               {
>>                   if(gettimeofday(&_timeval_ts, NULL)!=0)
>>                   {
>>                       LM_ERR("unable to get time val attributes\n");
>>                       return -1;
>>                   }
>> -                _timeval_msg_id = msg->id;
>> +                msg_ctx_id_set(msg,&_timeval_msgid);
>>               }
>>               return pv_get_uintval(msg, param, res, (unsigned 
>> int)_timeval_ts.tv_usec);
>>           case 2:
>> @@ -319,14 +317,14 @@ int pv_get_timeval(struct sip_msg *msg, 
>> pv_param_t *param,
>>               s.s = _timeval_ts_buf;
>>               return pv_get_strval(msg, param, res,&s);
>>           default:
>> -            if(_timeval_msg_id != msg->id)
>> +            if(msg_ctx_id_match(msg,&_timeval_msgid)!=1)
>>               {
>>                   if(gettimeofday(&_timeval_ts, NULL)!=0)
>>                   {
>>                       LM_ERR("unable to get time val attributes\n");
>>                       return -1;
>>                   }
>> -                _timeval_msg_id = msg->id;
>> +                msg_ctx_id_set(msg,&_timeval_msgid);
>>               }
>>               return pv_get_uintval(msg, param, res, (unsigned 
>> int)_timeval_ts.tv_sec);
>>       }
>>
>>
>> _______________________________________________
>> sr-dev mailing list
>> sr-dev at lists.sip-router.org
>> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
>
> _______________________________________________
> sr-dev mailing list
> sr-dev at lists.sip-router.org
> http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev

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




More information about the sr-dev mailing list