sr-dev August 2012

sr-dev@lists.kamailio.org

30 participants
170 discussions

[ openser-Feature Requests-1891148 ] protection of open fifo/ socket files against removing
by SourceForge.net 26 Aug '12

26 Aug '12

Feature Requests item #1891148, was opened at 2008-02-11 05:58 Message generated for change (Comment added) made by nobody You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=743023&aid=1891148&group_… Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: modules Group: ver devel Status: Open Resolution: None Priority: 5 Private: No Submitted By: Helmut Kuper (hekuper) Assigned to: Nobody/Anonymous (nobody) Summary: protection of open fifo/ socket files against removing Initial Comment: Hi, I found that starting a 2nd openser with same config as an already running openser on same machine using module mi_datagram leads to a deletion of the socket file of the already running openser process. This is very bad when u use openser as a frontend for sems for example. :( If this is a bug, maybe the check, whether there is an already running openser using same network resources, should be placed *befor* starting modules. Alternative modules should be able to handle those problematic scenarios ... My mi_datagram config is this: loadmodule "mi_datagram.so" modparam("mi_datagram", "socket_name", "/tmp/openser_ansagen_sock") modparam("mi_datagram", "children_count", 1) modparam("mi_datagram", "unix_socket_mode", 0660) modparam("mi_datagram", "unix_socket_group", "voip") modparam("mi_datagram", "unix_socket_user", "ruth") modparam("mi_datagram", "socket_timeout", 2000) regards Helmut ---------------------------------------------------------------------- Comment By: Nobody/Anonymous (nobody) Date: 2012-08-26 01:24 Message: Hey there this is kind of of off topic but I was wondering if blogs use WYSIWYG editors or if you have to manually code with HTML. I'm starting a blog soon but have no coding expertise so I wanted to get advice from someone with experience. Any help would be enormously appreciated! http://www.sunglasses4summer.net/ ---------------------------------------------------------------------- Comment By: Henning Westerholt (henningw) Date: 2008-07-08 06:47 Message: Logged In: YES user_id=337916 Originator: NO Hi all, the problem with checking the existence of the fifo/ socket is that the old file is not deleted during the shutdown. Even if we change this then the problem would be still present in the event of a server crash. As Helmut aggreed that this can be closed, i've moved this to the feature request section. The startup code in main.c could be modified to check for available network resources before doing the module initialization to achieve this functionality. Cheers, Henning ---------------------------------------------------------------------- Comment By: Ovidiu Sas (osas) Date: 2008-02-12 07:58 Message: Logged In: YES user_id=1395524 Originator: NO It happened to me too. I wanted to check the version of openser and instead of running 'openser -V' I ran 'openser -v' and my fifo file was gone. Maybe a quick fix would be to check the existence of the fifo file. If exists, exit with an error, if not, proceed as normal. Just my 2c. Regards, Ovidiu sas ---------------------------------------------------------------------- Comment By: Nobody/Anonymous (nobody) Date: 2008-02-12 07:53 Message: Logged In: NO Hi, well, I agree too. This is an operation error by admin. But it is annoying that it is so easy to destroy the media server. This will quit often occurs if you are using bash shell with command history combined with nervous fingers ;) ... I think that checking network resources in openser befor doing rest of configuration is safer. It should work allways, cause no second openser will user same address and port on same machine at same time. On the other Hand saving the PID of first openser in a PID should be an acceptable work around. So we can close this case if you want. regards Helmut ---------------------------------------------------------------------- Comment By: Nobody/Anonymous (nobody) Date: 2008-02-12 07:40 Message: Logged In: NO is it possible to delete a FIFO which is currently used? E.g by mistake I start openser a second time. If it deletes the FIFO then it may cause problems to the current running openser instance. klaus ---------------------------------------------------------------------- Comment By: Bogdan-Andrei Iancu (bogdan_iancu) Date: 2008-02-12 05:33 Message: Logged In: YES user_id=1275325 Originator: NO Hi Henning, I agree here - the fix should not be in openser, but rather in whatever scripts/admins are managing openser. From my point of view, this is a openser operation error. Regards, Bogdan ---------------------------------------------------------------------- Comment By: Henning Westerholt (henningw) Date: 2008-02-12 03:24 Message: Logged In: YES user_id=337916 Originator: NO I think this can be fixed by other means: Just add some checks to openserctl to not start openser two times, like the init script does (i think). This would be easier than to fix the OpenSER start process to not start at all (or fail just in the beginning) if another process is already running. Henning ---------------------------------------------------------------------- Comment By: Bogdan-Andrei Iancu (bogdan_iancu) Date: 2008-02-12 03:06 Message: Logged In: YES user_id=1275325 Originator: NO Hi Helmut, This is the normal/expected bahviour of the module - to avoid any security problems (using a fake or malicious already existing unixsocket) openser removes it first (if exists) and then creates its own with proper permissions and security restrictions. So, it is not a bug :). Regards, Bogdan ---------------------------------------------------------------------- Comment By: Helmut Kuper (hekuper) Date: 2008-02-11 06:53 Message: Logged In: YES user_id=1851349 Originator: YES Hi Bogdan, yes I use same socket file name for both opensers, cause both are using same config file. Befor we misunderstand: I don't want to run 2 opensers with same config on same machine!! I rather strumbled about that bug/problem, when I accidently started same openser without openserctl twice ---------------------------------------------------------------------- Comment By: Bogdan-Andrei Iancu (bogdan_iancu) Date: 2008-02-11 06:45 Message: Logged In: YES user_id=1275325 Originator: NO Hi Helmut, so you use the same socket_name for both openser's ?? Regards, Bogdan ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=743023&aid=1891148&group_…

