[Asterisk-code-review] func odbc: single database connection should be optional (asterisk[13])

Alexei Gradinari asteriskteam at digium.com
Tue May 10 14:35:50 CDT 2016


Alexei Gradinari has uploaded a new change for review.

  https://gerrit.asterisk.org/2800

Change subject: func_odbc: single database connection should be optional
......................................................................

func_odbc: single database connection should be optional

func_odbc was changed in Asterisk 13.9.0 (ASTERISK-25938)
to make func_odbc use a single database connection per DSN
because of reported bug with MySQL/MariaDB LAST_INSERT_ID().

This is drawback in performance when func_odbc is used
very often in dialplan.

Single database connection should be optional and disabled
by default.

Meanwhile... even with not persistent connection the stored
routine (procedure or function) can be used
to get correct LAST_INSERT_ID value.

ASTERISK-26010

Change-Id: I57d990616c957dabf7597dea5d5c3148f459dfb6
---
M CHANGES
M configs/samples/func_odbc.conf.sample
M funcs/func_odbc.c
3 files changed, 277 insertions(+), 52 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/00/2800/1

diff --git a/CHANGES b/CHANGES
index 965b1b4..be7153b 100644
--- a/CHANGES
+++ b/CHANGES
@@ -12,6 +12,12 @@
 --- Functionality changes from Asterisk 13.9.0 to Asterisk 13.10.0 -----------
 ------------------------------------------------------------------------------
 
+func_odbc
+------------------
+ * Added new global option "single_db_connection".
+   Enabling this option func_odbc will use a single database connection per DSN.
+   This option is disabled by default.
+
 res_fax
 ------------------
  * Added FAXMODE variable to let dialplan know what fax transport was used.
diff --git a/configs/samples/func_odbc.conf.sample b/configs/samples/func_odbc.conf.sample
index fd528d2..2863c71 100644
--- a/configs/samples/func_odbc.conf.sample
+++ b/configs/samples/func_odbc.conf.sample
@@ -1,6 +1,14 @@
 ;
 ; func_odbc.conf
 ;
+[general]
+; Asterisk uses separate connections for every database operation.
+; If single_db_connection is enabled then func_odbc will use a single
+; database connection per DSN.
+; This option is disabled by default.
+;single_db_connection=no
+;
+;
 ; Each context is a separately defined function.  By convention, all
 ; functions are entirely uppercase, so the defined contexts should also
 ; be all-uppercase, but there is nothing that enforces this.  All functions
diff --git a/funcs/func_odbc.c b/funcs/func_odbc.c
index aa84c00..2e6aa76 100644
--- a/funcs/func_odbc.c
+++ b/funcs/func_odbc.c
@@ -101,6 +101,10 @@
 
 static char *config = "func_odbc.conf";
 
+#define DEFAULT_SINGLE_DB_CONNECTION 0
+
+static int single_db_connection = DEFAULT_SINGLE_DB_CONNECTION;
+
 enum odbc_option_flags {
 	OPT_ESCAPECOMMAS =	(1 << 0),
 	OPT_MULTIROW     =	(1 << 1),
@@ -568,19 +572,29 @@
 			if ((obj = ast_odbc_retrieve_transaction_obj(chan, query->writehandle[dsn_num]))) {
 				transactional = 1;
 			} else {
-				dsn = get_dsn(query->writehandle[dsn_num]);
-				if (!dsn) {
-					continue;
+				if (single_db_connection) {
+					dsn = get_dsn(query->writehandle[dsn_num]);
+					if (!dsn) {
+						continue;
+					}
+					obj = dsn->connection;
+				} else {
+					obj = ast_odbc_request_obj(query->writehandle[dsn_num], 0);
 				}
-				obj = dsn->connection;
 				transactional = 0;
 			}
 
 			if (obj && (stmt = ast_odbc_direct_execute(obj, generic_execute, ast_str_buffer(buf)))) {
 				break;
 			}
-
-			dsn = release_dsn(dsn);
+			if (single_db_connection) {
+				dsn = release_dsn(dsn);
+			} else {
+				if (obj && !transactional) {
+					ast_odbc_release_obj(obj);
+					obj = NULL;
+				}
+			}
 		}
 	}
 
