[Home]

Summary:ASTERISK-26177: func_odbc: Database handle is kept when it should be released
Reporter:Leandro Dardini (ldardini)Labels:
Date Opened:2016-07-06 15:43:35Date Closed:2016-07-12 15:15:51
Priority:MajorRegression?Yes
Status:Closed/CompleteComponents:Functions/func_odbc
Versions:13.10.0-rc1 Frequency of
Occurrence
Constant
Related
Issues:
Environment:CentOS 6.8 64 bit with unixODBC-2.3.4-1 backported from fc23 and Oracle Mysql connector 5.3.6Attachments:( 0) backtrace-threads.txt
( 1) coreshowlocks.txt
( 2) full-lock.txt
Description:When processing normal dialplan commands, including several ODBC commands, at some point, just running a simple command like "FollowMe" locks up asterisk. Running the FollowMe command with the same parameter, but without running anything before, produces no lock. The same dialplan run under asterisk 13.2.0 produces no issues.
Comments:By: Asterisk Team (asteriskteam) 2016-07-06 15:43:37.079-0500

Thanks for creating a report! The issue has entered the triage process. That means the issue will wait in this status until a Bug Marshal has an opportunity to review the issue. Once the issue has been reviewed you will receive comments regarding the next steps towards resolution.

A good first step is for you to review the [Asterisk Issue Guidelines|https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines] if you haven't already. The guidelines detail what is expected from an Asterisk issue report.

Then, if you are submitting a patch, please review the [Patch Contribution Process|https://wiki.asterisk.org/wiki/display/AST/Patch+Contribution+Process].

By: Leandro Dardini (ldardini) 2016-07-06 15:48:30.221-0500

full-lock is the result of "core set debug 3" saved in /var/log/asterisk/full
coreshowlocks.txt is the result of "core show locks" run while the system was locked
backtrace-threads.txt has been produced as info on wiki.asterisk.org

By: Joshua C. Colp (jcolp) 2016-07-06 15:58:51.799-0500

The underlying issue here is that func_odbc is keeping a database handle when it should not. It should request and release the handle for each use.

By: Alexei Gradinari (alexei gradinari) 2016-07-07 10:03:16.788-0500

Leandro,

What's value of 'single_db_connection' in func_odbc.conf?
If it's not set or set to 'yes', could you, please set to 'no' and try again.


By: Alexei Gradinari (alexei gradinari) 2016-07-07 10:09:56.670-0500

Joshua,

If single_db_connection=yes then func_odbc should keep database handle.

By: Joshua C. Colp (jcolp) 2016-07-07 10:13:18.436-0500

[~alexei gradinari] It can't, that will starve every other thread from using that database handle. It should ensure that only one is used, but it can't keep it.

By: Alexei Gradinari (alexei gradinari) 2016-07-07 11:06:24.160-0500

Joshua,

I think If single_db_connection then func_odbc should keep and lock the connection to avoid other threads to use this connection.
Now it only keeps without locking.
The single_db_connection was added because MySQL's LAST_INSERTED_ID() works per-connection and returns
"value representing the first automatically generated value successfully inserted for an AUTO_INCREMENT column as a result of the MOST RECENTLY executed INSERT statement".
If another thread executes INSERT statement into the table with the AUTO_INCREMENT column on the same connection then the result of LAST_INSERTED_ID() will be incorrect.


By: Alexei Gradinari (alexei gradinari) 2016-07-07 11:22:44.411-0500

Leandro,

What's the value of max_connections in your res_odbc.conf?

By: Alexei Gradinari (alexei gradinari) 2016-07-07 11:27:47.785-0500

Joshua,

It seems because of max_connections reached.

/* Otherwise if we're not allowed to create a new one we
* wait for another thread to give up the connection they
* own.
*/
ast_cond_wait(&class->cond, &class->lock);



By: Joshua C. Colp (jcolp) 2016-07-07 11:31:30.690-0500

Yes, it's expected that once a thread has used a connection they release it back. This is done everywhere else.

By: Alexei Gradinari (alexei gradinari) 2016-07-07 11:46:57.914-0500

But in case of func_odbc single connection we shouldn't release it because we can get incorrect value executing LAST_INSERTED_ID.
Maybe Leonardo did not pay attention to CHANGES about func_odbc and res_odbc and keep the default settings for
single_db_connection = yes
max_connections =1
Of course in this case func_odbc holds 1 connection and other threads wait for releasing it.

Maybe we should disable single_db_connection by default?


By: Joshua C. Colp (jcolp) 2016-07-07 11:51:12.468-0500

I'll be working on this issue soon and will be investigating the right path forward.

By: Leandro Dardini (ldardini) 2016-07-07 14:10:59.910-0500

I think Alexei is right. I didn't pay attention to CHANGES and left max_connections to its default value (1). Setting max_connections to 20, produces no locks.

About single_db_connection, by setting it to "no", there is no more locks, regardless the value of max_connections. Turning it back to "yes" produces immediately a lock (with max_connections to 1). From a performance point of view, having asterisk working to setup a new connection for each ODBC_ command (if I have understood correctly) then can be a big penalty.

I confirm the problem with max_connections, raising it to 20 with single_db_connection to yes produces no locks.



By: Joshua C. Colp (jcolp) 2016-07-07 14:15:54.395-0500

Current res_odbc doesn't connect/disconnect, it maintains a pool of active connections. By default it's set to previous behavior, which is 1 connection. Due to the implementation of both res_odbc and func_odbc the combination of the defaults causes the deadlock.

By: Alexei Gradinari (alexei gradinari) 2016-07-07 14:35:47.226-0500

Leandro,

The single_db_connection option should be set to 'yes' only if LAST_INSERTED_ID is used.
If you doesn't use LAST_INSERTED_ID your should set single_db_connection to 'no' for better performance.