sr-dev August 2012

sr-dev@lists.kamailio.org

30 participants
169 discussions

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

git:3.3: modules/db_postgres: libpq requires null terminated string, but Kamailio internal str type is not (necessarily) null-terminated
by Peter Dunkley 21 Aug '12

21 Aug '12

Module: sip-router Branch: 3.3 Commit: fb51d3815292c27245b26d1f4a4f6c9b190f9200 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=fb51d38… Author: Peter Dunkley <peter.dunkley(a)crocodile-rcs.com> Committer: Peter Dunkley <peter.dunkley(a)crocodile-rcs.com> Date: Tue Aug 21 15:19:51 2012 +0100 modules/db_postgres: libpq requires null terminated string, but Kamailio internal str type is not (necessarily) null-terminated - Found and fixed by Paul Pankhurst @ Crocodile RCS (cherry picked from commit f20713462410211370e762abb463f0ceafd36d8d) --- modules/db_postgres/km_dbase.c | 16 +++++++++++++++- 1 files changed, 15 insertions(+), 1 deletions(-) diff --git a/modules/db_postgres/km_dbase.c b/modules/db_postgres/km_dbase.c index d2be696..6c21b05 100644 --- a/modules/db_postgres/km_dbase.c +++ b/modules/db_postgres/km_dbase.c @@ -163,6 +163,7 @@ void db_postgres_close(db1_con_t* _h) */ static int db_postgres_submit_query(const db1_con_t* _con, const str* _s) { + char *s=NULL; int i, retries; ExecStatusType pqresult; @@ -199,17 +200,29 @@ static int db_postgres_submit_query(const db1_con_t* _con, const str* _s) else retries = pg_retries; + s = pkg_malloc((_s->len+1)*sizeof(char)); + if (s==NULL) + { + LM_ERR("%p db_postgres_submit_query Out of Memory: Query: %.*s\n", _con, _s->len, _s->s); + return -1; + } + + memcpy( s, _s->s, _s->len ); + s[_s->len] = '\0'; + for(i = 0; i <= retries; i++) { /* free any previous query that is laying about */ db_postgres_free_query(_con); /* exec the query */ - if (PQsendQuery(CON_CONNECTION(_con), _s->s)) { + + if (PQsendQuery(CON_CONNECTION(_con), s)) { pqresult = PQresultStatus(CON_RESULT(_con)); if((pqresult!=PGRES_FATAL_ERROR) || (PQstatus(CON_CONNECTION(_con))==CONNECTION_OK)) { LM_DBG("sending query ok: %p (%d) - [%.*s]\n", _con, pqresult, _s->len, _s->s); + pkg_free(s); return 0; } LM_WARN("postgres result check failed with code %d (%s)\n", @@ -226,6 +239,7 @@ static int db_postgres_submit_query(const db1_con_t* _con, const str* _s) } LM_ERR("%p PQsendQuery Error: %s Query: %.*s\n", _con, PQerrorMessage(CON_CONNECTION(_con)), _s->len, _s->s); + pkg_free(s); return -1; }

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