[Asterisk-code-review] res odbc: Remove connection management (asterisk[13])

[Mark Michelson]

Hello Anonymous Coward #1000019, Joshua Colp,

I'd like you to reexamine a change.  Please visit

    https://gerrit.asterisk.org/2005

to look at the new patch set (#2).

Change subject: res_odbc: Remove connection management
......................................................................

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
---
M UPGRADE.txt
M funcs/func_odbc.c
M include/asterisk/config.h
M include/asterisk/res_odbc.h
A include/asterisk/res_odbc_transaction.h
M res/res_odbc.c
M res/res_odbc.exports.in
A res/res_odbc_transaction.c
A res/res_odbc_transaction.exports.in
9 files changed, 741 insertions(+), 1,093 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/05/2005/2
-- 
To view, visit https://gerrit.asterisk.org/2005
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I37a84def5ea4ddf93868ce8105f39de078297fbf
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>