func_odbc: single database connection should be optional

func_odbc was changed in Asterisk 13.9.0
to make func_odbc use a single database connection per DSN
because of reported bug ASTERISK-25938
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.

ASTERISK-26010

Change-Id: I7091783a7150252de8eeb455115bd00514dfe843

CHANGES

@@ -183,6 +183,8 @@ Core
 
 Functions
 ------------------
+ * The func_odbc global option "single_db_connection" default value has been
+   changed to 'no'.
 
 CHANNEL
 ------------------
@@ -275,6 +277,12 @@ Queue
 --- 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 enabled by default.
+
 res_fax
 ------------------
  * Added FAXMODE variable to let dialplan know what fax transport was used.

configs/samples/func_odbc.conf.sample

@@ -1,6 +1,20 @@
 ;
 ; 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 exists for those who expect that a second func_odbc call
+; works on the same connection. That allows you to do a LAST_INSERT_ID()
+; in a second func_odbc call.
+; Note that you'll need additional dialplan locks for this behaviour to work.
+; There are better ways: using stored procedures/functions instead.
+; 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

funcs/func_odbc.c

@@ -101,6 +101,12 @@ ASTERISK_REGISTER_FILE()
 
 static char *config = "func_odbc.conf";
 
+#define DEFAULT_SINGLE_DB_CONNECTION 0
+
+static int single_db_connection;
+
+AST_RWLOCK_DEFINE_STATIC(single_db_connection_lock);
+
 enum odbc_option_flags {
 	OPT_ESCAPECOMMAS =	(1 << 0),
 	OPT_MULTIROW     =	(1 << 1),
@@ -221,6 +227,10 @@ static struct dsn *create_dsn(const char *name)
 {
 	struct dsn *dsn;
 
+	if (!dsns) {
+		return NULL;
+	}
+
 	dsn = ao2_alloc(sizeof(*dsn) + strlen(name) + 1, dsn_destructor);
 	if (!dsn) {
 		return NULL;
@@ -296,6 +306,10 @@ static struct dsn *get_dsn(const char *name)
 {
 	struct dsn *dsn;
 
+	if (!dsns) {
+		return NULL;
+	}
+
 	ao2_lock(dsns);
 	dsn = ao2_find(dsns, name, OBJ_SEARCH_KEY | OBJ_NOLOCK);
 	if (!dsn) {
@@ -332,21 +346,55 @@ static struct dsn *get_dsn(const char *name)
 }
 
 /*!
- * \brief Unlock and unreference a DSN
+ * \brief Get a DB handle via a DSN or directly
  *
- * \param dsn The dsn to unlock and unreference
+ * If single db connection then get the DB handle via DSN
- * \return NULL
+ * else by requesting a connection directly
+ *
+ * \param dsn_name Name of the DSN as found in res_odbc.conf
+ * \param dsn The pointer to the DSN
+ * \retval NULL Unable to retrieve the DB handle
+ * \retval non-NULL The retrieved DB handle
  */
-static void *release_dsn(struct dsn *dsn)
+static struct odbc_obj *get_odbc_obj(const char *dsn_name, struct dsn **dsn)
 {
-	if (!dsn) {
+	struct odbc_obj *obj = NULL;
-		return NULL;
+
+	ast_rwlock_rdlock(&single_db_connection_lock);
+	if (single_db_connection) {
+		if (dsn) {
+			*dsn = get_dsn(dsn_name);
+			if (*dsn) {
+				obj = (*dsn)->connection;
+			}
+		}
+	} else {
+		obj = ast_odbc_request_obj(dsn_name, 0);
 	}
+	ast_rwlock_unlock(&single_db_connection_lock);
 
-	ao2_unlock(dsn);
+	return obj;
-	ao2_ref(dsn, -1);
+}
 
-	return NULL;
+/*!
+ * \brief Release an ODBC obj or a DSN
+ *
+ * If single db connection then unlock and unreference the DSN
+ * else release the ODBC obj
+ *
+ * \param obj The pointer to the ODBC obj to release
+ * \param dsn The pointer to the dsn to unlock and unreference
+ */
+static inline void release_obj_or_dsn(struct odbc_obj **obj, struct dsn **dsn)
+{
+	if (dsn && *dsn) {
+		ao2_unlock(*dsn);
+		ao2_ref(*dsn, -1);
+		*dsn = NULL;
+	} else if (obj && *obj) {
+		ast_odbc_release_obj(*obj);
+		*obj = NULL;
+	}
 }
 
 static AST_RWLIST_HEAD_STATIC(queries, acf_odbc_query);
@@ -568,19 +616,16 @@ static int acf_odbc_write(struct ast_channel *chan, const char *cmd, char *s, co
 			if ((obj = ast_odbc_retrieve_transaction_obj(chan, query->writehandle[dsn_num]))) {
 				transactional = 1;
 			} else {
-				dsn = get_dsn(query->writehandle[dsn_num]);
+				obj = get_odbc_obj(query->writehandle[dsn_num], &dsn);
-				if (!dsn) {
-					continue;
-				}
-				obj = dsn->connection;
 				transactional = 0;
 			}
 
 			if (obj && (stmt = ast_odbc_direct_execute(obj, generic_execute, ast_str_buffer(buf)))) {
 				break;
 			}
-
+			if (!transactional) {
-			dsn = release_dsn(dsn);
+				release_obj_or_dsn (&obj, &dsn);
+			}
 		}
 	}
 
@@ -593,25 +638,23 @@ static int acf_odbc_write(struct ast_channel *chan, const char *cmd, char *s, co
 			status = "SUCCESS";
 
 		} else if (query->sql_insert) {
-			dsn = release_dsn(dsn);
+			if (!transactional) {
+				release_obj_or_dsn (&obj, &dsn);
+			}
 
 			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) {
+					} else {
-						dsn = release_dsn(dsn);
+						release_obj_or_dsn (&obj, &dsn);
 					}
 
 					if ((obj = ast_odbc_retrieve_transaction_obj(chan, query->writehandle[dsn_num]))) {
 						transactional = 1;
 					} else {
-						dsn = get_dsn(query->writehandle[dsn_num]);
+						obj = get_odbc_obj(query->writehandle[dsn_num], &dsn);
-						if (!dsn) {
-							continue;
-						}
-						obj = dsn->connection;
 						transactional = 0;
 					}
 					if (obj) {
@@ -641,7 +684,9 @@ static int acf_odbc_write(struct ast_channel *chan, const char *cmd, char *s, co
 		pbx_builtin_setvar_helper(chan, "ODBCSTATUS", status);
 	}
 
-	dsn = release_dsn(dsn);
+	if (!transactional) {
+		release_obj_or_dsn (&obj, &dsn);
+	}
 
 	if (!bogus_chan) {
 		ast_autoservice_stop(chan);
@@ -652,6 +697,7 @@ static int acf_odbc_write(struct ast_channel *chan, const char *cmd, char *s, co
 
 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 +803,21 @@ static int acf_odbc_read(struct ast_channel *chan, const char *cmd, char *s, cha
 
 	for (dsn_num = 0; dsn_num < 5; dsn_num++) {
 		if (!ast_strlen_zero(query->readhandle[dsn_num])) {
-			dsn = get_dsn(query->readhandle[dsn_num]);
+			obj = get_odbc_obj(query->readhandle[dsn_num], &dsn);
-			if (!dsn) {
+			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);
+		release_obj_or_dsn (&obj, &dsn);
 	}
 
 	if (!stmt) {
 		ast_log(LOG_ERROR, "Unable to execute query [%s]\n", ast_str_buffer(sql));
-		dsn = release_dsn(dsn);
+		release_obj_or_dsn (&obj, &dsn);
 		if (!bogus_chan) {
 			pbx_builtin_setvar_helper(chan, "ODBCROWS", rowcount);
 			ast_autoservice_stop(chan);
@@ -785,7 +831,7 @@ static int acf_odbc_read(struct ast_channel *chan, const char *cmd, char *s, cha
 		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);
+		release_obj_or_dsn (&obj, &dsn);
 		if (!bogus_chan) {
 			pbx_builtin_setvar_helper(chan, "ODBCROWS", rowcount);
 			ast_autoservice_stop(chan);
@@ -809,7 +855,7 @@ static int acf_odbc_read(struct ast_channel *chan, const char *cmd, char *s, cha
 		}
 		SQLCloseCursor(stmt);
 		SQLFreeHandle(SQL_HANDLE_STMT, stmt);
-		dsn = release_dsn(dsn);
+		release_obj_or_dsn (&obj, &dsn);
 		if (!bogus_chan) {
 			pbx_builtin_setvar_helper(chan, "ODBCROWS", rowcount);
 			pbx_builtin_setvar_helper(chan, "ODBCSTATUS", status);
@@ -832,7 +878,7 @@ static int acf_odbc_read(struct ast_channel *chan, const char *cmd, char *s, cha
 				odbc_datastore_free(resultset);
 				SQLCloseCursor(stmt);
 				SQLFreeHandle(SQL_HANDLE_STMT, stmt);
-				dsn = release_dsn(dsn);
+				release_obj_or_dsn (&obj, &dsn);
 				if (!bogus_chan) {
 					pbx_builtin_setvar_helper(chan, "ODBCSTATUS", "MEMERROR");
 					ast_autoservice_stop(chan);
@@ -864,7 +910,7 @@ static int acf_odbc_read(struct ast_channel *chan, const char *cmd, char *s, cha
 						odbc_datastore_free(resultset);
 						SQLCloseCursor(stmt);
 						SQLFreeHandle(SQL_HANDLE_STMT, stmt);
-						dsn = release_dsn(dsn);
+						release_obj_or_dsn (&obj, &dsn);
 						if (!bogus_chan) {
 							pbx_builtin_setvar_helper(chan, "ODBCROWS", rowcount);
 							pbx_builtin_setvar_helper(chan, "ODBCSTATUS", "MEMERROR");
@@ -973,7 +1019,7 @@ end_acf_read:
 				odbc_datastore_free(resultset);
 				SQLCloseCursor(stmt);
 				SQLFreeHandle(SQL_HANDLE_STMT, stmt);
-				dsn = release_dsn(dsn);
+				release_obj_or_dsn (&obj, &dsn);
 				pbx_builtin_setvar_helper(chan, "ODBCSTATUS", "MEMERROR");
 				ast_autoservice_stop(chan);
 				return -1;
@@ -986,7 +1032,7 @@ end_acf_read:
 	}
 	SQLCloseCursor(stmt);
 	SQLFreeHandle(SQL_HANDLE_STMT, stmt);
-	dsn = release_dsn(dsn);
+	release_obj_or_dsn (&obj, &dsn);
 	if (resultset && !multirow) {
 		/* Fetch the first resultset */
 		if (!acf_fetch(chan, "", buf, buf, len)) {
@@ -1413,6 +1459,7 @@ static char *cli_odbc_read(struct ast_cli_entry *e, int cmd, struct ast_cli_args
 
 	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 +1479,14 @@ static char *cli_odbc_read(struct ast_cli_entry *e, int cmd, struct ast_cli_args
 			if (ast_strlen_zero(query->readhandle[dsn_num])) {
 				continue;
 			}
-			dsn = get_dsn(query->readhandle[dsn_num]);
+			obj = get_odbc_obj(query->readhandle[dsn_num], &dsn);
-			if (!dsn) {
+			if (!obj) {
 				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)))) {
+			if (!(stmt = ast_odbc_direct_execute(obj, generic_execute, ast_str_buffer(sql)))) {
-				dsn = release_dsn(dsn);
+				release_obj_or_dsn (&obj, &dsn);
 				continue;
 			}
 
@@ -1450,7 +1497,7 @@ static char *cli_odbc_read(struct ast_cli_entry *e, int cmd, struct ast_cli_args
 				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);
+				release_obj_or_dsn (&obj, &dsn);
 				AST_RWLIST_UNLOCK(&queries);
 				return CLI_SUCCESS;
 			}
@@ -1459,7 +1506,7 @@ static char *cli_odbc_read(struct ast_cli_entry *e, int cmd, struct ast_cli_args
 			if ((res != SQL_SUCCESS) && (res != SQL_SUCCESS_WITH_INFO)) {
 				SQLCloseCursor(stmt);
 				SQLFreeHandle(SQL_HANDLE_STMT, stmt);
-				dsn = release_dsn(dsn);
+				release_obj_or_dsn (&obj, &dsn);
 				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 +1535,7 @@ static char *cli_odbc_read(struct ast_cli_entry *e, int cmd, struct ast_cli_args
 						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);
+						release_obj_or_dsn (&obj, &dsn);
 						AST_RWLIST_UNLOCK(&queries);
 						return CLI_SUCCESS;
 					}
@@ -1506,11 +1553,11 @@ static char *cli_odbc_read(struct ast_cli_entry *e, int cmd, struct ast_cli_args
 			}
 			SQLCloseCursor(stmt);
 			SQLFreeHandle(SQL_HANDLE_STMT, stmt);
-			dsn = release_dsn(dsn);
+			release_obj_or_dsn (&obj, &dsn);
 			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);
+		release_obj_or_dsn (&obj, &dsn);
 
 		if (!executed) {
 			ast_cli(a->fd, "Failed to execute query. [%s]\n", ast_str_buffer(sql));
@@ -1633,7 +1680,8 @@ static char *cli_odbc_write(struct ast_cli_entry *e, int cmd, struct ast_cli_arg
 
 	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 +1690,19 @@ static char *cli_odbc_write(struct ast_cli_entry *e, int cmd, struct ast_cli_arg
 			if (ast_strlen_zero(query->writehandle[dsn_num])) {
 				continue;
 			}
-			dsn = get_dsn(query->writehandle[dsn_num]);
+			obj = get_odbc_obj(query->writehandle[dsn_num], &dsn);
-			if (!dsn) {
+			if (!obj) {
 				continue;
 			}
-			if (!(stmt = ast_odbc_direct_execute(dsn->connection, generic_execute, ast_str_buffer(sql)))) {
+			if (!(stmt = ast_odbc_direct_execute(obj, generic_execute, ast_str_buffer(sql)))) {
-				dsn = release_dsn(dsn);
+				release_obj_or_dsn (&obj, &dsn);
 				continue;
 			}
 
 			SQLRowCount(stmt, &rows);
 			SQLCloseCursor(stmt);
 			SQLFreeHandle(SQL_HANDLE_STMT, stmt);
-			dsn = release_dsn(dsn);
+			release_obj_or_dsn (&obj, &dsn);
 			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,31 +1728,48 @@ static int load_module(void)
 	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;
 	}
 
+	ast_rwlock_wrlock(&single_db_connection_lock);
+	if ((s = ast_variable_retrieve(cfg, "general", "single_db_connection"))) {
+		single_db_connection = ast_true(s);
+	} else {
+		single_db_connection = DEFAULT_SINGLE_DB_CONNECTION;
+	}
+
+	dsns = NULL;
+
+	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");
+			ast_rwlock_unlock(&single_db_connection_lock);
+			return AST_MODULE_LOAD_DECLINE;
+		}
+	}
+	ast_rwlock_unlock(&single_db_connection_lock);
+
+	AST_RWLIST_WRLOCK(&queries);
 	for (catg = ast_category_browse(cfg, NULL);
 	     catg;
 	     catg = ast_category_browse(cfg, catg)) {
 		struct acf_odbc_query *query = NULL;
 		int err;
 
+		if (!strcasecmp(catg, "general")) {
+			continue;
+		}
+
 		if ((err = init_acf_query(cfg, catg, &query))) {
 			if (err == ENOMEM)
 				ast_log(LOG_ERROR, "Out of memory\n");
@@ -1750,7 +1815,9 @@ static int unload_module(void)
 
 	AST_RWLIST_UNLOCK(&queries);
 
-	ao2_ref(dsns, -1);
+	if (dsns) {
+		ao2_ref(dsns, -1);
+	}
 	return res;
 }
 
@@ -1760,12 +1827,36 @@ static int reload(void)
 	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;
 
+	ast_rwlock_wrlock(&single_db_connection_lock);
+
+	if (dsns) {
+		ao2_ref(dsns, -1);
+		dsns = NULL;
+	}
+
+	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");
+			ast_rwlock_unlock(&single_db_connection_lock);
+			return 0;
+		}
+	}
+	ast_rwlock_unlock(&single_db_connection_lock);
+
 	AST_RWLIST_WRLOCK(&queries);
 
 	while (!AST_RWLIST_EMPTY(&queries)) {
@@ -1784,6 +1875,10 @@ static int reload(void)
 	     catg = ast_category_browse(cfg, catg)) {
 		struct acf_odbc_query *query = NULL;
 
+		if (!strcasecmp(catg, "general")) {
+			continue;
+		}
+
 		if (init_acf_query(cfg, catg, &query)) {
 			ast_log(LOG_ERROR, "Cannot initialize query %s\n", catg);
 		} else {