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-0600 | Date Closed: | 2014-03-19 15:30:04 |
Priority: | Minor | Regression? | No |
Status: | Closed/Complete | Components: | 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. |