res_odbc: Remove connection management

Asterisk by default will create a single database connection and share
it among all threads that attempt to access the database. In previous
versions of Asterisk, this was tolerable, because the most used channel
driver, chan_sip, mostly accessed the database from a single thread.
With PJSIP, however, many threads may be attempting to perform database
operations, and there is the potential for many more database accesses,
meaning the concurrency is a horrible bottleneck if only one connection
is shared.

Asterisk has a connection pooling facility built into it, but the
implementation has flaws. For one, there is a strict limit on the number
of simultaneous connections that could be made to the database. Anything
beyond the maximum would result in a failed operation. Attempting to
predict what the maximum should be is nearly impossible even for someone
intimately familiar with Asterisk's threading model. In addition, use of
transactions in the dialplan can cause some severe bugs if connection
pooling is enabled.

This commit seeks to fix the concurrency problem by removing all
connection management code from Asterisk and leaving that to the
underlying unixODBC code instead. Now, Asterisk does not share a single
connection, nor does it try to maintain a connection pool. Instead, all
Asterisk ever does is request a connection from unixODBC and allow
unixODBC to either allocate those connections or retrieve them from a
pool.

Doing this has a bit of a ripple effect. For one, since connections are
not long-lived objects, several of the safeguards that previously
existed have been removed. We don't have to worry about trying to use a
connection that has gone stale. In every case, when we request a
connection, it has just been made and we don't need to perform any
sanity checks to be sure it's still active.

Another major player affected by this change is transactions.
Transactions and their respective connections were so tightly coupled
that it was almost pornographic. This code change moves
transaction-related code to its own file separate from the core ODBC
functionality. This way, the core of ODBC does not even have to know
that transactions exist.

In making this large change, I had to look at a lot of code and
understand it. When making this change, I discovered several places
where the behavior is definitely not ideal, but it seemed outside the
scope of this change to be fixing it. Instead, any place where I saw
some sort of room for improvement has had a XXX comment added explaining
what could be altered to improve it.

Change-Id: I37a84def5ea4ddf93868ce8105f39de078297fbf

3 files changed, 114 insertions, 29 deletions

diff --git a/include/asterisk/config.h b/include/asterisk/config.h
index bd268a333..9a899d8d0 100644
--- a/include/asterisk/config.h
+++ b/include/asterisk/config.h
@@ -495,6 +495,17 @@ int ast_unload_realtime(const char *family);
  *
  * \note You should use the constant SENTINEL to terminate arguments, in
  * order to preserve cross-platform compatibility.
+ *
+ * TODO The return value of this function is routinely ignored. Ignoring
+ * the return value means that it's mostly pointless to be calling this.
+ * You'll see some warning messages potentially, but that's it.
+ *
+ * XXX This function is super useful for detecting configuration problems
+ * early, but unfortunately, the latest in configuration management, sorcery,
+ * doesn't work well with this. Users of sorcery are familiar with the fields
+ * they will need to write but don't know if realtime is being used. Sorcery
+ * knows what storage mechanism is being used but has no high-level knowledge
+ * of what sort of data is going to be written.
  */
 int ast_realtime_require_field(const char *family, ...) attribute_sentinel;
 