1 0

git:master: xmlrpc: more correct re XMLRPC spec re. no type specified, defaults to string
by Jason Penton 24 Aug '12

24 Aug '12

Module: sip-router Branch: master Commit: fbc6ce9c0792408877166a3e35b1c6cc2dc645e3 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=fbc6ce9… Author: Jason Penton <jason.penton(a)smilecoms.com> Committer: Jason Penton <jason.penton(a)smilecoms.com> Date: Fri Aug 24 09:51:06 2012 +0200 xmlrpc: more correct re XMLRPC spec re. no type specified, defaults to string - This will allow you to receive docs like <value>mystringvalue</value> - instead of only allowing <value><string>mystringvalue</string></value> - according to spec if no type specified, assume string - allows kamailio to receive messages from apache ws-xmlrpc which doesnt specify string type --- modules/xmlrpc/xmlrpc.c | 29 +++++++++++++++++++++++------ 1 files changed, 23 insertions(+), 6 deletions(-) diff --git a/modules/xmlrpc/xmlrpc.c b/modules/xmlrpc/xmlrpc.c index aa5d3aa..b840131 100644 --- a/modules/xmlrpc/xmlrpc.c +++ b/modules/xmlrpc/xmlrpc.c @@ -1126,6 +1126,7 @@ static time_t xmlrpc2time(const char* str) /* xml value types */ enum xmlrpc_val_type{ XML_T_STR, + XML_T_TXT, XML_T_INT, XML_T_BOOL, XML_T_DATE, @@ -1142,6 +1143,8 @@ static enum xmlrpc_val_type xml_get_type(xmlNodePtr value) { if (!xmlStrcmp(value->name, BAD_CAST "string")){ return XML_T_STR; + } else if (!xmlStrcmp(value->name, BAD_CAST "text")) { + return XML_T_TXT; } else if ( !xmlStrcmp(value->name, BAD_CAST "i4") || !xmlStrcmp(value->name, BAD_CAST "int")) { return XML_T_INT; @@ -1204,6 +1207,7 @@ static int get_int(int* val, struct xmlrpc_reply* reply, break; case XML_T_DOUBLE: case XML_T_STR: + case XML_T_TXT: if (flags & GET_X_AUTOCONV) break; case XML_T_ERR: @@ -1211,8 +1215,10 @@ static int get_int(int* val, struct xmlrpc_reply* reply, set_fault(reply, 400, "Invalid Parameter Type"); return -1; } - - val_str = (char*)xmlNodeListGetString(doc, i4->xmlChildrenNode, 1); + if (type == XML_T_TXT) + val_str = (char*)i4->content; + else + val_str = (char*)xmlNodeListGetString(doc, i4->xmlChildrenNode, 1); if (!val_str) { if (!(flags & GET_X_NOREPLY)) set_fault(reply, 400, "Empty Parameter Value"); @@ -1223,6 +1229,7 @@ static int get_int(int* val, struct xmlrpc_reply* reply, case XML_T_INT: case XML_T_BOOL: case XML_T_STR: + case XML_T_TXT: /* Integer/bool conversion */ *val = strtol(val_str, &end_ptr, 10); if (val_str==end_ptr) @@ -1295,6 +1302,7 @@ static int get_double(double* val, struct xmlrpc_reply* reply, case XML_T_BOOL: case XML_T_DATE: case XML_T_STR: + case XML_T_TXT: if (flags & GET_X_AUTOCONV) break; case XML_T_ERR: @@ -1302,8 +1310,10 @@ static int get_double(double* val, struct xmlrpc_reply* reply, set_fault(reply, 400, "Invalid Parameter Type"); return -1; } - - val_str = (char*)xmlNodeListGetString(doc, dbl->xmlChildrenNode, 1); + if (type == XML_T_TXT) + val_str = (char*)dbl->content; + else + val_str = (char*)xmlNodeListGetString(doc, dbl->xmlChildrenNode, 1); if (!val_str) { if (!(flags & GET_X_NOREPLY)) set_fault(reply, 400, "Empty Double Parameter"); @@ -1315,6 +1325,7 @@ static int get_double(double* val, struct xmlrpc_reply* reply, case XML_T_INT: case XML_T_BOOL: case XML_T_STR: + case XML_T_TXT: *val = strtod(val_str, &end_ptr); if (val_str==end_ptr) ret=-1; @@ -1379,6 +1390,7 @@ static int get_string(char** val, struct xmlrpc_reply* reply, type=xml_get_type(dbl); switch(type){ case XML_T_STR: + case XML_T_TXT: break; case XML_T_INT: case XML_T_BOOL: @@ -1391,9 +1403,13 @@ static int get_string(char** val, struct xmlrpc_reply* reply, set_fault(reply, 400, "Invalid Parameter Type"); return -1; } - val_str = (char*)xmlNodeListGetString(doc, dbl->xmlChildrenNode, 1); + if (type == XML_T_TXT) + val_str = (char*)dbl->content; + else + val_str = (char*)xmlNodeListGetString(doc, dbl->xmlChildrenNode, 1); + if (!val_str) { - if (type==XML_T_STR){ + if (type==XML_T_STR || type==XML_T_TXT){ *val = null_str; return 0; }else{ @@ -1405,6 +1421,7 @@ static int get_string(char** val, struct xmlrpc_reply* reply, ret=0; switch(type){ case XML_T_STR: + case XML_T_TXT: if (flags & GET_X_LFLF2CRLF){ p=val_str; while(*p){

1 0

git:master: Revert "xmlrpc: more correct re XMLRPC spec re. no type specified, defaults to string"
by Jason Penton 24 Aug '12

24 Aug '12

Module: sip-router Branch: master Commit: 881ee61c85f66767e0e8988e0a98205abbc7f9e9 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=881ee61… Author: Jason Penton <jason.penton(a)smilecoms.com> Committer: Jason Penton <jason.penton(a)smilecoms.com> Date: Fri Aug 24 09:48:46 2012 +0200 Revert "xmlrpc: more correct re XMLRPC spec re. no type specified, defaults to string" This reverts commit 35e5e50c8f624ab70ef095bdbc729861b4a0a749. --- modules/xmlrpc/xmlrpc.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/modules/xmlrpc/xmlrpc.c b/modules/xmlrpc/xmlrpc.c index 8f865ba..aa5d3aa 100644 --- a/modules/xmlrpc/xmlrpc.c +++ b/modules/xmlrpc/xmlrpc.c @@ -1140,7 +1140,7 @@ enum xmlrpc_val_type{ */ static enum xmlrpc_val_type xml_get_type(xmlNodePtr value) { - if (!xmlStrcmp(value->name, BAD_CAST "string") || !xmlStrcmp(value->name, BAD_CAST "text")){ + if (!xmlStrcmp(value->name, BAD_CAST "string")){ return XML_T_STR; } else if ( !xmlStrcmp(value->name, BAD_CAST "i4") || !xmlStrcmp(value->name, BAD_CAST "int")) {

1 0

git:master: xmlrpc: more correct re XMLRPC spec re. no type specified, defaults to string
by Jason Penton 24 Aug '12

24 Aug '12

Module: sip-router Branch: master Commit: 35e5e50c8f624ab70ef095bdbc729861b4a0a749 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=35e5e50… Author: Jason Penton <jason.penton(a)smilecoms.com> Committer: Jason Penton <jason.penton(a)smilecoms.com> Date: Fri Aug 24 09:21:40 2012 +0200 xmlrpc: more correct re XMLRPC spec re. no type specified, defaults to string - This will allow you to receive docs like <value>mystringvalue</value> - instead of only allowing <value><string>mystringvalue</string></value> - according to spec if no type specified, assume string - allows kamailio to receive messages from apache ws-xmlrpc which doesnt specify string type --- modules/xmlrpc/xmlrpc.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/modules/xmlrpc/xmlrpc.c b/modules/xmlrpc/xmlrpc.c index aa5d3aa..8f865ba 100644 --- a/modules/xmlrpc/xmlrpc.c +++ b/modules/xmlrpc/xmlrpc.c @@ -1140,7 +1140,7 @@ enum xmlrpc_val_type{ */ static enum xmlrpc_val_type xml_get_type(xmlNodePtr value) { - if (!xmlStrcmp(value->name, BAD_CAST "string")){ + if (!xmlStrcmp(value->name, BAD_CAST "string") || !xmlStrcmp(value->name, BAD_CAST "text")){ return XML_T_STR; } else if ( !xmlStrcmp(value->name, BAD_CAST "i4") || !xmlStrcmp(value->name, BAD_CAST "int")) {

1 0

git:master: db_cluster: use connection from write structure for db updates
by Daniel-Constantin Mierla 24 Aug '12

24 Aug '12

Module: sip-router Branch: master Commit: 4d15ba97bab58108cfedc45158d90583f67cdadf URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=4d15ba9… Author: Daniel-Constantin Mierla <miconda(a)gmail.com> Committer: Daniel-Constantin Mierla <miconda(a)gmail.com> Date: Fri Aug 24 09:14:28 2012 +0200 db_cluster: use connection from write structure for db updates - could be related to an issue reported by Øyvind Kolbu --- modules_k/db_cluster/dbcl_api.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/modules_k/db_cluster/dbcl_api.c b/modules_k/db_cluster/dbcl_api.c index d92a8ca..20d689c 100644 --- a/modules_k/db_cluster/dbcl_api.c +++ b/modules_k/db_cluster/dbcl_api.c @@ -136,7 +136,7 @@ {\ LM_DBG("serial operation - cluster [%.*s] (%d/%d)\n",\ cls->name.len, cls->name.s, i, j);\ - dbh = cls->rlist[i].clist[j]->dbh;\ + dbh = cls->wlist[i].clist[j]->dbh;\ ret = cls->wlist[i].clist[j]->dbf.command;\ if (ret==0) {\ cls->usedcon = cls->wlist[i].clist[j];\ @@ -159,7 +159,7 @@ {\ LM_DBG("round robin operation - cluster [%.*s] (%d/%d)\n",\ cls->name.len, cls->name.s, i, j);\ - dbh = cls->rlist[i].clist[j]->dbh;\ + dbh = cls->wlist[i].clist[j]->dbh;\ ret = cls->wlist[i].clist[j]->dbf.command;\ if (ret==0)\ {\ @@ -183,7 +183,7 @@ {\ LM_DBG("parallel operation - cluster [%.*s] (%d/%d)\n",\ cls->name.len, cls->name.s, i, j);\ - dbh = cls->rlist[i].clist[j]->dbh;\ + dbh = cls->wlist[i].clist[j]->dbh;\ rc = cls->wlist[i].clist[j]->dbf.command;\ if(rc==0) {\ cls->usedcon = cls->wlist[i].clist[j];\

1 0

git:master: modules_k/rls: Use database row/ table locking where supported in DB only mode
by Peter Dunkley 21 Aug '12

21 Aug '12

Module: sip-router Branch: master Commit: 10327c61d35e034f31c49a27f11f81ec82c22055 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=10327c6… Author: Peter Dunkley <peter.dunkley(a)crocodile-rcs.com> Committer: Peter Dunkley <peter.dunkley(a)crocodile-rcs.com> Date: Tue Aug 21 15:41:27 2012 +0100 modules_k/rls: Use database row/table locking where supported in DB only mode - Under load there are lots of DB deadlocks when using (start|end)_transaction() with multiple presence processes and/or servers. - Without using (start|end)_transaction() multiple processes/servers overwrite each others changes. - Using row locking (where possible) and table locking (where required) fixes these problems. - IMPORTANT NOTE: DB only, multi-process/multi-server, presence will only work properly under high-load when using a database driver that supports transactions and locking (currently just db_postgres). --- modules_k/rls/notify.c | 10 ++++---- modules_k/rls/resource_notify.c | 40 +++++++++++++++++++++++++++++------ modules_k/rls/rls_db.c | 37 +++++++++++++++++++++++++++++--- modules_k/rls/subscribe.c | 44 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 115 insertions(+), 16 deletions(-) diff --git a/modules_k/rls/notify.c b/modules_k/rls/notify.c index c8eb230..f0ec0aa 100644 --- a/modules_k/rls/notify.c +++ b/modules_k/rls/notify.c @@ -101,10 +101,10 @@ int send_full_notify(subs_t* subs, xmlNodePtr rl_node, str* rl_uri, int len_est; res_param_t param; int resource_added = 0; /* Flag to indicate that we have added at least one resource */ - multipart_body = NULL; + multipart_body=NULL; + db_query_f query_fn = rlpres_dbf.query_lock ? rlpres_dbf.query_lock : rlpres_dbf.query; LM_DBG("start\n"); - /* query in alfabetical order */ if(CONSTR_RLSUBS_DID(subs, &rlsubs_did)<0) { @@ -136,15 +136,15 @@ int send_full_notify(subs_t* subs, xmlNodePtr rl_node, str* rl_uri, if (dbmode == RLS_DB_ONLY && rlpres_dbf.start_transaction) { - if (rlpres_dbf.start_transaction(rlpres_db) < 0) + if (rlpres_dbf.start_transaction(rlpres_db, DB_LOCKING_WRITE) < 0) { LM_ERR("in start_transaction\n"); goto error; } } - if(rlpres_dbf.query(rlpres_db, query_cols, 0, query_vals, result_cols, - 1, n_result_cols, &str_resource_uri_col, &result )< 0) + if(query_fn(rlpres_db, query_cols, 0, query_vals, result_cols, + 1, n_result_cols, NULL, &result )< 0) { LM_ERR("in sql query\n"); goto error; diff --git a/modules_k/rls/resource_notify.c b/modules_k/rls/resource_notify.c index 8937131..1b9b47c 100644 --- a/modules_k/rls/resource_notify.c +++ b/modules_k/rls/resource_notify.c @@ -203,6 +203,15 @@ static void send_notifies(db1_res_t *result, int did_col, int resource_uri_col, ERR_MEM(PKG_MEM_STR); } + if (dbmode == RLS_DB_ONLY && rls_dbf.start_transaction) + { + if (rls_dbf.start_transaction(rls_db, DB_LOCKING_WRITE) < 0) + { + LM_ERR("in start_transaction\n"); + goto error; + } + } + LM_DBG("found %d records with updated state\n", result->n); for(i= 0; i< result->n; i++) { @@ -420,9 +429,17 @@ static void send_notifies(db1_res_t *result, int did_col, int resource_uri_col, dialog= NULL; } - -error: done: + if (dbmode == RLS_DB_ONLY && rls_dbf.end_transaction) + { + if (rls_dbf.end_transaction(rls_db) < 0) + { + LM_ERR("in end_transaction\n"); + goto error; + } + } + +error: if(bstr.s) pkg_free(bstr.s); @@ -430,6 +447,13 @@ done: pkg_free(buf); if(dialog) pkg_free(dialog); + + if (dbmode == RLS_DB_ONLY && rls_dbf.abort_transaction) + { + if (rls_dbf.abort_transaction(rls_db) < 0) + LM_ERR("in abort_transaction\n"); + } + return; } @@ -769,7 +793,7 @@ int rls_handle_notify(struct sip_msg* msg, char* c1, char* c2) if (dbmode == RLS_DB_ONLY && rlpres_dbf.start_transaction) { - if (rlpres_dbf.start_transaction(rlpres_db) < 0) + if (rlpres_dbf.start_transaction(rlpres_db, DB_LOCKING_WRITE) < 0) { LM_ERR("in start_transaction\n"); goto error; @@ -883,6 +907,7 @@ static void timer_send_full_state_notifies(int round) xmlDocPtr doc = NULL; xmlNodePtr service_node = NULL; int now = (int)time(NULL); + db_query_f query_fn = rls_dbf.query_lock ? rls_dbf.query_lock : rls_dbf.query; query_cols[0] = &str_updated_col; query_vals[0].type = DB1_INT; @@ -925,7 +950,7 @@ static void timer_send_full_state_notifies(int round) if (dbmode == RLS_DB_ONLY && rls_dbf.start_transaction) { - if (rls_dbf.start_transaction(rls_db) < 0) + if (rls_dbf.start_transaction(rls_db, DB_LOCKING_WRITE) < 0) { LM_ERR("in start_transaction\n"); goto done; @@ -933,7 +958,7 @@ static void timer_send_full_state_notifies(int round) } /* Step 1: Find rls_watchers that require full-state notification */ - if (rls_dbf.query(rls_db, query_cols, 0, query_vals, result_cols, + if (query_fn(rls_db, query_cols, 0, query_vals, result_cols, 1, n_result_cols, 0, &result) < 0) { LM_ERR("in sql query\n"); @@ -1051,6 +1076,7 @@ static void timer_send_update_notifies(int round) pres_state_col, content_type_col; int n_result_cols= 0; db1_res_t *result= NULL; + db_query_f query_fn = rlpres_dbf.query_lock ? rlpres_dbf.query_lock : rlpres_dbf.query; query_cols[0]= &str_updated_col; query_vals[0].type = DB1_INT; @@ -1080,14 +1106,14 @@ static void timer_send_update_notifies(int round) if (dbmode == RLS_DB_ONLY && rlpres_dbf.start_transaction) { - if (rlpres_dbf.start_transaction(rlpres_db) < 0) + if (rlpres_dbf.start_transaction(rlpres_db, DB_LOCKING_WRITE) < 0) { LM_ERR("in start_transaction\n"); goto done; } } - if(rlpres_dbf.query(rlpres_db, query_cols, 0, query_vals, result_cols, + if(query_fn(rlpres_db, query_cols, 0, query_vals, result_cols, 1, n_result_cols, &str_rlsubs_did_col, &result)< 0) { LM_ERR("in sql query\n"); diff --git a/modules_k/rls/rls_db.c b/modules_k/rls/rls_db.c index 97ac7d6..ee406de 100644 --- a/modules_k/rls/rls_db.c +++ b/modules_k/rls/rls_db.c @@ -124,6 +124,7 @@ int delete_expired_subs_rlsdb( void ) int i; subs_t subs; str rlsubs_did = {0, 0}; + db_query_f query_fn = rls_dbf.query_lock ? rls_dbf.query_lock : rls_dbf.query; if(rls_db == NULL) { @@ -148,7 +149,16 @@ int delete_expired_subs_rlsdb( void ) result_cols[r_to_tag_col=n_result_cols++] = &str_to_tag_col; result_cols[r_from_tag_col=n_result_cols++] = &str_from_tag_col; - if(rls_dbf.query(rls_db, query_cols, query_ops, query_vals, result_cols, + if (rls_dbf.start_transaction) + { + if (rls_dbf.start_transaction(rls_db, DB_LOCKING_WRITE) < 0) + { + LM_ERR("in start_transaction\n"); + goto error; + } + } + + if(query_fn(rls_db, query_cols, query_ops, query_vals, result_cols, n_query_cols, n_result_cols, 0, &result )< 0) { LM_ERR("Can't query db\n"); @@ -213,11 +223,28 @@ int delete_expired_subs_rlsdb( void ) } rls_dbf.free_result(rls_db, result); + + if (rls_dbf.end_transaction) + { + if (rls_dbf.end_transaction(rls_db) < 0) + { + LM_ERR("in end_transaction\n"); + goto error; + } + } + return 1; error: if (result) rls_dbf.free_result(rls_db, result); if (rlsubs_did.s) pkg_free(rlsubs_did.s); + + if (rls_dbf.abort_transaction) + { + if (rls_dbf.abort_transaction(rls_db) < 0) + LM_ERR("in abort_transaction\n"); + } + return -1; } @@ -718,7 +745,8 @@ int get_dialog_subscribe_rlsdb(subs_t *subs) int nr_rows; int r_remote_cseq, r_local_cseq, r_version; char *r_pres_uri, *r_record_route; - + db_query_f query_fn = rls_dbf.query_lock ? rls_dbf.query_lock : rls_dbf.query; + if(rls_db == NULL) { LM_ERR("null database connection\n"); @@ -761,7 +789,7 @@ int get_dialog_subscribe_rlsdb(subs_t *subs) result_cols[version_col = n_result_cols++] = &str_version_col; result_cols[rroute_col = n_result_cols++] = &str_record_route_col; - if(rls_dbf.query(rls_db, query_cols, 0, query_vals, result_cols, + if(query_fn(rls_db, query_cols, 0, query_vals, result_cols, n_query_cols, n_result_cols, 0, &result )< 0) { LM_ERR("Can't query db\n"); @@ -865,6 +893,7 @@ subs_t *get_dialog_notify_rlsdb(str callid, str to_tag, str from_tag) subs_t *dest; event_t parsed_event; str ev_sname; + db_query_f query_fn = rls_dbf.query_lock ? rls_dbf.query_lock : rls_dbf.query; if(rls_db == NULL) { @@ -919,7 +948,7 @@ subs_t *get_dialog_notify_rlsdb(str callid, str to_tag, str from_tag) result_cols[r_version_col=n_result_cols++] = &str_version_col; result_cols[r_expires_col=n_result_cols++] = &str_expires_col; - if(rls_dbf.query(rls_db, query_cols, 0, query_vals, result_cols, + if(query_fn(rls_db, query_cols, 0, query_vals, result_cols, n_query_cols, n_result_cols, 0, &result )< 0) { LM_ERR("Can't query db\n"); diff --git a/modules_k/rls/subscribe.c b/modules_k/rls/subscribe.c index f719754..c770cbf 100644 --- a/modules_k/rls/subscribe.c +++ b/modules_k/rls/subscribe.c @@ -639,6 +639,15 @@ int rls_handle_subscribe(struct sip_msg* msg, str watcher_user, str watcher_doma /* search if a stored dialog */ if ( dbmode == RLS_DB_ONLY ) { + if (rls_dbf.start_transaction) + { + if (rls_dbf.start_transaction(rls_db, DB_LOCKING_WRITE) < 0) + { + LM_ERR("in start_transaction\n"); + goto error; + } + } + rt = get_dialog_subscribe_rlsdb(&subs); if (rt <= 0) @@ -646,6 +655,16 @@ int rls_handle_subscribe(struct sip_msg* msg, str watcher_user, str watcher_doma LM_DBG("subscription dialog not found for <%.*s(a)%.*s>\n", subs.watcher_user.len, subs.watcher_user.s, subs.watcher_domain.len, subs.watcher_domain.s); + + if (rls_dbf.end_transaction) + { + if (rls_dbf.end_transaction(rls_db) < 0) + { + LM_ERR("in end_transaction\n"); + goto error; + } + } + goto forpresence; } else if(rt>=400) @@ -657,6 +676,16 @@ int rls_handle_subscribe(struct sip_msg* msg, str watcher_user, str watcher_doma LM_ERR("while sending reply\n"); goto error; } + + if (rls_dbf.end_transaction) + { + if (rls_dbf.end_transaction(rls_db) < 0) + { + LM_ERR("in end_transaction\n"); + goto error; + } + } + ret = 0; goto stop; } @@ -670,6 +699,15 @@ int rls_handle_subscribe(struct sip_msg* msg, str watcher_user, str watcher_doma LM_ERR("while updating resource list subscription\n"); goto error; } + + if (rls_dbf.end_transaction) + { + if (rls_dbf.end_transaction(rls_db) < 0) + { + LM_ERR("in end_transaction\n"); + goto error; + } + } } else { @@ -795,6 +833,12 @@ error: if (rlsubs_did.s != NULL) pkg_free(rlsubs_did.s); + if (rls_dbf.abort_transaction) + { + if (rls_dbf.abort_transaction(rls_db) < 0) + LM_ERR("in abort_transaction\n"); + } + return err_ret; }

1 0

git:master: modules_k/pua: Use database row/ table locking where supported in DB only mode
by Peter Dunkley 21 Aug '12

21 Aug '12

Module: sip-router Branch: master Commit: 92aedbb83f5d6cbfc7c4b5e68f260f7ecccc992e URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=92aedbb… Author: Peter Dunkley <peter.dunkley(a)crocodile-rcs.com> Committer: Peter Dunkley <peter.dunkley(a)crocodile-rcs.com> Date: Tue Aug 21 15:40:56 2012 +0100 modules_k/pua: Use database row/table locking where supported in DB only mode - Under load there are lots of DB deadlocks when using (start|end)_transaction() with multiple presence processes and/or servers. - Without using (start|end)_transaction() multiple processes/servers overwrite each others changes. - Using row locking (where possible) and table locking (where required) fixes these problems. - IMPORTANT NOTE: DB only, multi-process/multi-server, presence will only work properly under high-load when using a database driver that supports transactions and locking (currently just db_postgres). --- modules_k/pua/pua_db.c | 6 ++++-- modules_k/pua/send_publish.c | 4 ++-- modules_k/pua/send_subscribe.c | 22 ++++++++++++++++++++-- 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/modules_k/pua/pua_db.c b/modules_k/pua/pua_db.c index 05dd1e3..4be51bf 100644 --- a/modules_k/pua/pua_db.c +++ b/modules_k/pua/pua_db.c @@ -817,6 +817,7 @@ ua_pres_t *get_record_puadb(str pres_id, str *etag, ua_pres_t *result, db1_res_t db_row_t *rows; db1_res_t *res; int n_query_cols = 0, nr_rows; + db_query_f query_fn = pua_dbf.query_lock ? pua_dbf.query_lock : pua_dbf.query; q_cols[n_query_cols] = &str_pres_id_col; q_vals[n_query_cols].type = DB1_STR; @@ -840,7 +841,7 @@ ua_pres_t *get_record_puadb(str pres_id, str *etag, ua_pres_t *result, db1_res_t return(NULL); } - if(pua_dbf.query(pua_db, q_cols, 0, q_vals, + if(query_fn(pua_db, q_cols, 0, q_vals, NULL,n_query_cols,0,0,&res) < 0) { LM_ERR("DB query error\n"); @@ -1162,6 +1163,7 @@ ua_pres_t *get_dialog_puadb(str pres_id, str *pres_uri, ua_pres_t *result, db1_r db_row_t *rows; db1_res_t *res; int n_query_cols = 0, nr_rows; + db_query_f query_fn = pua_dbf.query_lock ? pua_dbf.query_lock : pua_dbf.query; if (pres_uri == NULL) { @@ -1188,7 +1190,7 @@ ua_pres_t *get_dialog_puadb(str pres_id, str *pres_uri, ua_pres_t *result, db1_r return(NULL); } - if(pua_dbf.query(pua_db, q_cols, 0, q_vals, + if(query_fn(pua_db, q_cols, 0, q_vals, NULL,n_query_cols,0,0,&res) < 0) { LM_ERR("DB query error\n"); diff --git a/modules_k/pua/send_publish.c b/modules_k/pua/send_publish.c index 97935c1..1f4ed37 100644 --- a/modules_k/pua/send_publish.c +++ b/modules_k/pua/send_publish.c @@ -216,7 +216,7 @@ void publ_cback_func(struct cell *t, int type, struct tmcb_params *ps) if (dbmode == PUA_DB_ONLY && pua_dbf.start_transaction) { - if (pua_dbf.start_transaction(pua_db) < 0) + if (pua_dbf.start_transaction(pua_db, DB_LOCKING_WRITE) < 0) { LM_ERR("in start_transaction\n"); goto error; @@ -498,7 +498,7 @@ int send_publish( publ_info_t* publ ) if (dbmode == PUA_DB_ONLY && pua_dbf.start_transaction) { - if (pua_dbf.start_transaction(pua_db) < 0) + if (pua_dbf.start_transaction(pua_db, DB_LOCKING_WRITE) < 0) { LM_ERR("in start_transaction\n"); goto error; diff --git a/modules_k/pua/send_subscribe.c b/modules_k/pua/send_subscribe.c index 89a8141..1c10bf0 100644 --- a/modules_k/pua/send_subscribe.c +++ b/modules_k/pua/send_subscribe.c @@ -301,7 +301,7 @@ void subs_cback_func(struct cell *t, int cb_type, struct tmcb_params *ps) if (dbmode == PUA_DB_ONLY && pua_dbf.start_transaction) { - if (pua_dbf.start_transaction(pua_db) < 0) + if (pua_dbf.start_transaction(pua_db, DB_LOCKING_WRITE) < 0) { LM_ERR("in start_transaction\n"); goto error; @@ -676,6 +676,24 @@ faked_error: if (dbmode==PUA_DB_ONLY) { + if (pua_dbf.end_transaction) + { + if (pua_dbf.end_transaction(pua_db) < 0) + { + LM_ERR("in end_transaction\n"); + goto error; + } + } + + if (pua_dbf.start_transaction) + { + if (pua_dbf.start_transaction(pua_db, DB_LOCKING_WRITE) < 0) + { + LM_ERR("in start_transaction\n"); + goto error; + } + } + if (convert_temporary_dialog_puadb(presentity) < 0) { LM_ERR("Could not convert temporary dialog into a dialog\n"); @@ -968,7 +986,7 @@ int send_subscribe(subs_info_t* subs) if (dbmode == PUA_DB_ONLY && pua_dbf.start_transaction) { - if (pua_dbf.start_transaction(pua_db) < 0) + if (pua_dbf.start_transaction(pua_db, DB_LOCKING_WRITE) < 0) { LM_ERR("in start_transaction\n"); goto error;

1 0

git:master: modules_k/presence: Use database row/ table locking where supported in DB only mode
by Peter Dunkley 21 Aug '12

21 Aug '12

Module: sip-router Branch: master Commit: 34cd2acb5370f36238f8765765cee6d97fd18209 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=34cd2ac… Author: Peter Dunkley <peter.dunkley(a)crocodile-rcs.com> Committer: Peter Dunkley <peter.dunkley(a)crocodile-rcs.com> Date: Tue Aug 21 15:36:34 2012 +0100 modules_k/presence: Use database row/table locking where supported in DB only mode - Under load there are lots of DB deadlocks when using (start|end)_transaction() with multiple presence processes and/or servers. - Without using (start|end)_transaction() multiple processes/servers overwrite each others changes. - Using row locking (where possible) and table locking (where required) fixes these problems. - IMPORTANT NOTE: DB only, multi-process/multi-server, presence will only work properly under high-load when using a database driver that supports transactions and locking (currently just db_postgres). --- modules_k/presence/notify.c | 47 +++++++++++++++++++---- modules_k/presence/presentity.c | 53 +++++++++++++++++++++++++- modules_k/presence/subscribe.c | 81 +++++++++++++++++++++++++++++++++++--- 3 files changed, 165 insertions(+), 16 deletions(-) Diff: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commitdiff;h=34c…

1 0

git:master: modules/db_postgres: Added support for database row and table locking to PostgreSQL database module
by Peter Dunkley 21 Aug '12

21 Aug '12

Module: sip-router Branch: master Commit: f1f39db4f8a73f16497c4cc2658e9b4e68bac0fc URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=f1f39db… Author: Peter Dunkley <peter.dunkley(a)crocodile-rcs.com> Committer: Peter Dunkley <peter.dunkley(a)crocodile-rcs.com> Date: Tue Aug 21 15:27:29 2012 +0100 modules/db_postgres: Added support for database row and table locking to PostgreSQL database module - start_transaction() now takes an argument allowing the type of locking (none, read, or full) to be specified. - new query_lock() API will use a SELECT ... FOR UPDATE query instead of just a SELECT ... --- modules/db_postgres/km_db_postgres.c | 1 + modules/db_postgres/km_dbase.c | 80 ++++++++++++++++++++++++++++++++- modules/db_postgres/km_dbase.h | 11 ++++- 3 files changed, 88 insertions(+), 4 deletions(-) diff --git a/modules/db_postgres/km_db_postgres.c b/modules/db_postgres/km_db_postgres.c index 34edca7..a8f0130 100644 --- a/modules/db_postgres/km_db_postgres.c +++ b/modules/db_postgres/km_db_postgres.c @@ -99,6 +99,7 @@ int db_postgres_bind_api(db_func_t *dbb) dbb->start_transaction= db_postgres_start_transaction; dbb->end_transaction = db_postgres_end_transaction; dbb->abort_transaction= db_postgres_abort_transaction; + dbb->query_lock = db_postgres_query_lock; return 0; } diff --git a/modules/db_postgres/km_dbase.c b/modules/db_postgres/km_dbase.c index 6c21b05..bf393ed 100644 --- a/modules/db_postgres/km_dbase.c +++ b/modules/db_postgres/km_dbase.c @@ -441,6 +441,33 @@ int db_postgres_query(const db1_con_t* _h, const db_key_t* _k, const db_op_t* _o /*! + * \brief Query table for specified rows and lock them + * \param _h structure representing database connection + * \param _k key names + * \param _op operators + * \param _v values of the keys that must match + * \param _c column names to return + * \param _n nmber of key=values pairs to compare + * \param _nc number of columns to return + * \param _o order by the specified column + * \param _r result set + * \return 0 on success, negative on failure + */ +int db_postgres_query_lock(const db1_con_t* _h, const db_key_t* _k, const db_op_t* _op, + const db_val_t* _v, const db_key_t* _c, const int _n, const int _nc, + const db_key_t _o, db1_res_t** _r) +{ + if (CON_TRANSACTION(_h) == 0) + { + LM_ERR("transaction not in progress\n"); + return -1; + } + return db_do_query_lock(_h, _k, _op, _v, _c, _n, _nc, _o, _r, db_postgres_val2str, + db_postgres_submit_query, db_postgres_store_result); +} + + +/*! * Execute a raw SQL query * \param _h database connection * \param _s raw query string @@ -658,10 +685,15 @@ int db_postgres_affected_rows(const db1_con_t* _h) * \param _h database handle * \return 0 on success, negative on failure */ -int db_postgres_start_transaction(db1_con_t* _h) +int db_postgres_start_transaction(db1_con_t* _h, db_locking_t _l) { db1_res_t *res = NULL; - str query_str = str_init("BEGIN"); + str begin_str = str_init("BEGIN"); + str lock_start_str = str_init("LOCK TABLE "); + str lock_write_end_str = str_init(" IN EXCLUSIVE MODE"); + str lock_full_end_str = str_init(" IN ACCESS EXCLUSIVE MODE"); + str *lock_end_str = &lock_write_end_str; + str lock_str = {0, 0}; if (!_h) { LM_ERR("invalid parameter value\n"); @@ -673,7 +705,7 @@ int db_postgres_start_transaction(db1_con_t* _h) return -1; } - if (db_postgres_raw_query(_h, &query_str, &res) < 0) + if (db_postgres_raw_query(_h, &begin_str, &res) < 0) { LM_ERR("executing raw_query\n"); return -1; @@ -682,7 +714,49 @@ int db_postgres_start_transaction(db1_con_t* _h) if (res) db_postgres_free_result(_h, res); CON_TRANSACTION(_h) = 1; + + switch(_l) + { + case DB_LOCKING_NONE: + break; + case DB_LOCKING_FULL: + lock_end_str = &lock_full_end_str; + /* Fall-thru */ + case DB_LOCKING_WRITE: + if ((lock_str.s = pkg_malloc((lock_start_str.len + CON_TABLE(_h)->len + lock_end_str->len) * sizeof(char))) == NULL) + { + LM_ERR("allocating pkg memory\n"); + goto error; + } + + memcpy(lock_str.s, lock_start_str.s, lock_start_str.len); + lock_str.len += lock_start_str.len; + memcpy(lock_str.s + lock_str.len, CON_TABLE(_h)->s, CON_TABLE(_h)->len); + lock_str.len += CON_TABLE(_h)->len; + memcpy(lock_str.s + lock_str.len, lock_end_str->s, lock_end_str->len); + lock_str.len += lock_end_str->len; + + if (db_postgres_raw_query(_h, &lock_str, &res) < 0) + { + LM_ERR("executing raw_query\n"); + goto error; + } + + if (res) db_postgres_free_result(_h, res); + if (lock_str.s) pkg_free(lock_str.s); + break; + + default: + LM_WARN("unrecognised lock type\n"); + goto error; + } + return 0; + +error: + if (lock_str.s) pkg_free(lock_str.s); + db_postgres_abort_transaction(_h); + return -1; } /** diff --git a/modules/db_postgres/km_dbase.h b/modules/db_postgres/km_dbase.h index 4598139..24ae484 100644 --- a/modules/db_postgres/km_dbase.h +++ b/modules/db_postgres/km_dbase.h @@ -75,6 +75,15 @@ int db_postgres_query(const db1_con_t* _h, const db_key_t* _k, const db_op_t* _o const db_val_t* _v, const db_key_t* _c, const int _n, const int _nc, const db_key_t _o, db1_res_t** _r); + +/* + * Do a query and lock rows for update + */ +int db_postgres_query_lock(const db1_con_t* _h, const db_key_t* _k, const db_op_t* _op, + const db_val_t* _v, const db_key_t* _c, const int _n, const int _nc, + const db_key_t _o, db1_res_t** _r); + + /* * Raw SQL query */ @@ -115,7 +124,7 @@ int db_postgres_affected_rows(const db1_con_t* _h); /* * SQL BEGIN */ -int db_postgres_start_transaction(db1_con_t* _h); +int db_postgres_start_transaction(db1_con_t* _h, db_locking_t _l); /* * SQL COMMIT

1 0

git:master: lib/srdb1: Added support for database row and table locking to SRDB1
by Peter Dunkley 21 Aug '12

21 Aug '12

Module: sip-router Branch: master Commit: 6c39a678a5b1bf00e0a138daa90ac6401b017d94 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=6c39a67… Author: Peter Dunkley <peter.dunkley(a)crocodile-rcs.com> Committer: Peter Dunkley <peter.dunkley(a)crocodile-rcs.com> Date: Tue Aug 21 15:26:44 2012 +0100 lib/srdb1: Added support for database row and table locking to SRDB1 - Requires support within the database module --- lib/srdb1/db.c | 3 +- lib/srdb1/db.h | 13 +++++++- lib/srdb1/db_locking.h | 39 ++++++++++++++++++++++++ lib/srdb1/db_query.c | 78 +++++++++++++++++++++++++++++++++++++++++------ lib/srdb1/db_query.h | 27 ++++++++++++++++ 5 files changed, 148 insertions(+), 12 deletions(-) Diff: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commitdiff;h=6c3…

1 0

← Newer
1
2
3
4
5
6
7
8
...
17
Older →

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

sr-dev August 2012