[Home]

Summary:ASTERISK-23111: astdb entries added just before shutdown are not saved
Reporter:Tzafrir Cohen (tzafrir)Labels:
Date Opened:2014-01-07 10:39:43.000-0600Date Closed:2014-03-19 15:30:04
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Core/AstDB
Versions:SVN 11.2.0 11.7.0 Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) astdb-shutdown-commit-r2.patch
( 1) test_ast_add_exit.sh
Description:If I add a large number of entries to astdb and then stop asterisk, all or most of the entries will not be saved into the sqlite database on disk.

Attached a script that demonstrates the issue. Use the option 'clean' to delete the entries. Unrem the line with 'sleep 5' to work around the issue.

Tested currently on branch 11: both 11.7.0 and SVN (branch 11 rev. 404888  - before and after recent astdb fix and trunk rev. 371521) as well as with 11.2.0 and 10.12.1 .

Attached a script that tests the issue.
Comments:By: Tzafrir Cohen (tzafrir) 2014-01-08 10:47:23.271-0600

Also: if this is considered a feature and not a bug, an extra command along the lines of 'database sync' would be needed.
By: Matt Jordan (mjordan) 2014-01-08 13:42:57.921-0600

I would say this is a bug. We already have the opportunity on shutdown to clean up and sync with the database.

In fact, it looks like we're trying to do this right now:

{noformat}
/*! \internal \brief Clean up resources on Asterisk shutdown */
static void astdb_atexit(void)
{
       ...

/* Set doexit to 1 to kill thread. db_sync must be called with
* mutex held. */
doexit = 1;
ast_mutex_lock(&dblock);
db_sync();
ast_mutex_unlock(&dblock);

pthread_join(syncthread, NULL);
ast_mutex_lock(&dblock);
clean_statements();
if (sqlite3_close(astdb) == SQLITE_OK) {
astdb = NULL;
}
ast_mutex_unlock(&dblock);
}
{noformat}

I have to wonder if we're not giving the {{db_sync_thread}} enough time to commit the transaction before closing the SQLite connection. We may want to actually inform the thread that we're terminating and join on it before closing the SQLite connection - that would ensure that we don't close the database connection until we're actually sure that everything has been written to it.
By: Tzafrir Cohen (tzafrir) 2014-01-09 16:25:05.072-0600

The name "db_sync()" is misleading. It signals the DB thread and doesn't actually sync on its own. And indeed I see that in my case db_sync_thread() doesn't wake from ast_cond_wait(&dbcond, &dblock) in time for the shutdown.
By: Corey Farrell (coreyfarrell) 2014-01-15 19:35:41.960-0600

Please try astdb-shutdown-commit.patch.  It worked for me, tested with your script against branches/11 only.
By: Corey Farrell (coreyfarrell) 2014-01-16 02:44:11.126-0600

First patch incorrectly unlocked dblock before the final ast_db_commit_transaction.
By: Corey Farrell (coreyfarrell) 2014-02-24 18:38:34.527-0600

My last comment was unclear, [^astdb-shutdown-commit-r2.patch] has locking in all the correct places.  It works on my system but since this could be timing dependent I'd like to get confirmation it fixes the issue for you before I post to RB.
By: Rusty Newton (rnewton) 2014-03-11 13:47:37.221-0500

This one has been silent a couple weeks. Recycling status to ping Tzafrir.
By: Tzafrir Cohen (tzafrir) 2014-03-19 12:19:51.337-0500

Branch 11 and trunk work well. Problem fixed, thanks.
By: Corey Farrell (coreyfarrell) 2014-03-19 12:52:51.761-0500

Looks like mmichelson committed a fix for this (r410556 & r410606 in Branch 11), so it appears my patch is unneeded?
By: Richard Mudgett (rmudgett) 2014-03-19 15:30:04.324-0500

Closing issue as fixed because of mmichelson's commit of a fix.