Hi Daniel,

Thanks for that change.  I have just tested here (the PUA and RLS parts) against PostgreSQL and it seems to work fine.

There will be some soak and load tests of presence and RLS done here over the next couple of weeks so that should give the change a good hammering.

Thanks again,

Peter

On Tue, 2012-03-13 at 14:24 +0100, Daniel-Constantin Mierla wrote:
Hello,

On 3/13/12 12:28 AM, Peter Dunkley wrote:
> Hi,
>
> I can test the PostgreSQL "replace" function if/when it becomes available.
just pushed the code in master. I updated the existing code to compile, 
but had no way of playing with it so far. It will be great if you can 
test it and provide feedback.

All modules using DB replace were updated, just p_usrloc requires 
further review and fix to make it actually working with postgres replace 
command. rls and pua should be ready, but another eye review (and fix if 
something was done wrong) would be good.

Cheers,
Daniel
>   I did think about using locks but was worried about the performance
> implications of locking the whole DB.  I didn't think of hashing on the
> query and just locking for similar queries.
>
> The PostgreSQL rules work for me at the moment and do have the advantage
> that no locking is required, any problems are neatly and quietly handled
> within the DB.  However, a generic solution would be good as I have no
> doubt there are other tables (you already mentioned location) that could
> suffer from the same problems.
>
> Peter
>
>
>> Hello,
>>
>> looking at previous commits and the utility of a "replace" SQL command,
>> I think it can be implemented in kamailio for those modules providing
>> "affected_rows" but no native "replace".
>>
>> Pretty much like done for postgres case inside rls/pua, but done within
>> a mutex lock, to avoid any kind of race. For proper concurrency, I think
>> there should be an array of mutexes (gen_lock_set_t) , the exact mutex
>> to be used will be selected based on a hash value over the unique key
>> values of the record.
>>
>> That means the "replace" DB API method should be extended with two new
>> parameters to specify the unique key values, a db_val_t array (kvals)
>> and its number of elements (kn):
>>
>> typedef int (*db_replace_f) (const db1_con_t* handle, const db_key_t*
>> keys,
>>                   const db_val_t* vals, const int n, const db_val_t*
>> kvals, const int kn);
>>
>> In the case the unique key values are at the beginning of the vals
>> array, then vals can be reused for kvals and kn set accordingly.
>>
>> When the kvals array is null, no mutex will be used. Of course, if db
>> library has native "replace" function, that will be used and the last
>> two parameters will be ignored.
>>
>> In short, to implement replace() for postgres:
>> - DB API replace method will be extended with two parameters
>> - db_postgres will allocate at startup an array of mutex locks (AS) -
>> its size (LS) to be set by module parameter
>> - postgres replace() will compute the hash as: H =
>> get_hash1_raw(kvals[0]) + ... + get_hash1_raw(kvals[kn-1]) -- if kval[N]
>> is not string, but integer, its value can be used directly instead of
>> getting to string and then hashing it
>> - the mutex to be used will be M = AS[H % LS]
>> - the rest of replace() as pseudocode:
>>
>> lock(M)
>> do DB UPDATE
>> if affected rows==0 do DB INSERT
>> unlock(M)
>>
>> or, as the chance to have insert succeed in most of the cases can be
>> relevant in some cases, the order can be:
>>
>> lock(M)
>> do DB INSERT
>> if insert error do DB UPDATE
>> unlock(M)
>>
>> The way to be done can be a third new param to replace method -- this
>> implementation does not require "affected rows", eventually to detect
>> based on error code that it was a duplicate key failure.
>>
>> Then some other modules can use replace() mechanism easier (e.g, as
>> discussed on IRC devel meeting, usrloc can solve inconsistency when db
>> was not available for some time) -- such usage (insert or replace) can
>> be a matter of module parameters, if wanted, from case to case.
>>
>> Any other opinions?
>>
>> I am not a postgres user, but I can try to make a patch if there are
>> testers and nobody else wants to take over.
>>
>> Cheers,
>> Daniel
>>
>> --
>> Daniel-Constantin Mierla
>> Kamailio Advanced Training, April 23-26, 2012, Berlin, Germany
>> http://www.asipto.com/index.php/kamailio-advanced-training/
>>
>>
>


-- 
Peter Dunkley
Technical Director
Crocodile RCS Ltd