func_odbc: single database connection should be optional
authorAlexei Gradinari <alex2grad@gmail.com>
Thu, 12 May 2016 20:18:22 +0000 (16:18 -0400)
committerAlexei Gradinari <alex2grad@gmail.com>
Fri, 20 May 2016 17:46:03 +0000 (13:46 -0400)
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
configs/samples/func_odbc.conf.sample
funcs/func_odbc.c

diff --git a/CHANGES b/CHANGES
index ec44e30..0e99ab9 100644 (file)
--- a/CHANGES
+++ b/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.
index fd528d2..f4d90b7 100644 (file)
@@ -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
index d8ffd90..489f373 100644 (file)
@@ -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
- * \return NULL
+ * If single db connection then get the DB handle via DSN
+ * 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) {
-               return NULL;
+       struct odbc_obj *obj = 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);
-       ao2_ref(dsn, -1);
+       return obj;
+}
 
-       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]);
-                               if (!dsn) {
-                                       continue;
-                               }
-                               obj = dsn->connection;
+                               obj = get_odbc_obj(query->writehandle[dsn_num], &dsn);
                                transactional = 0;
                        }
 
                        if (obj && (stmt = ast_odbc_direct_execute(obj, generic_execute, ast_str_buffer(buf)))) {
                                break;
                        }
-
-                       dsn = release_dsn(dsn);
+                       if (!transactional) {
+                               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) {
-                                               dsn = release_dsn(dsn);
+                                       } else {
+                                               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]);
-                                               if (!dsn) {
-                                                       continue;
-                                               }
-                                               obj = dsn->connection;
+                                               obj = get_odbc_obj(query->writehandle[dsn_num], &dsn);
                                                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]);
-                       if (!dsn) {
+                       obj = get_odbc_obj(query->readhandle[dsn_num], &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]);
-                       if (!dsn) {
+                       obj = get_odbc_obj(query->readhandle[dsn_num], &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)))) {
-                               dsn = release_dsn(dsn);
+                       if (!(stmt = ast_odbc_direct_execute(obj, generic_execute, ast_str_buffer(sql)))) {
+                               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]);
-                       if (!dsn) {
+                       obj = get_odbc_obj(query->writehandle[dsn_num], &dsn);
+                       if (!obj) {
                                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)))) {
+                               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 {