From 85d1a971290f059e840f8d8c16ef47234853b0be Mon Sep 17 00:00:00 2001 From: Charles Lindsay Date: Mon, 3 Mar 2014 17:40:53 -0800 Subject: [PATCH] Don't multithread db upgrades Turns out for long-running upgrades we were running them all in parallel, which thrashes the disk pretty hard. This adds a simple mutex lock around each upgrade, so at least the computer is usable while it's going on. A more robust solution would be to have a single-thread thread pool where we enqueue upgrades, but that's too much change this late in the release cycle. Also it turns out that the nullifying of the internaldate_time_t column before we repopulate it was very costly, and unnecessary. So, this also should speed things up for upgrading users. Closes: bgo #724475 --- sql/version-018.sql | 6 ++---- src/engine/db/db-versioned-database.vala | 13 +++++++++++++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/sql/version-018.sql b/sql/version-018.sql index eda51260..c0a38dfc 100644 --- a/sql/version-018.sql +++ b/sql/version-018.sql @@ -1,6 +1,4 @@ -- --- Nuke the internaldate_time_t column, because it had the wrong values. It'll --- be repopulated in code, in imap-db-database.vala. +-- Dummy upgrade to nuke the internaldate_time_t column, because it had the +-- wrong values. It'll be repopulated in code, in imap-db-database.vala. -- - -UPDATE MessageTable SET internaldate_time_t = NULL; diff --git a/src/engine/db/db-versioned-database.vala b/src/engine/db/db-versioned-database.vala index 311438d3..850738d8 100644 --- a/src/engine/db/db-versioned-database.vala +++ b/src/engine/db/db-versioned-database.vala @@ -7,6 +7,8 @@ public class Geary.Db.VersionedDatabase : Geary.Db.Database { public delegate void WorkCallback(); + private static Mutex upgrade_mutex = new Mutex(); + public File schema_dir { get; private set; } public VersionedDatabase(File db_file, File schema_dir) { @@ -107,6 +109,14 @@ public class Geary.Db.VersionedDatabase : Geary.Db.Database { started = true; } + // Since these upgrades run in a background thread, there's a possibility they + // can run in parallel. That leads to Geary absolutely taking over the machine, + // with potentially several threads all doing heavy database manipulation at + // once. So, we wrap this bit in a mutex lock so that only one database is + // updating at once. It means overall it might take a bit longer, but it keeps + // things usable in the meantime. See . + upgrade_mutex.@lock(); + pre_upgrade(db_version); check_cancelled("VersionedDatabase.open", cancellable); @@ -121,11 +131,14 @@ public class Geary.Db.VersionedDatabase : Geary.Db.Database { }, cancellable); } catch (Error err) { warning("Error upgrading database to version %d: %s", db_version, err.message); + upgrade_mutex.unlock(); throw err; } post_upgrade(db_version); + + upgrade_mutex.unlock(); } if (started)