gnucash maint: Multiple changes pushed
John Ralls
jralls at code.gnucash.org
Sat Feb 4 20:37:11 EST 2017
Updated via https://github.com/Gnucash/gnucash/commit/93301cd2 (commit)
via https://github.com/Gnucash/gnucash/commit/a70637f3 (commit)
via https://github.com/Gnucash/gnucash/commit/979e6397 (commit)
via https://github.com/Gnucash/gnucash/commit/db73c39b (commit)
from https://github.com/Gnucash/gnucash/commit/b682fe6b (commit)
commit 93301cd285a88eb5b58b2902d23118bfa9edbc44
Author: John Ralls <jralls at ceridwen.us>
Date: Sat Feb 4 17:36:09 2017 -0800
Make gnc_dbi_safe_sync_all safer.
Creates a new safe_sync function in struct provider and a new
gnc_db_do_safe_sync_all function with the guts of gnc_dbi_do_safe_sync_all.
The last calls the provider’s safe_sync function, which for SQLite3 and
PGSql wraps the call to gnc_dbi_do_safe_sync_all in a SQL transaction.
Unfortunately MySQL commits the transaction on the first schema-altering
query (CREATE_TABLE in this case) without decrementing sql_savepoint, so
raising an error when we try to release the (non-existent) save point at
the end of writing the tables.
Add a gnc_dbi_check_and_rollback_failed_save() to restore the database
after a failed safe_save; this is performed at the next connection.
diff --git a/src/backend/dbi/gnc-backend-dbi-priv.h b/src/backend/dbi/gnc-backend-dbi-priv.h
index 6e816e8..dab3331 100644
--- a/src/backend/dbi/gnc-backend-dbi-priv.h
+++ b/src/backend/dbi/gnc-backend-dbi-priv.h
@@ -63,6 +63,7 @@ typedef GSList* (*GET_TABLE_LIST_FN) ( dbi_conn conn, const gchar* dbname );
typedef void (*APPEND_COLUMN_DEF_FN) ( GString* ddl, GncSqlColumnInfo* info );
typedef GSList* (*GET_INDEX_LIST_FN) ( dbi_conn conn );
typedef void (*DROP_INDEX_FN) ( dbi_conn conn, const gchar* index );
+typedef void (*SAFE_SYNC) ( QofBackend *qbe, QofBook * book );
typedef struct
{
CREATE_TABLE_DDL_FN create_table_ddl;
@@ -70,6 +71,7 @@ typedef struct
APPEND_COLUMN_DEF_FN append_col_def;
GET_INDEX_LIST_FN get_index_list;
DROP_INDEX_FN drop_index;
+ SAFE_SYNC safe_sync;
} provider_functions_t;
diff --git a/src/backend/dbi/gnc-backend-dbi.c b/src/backend/dbi/gnc-backend-dbi.c
index 215c658..912a9fa 100644
--- a/src/backend/dbi/gnc-backend-dbi.c
+++ b/src/backend/dbi/gnc-backend-dbi.c
@@ -101,13 +101,16 @@ static GSList* conn_get_table_list_sqlite3( dbi_conn conn, const gchar* dbname )
static void append_sqlite3_col_def( GString* ddl, GncSqlColumnInfo* info );
static GSList *conn_get_index_list_sqlite3( dbi_conn conn );
static void conn_drop_index_sqlite3 (dbi_conn conn, const gchar *index );
+static void conn_safe_sync_sqlite3 (QofBackend* qbe, QofBook *book);
+
static provider_functions_t provider_sqlite3 =
{
conn_create_table_ddl_sqlite3,
conn_get_table_list_sqlite3,
append_sqlite3_col_def,
conn_get_index_list_sqlite3,
- conn_drop_index_sqlite3
+ conn_drop_index_sqlite3,
+ conn_safe_sync_sqlite3
};
#define SQLITE3_TIMESPEC_STR_FORMAT "%04d%02d%02d%02d%02d%02d"
@@ -117,13 +120,16 @@ static /*@ null @*/ gchar* conn_create_table_ddl_mysql( GncSqlConnection* conn,
static void append_mysql_col_def( GString* ddl, GncSqlColumnInfo* info );
static GSList *conn_get_index_list_mysql( dbi_conn conn );
static void conn_drop_index_mysql (dbi_conn conn, const gchar *index );
+static void conn_safe_sync_mysql (QofBackend* qbe, QofBook *book);
+
static provider_functions_t provider_mysql =
{
conn_create_table_ddl_mysql,
conn_get_table_list,
append_mysql_col_def,
conn_get_index_list_mysql,
- conn_drop_index_mysql
+ conn_drop_index_mysql,
+ conn_safe_sync_mysql
};
#define MYSQL_TIMESPEC_STR_FORMAT "%04d%02d%02d%02d%02d%02d"
@@ -134,6 +140,7 @@ static GSList* conn_get_table_list_pgsql( dbi_conn conn, const gchar* dbname );
static void append_pgsql_col_def( GString* ddl, GncSqlColumnInfo* info );
static GSList *conn_get_index_list_pgsql( dbi_conn conn );
static void conn_drop_index_pgsql (dbi_conn conn, const gchar *index );
+static void conn_safe_sync_pgsql (QofBackend* qbe, QofBook *book);
static provider_functions_t provider_pgsql =
{
@@ -141,14 +148,18 @@ static provider_functions_t provider_pgsql =
conn_get_table_list_pgsql,
append_pgsql_col_def,
conn_get_index_list_pgsql,
- conn_drop_index_pgsql
+ conn_drop_index_pgsql,
+ conn_safe_sync_pgsql
};
#define PGSQL_TIMESPEC_STR_FORMAT "%04d%02d%02d %02d%02d%02d"
static gboolean gnc_dbi_lock_database( QofBackend *qbe, gboolean ignore_lock );
static void gnc_dbi_unlock( QofBackend *qbe );
+static gboolean gnc_dbi_check_and_rollback_failed_save(QofBackend *qbe);
static gboolean save_may_clobber_data( QofBackend* qbe );
-
+static gboolean gnc_dbi_do_safe_sync_all(QofBackend* qbe, QofBook *book);
+static gboolean conn_table_operation(GncSqlConnection *sql_conn,
+ GSList *table_name_list, TableOpType op);
static /*@ null @*/ gchar* create_index_ddl( GncSqlConnection* conn,
const gchar* index_name,
const gchar* table_name,
@@ -227,7 +238,7 @@ gnc_dbi_transaction_begin(QofBackend* qbe, dbi_conn conn)
{
dbi_result result;
if (sql_savepoint == 0)
- result = dbi_conn_queryf(conn, "BEGIN");
+ result = dbi_conn_queryf(conn, "BEGIN");
else
{
char *savepoint = g_strdup_printf("savepoint_%d", sql_savepoint);
@@ -255,7 +266,7 @@ gnc_dbi_transaction_commit(QofBackend *qbe, dbi_conn conn)
dbi_result result;
g_return_val_if_fail(sql_savepoint > 0, FALSE);
if (sql_savepoint == 1)
- result = dbi_conn_queryf(conn, "COMMIT");
+ result = dbi_conn_queryf(conn, "COMMIT");
else
{
char *savepoint = g_strdup_printf("savepoint_%d", sql_savepoint - 1);
@@ -285,7 +296,7 @@ gnc_dbi_transaction_rollback(QofBackend *qbe, dbi_conn conn)
DEBUG( "ROLLBACK\n" );
g_return_val_if_fail(sql_savepoint > 0, FALSE);
if (sql_savepoint == 1)
- result = dbi_conn_queryf(conn, "ROLLBACK");
+ result = dbi_conn_queryf(conn, "ROLLBACK");
else
{
char *savepoint = g_strdup_printf("savepoint_%d", sql_savepoint - 1);
@@ -308,6 +319,60 @@ gnc_dbi_transaction_rollback(QofBackend *qbe, dbi_conn conn)
return FALSE;
}
+static gboolean
+gnc_dbi_do_safe_sync_all(QofBackend* qbe, QofBook* book)
+{
+ GncDbiBackend *be = (GncDbiBackend*)qbe;
+ GncDbiSqlConnection *conn = (GncDbiSqlConnection*)(((GncSqlBackend*)be)->conn);
+ GSList *table_list, *index_list, *iter;
+ const gchar* dbname = NULL;
+ dbname = dbi_conn_get_option( be->conn, "dbname" );
+ table_list = conn->provider->get_table_list( conn->conn, dbname );
+ if ( !conn_table_operation( (GncSqlConnection*)conn, table_list,
+ backup ) )
+ {
+ qof_backend_set_error( qbe, ERR_BACKEND_SERVER_ERR );
+ conn_table_operation( (GncSqlConnection*)conn, table_list,
+ rollback );
+ LEAVE( "Failed to rename tables" );
+ gnc_table_slist_free( table_list );
+ return FALSE;
+ }
+ index_list = conn->provider->get_index_list( conn->conn );
+ for ( iter = index_list; iter != NULL; iter = g_slist_next( iter) )
+ {
+ const char *errmsg;
+ conn->provider->drop_index (conn->conn, iter->data);
+ if ( DBI_ERROR_NONE != dbi_conn_error( conn->conn, &errmsg ) )
+ {
+ qof_backend_set_error( qbe, ERR_BACKEND_SERVER_ERR );
+ gnc_table_slist_free( index_list );
+ conn_table_operation( (GncSqlConnection*)conn, table_list,
+ rollback );
+ gnc_table_slist_free( table_list );
+ LEAVE( "Failed to drop indexes %s", errmsg );
+ return FALSE;
+ }
+ }
+ gnc_table_slist_free( index_list );
+
+ be->is_pristine_db = TRUE;
+ be->primary_book = book;
+
+ gnc_sql_sync_all( &be->sql_be, book );
+ if (qof_backend_check_error (qbe))
+ {
+ conn_table_operation( (GncSqlConnection*)conn, table_list,
+ rollback );
+ gnc_dbi_transaction_rollback(qbe, be->conn);
+ LEAVE( "Failed to create new database tables" );
+ return FALSE;
+ }
+ conn_table_operation( (GncSqlConnection*)conn, table_list,
+ drop_backup );
+ gnc_table_slist_free( table_list );
+ return TRUE;
+}
/* ================================================================= */
static void
@@ -469,14 +534,17 @@ gnc_dbi_sqlite3_session_begin( QofBackend *qbe, QofSession *session,
msg = "Locked";
goto exit;
}
-
if ( be->sql_be.conn != NULL )
{
gnc_sql_connection_dispose( be->sql_be.conn );
}
be->sql_be.conn = create_dbi_connection( GNC_DBI_PROVIDER_SQLITE, qbe, be->conn );
be->sql_be.timespec_format = SQLITE3_TIMESPEC_STR_FORMAT;
-
+ if (! gnc_dbi_check_and_rollback_failed_save(qbe))
+ {
+ gnc_sql_connection_dispose(be->sql_be.conn);
+ goto exit;
+ }
/* We should now have a proper session set up.
* Let's start logging */
xaccLogSetBaseName (filepath);
@@ -520,6 +588,24 @@ conn_drop_index_sqlite3 (dbi_conn conn, const gchar *index )
}
static void
+conn_safe_sync_sqlite3 (QofBackend* qbe, QofBook *book)
+{
+ GncDbiBackend *be = (GncDbiBackend*)qbe;
+ if (!gnc_dbi_transaction_begin(qbe, be->conn))
+ {
+ qof_backend_set_error( qbe, ERR_BACKEND_SERVER_ERR );
+ qof_backend_set_message(qbe, "Backend save failed, couldn't obtain the lock.");
+ }
+ if (!gnc_dbi_do_safe_sync_all(qbe, book))
+ gnc_dbi_transaction_rollback(qbe, be->conn);
+ else if (!gnc_dbi_transaction_commit(qbe, be->conn))
+ {
+ qof_backend_set_error(qbe, ERR_BACKEND_SERVER_ERR);
+ qof_backend_set_message(qbe, "Save failed, unable to commit the SQL transaction.");
+ }
+}
+
+static void
mysql_error_fn( dbi_conn conn, void* user_data )
{
GncDbiBackend *be = (GncDbiBackend*)user_data;
@@ -716,43 +802,24 @@ gnc_dbi_lock_database ( QofBackend* qbe, gboolean ignore_lock )
result = NULL;
}
- /* Check for an existing entry; delete it if ignore_lock is true, otherwise fail */
- result = dbi_conn_queryf( dcon, "SELECT * FROM %s", lock_table );
- if ( result && dbi_result_get_numrows( result ) )
+ /* Check for an existing entry; delete it if ignore_lock is true, otherwise fail */
+ result = dbi_conn_queryf( dcon, "SELECT * FROM %s", lock_table );
+ if ( result && dbi_result_get_numrows( result ) )
+ {
+ dbi_result_free( result );
+ result = NULL;
+ if ( !ignore_lock )
{
- dbi_result_free( result );
- result = NULL;
- if ( !ignore_lock )
- {
- qof_backend_set_error( qbe, ERR_BACKEND_LOCKED );
- /* FIXME: After enhancing the qof_backend_error mechanism, report in the dialog what is the hostname of the machine holding the lock. */
- gnc_dbi_transaction_rollback(qbe, dcon);
- return FALSE;
- }
- result = dbi_conn_queryf( dcon, "DELETE FROM %s", lock_table );
- if ( !result)
- {
- qof_backend_set_error( qbe, ERR_BACKEND_SERVER_ERR );
- qof_backend_set_message( qbe, "Failed to delete lock record" );
- gnc_dbi_transaction_rollback(qbe, dcon);
- return FALSE;
- }
- if (result)
- {
- dbi_result_free( result );
- result = NULL;
- }
+ qof_backend_set_error( qbe, ERR_BACKEND_LOCKED );
+ /* FIXME: After enhancing the qof_backend_error mechanism, report in the dialog what is the hostname of the machine holding the lock. */
+ gnc_dbi_transaction_rollback(qbe, dcon);
+ return FALSE;
}
- /* Add an entry and commit the transaction */
- memset( hostname, 0, sizeof(hostname) );
- gethostname( hostname, GNC_HOST_NAME_MAX );
- result = dbi_conn_queryf( dcon,
- "INSERT INTO %s VALUES ('%s', '%d')",
- lock_table, hostname, (int)GETPID() );
+ result = dbi_conn_queryf( dcon, "DELETE FROM %s", lock_table );
if ( !result)
{
qof_backend_set_error( qbe, ERR_BACKEND_SERVER_ERR );
- qof_backend_set_message( qbe, "Failed to create lock record" );
+ qof_backend_set_message( qbe, "Failed to delete lock record" );
gnc_dbi_transaction_rollback(qbe, dcon);
return FALSE;
}
@@ -761,10 +828,29 @@ gnc_dbi_lock_database ( QofBackend* qbe, gboolean ignore_lock )
dbi_result_free( result );
result = NULL;
}
- if (gnc_dbi_transaction_commit(qbe, dcon))
- return TRUE;
- /* Uh-oh, abort! */
+ }
+ /* Add an entry and commit the transaction */
+ memset( hostname, 0, sizeof(hostname) );
+ gethostname( hostname, GNC_HOST_NAME_MAX );
+ result = dbi_conn_queryf( dcon,
+ "INSERT INTO %s VALUES ('%s', '%d')",
+ lock_table, hostname, (int)GETPID() );
+ if ( !result)
+ {
+ qof_backend_set_error( qbe, ERR_BACKEND_SERVER_ERR );
+ qof_backend_set_message( qbe, "Failed to create lock record" );
gnc_dbi_transaction_rollback(qbe, dcon);
+ return FALSE;
+ }
+ if (result)
+ {
+ dbi_result_free( result );
+ result = NULL;
+ }
+ if (gnc_dbi_transaction_commit(qbe, dcon))
+ return TRUE;
+ /* Uh-oh, abort! */
+ gnc_dbi_transaction_rollback(qbe, dcon);
/* Couldn't commit the transaction, database isn't locked! */
qof_backend_set_error( qbe, ERR_BACKEND_SERVER_ERR );
qof_backend_set_message( qbe, "SQL Backend failed to lock the database, it was unable to commit the SQL transaction." );
@@ -1134,14 +1220,18 @@ gnc_dbi_mysql_session_begin( QofBackend* qbe, QofSession *session,
gnc_sql_connection_dispose( be->sql_be.conn );
}
be->sql_be.conn = create_dbi_connection( GNC_DBI_PROVIDER_MYSQL, qbe, be->conn );
- be->sql_be.timespec_format = MYSQL_TIMESPEC_STR_FORMAT;
-
- /* We should now have a proper session set up.
- * Let's start logging */
- basename = g_strjoin("_", protocol, host, username, dbname, NULL);
- translog_path = gnc_build_translog_path (basename);
- xaccLogSetBaseName (translog_path);
- PINFO ("logpath=%s", translog_path ? translog_path : "(null)");
+ be->sql_be.timespec_format = MYSQL_TIMESPEC_STR_FORMAT;
+ if (! gnc_dbi_check_and_rollback_failed_save(qbe))
+ {
+ gnc_sql_connection_dispose(be->sql_be.conn);
+ goto exit;
+ }
+ /* We should now have a proper session set up.
+ * Let's start logging */
+ basename = g_strjoin("_", protocol, host, username, dbname, NULL);
+ translog_path = gnc_build_translog_path (basename);
+ xaccLogSetBaseName (translog_path);
+ PINFO ("logpath=%s", translog_path ? translog_path : "(null)");
}
exit:
g_free( protocol );
@@ -1218,6 +1308,18 @@ conn_drop_index_mysql (dbi_conn conn, const gchar *index )
g_strfreev (index_table_split);
}
+/* MySQL automatically commits after any operation that changes a
+ * database's structure, like CREATE_TABLE or DROP_TABLE. Even if one
+ * disables that the changes can't be rolled back, so there's
+ * no way to make the save atomic. If the save doesn't complete we
+ * must rely on gnc_dbi_check_and_rollback_failed_save.
+ */
+static void
+conn_safe_sync_mysql (QofBackend* qbe, QofBook *book)
+{
+ gnc_dbi_do_safe_sync_all(qbe, book);
+}
+
static void
pgsql_error_fn( dbi_conn conn, void* user_data )
{
@@ -1480,19 +1582,23 @@ gnc_dbi_postgres_session_begin( QofBackend *qbe, QofSession *session,
}
if ( success )
{
- if ( be->sql_be.conn != NULL )
- {
- gnc_sql_connection_dispose( be->sql_be.conn );
- }
- be->sql_be.conn = create_dbi_connection( GNC_DBI_PROVIDER_PGSQL, qbe, be->conn );
- be->sql_be.timespec_format = PGSQL_TIMESPEC_STR_FORMAT;
-
- /* We should now have a proper session set up.
- * Let's start logging */
- basename = g_strjoin("_", protocol, host, username, dbname, NULL);
- translog_path = gnc_build_translog_path (basename);
- xaccLogSetBaseName (translog_path);
- PINFO ("logpath=%s", translog_path ? translog_path : "(null)");
+ if ( be->sql_be.conn != NULL )
+ {
+ gnc_sql_connection_dispose( be->sql_be.conn );
+ }
+ be->sql_be.conn = create_dbi_connection( GNC_DBI_PROVIDER_PGSQL, qbe, be->conn );
+ be->sql_be.timespec_format = PGSQL_TIMESPEC_STR_FORMAT;
+ if (! gnc_dbi_check_and_rollback_failed_save(qbe))
+ {
+ gnc_sql_connection_dispose(be->sql_be.conn);
+ goto exit;
+ }
+ /* We should now have a proper session set up.
+ * Let's start logging */
+ basename = g_strjoin("_", protocol, host, username, dbname, NULL);
+ translog_path = gnc_build_translog_path (basename);
+ xaccLogSetBaseName (translog_path);
+ PINFO ("logpath=%s", translog_path ? translog_path : "(null)");
}
exit:
g_free( protocol );
@@ -1539,6 +1645,23 @@ conn_drop_index_pgsql (dbi_conn conn, const gchar *index )
dbi_result_free( result );
}
+static void
+conn_safe_sync_pgsql (QofBackend* qbe, QofBook *book)
+{
+ GncDbiBackend *be = (GncDbiBackend*)qbe;
+ if (!gnc_dbi_transaction_begin(qbe, be->conn))
+ {
+ qof_backend_set_error( qbe, ERR_BACKEND_SERVER_ERR );
+ qof_backend_set_message(qbe, "Backend save failed, couldn't obtain the lock.");
+ }
+ if (!gnc_dbi_do_safe_sync_all(qbe, book))
+ gnc_dbi_transaction_rollback(qbe, be->conn);
+ else if (!gnc_dbi_transaction_commit(qbe, be->conn))
+ {
+ qof_backend_set_error(qbe, ERR_BACKEND_SERVER_ERR);
+ qof_backend_set_message(qbe, "Save failed, unable to commit the SQL transaction.");
+ }
+}
/* ================================================================= */
@@ -1791,59 +1914,63 @@ gnc_dbi_safe_sync_all( QofBackend *qbe, QofBook *book )
{
GncDbiBackend *be = (GncDbiBackend*)qbe;
GncDbiSqlConnection *conn = (GncDbiSqlConnection*)(((GncSqlBackend*)be)->conn);
- GSList *table_list, *index_list, *iter;
- const gchar* dbname = NULL;
-
g_return_if_fail( be != NULL );
g_return_if_fail( book != NULL );
-
+ conn->provider->safe_sync(qbe, book);
ENTER( "book=%p, primary=%p", book, be->primary_book );
- dbname = dbi_conn_get_option( be->conn, "dbname" );
- table_list = conn->provider->get_table_list( conn->conn, dbname );
- if ( !conn_table_operation( (GncSqlConnection*)conn, table_list,
- backup ) )
+ LEAVE("book=%p", book);
+}
+
+static int
+find_backup_cb(gconstpointer data, gconstpointer user)
+{
+ char* table_name = (char*)data;
+ char* suffix = (char*)user;
+ if (g_str_has_suffix(table_name, suffix))
+ return 0;
+ return 1;
+}
+
+static gboolean
+gnc_dbi_check_and_rollback_failed_save(QofBackend *qbe)
+{
+ GncDbiBackend *be = (GncDbiBackend*)qbe;
+ GncSqlConnection *conn = (GncSqlConnection*)(((GncSqlBackend*)be)->conn);
+ dbi_result result;
+ GSList *table_list = NULL;
+ const gchar* dbname = NULL;
+ static const char* suffix = "%back";
+ g_return_val_if_fail(be != NULL, FALSE);
+ g_return_val_if_fail(conn != NULL, FALSE);
+
+ dbname = dbi_conn_get_option(be->conn, "dbname");
+ result = dbi_conn_get_table_list(be->conn, dbname, suffix);
+ while (dbi_result_next_row(result) != 0)
{
- qof_backend_set_error( qbe, ERR_BACKEND_SERVER_ERR );
- conn_table_operation( (GncSqlConnection*)conn, table_list,
- rollback );
- LEAVE( "Failed to rename tables" );
- gnc_table_slist_free( table_list );
- return;
+ const char *table_name = dbi_result_get_string_idx(result, 1);
+ table_list = g_slist_prepend(table_list, g_strdup(table_name));
}
- index_list = conn->provider->get_index_list( conn->conn );
- for ( iter = index_list; iter != NULL; iter = g_slist_next( iter) )
+ if (result)
+ dbi_result_free(result);
+ if (table_list == NULL)
+ return TRUE;
+ if (!gnc_dbi_transaction_begin(qbe, be->conn))
{
- const char *errmsg;
- conn->provider->drop_index (conn->conn, iter->data);
- if ( DBI_ERROR_NONE != dbi_conn_error( conn->conn, &errmsg ) )
- {
- qof_backend_set_error( qbe, ERR_BACKEND_SERVER_ERR );
- gnc_table_slist_free( index_list );
- conn_table_operation( (GncSqlConnection*)conn, table_list,
- rollback );
- gnc_table_slist_free( table_list );
- LEAVE( "Failed to drop indexes %s", errmsg );
- return;
- }
+ qof_backend_set_message(qbe, "Backup tables found from a failed safe sync, unable to lock the database to restore them.");
+ g_slist_free_full(table_list, g_free);
+ return FALSE;
}
- gnc_table_slist_free( index_list );
-
- be->is_pristine_db = TRUE;
- be->primary_book = book;
-
- gnc_sql_sync_all( &be->sql_be, book );
- if (qof_backend_check_error (qbe))
+ conn_table_operation((GncSqlConnection*)conn, table_list, rollback);
+ g_slist_free_full(table_list, g_free);
+ if (!gnc_dbi_transaction_commit(qbe, be->conn))
{
- conn_table_operation( (GncSqlConnection*)conn, table_list,
- rollback );
- LEAVE( "Failed to create new database tables" );
- return;
+ qof_backend_set_message(qbe, "Backup tables found from a failed safe sync, unable to commit the restoration transaction.");
+ gnc_dbi_transaction_rollback(qbe, be->conn);
+ return FALSE;
}
- conn_table_operation( (GncSqlConnection*)conn, table_list,
- drop_backup );
- gnc_table_slist_free( table_list );
- LEAVE("book=%p", book);
+ return TRUE;
}
+
/* ================================================================= */
static void
gnc_dbi_begin_edit( QofBackend *qbe, QofInstance *inst )
commit a70637f34d48461cb8551d205935d8b3663869af
Author: John Ralls <jralls at ceridwen.us>
Date: Sat Feb 4 09:58:27 2017 -0800
Move the transaction-lock on obtaining the database lock earlier.
So that opening/creating the table is included. Also modify the condition
to reduce code nesting.
diff --git a/src/backend/dbi/gnc-backend-dbi.c b/src/backend/dbi/gnc-backend-dbi.c
index 4ef104d..215c658 100644
--- a/src/backend/dbi/gnc-backend-dbi.c
+++ b/src/backend/dbi/gnc-backend-dbi.c
@@ -673,6 +673,14 @@ gnc_dbi_lock_database ( QofBackend* qbe, gboolean ignore_lock )
dbi_conn dcon = qe->conn;
dbi_result result;
const gchar *dbname = dbi_conn_get_option( dcon, "dbname" );
+ gchar hostname[ GNC_HOST_NAME_MAX + 1 ];
+
+ if (!gnc_dbi_transaction_begin(qbe, dcon))
+ {
+ qof_backend_set_error( qbe, ERR_BACKEND_SERVER_ERR );
+ qof_backend_set_message( qbe, "SQL Backend lock database failed, couldn't obtain a transaction." );
+ return FALSE;
+ }
/* Create the table if it doesn't exist */
result = dbi_conn_get_table_list( dcon, dbname, lock_table);
if (!( result && dbi_result_get_numrows( result ) ))
@@ -707,16 +715,8 @@ gnc_dbi_lock_database ( QofBackend* qbe, gboolean ignore_lock )
dbi_result_free( result );
result = NULL;
}
- /* Protect everything with a single transaction to prevent races */
- if (gnc_dbi_transaction_begin(qbe, dcon))
- {
+
/* Check for an existing entry; delete it if ignore_lock is true, otherwise fail */
- gchar hostname[ GNC_HOST_NAME_MAX + 1 ];
- if (result)
- {
- dbi_result_free( result );
- result = NULL;
- }
result = dbi_conn_queryf( dcon, "SELECT * FROM %s", lock_table );
if ( result && dbi_result_get_numrows( result ) )
{
@@ -765,12 +765,12 @@ gnc_dbi_lock_database ( QofBackend* qbe, gboolean ignore_lock )
return TRUE;
/* Uh-oh, abort! */
gnc_dbi_transaction_rollback(qbe, dcon);
- }
- /* Couldn't get a transaction (probably couldn't get a lock) so fail */
+ /* Couldn't commit the transaction, database isn't locked! */
qof_backend_set_error( qbe, ERR_BACKEND_SERVER_ERR );
- qof_backend_set_message( qbe, "SQL Backend failed to get a transaction");
+ qof_backend_set_message( qbe, "SQL Backend failed to lock the database, it was unable to commit the SQL transaction." );
return FALSE;
}
+
static void
gnc_dbi_unlock( QofBackend *qbe )
{
@@ -778,6 +778,7 @@ gnc_dbi_unlock( QofBackend *qbe )
dbi_conn dcon = qe->conn;
dbi_result result;
const gchar *dbname = NULL;
+ gchar hostname[ GNC_HOST_NAME_MAX + 1 ];
g_return_if_fail( dcon != NULL );
g_return_if_fail( dbi_conn_error( dcon, NULL ) == 0 );
@@ -806,7 +807,6 @@ gnc_dbi_unlock( QofBackend *qbe )
}
/* Delete the entry if it's our hostname and PID */
- gchar hostname[ GNC_HOST_NAME_MAX + 1 ];
memset( hostname, 0, sizeof(hostname) );
gethostname( hostname, GNC_HOST_NAME_MAX );
result = dbi_conn_queryf( dcon, "SELECT * FROM %s WHERE Hostname = '%s' AND PID = '%d'", lock_table, hostname, (int)GETPID() );
@@ -1134,7 +1134,6 @@ gnc_dbi_mysql_session_begin( QofBackend* qbe, QofSession *session,
gnc_sql_connection_dispose( be->sql_be.conn );
}
be->sql_be.conn = create_dbi_connection( GNC_DBI_PROVIDER_MYSQL, qbe, be->conn );
- }
be->sql_be.timespec_format = MYSQL_TIMESPEC_STR_FORMAT;
/* We should now have a proper session set up.
@@ -1143,7 +1142,7 @@ gnc_dbi_mysql_session_begin( QofBackend* qbe, QofSession *session,
translog_path = gnc_build_translog_path (basename);
xaccLogSetBaseName (translog_path);
PINFO ("logpath=%s", translog_path ? translog_path : "(null)");
-
+ }
exit:
g_free( protocol );
g_free( host );
@@ -1486,7 +1485,6 @@ gnc_dbi_postgres_session_begin( QofBackend *qbe, QofSession *session,
gnc_sql_connection_dispose( be->sql_be.conn );
}
be->sql_be.conn = create_dbi_connection( GNC_DBI_PROVIDER_PGSQL, qbe, be->conn );
- }
be->sql_be.timespec_format = PGSQL_TIMESPEC_STR_FORMAT;
/* We should now have a proper session set up.
@@ -1495,7 +1493,7 @@ gnc_dbi_postgres_session_begin( QofBackend *qbe, QofSession *session,
translog_path = gnc_build_translog_path (basename);
xaccLogSetBaseName (translog_path);
PINFO ("logpath=%s", translog_path ? translog_path : "(null)");
-
+ }
exit:
g_free( protocol );
g_free( host );
commit 979e6397c15290550231d3c8ce18a1097157ccc0
Author: John Ralls <jralls at ceridwen.us>
Date: Sat Feb 4 09:50:50 2017 -0800
Add SAVEPOINT support to enable nested gnc_dbi_transaction calls.
diff --git a/src/backend/dbi/gnc-backend-dbi.c b/src/backend/dbi/gnc-backend-dbi.c
index 26fa9e1..4ef104d 100644
--- a/src/backend/dbi/gnc-backend-dbi.c
+++ b/src/backend/dbi/gnc-backend-dbi.c
@@ -220,12 +220,20 @@ gnc_dbi_verify_conn( GncDbiSqlConnection* dbi_conn )
return dbi_conn->conn_ok;
}
+static unsigned int sql_savepoint = 0;
static gboolean
gnc_dbi_transaction_begin(QofBackend* qbe, dbi_conn conn)
{
dbi_result result;
+ if (sql_savepoint == 0)
result = dbi_conn_queryf(conn, "BEGIN");
+ else
+ {
+ char *savepoint = g_strdup_printf("savepoint_%d", sql_savepoint);
+ result = dbi_conn_queryf(conn, "SAVEPOINT %s", savepoint);
+ g_free(savepoint);
+ }
if (result != NULL)
{
if(dbi_result_free(result) != 0)
@@ -233,6 +241,7 @@ gnc_dbi_transaction_begin(QofBackend* qbe, dbi_conn conn)
PERR( "Error in dbi_result_free() result\n" );
qof_backend_set_error(qbe, ERR_BACKEND_SERVER_ERR);
}
+ ++sql_savepoint;
return TRUE;
}
PERR( "BEGIN transaction failed()\n" );
@@ -244,7 +253,16 @@ static gboolean
gnc_dbi_transaction_commit(QofBackend *qbe, dbi_conn conn)
{
dbi_result result;
+ g_return_val_if_fail(sql_savepoint > 0, FALSE);
+ if (sql_savepoint == 1)
result = dbi_conn_queryf(conn, "COMMIT");
+ else
+ {
+ char *savepoint = g_strdup_printf("savepoint_%d", sql_savepoint - 1);
+ result = dbi_conn_queryf(conn, "RELEASE SAVEPOINT %s",
+ savepoint);
+ g_free(savepoint);
+ }
if (result != NULL)
{
if(dbi_result_free(result) != 0)
@@ -252,6 +270,7 @@ gnc_dbi_transaction_commit(QofBackend *qbe, dbi_conn conn)
PERR( "Error in dbi_result_free() result\n" );
qof_backend_set_error(qbe, ERR_BACKEND_SERVER_ERR);
}
+ --sql_savepoint;
return TRUE;
}
PERR( "COMMIT transaction failed()\n" );
@@ -264,7 +283,16 @@ gnc_dbi_transaction_rollback(QofBackend *qbe, dbi_conn conn)
{
dbi_result result;
DEBUG( "ROLLBACK\n" );
+ g_return_val_if_fail(sql_savepoint > 0, FALSE);
+ if (sql_savepoint == 1)
result = dbi_conn_queryf(conn, "ROLLBACK");
+ else
+ {
+ char *savepoint = g_strdup_printf("savepoint_%d", sql_savepoint - 1);
+ result = dbi_conn_queryf(conn, "ROLLBACK TO SAVEPOINT %s",
+ savepoint);
+ g_free(savepoint);
+ }
if (result != NULL)
{
if(dbi_result_free(result) != 0)
@@ -272,6 +300,7 @@ gnc_dbi_transaction_rollback(QofBackend *qbe, dbi_conn conn)
PERR( "Error in dbi_result_free() result\n" );
qof_backend_set_error(qbe, ERR_BACKEND_SERVER_ERR );
}
+ --sql_savepoint;
return TRUE;
}
PERR( "ROLLBACK transaction failed()\n" );
commit db73c39bf182904d01b3531524481aa274a2462a
Author: John Ralls <jralls at ceridwen.us>
Date: Sat Feb 4 09:41:45 2017 -0800
Extract static gnc_dbi_transaction functions.
To enable local calls (as opposed to just virtual calls via
GncDbiSqlConnection) and replace all direct transaction queries with the
new functions.
diff --git a/src/backend/dbi/gnc-backend-dbi.c b/src/backend/dbi/gnc-backend-dbi.c
index a716ad2..26fa9e1 100644
--- a/src/backend/dbi/gnc-backend-dbi.c
+++ b/src/backend/dbi/gnc-backend-dbi.c
@@ -221,6 +221,64 @@ gnc_dbi_verify_conn( GncDbiSqlConnection* dbi_conn )
return dbi_conn->conn_ok;
}
+static gboolean
+gnc_dbi_transaction_begin(QofBackend* qbe, dbi_conn conn)
+{
+ dbi_result result;
+ result = dbi_conn_queryf(conn, "BEGIN");
+ if (result != NULL)
+ {
+ if(dbi_result_free(result) != 0)
+ {
+ PERR( "Error in dbi_result_free() result\n" );
+ qof_backend_set_error(qbe, ERR_BACKEND_SERVER_ERR);
+ }
+ return TRUE;
+ }
+ PERR( "BEGIN transaction failed()\n" );
+ qof_backend_set_error(qbe, ERR_BACKEND_SERVER_ERR);
+ return FALSE;
+}
+
+static gboolean
+gnc_dbi_transaction_commit(QofBackend *qbe, dbi_conn conn)
+{
+ dbi_result result;
+ result = dbi_conn_queryf(conn, "COMMIT");
+ if (result != NULL)
+ {
+ if(dbi_result_free(result) != 0)
+ {
+ PERR( "Error in dbi_result_free() result\n" );
+ qof_backend_set_error(qbe, ERR_BACKEND_SERVER_ERR);
+ }
+ return TRUE;
+ }
+ PERR( "COMMIT transaction failed()\n" );
+ qof_backend_set_error(qbe, ERR_BACKEND_SERVER_ERR);
+ return FALSE;
+}
+
+static gboolean
+gnc_dbi_transaction_rollback(QofBackend *qbe, dbi_conn conn)
+{
+ dbi_result result;
+ DEBUG( "ROLLBACK\n" );
+ result = dbi_conn_queryf(conn, "ROLLBACK");
+ if (result != NULL)
+ {
+ if(dbi_result_free(result) != 0)
+ {
+ PERR( "Error in dbi_result_free() result\n" );
+ qof_backend_set_error(qbe, ERR_BACKEND_SERVER_ERR );
+ }
+ return TRUE;
+ }
+ PERR( "ROLLBACK transaction failed()\n" );
+ qof_backend_set_error(qbe, ERR_BACKEND_SERVER_ERR );
+ return FALSE;
+}
+
/* ================================================================= */
static void
@@ -620,9 +678,8 @@ gnc_dbi_lock_database ( QofBackend* qbe, gboolean ignore_lock )
dbi_result_free( result );
result = NULL;
}
-
/* Protect everything with a single transaction to prevent races */
- if ( (result = dbi_conn_query( dcon, "BEGIN" )) )
+ if (gnc_dbi_transaction_begin(qbe, dcon))
{
/* Check for an existing entry; delete it if ignore_lock is true, otherwise fail */
gchar hostname[ GNC_HOST_NAME_MAX + 1 ];
@@ -640,7 +697,7 @@ gnc_dbi_lock_database ( QofBackend* qbe, gboolean ignore_lock )
{
qof_backend_set_error( qbe, ERR_BACKEND_LOCKED );
/* FIXME: After enhancing the qof_backend_error mechanism, report in the dialog what is the hostname of the machine holding the lock. */
- dbi_conn_query( dcon, "ROLLBACK" );
+ gnc_dbi_transaction_rollback(qbe, dcon);
return FALSE;
}
result = dbi_conn_queryf( dcon, "DELETE FROM %s", lock_table );
@@ -648,12 +705,7 @@ gnc_dbi_lock_database ( QofBackend* qbe, gboolean ignore_lock )
{
qof_backend_set_error( qbe, ERR_BACKEND_SERVER_ERR );
qof_backend_set_message( qbe, "Failed to delete lock record" );
- result = dbi_conn_query( dcon, "ROLLBACK" );
- if (result)
- {
- dbi_result_free( result );
- result = NULL;
- }
+ gnc_dbi_transaction_rollback(qbe, dcon);
return FALSE;
}
if (result)
@@ -672,12 +724,7 @@ gnc_dbi_lock_database ( QofBackend* qbe, gboolean ignore_lock )
{
qof_backend_set_error( qbe, ERR_BACKEND_SERVER_ERR );
qof_backend_set_message( qbe, "Failed to create lock record" );
- result = dbi_conn_query( dcon, "ROLLBACK" );
- if (result)
- {
- dbi_result_free( result );
- result = NULL;
- }
+ gnc_dbi_transaction_rollback(qbe, dcon);
return FALSE;
}
if (result)
@@ -685,22 +732,14 @@ gnc_dbi_lock_database ( QofBackend* qbe, gboolean ignore_lock )
dbi_result_free( result );
result = NULL;
}
- result = dbi_conn_query( dcon, "COMMIT" );
- if (result)
- {
- dbi_result_free( result );
- result = NULL;
- }
- return TRUE;
+ if (gnc_dbi_transaction_commit(qbe, dcon))
+ return TRUE;
+ /* Uh-oh, abort! */
+ gnc_dbi_transaction_rollback(qbe, dcon);
}
- /* Couldn't get a transaction (probably couldn't get a lock), so fail */
+ /* Couldn't get a transaction (probably couldn't get a lock) so fail */
qof_backend_set_error( qbe, ERR_BACKEND_SERVER_ERR );
- qof_backend_set_message( qbe, "SQL Backend failed to obtain a transaction" );
- if (result)
- {
- dbi_result_free( result );
- result = NULL;
- }
+ qof_backend_set_message( qbe, "SQL Backend failed to get a transaction");
return FALSE;
}
static void
@@ -729,67 +768,49 @@ gnc_dbi_unlock( QofBackend *qbe )
return;
}
dbi_result_free( result );
+ result = NULL;
- result = dbi_conn_query( dcon, "BEGIN" );
- if ( result )
+ if (!gnc_dbi_transaction_begin(qbe, dcon))
{
- /* Delete the entry if it's our hostname and PID */
- gchar hostname[ GNC_HOST_NAME_MAX + 1 ];
+ PWARN("Unable to get a lock on LOCK, so failed to clear the lock entry.");
+ qof_backend_set_error( qbe, ERR_BACKEND_SERVER_ERR );
+ }
- dbi_result_free( result );
- result = NULL;
- memset( hostname, 0, sizeof(hostname) );
- gethostname( hostname, GNC_HOST_NAME_MAX );
- result = dbi_conn_queryf( dcon, "SELECT * FROM %s WHERE Hostname = '%s' AND PID = '%d'", lock_table, hostname, (int)GETPID() );
- if ( result && dbi_result_get_numrows( result ) )
+ /* Delete the entry if it's our hostname and PID */
+ gchar hostname[ GNC_HOST_NAME_MAX + 1 ];
+ memset( hostname, 0, sizeof(hostname) );
+ gethostname( hostname, GNC_HOST_NAME_MAX );
+ result = dbi_conn_queryf( dcon, "SELECT * FROM %s WHERE Hostname = '%s' AND PID = '%d'", lock_table, hostname, (int)GETPID() );
+ if ( result && dbi_result_get_numrows( result ) )
+ {
+ if (result)
{
- if (result)
- {
- dbi_result_free( result );
- result = NULL;
- }
- result = dbi_conn_queryf( dcon, "DELETE FROM %s", lock_table );
- if ( !result)
- {
- PERR("Failed to delete the lock entry");
- qof_backend_set_error( qbe, ERR_BACKEND_SERVER_ERR );
- result = dbi_conn_query( dcon, "ROLLBACK" );
- if (result)
- {
- dbi_result_free( result );
- result = NULL;
- }
- return;
- }
- else
- {
- dbi_result_free( result );
- result = NULL;
- }
- result = dbi_conn_query( dcon, "COMMIT" );
- if (result)
- {
- dbi_result_free( result );
- result = NULL;
- }
- return;
+ dbi_result_free( result );
+ result = NULL;
}
- result = dbi_conn_query( dcon, "ROLLBACK" );
- if (result)
+ result = dbi_conn_queryf( dcon, "DELETE FROM %s", lock_table );
+ if ( !result)
+ {
+ PERR("Failed to delete the lock entry");
+ qof_backend_set_error( qbe, ERR_BACKEND_SERVER_ERR );
+ gnc_dbi_transaction_rollback(qbe, dcon);
+ }
+ else
{
dbi_result_free( result );
result = NULL;
}
- PWARN("There was no lock entry in the Lock table");
+ if (!gnc_dbi_transaction_commit(qbe, dcon))
+ {
+ gnc_dbi_transaction_rollback(qbe, dcon);
+ PWARN("Failed to unlock the database, unable to commit the SQL transaction.");
+ }
return;
}
- if (result)
- {
- dbi_result_free( result );
- result = NULL;
- }
- PWARN("Unable to get a lock on LOCK, so failed to clear the lock entry.");
- qof_backend_set_error( qbe, ERR_BACKEND_SERVER_ERR );
+
+ gnc_dbi_transaction_rollback(qbe, dcon);
+ PWARN("There was no lock entry in the Lock table");
+ return;
}
#define SQL_OPTION_TO_REMOVE "NO_ZERO_DATE"
@@ -2543,39 +2564,23 @@ static gboolean
conn_begin_transaction( /*@ unused @*/ GncSqlConnection* conn )
{
GncDbiSqlConnection* dbi_conn = (GncDbiSqlConnection*)conn;
- dbi_result result;
gint status;
gboolean success = FALSE;
DEBUG( "BEGIN\n" );
-
if ( !gnc_dbi_verify_conn (dbi_conn) )
{
PERR( "gnc_dbi_verify_conn() failed\n" );
qof_backend_set_error( dbi_conn->qbe, ERR_BACKEND_SERVER_ERR );
return FALSE;
}
-
do
{
gnc_dbi_init_error( dbi_conn );
- result = dbi_conn_queryf( dbi_conn->conn, "BEGIN" );
+ success = gnc_dbi_transaction_begin(dbi_conn->qbe, dbi_conn->conn);
}
while ( dbi_conn->retry );
- success = ( result != NULL );
- status = dbi_result_free( result );
- if ( status < 0 )
- {
- PERR( "Error in dbi_result_free() result\n" );
- qof_backend_set_error( dbi_conn->qbe, ERR_BACKEND_SERVER_ERR );
- }
- if ( !success )
- {
- PERR( "BEGIN transaction failed()\n" );
- qof_backend_set_error( dbi_conn->qbe, ERR_BACKEND_SERVER_ERR );
- }
-
return success;
}
@@ -2583,54 +2588,14 @@ static gboolean
conn_rollback_transaction( /*@ unused @*/ GncSqlConnection* conn )
{
GncDbiSqlConnection* dbi_conn = (GncDbiSqlConnection*)conn;
- dbi_result result;
- gint status;
- gboolean success = FALSE;
-
- DEBUG( "ROLLBACK\n" );
- result = dbi_conn_queryf( dbi_conn->conn, "ROLLBACK" );
- success = ( result != NULL );
-
- status = dbi_result_free( result );
- if ( status < 0 )
- {
- PERR( "Error in dbi_result_free() result\n" );
- qof_backend_set_error( dbi_conn->qbe, ERR_BACKEND_SERVER_ERR );
- }
- if ( !success )
- {
- PERR( "Error in conn_rollback_transaction()\n" );
- qof_backend_set_error( dbi_conn->qbe, ERR_BACKEND_SERVER_ERR );
- }
-
- return success;
+ return gnc_dbi_transaction_rollback(dbi_conn->qbe, dbi_conn->conn);
}
static gboolean
conn_commit_transaction( /*@ unused @*/ GncSqlConnection* conn )
{
GncDbiSqlConnection* dbi_conn = (GncDbiSqlConnection*)conn;
- dbi_result result;
- gint status;
- gboolean success = FALSE;
-
- DEBUG( "COMMIT\n" );
- result = dbi_conn_queryf( dbi_conn->conn, "COMMIT" );
- success = ( result != NULL );
-
- status = dbi_result_free( result );
- if ( status < 0 )
- {
- PERR( "Error in dbi_result_free() result\n" );
- qof_backend_set_error( dbi_conn->qbe, ERR_BACKEND_SERVER_ERR );
- }
- if ( !success )
- {
- PERR( "Error in conn_commit_transaction()\n" );
- qof_backend_set_error( dbi_conn->qbe, ERR_BACKEND_SERVER_ERR );
- }
-
- return success;
+ return gnc_dbi_transaction_commit(dbi_conn->qbe, dbi_conn->conn);
}
static /*@ null @*/ gchar*
Summary of changes:
src/backend/dbi/gnc-backend-dbi-priv.h | 2 +
src/backend/dbi/gnc-backend-dbi.c | 603 ++++++++++++++++++++-------------
2 files changed, 363 insertions(+), 242 deletions(-)
More information about the gnucash-changes
mailing list