diff --git a/include/asterisk/res_odbc.h b/include/asterisk/res_odbc.h
index 7d9d4a19e..8c7b54950 100644
--- a/include/asterisk/res_odbc.h
+++ b/include/asterisk/res_odbc.h
@@ -46,16 +46,11 @@ enum {
 struct odbc_obj {
 	SQLHDBC  con;                   /*!< ODBC Connection Handle */
 	struct odbc_class *parent;      /*!< Information about the connection is protected */
-	struct timeval last_used;       /*!< Used by idlecheck to determine if the connection should be renegotiated */
 #ifdef DEBUG_THREADS
 	char file[80];
 	char function[80];
 	int lineno;
 #endif
-	unsigned int used:1;            /*!< Is this connection currently in use? */
-	unsigned int up:1;
-	unsigned int tx:1;              /*!< Should this connection be unshared, regardless of the class setting? */
-	struct odbc_txn_frame *txf;     /*!< Reference back to the transaction frame, if applicable */
 	AST_LIST_ENTRY(odbc_obj) list;
 };
 
@@ -102,39 +97,29 @@ int ast_odbc_smart_execute(struct odbc_obj *obj, SQLHSTMT stmt) __attribute__((d
 
 /*!
  * \brief Retrieves a connected ODBC object
- * \param name The name of the ODBC class for which a connection is needed.
- * \param flags One or more of the following flags:
- *	\li RES_ODBC_SANITY_CHECK Whether to ensure that a connection is valid before returning the handle.  Usually unnecessary.
- *	\li RES_ODBC_INDEPENDENT_CONNECTION Return a handle which is independent from all others.  Usually used when starting a transaction.
- *	\li RES_ODBC_CONNECTED Only return a connected handle.  Intended for use with peers which use idlecheck, which are checked periodically for reachability.
- * \param  file, function, lineno
  *
- * \return ODBC object
- * \retval NULL if there is no connection available with the requested name.
+ * \deprecated
  *
- * Connection classes may, in fact, contain multiple connection handles.  If
- * the connection is pooled, then each connection will be dedicated to the
- * thread which requests it.  Note that all connections should be released
- * when the thread is done by calling ast_odbc_release_obj(), below.
+ * This is only around for backwards-compatibility with older versions of Asterisk.
  */
 struct odbc_obj *_ast_odbc_request_obj2(const char *name, struct ast_flags flags, const char *file, const char *function, int lineno);
+
+/*!
+ * \brief Get a ODBC connection object
+ *
+ * The "check" parameter is leftover from an earlier implementation where database connections
+ * were cached by res_odbc. Since connections are managed by unixODBC now, this parameter is
+ * only kept around for API compatibility.
+ *
+ * \param name The name of the res_odbc.conf section describing the database to connect to
+ * \param check unused
+ * \return A connection to the database. Call ast_odbc_release_obj() when finished.
+ */
 struct odbc_obj *_ast_odbc_request_obj(const char *name, int check, const char *file, const char *function, int lineno);
 
 #define ast_odbc_request_obj2(a, b)	_ast_odbc_request_obj2(a, b, __FILE__, __PRETTY_FUNCTION__, __LINE__)
 #define ast_odbc_request_obj(a, b)	_ast_odbc_request_obj(a, b, __FILE__, __PRETTY_FUNCTION__, __LINE__)
 
-/*!
- * \brief Retrieve a stored ODBC object, if a transaction has been started.
- * \param chan Channel associated with the transaction.
- * \param objname Name of the database handle.  This name corresponds to the name passed
- * to \see ast_odbc_request_obj2 (or formerly, to ast_odbc_request_obj).  Note that the
- * existence of this parameter name explicitly allows for multiple transactions to be open
- * at once, albeit to different databases.
- * \retval A stored ODBC object, if a transaction was already started.
- * \retval NULL, if no transaction yet exists.
- */
-struct odbc_obj *ast_odbc_retrieve_transaction_obj(struct ast_channel *chan, const char *objname);
-
 /*! 
  * \brief Releases an ODBC object previously allocated by ast_odbc_request_obj()
  * \param obj The ODBC object
@@ -223,4 +208,39 @@ int ast_odbc_clear_cache(const char *database, const char *tablename);
  */
 SQLRETURN ast_odbc_ast_str_SQLGetData(struct ast_str **buf, int pmaxlen, SQLHSTMT StatementHandle, SQLUSMALLINT ColumnNumber, SQLSMALLINT TargetType, SQLLEN *StrLen_or_Ind);
 
+/*!
+ * \brief Shortcut for printing errors to logs after a failed SQL operation.
+ *
+ * \param handle_type The type of SQL handle on which to gather diagnostics
+ * \param handle The SQL handle to gather diagnostics from
+ * \param operation The name of the failed operation.
+ * \return The error string that was printed to the logs
+ */
+struct ast_str *ast_odbc_print_errors(SQLSMALLINT handle_type, SQLHANDLE handle, const char *operation);
+
+/*!
+ * \brief Get the transaction isolation setting for an ODBC class
+ */
+unsigned int ast_odbc_class_get_isolation(struct odbc_class *class);
+
+/*!
+ * \brief Get the transaction forcecommit setting for an ODBC class
+ */
+unsigned int ast_odbc_class_get_forcecommit(struct odbc_class *class);
+
+/*!
+ * \brief Get the name of an ODBC class.
+ */
+const char *ast_odbc_class_get_name(struct odbc_class *class);
+
+/*!
+ * \brief Convert from textual transaction isolation values to their numeric constants
+ */
+int ast_odbc_text2isolation(const char *txt);
+
+/*!
+ * \brief Convert from numeric transaction isolation values to their textual counterparts
+ */
+const char *ast_odbc_isolation2text(int iso);
+
 #endif /* _ASTERISK_RES_ODBC_H */
diff --git a/include/asterisk/res_odbc_transaction.h b/include/asterisk/res_odbc_transaction.h
new file mode 100644
index 000000000..b0f9316a1
--- /dev/null
+++ b/include/asterisk/res_odbc_transaction.h
@@ -0,0 +1,54 @@
+/*
+ * Asterisk -- An open source telephony toolkit.
+ *
+ * Copyright (C) 2016, Digium, Inc.
+ *
+ * Mark Michelson <mmichelson@digium.com>
+ *
+ * See http://www.asterisk.org for more information about
+ * the Asterisk project. Please do not directly contact
+ * any of the maintainers of this project for assistance;
+ * the project provides a web site, mailing lists and IRC
+ * channels for your use.
+ *
+ * This program is free software, distributed under the terms of
+ * the GNU General Public License Version 2. See the LICENSE file
+ * at the top of the source tree.
+ */
+
+#ifndef RES_ODBC_TRANSACTION_H
+#define RES_ODBC_TRANSACTION_H
+
+/*!
+ * \brief
+ *
+ * Retrieve an ODBC transaction connection with the given ODBC class name.
+ *
+ * \note The name passed here is *not* the name of the transaction but the name of the
+ * ODBC class defined in res_odbc.conf.
+ *
+ * \note Do not call ast_odbc_release_obj() on the retrieved connection. Calling this function
+ * does not make you the owner of the connection.
+ *
+ * XXX This function is majorly flawed because it ignores properties of transactions and simply
+ * finds one that corresponds to the given DSN. The problem here is that transactions have names
+ * and they maintain which transaction is "active" for operations like transaction creation,
+ * commit, and rollback. However, when it comes to intermediary operations to be made on the
+ * transactions, all that is ignored. It means that if a channel has created multiple transactions
+ * for the same DSN, it's a crapshoot which of those transactions the operation will be performed
+ * on. This can potentially lead to baffling errors under the right circumstances.
+ *
+ * XXX The semantics of this function make for writing some awkward code. If you use func_odbc as
+ * an example, it has to first try to retrieve a transactional connection, then failing that, create
+ * a non-transactional connection. The result is that it has to remember which type of connection it's
+ * using and know whether to release the connection when completed or not. It would be much better
+ * if callers did not have to jump through such hoops.
+ *
+ * \param chan Channel on which the ODBC transaction was created
+ * \param objname The name of the ODBC class configured in res_odbc.conf
+ * \retval NULL Transaction connection could not be found.
+ * \retval non-NULL A transactional connection
+ */
+struct odbc_obj *ast_odbc_retrieve_transaction_obj(struct ast_channel *chan, const char *objname);
+
+#endif /* RES_ODBC_TRANSACTION_H */