@@ -593,25 +607,43 @@
 			status = "SUCCESS";
 
 		} else if (query->sql_insert) {
-			dsn = release_dsn(dsn);
+			if (single_db_connection) {
+				dsn = release_dsn(dsn);
+			} else {
+				if (obj && !transactional) {
+					ast_odbc_release_obj(obj);
+					obj = NULL;
+				}
+			}
 
 			for (transactional = 0, dsn_num = 0; dsn_num < 5; dsn_num++) {
 				if (!ast_strlen_zero(query->writehandle[dsn_num])) {
 					if (transactional) {
 						/* This can only happen second time through or greater. */
 						ast_log(LOG_WARNING, "Transactions do not work well with multiple DSNs for 'writehandle'\n");
-					} else if (obj) {
-						dsn = release_dsn(dsn);
+					} else {
+						if (single_db_connection) {
+							dsn = release_dsn(dsn);
+						} else {
+							if (obj) {
+								ast_odbc_release_obj(obj);
+								obj = NULL;
+							}
+						}
 					}
 
 					if ((obj = ast_odbc_retrieve_transaction_obj(chan, query->writehandle[dsn_num]))) {
 						transactional = 1;
 					} else {
-						dsn = get_dsn(query->writehandle[dsn_num]);
-						if (!dsn) {
-							continue;
+						if (single_db_connection) {
+							dsn = get_dsn(query->writehandle[dsn_num]);
+							if (!dsn) {
+								continue;
+							}
+							obj = dsn->connection;
+						} else {
+							obj = ast_odbc_request_obj(query->writehandle[dsn_num], 0);
 						}
-						obj = dsn->connection;
 						transactional = 0;
 					}
 					if (obj) {
@@ -641,7 +673,14 @@
 		pbx_builtin_setvar_helper(chan, "ODBCSTATUS", status);
 	}
 
-	dsn = release_dsn(dsn);
+	if (single_db_connection) {
+		dsn = release_dsn(dsn);
+	} else {
+		if (obj && !transactional) {
+			ast_odbc_release_obj(obj);
+			obj = NULL;
+		}
+	}
 
 	if (!bogus_chan) {
 		ast_autoservice_stop(chan);
@@ -652,6 +691,7 @@
 
 static int acf_odbc_read(struct ast_channel *chan, const char *cmd, char *s, char *buf, size_t len)
 {
+	struct odbc_obj *obj = NULL;
 	struct acf_odbc_query *query;
 	char varname[15], rowcount[12] = "-1";
 	struct ast_str *colnames = ast_str_thread_get(&colnames_buf, 16);
@@ -757,21 +797,43 @@
 
 	for (dsn_num = 0; dsn_num < 5; dsn_num++) {
 		if (!ast_strlen_zero(query->readhandle[dsn_num])) {
-			dsn = get_dsn(query->readhandle[dsn_num]);
-			if (!dsn) {
-				continue;
+			if (single_db_connection) {
+				dsn = get_dsn(query->readhandle[dsn_num]);
+				if (!dsn) {
+					continue;
+				}
+				obj = dsn->connection;
+			} else {
+				obj = ast_odbc_request_obj(query->readhandle[dsn_num], 0);
+				if (!obj) {
+					continue;
+				}
 			}
-			stmt = ast_odbc_direct_execute(dsn->connection, generic_execute, ast_str_buffer(sql));
+			stmt = ast_odbc_direct_execute(obj, generic_execute, ast_str_buffer(sql));
 		}
 		if (stmt) {
 			break;
 		}
-		dsn = release_dsn(dsn);
+		if (single_db_connection) {
+			dsn = release_dsn(dsn);
+		} else {
+			if (obj) {
+				ast_odbc_release_obj(obj);
+				obj = NULL;
+			}
+		}
 	}
 
 	if (!stmt) {
 		ast_log(LOG_ERROR, "Unable to execute query [%s]\n", ast_str_buffer(sql));
-		dsn = release_dsn(dsn);
+		if (single_db_connection) {
+			dsn = release_dsn(dsn);
+		} else {
+			if (obj) {
+				ast_odbc_release_obj(obj);
+				obj = NULL;
+			}
+		}
 		if (!bogus_chan) {
 			pbx_builtin_setvar_helper(chan, "ODBCROWS", rowcount);
 			ast_autoservice_stop(chan);
@@ -785,7 +847,14 @@
 		ast_log(LOG_WARNING, "SQL Column Count error!\n[%s]\n\n", ast_str_buffer(sql));
 		SQLCloseCursor(stmt);
 		SQLFreeHandle (SQL_HANDLE_STMT, stmt);
-		dsn = release_dsn(dsn);
+		if (single_db_connection) {
+			dsn = release_dsn(dsn);
+		} else {
+			if (obj) {
+				ast_odbc_release_obj(obj);
+				obj = NULL;
+			}
+		}
 		if (!bogus_chan) {
 			pbx_builtin_setvar_helper(chan, "ODBCROWS", rowcount);
 			ast_autoservice_stop(chan);
@@ -809,7 +878,14 @@
 		}
 		SQLCloseCursor(stmt);
 		SQLFreeHandle(SQL_HANDLE_STMT, stmt);
-		dsn = release_dsn(dsn);
+		if (single_db_connection) {
+			dsn = release_dsn(dsn);
+		} else {
+			if (obj) {
+				ast_odbc_release_obj(obj);
+				obj = NULL;
+			}
+		}
 		if (!bogus_chan) {
 			pbx_builtin_setvar_helper(chan, "ODBCROWS", rowcount);
 			pbx_builtin_setvar_helper(chan, "ODBCSTATUS", status);
@@ -832,7 +908,14 @@
 				odbc_datastore_free(resultset);
 				SQLCloseCursor(stmt);
 				SQLFreeHandle(SQL_HANDLE_STMT, stmt);
-				dsn = release_dsn(dsn);
+				if (single_db_connection) {
+					dsn = release_dsn(dsn);
+				} else {
+					if (obj) {
+						ast_odbc_release_obj(obj);
+						obj = NULL;
+					}
+				}
 				if (!bogus_chan) {
 					pbx_builtin_setvar_helper(chan, "ODBCSTATUS", "MEMERROR");
 					ast_autoservice_stop(chan);
@@ -864,7 +947,14 @@
 						odbc_datastore_free(resultset);
 						SQLCloseCursor(stmt);
 						SQLFreeHandle(SQL_HANDLE_STMT, stmt);
-						dsn = release_dsn(dsn);
+						if (single_db_connection) {
+							dsn = release_dsn(dsn);
+						} else {
+							if (obj) {
+								ast_odbc_release_obj(obj);
+								obj = NULL;
+							}
+						}
 						if (!bogus_chan) {
 							pbx_builtin_setvar_helper(chan, "ODBCROWS", rowcount);
 							pbx_builtin_setvar_helper(chan, "ODBCSTATUS", "MEMERROR");
@@ -973,7 +1063,14 @@
 				odbc_datastore_free(resultset);
 				SQLCloseCursor(stmt);
 				SQLFreeHandle(SQL_HANDLE_STMT, stmt);
-				dsn = release_dsn(dsn);
+				if (single_db_connection) {
+					dsn = release_dsn(dsn);
+				} else {
+					if (obj) {
+						ast_odbc_release_obj(obj);
+						obj = NULL;
+					}
+				}
 				pbx_builtin_setvar_helper(chan, "ODBCSTATUS", "MEMERROR");
 				ast_autoservice_stop(chan);
 				return -1;
@@ -986,7 +1083,14 @@
 	}
 	SQLCloseCursor(stmt);
 	SQLFreeHandle(SQL_HANDLE_STMT, stmt);
-	dsn = release_dsn(dsn);
+	if (single_db_connection) {
+		dsn = release_dsn(dsn);
+	} else {
+		if (obj) {
+			ast_odbc_release_obj(obj);
+			obj = NULL;
+		}
+	}
 	if (resultset && !multirow) {
 		/* Fetch the first resultset */
 		if (!acf_fetch(chan, "", buf, buf, len)) {
@@ -1413,6 +1517,7 @@
 
 	if (a->argc == 5 && !strcmp(a->argv[4], "exec")) {
 		/* Execute the query */
+		struct odbc_obj *obj = NULL;
 		struct dsn *dsn = NULL;
 		int dsn_num, executed = 0;
 		SQLHSTMT stmt;
@@ -1432,14 +1537,28 @@
 			if (ast_strlen_zero(query->readhandle[dsn_num])) {
 				continue;
 			}
-			dsn = get_dsn(query->readhandle[dsn_num]);
-			if (!dsn) {
-				continue;
+			if (single_db_connection) {
+				dsn = get_dsn(query->readhandle[dsn_num]);
+				if (!dsn) {
+					continue;
+				}
+				obj = dsn->connection;
+			} else {
+				if (!(obj = ast_odbc_request_obj(query->readhandle[dsn_num], 0))) {
+					continue;
+				}
 			}
 			ast_debug(1, "Found handle %s\n", query->readhandle[dsn_num]);
 
-			if (!(stmt = ast_odbc_direct_execute(dsn->connection, generic_execute, ast_str_buffer(sql)))) {
-				dsn = release_dsn(dsn);
+			if (!(stmt = ast_odbc_direct_execute(obj, generic_execute, ast_str_buffer(sql)))) {
+				if (single_db_connection) {
+					dsn = release_dsn(dsn);
+				} else {
+					if (obj) {
+						ast_odbc_release_obj(obj);
+						obj = NULL;
+					}
+				}
 				continue;
 			}
 
@@ -1450,7 +1569,14 @@
 				ast_cli(a->fd, "SQL Column Count error!\n[%s]\n\n", ast_str_buffer(sql));
 				SQLCloseCursor(stmt);
 				SQLFreeHandle (SQL_HANDLE_STMT, stmt);
-				dsn = release_dsn(dsn);
+				if (single_db_connection) {
+					dsn = release_dsn(dsn);
+				} else {
+					if (obj) {
+						ast_odbc_release_obj(obj);
+						obj = NULL;
+					}
+				}
 				AST_RWLIST_UNLOCK(&queries);
 				return CLI_SUCCESS;
 			}
@@ -1459,7 +1585,14 @@
 			if ((res != SQL_SUCCESS) && (res != SQL_SUCCESS_WITH_INFO)) {
 				SQLCloseCursor(stmt);
 				SQLFreeHandle(SQL_HANDLE_STMT, stmt);
-				dsn = release_dsn(dsn);
+				if (single_db_connection) {
+					dsn = release_dsn(dsn);
+				} else {
+					if (obj) {
+						ast_odbc_release_obj(obj);
+						obj = NULL;
+					}
+				}
 				if (res == SQL_NO_DATA) {
 					ast_cli(a->fd, "Returned %d rows.  Query executed on handle %d:%s [%s]\n", rows, dsn_num, query->readhandle[dsn_num], ast_str_buffer(sql));
 					break;
@@ -1488,7 +1621,14 @@
 						ast_cli(a->fd, "SQL Get Data error %d!\n[%s]\n\n", res, ast_str_buffer(sql));
 						SQLCloseCursor(stmt);
 						SQLFreeHandle(SQL_HANDLE_STMT, stmt);
-						dsn = release_dsn(dsn);
+						if (single_db_connection) {
+							dsn = release_dsn(dsn);
+						} else {
+							if (obj) {
+								ast_odbc_release_obj(obj);
+								obj = NULL;
+							}
+						}
 						AST_RWLIST_UNLOCK(&queries);
 						return CLI_SUCCESS;
 					}
@@ -1506,11 +1646,25 @@
 			}
 			SQLCloseCursor(stmt);
 			SQLFreeHandle(SQL_HANDLE_STMT, stmt);
-			dsn = release_dsn(dsn);
+			if (single_db_connection) {
+				dsn = release_dsn(dsn);
+			} else {
+				if (obj) {
+					ast_odbc_release_obj(obj);
+					obj = NULL;
+				}
+			}
 			ast_cli(a->fd, "Returned %d row%s.  Query executed on handle %d [%s]\n", rows, rows == 1 ? "" : "s", dsn_num, query->readhandle[dsn_num]);
 			break;
 		}
-		dsn = release_dsn(dsn);
+		if (single_db_connection) {
+			dsn = release_dsn(dsn);
+		} else {
+			if (obj) {
+				ast_odbc_release_obj(obj);
+				obj = NULL;
+			}
+		}
 
 		if (!executed) {
 			ast_cli(a->fd, "Failed to execute query. [%s]\n", ast_str_buffer(sql));
@@ -1633,7 +1787,8 @@
 
 	if (a->argc == 6 && !strcmp(a->argv[5], "exec")) {
 		/* Execute the query */
-		struct dsn *dsn;
+		struct odbc_obj *obj = NULL;
+		struct dsn *dsn = NULL;
 		int dsn_num, executed = 0;
 		SQLHSTMT stmt;
 		SQLLEN rows = -1;
@@ -1642,19 +1797,40 @@
 			if (ast_strlen_zero(query->writehandle[dsn_num])) {
 				continue;
 			}
-			dsn = get_dsn(query->writehandle[dsn_num]);
-			if (!dsn) {
-				continue;
+			if (single_db_connection) {
+				dsn = get_dsn(query->writehandle[dsn_num]);
+				if (!dsn) {
+					continue;
+				}
+				obj = dsn->connection;
+			} else {
+				if (!(obj = ast_odbc_request_obj(query->writehandle[dsn_num], 0))) {
+					continue;
+				}
 			}
-			if (!(stmt = ast_odbc_direct_execute(dsn->connection, generic_execute, ast_str_buffer(sql)))) {
-				dsn = release_dsn(dsn);
+			if (!(stmt = ast_odbc_direct_execute(obj, generic_execute, ast_str_buffer(sql)))) {
+				if (single_db_connection) {
+					dsn = release_dsn(dsn);
+				} else {
+					if (obj) {
+						ast_odbc_release_obj(obj);
+						obj = NULL;
+					}
+				}
 				continue;
 			}
 
 			SQLRowCount(stmt, &rows);
 			SQLCloseCursor(stmt);
 			SQLFreeHandle(SQL_HANDLE_STMT, stmt);
-			dsn = release_dsn(dsn);
+			if (single_db_connection) {
+				dsn = release_dsn(dsn);
+			} else {
+				if (obj) {
+					ast_odbc_release_obj(obj);
+					obj = NULL;
+				}
+			}
 			ast_cli(a->fd, "Affected %d rows.  Query executed on handle %d [%s]\n", (int)rows, dsn_num, query->writehandle[dsn_num]);
 			executed = 1;
 			break;
@@ -1680,28 +1856,38 @@
 	int res = 0;
 	struct ast_config *cfg;
 	char *catg;
+	const char *s;
 	struct ast_flags config_flags = { 0 };
-
-	dsns = ao2_container_alloc(DSN_BUCKETS, dsn_hash, dsn_cmp);
-	if (!dsns) {
-		return AST_MODULE_LOAD_DECLINE;
-	}
 
 	res |= ast_custom_function_register(&fetch_function);
 	res |= ast_register_application_xml(app_odbcfinish, exec_odbcfinish);
-	AST_RWLIST_WRLOCK(&queries);
 
 	cfg = ast_config_load(config, config_flags);
 	if (!cfg || cfg == CONFIG_STATUS_FILEINVALID) {
 		ast_log(LOG_NOTICE, "Unable to load config for func_odbc: %s\n", config);
-		AST_RWLIST_UNLOCK(&queries);
-		ao2_ref(dsns, -1);
 		return AST_MODULE_LOAD_DECLINE;
 	}
 
+	if ((s = ast_variable_retrieve(cfg, "general", "single_db_connection"))) {
+		single_db_connection = ast_true(s);
+	}
+
+	if (single_db_connection) {
+		dsns = ao2_container_alloc(DSN_BUCKETS, dsn_hash, dsn_cmp);
+		if (!dsns) {
+			ast_log(LOG_ERROR, "Could not initialize DSN container\n");
+			return AST_MODULE_LOAD_DECLINE;
+		}
+		ast_log(LOG_NOTICE, "Single database connection per DSN\n");
+	}
+
+	AST_RWLIST_WRLOCK(&queries);
 	for (catg = ast_category_browse(cfg, NULL);
 	     catg;
 	     catg = ast_category_browse(cfg, catg)) {
+		if (!strcasecmp(catg, "general")) {
+			continue;
+		}
 		struct acf_odbc_query *query = NULL;
 		int err;
 
@@ -1750,7 +1936,9 @@
 
 	AST_RWLIST_UNLOCK(&queries);
 
-	ao2_ref(dsns, -1);
+	if (single_db_connection) {
+		ao2_ref(dsns, -1);
+	}
 	return res;
 }
 
@@ -1760,11 +1948,31 @@
 	struct ast_config *cfg;
 	struct acf_odbc_query *oldquery;
 	char *catg;
+	const char *s;
 	struct ast_flags config_flags = { CONFIG_FLAG_FILEUNCHANGED };
 
 	cfg = ast_config_load(config, config_flags);
 	if (cfg == CONFIG_STATUS_FILEUNCHANGED || cfg == CONFIG_STATUS_FILEINVALID)
 		return 0;
+
+	if (single_db_connection) {
+		ao2_ref(dsns, -1);
+	}
+
+	if (cfg && (s = ast_variable_retrieve(cfg, "general", "single_db_connection"))) {
+		single_db_connection = ast_true(s);
+	} else {
+		single_db_connection = DEFAULT_SINGLE_DB_CONNECTION;
+	}
+
+	if (single_db_connection) {
+		dsns = ao2_container_alloc(DSN_BUCKETS, dsn_hash, dsn_cmp);
+		if (!dsns) {
+			ast_log(LOG_ERROR, "Could not initialize DSN container\n");
+			return 0;
+		}
+		ast_log(LOG_NOTICE, "Single database connection per DSN\n");
+	}
 
 	AST_RWLIST_WRLOCK(&queries);
 
@@ -1782,6 +1990,9 @@
 	for (catg = ast_category_browse(cfg, NULL);
 	     catg;
 	     catg = ast_category_browse(cfg, catg)) {
+		if (!strcasecmp(catg, "general")) {
+			continue;
+		}
 		struct acf_odbc_query *query = NULL;
 
 		if (init_acf_query(cfg, catg, &query)) {

-- 
To view, visit https://gerrit.asterisk.org/2800
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I57d990616c957dabf7597dea5d5c3148f459dfb6
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Alexei Gradinari <alex2grad at gmail.com>



More information about the asterisk-code-review mailing list