From: Shane Synan <digitalcircuit36939@gmail.com>
Date: Thu, 31 May 2018 23:21:33 +0000 (-0500)
Subject: core: Save schema version for intermediate steps
X-Git-Tag: travis-deploy-test~78
X-Git-Url: https://git.quassel-irc.org/?p=quassel.git;a=commitdiff_plain;h=f10304a35af0a7a4f8b812e467e69287d358ce7c

core: Save schema version for intermediate steps

When running upgrade queries, update the schema version for each
intermediate step.  This ensures that any interrupted upgrades have a
greater chance of resuming correctly after core restart.

Almost all databases make single queries atomic (fully works or fully
fails, no partial), and with many of the longest migrations being a
single query, this makes upgrade interruptions much more likely to
leave the database in a valid intermediate schema version.

As a side effect, downgrading from a partial upgrade will now require
setting schema version manually, instead of it possibly happening to
work depending on what changed.  An unsupported operation failing in
a known, recoverable way seems better than failing in an unknown,
possibly recoverable way.

Update logging to be more explicit about what fails.

Tested by kill -9'ng Quasselcore during migration, then re-running
core afterwards.

In the future, for databases that support it (e.g. almost only
PostgreSQL), we may want to wrap upgrades in a transaction.  This
will need careful testing of potential additional space requirements
and any database modifications that might not be allowed in a
transaction.

sqlite> select * from coreinfo;
schemaversion|26
$ (sleep 0.5 && pkill -9 quasselcore ) & \
  ./run-profile.sh master core local
sqlite> select * from coreinfo;
schemaversion|30
$ ./run-profile.sh master core local
sqlite> select * from coreinfo;
schemaversion|31
---

diff --git a/src/core/abstractsqlstorage.cpp b/src/core/abstractsqlstorage.cpp
index 7e436fce..fce1bb1d 100644
--- a/src/core/abstractsqlstorage.cpp
+++ b/src/core/abstractsqlstorage.cpp
@@ -250,16 +250,47 @@ bool AbstractSqlStorage::upgradeDb()
 
     QSqlDatabase db = logDb();
 
+    // TODO: For databases that support it (e.g. almost only PostgreSQL), wrap upgrades in a
+    // transaction.  This will need careful testing of potential additional space requirements and
+    // any database modifications that might not be allowed in a transaction.
+
     for (int ver = installedSchemaVersion() + 1; ver <= schemaVersion(); ver++) {
         foreach(QString queryString, upgradeQueries(ver)) {
             QSqlQuery query = db.exec(queryString);
             if (!watchQuery(query)) {
-                qCritical() << "Unable to upgrade Logging Backend!";
+                // Individual upgrade query failed, bail out
+                qCritical() << "Unable to upgrade Logging Backend!  Upgrade query in schema version"
+                            << ver << "failed.";
                 return false;
             }
         }
+
+        // Update the schema version for each intermediate step.  This ensures that any interrupted
+        // upgrades have a greater chance of resuming correctly after core restart.
+        //
+        // Almost all databases make single queries atomic (fully works or fully fails, no partial),
+        // and with many of the longest migrations being a single query, this makes upgrade
+        // interruptions much more likely to leave the database in a valid intermediate schema
+        // version.
+        if (!updateSchemaVersion(ver)) {
+            // Updating the schema version failed, bail out
+            qCritical() << "Unable to upgrade Logging Backend!  Setting schema version"
+                        << ver << "failed.";
+            return false;
+        }
     }
-    return updateSchemaVersion(schemaVersion());
+
+    // Update the schema version for the final step.  Split this out to offer more informative
+    // logging (though setting schema version really should not fail).
+    if (!updateSchemaVersion(schemaVersion())) {
+        // Updating the final schema version failed, bail out
+        qCritical() << "Unable to upgrade Logging Backend!  Setting final schema version"
+                    << schemaVersion() << "failed.";
+        return false;
+    }
+
+    // If we made it here, everything seems to have worked!
+    return true;
 }