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
Hi,
I can test the PostgreSQL "replace" function if/when it becomes available. 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/
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/
On 03/13/2012 03:24 PM, 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.
Hello,
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.
I will have a look how p_usrloc is affected by the new API changes, but I will do this the next week(in time for 3.3).
Cheers Marius
Cheers, Daniel
On 03/13/2012 03:31 PM, Marius Zbihlei wrote:
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. I will have a look how p_usrloc is affected by the new API changes, but I will do this the next week(in time for 3.3).
Hello all,
There were several changes that we fixed regarding postgresql support in p_usrloc. Looks like things are ok now (via commit
http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=5269ac4f...
)
Cheers, Marius
Cheers Marius
Cheers, Daniel
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/
-
Daniel-Constantin Mierla -
Marius Zbihlei -
Peter Dunkley