From f91f3faa1432894e6d6ecaaf0a1f60a483dd0129 Mon Sep 17 00:00:00 2001 From: Shane Synan Date: Mon, 28 Jan 2019 23:08:03 -0500 Subject: [PATCH] core: Track upgrade step within schema version Track the last successful upgrade step (upgrade_###_XXX.sql) within each schema version, storing it within 'coreinfo' table as 'schemaupgradestep'. When a schema upgrade finishes, clear 'schemaupgradestep' and set 'schemaversion'. This allows for resuming multi-step schema upgrades that were interrupted in the middle. Whenever starting a schema upgrade, also check the value of 'schemaupgradestep'. One of two states exist: 1. Empty ('') or nonexistent No interrupted schema upgrade, start the next schema version upgrade from the first query. 2. Contains text, e.g. 'upgrade_010_alter_sender_64bit_ids' The schema upgrade was interrupted, skip schema upgrade steps including the specified successful step, and resume from the next step. For case 2, if the schema upgrade step cannot be found, warn and bail out. This should only happen if: 1. The storage of successful query glitched, or the database was manually changed 2. Quassel changed the filenames of upgrade queries, and the local Quassel core version was replaced during an interrupted schema upgrade Modify SqliteStorage and PostgreSqlStorage to fetch/save the 'schemaupgradestep' key. Clearing this key is done atomically within updateSchemaVersion(). (Ideally, the whole upgrade would be wrapped in a transaction, but that doesn't seem to be easily possible.) Modify upgradeQueries() to return a list of query strings and resource filenames, used for tracking the upgrade step and providing clearer feedback on what steps fail. --- src/core/abstractsqlstorage.cpp | 71 +++++++++++++++++++++++++---- src/core/abstractsqlstorage.h | 47 ++++++++++++++++++- src/core/postgresqlstorage.cpp | 76 ++++++++++++++++++++++++++++--- src/core/postgresqlstorage.h | 18 +++++++- src/core/sqlitestorage.cpp | 80 +++++++++++++++++++++++++++++---- src/core/sqlitestorage.h | 18 +++++++- 6 files changed, 283 insertions(+), 27 deletions(-) diff --git a/src/core/abstractsqlstorage.cpp b/src/core/abstractsqlstorage.cpp index ba5a8f51..5aa7465c 100644 --- a/src/core/abstractsqlstorage.cpp +++ b/src/core/abstractsqlstorage.cpp @@ -217,13 +217,13 @@ bool AbstractSqlStorage::setup(const QVariantMap& settings, const QProcessEnviro return success; } -QStringList AbstractSqlStorage::upgradeQueries(int version) +QList AbstractSqlStorage::upgradeQueries(int version) { - QStringList queries; + QList queries; // Upgrade queries are stored in the 'version/##' subfolders. QDir dir = QDir(QString(":/SQL/%1/version/%2/").arg(displayName()).arg(version)); foreach (QFileInfo fileInfo, dir.entryInfoList(QStringList() << "upgrade*", QDir::NoFilter, QDir::Name)) { - queries << queryString(fileInfo.baseName(), version); + queries << SqlQueryResource(queryString(fileInfo.baseName(), version), fileInfo.baseName()); } return queries; } @@ -239,24 +239,69 @@ bool AbstractSqlStorage::upgradeDb() // transaction. This will need careful testing of potential additional space requirements and // any database modifications that might not be allowed in a transaction. + // Check if we're resuming an interrupted multi-step upgrade: is an upgrade step stored? + const QString previousLaunchUpgradeStep = schemaVersionUpgradeStep(); + bool resumingUpgrade = !previousLaunchUpgradeStep.isEmpty(); + for (int ver = installedSchemaVersion() + 1; ver <= schemaVersion(); ver++) { - foreach (QString queryString, upgradeQueries(ver)) { - QSqlQuery query = db.exec(queryString); + foreach (auto queryResource, upgradeQueries(ver)) { + if (resumingUpgrade) { + // An upgrade was interrupted. Check if this matches the the last successful query. + if (previousLaunchUpgradeStep == queryResource.queryFilename) { + // Found the matching query! + qInfo() << qPrintable(QString("Resuming interrupted upgrade for schema version %1 (last step: %2)") + .arg(QString::number(ver), previousLaunchUpgradeStep)); + + // Stop searching for queries + resumingUpgrade = false; + // Continue past the previous query with the next not-yet-tried query + continue; + } + else { + // Not yet matched, keep looking + continue; + } + } + + // Run the upgrade query + QSqlQuery query = db.exec(queryResource.queryString); if (!watchQuery(query)) { // Individual upgrade query failed, bail out - qCritical() << "Unable to upgrade Logging Backend! Upgrade query in schema version" << ver << "failed."; + qCritical() << qPrintable(QString("Unable to upgrade Logging Backend! Upgrade query in schema version %1 failed (step: %2).") + .arg(QString::number(ver), queryResource.queryFilename)); return false; } + else { + // Mark as successful + setSchemaVersionUpgradeStep(queryResource.queryFilename); + } } - // Update the schema version for each intermediate step. This ensures that any interrupted - // upgrades have a greater chance of resuming correctly after core restart. + if (resumingUpgrade) { + // Something went wrong and the last successful SQL query to resume from couldn't be + // found. + // 1. The storage of successful query glitched, or the database was manually changed + // 2. Quassel changed the filenames of upgrade queries, and the local Quassel core + // version was replaced during an interrupted schema upgrade + // + // Both are unlikely, but it's a good idea to handle it anyways. + + qCritical() << qPrintable(QString("Unable to resume interrupted upgrade in Logging " + "Backend! Missing upgrade step in schema version %1 " + "(expected step: %2)") + .arg(QString::number(ver), previousLaunchUpgradeStep)); + return false; + } + + // Update the schema version for each intermediate step and mark the step as done. 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)) { + if (!updateSchemaVersion(ver, true)) { // Updating the schema version failed, bail out qCritical() << "Unable to upgrade Logging Backend! Setting schema version" << ver << "failed."; return false; @@ -300,6 +345,14 @@ int AbstractSqlStorage::schemaVersion() return _schemaVersion; } + +QString AbstractSqlStorage::schemaVersionUpgradeStep() +{ + // By default, assume there's no pending upgrade + return {}; +} + + bool AbstractSqlStorage::watchQuery(QSqlQuery& query) { bool queryError = query.lastError().isValid(); diff --git a/src/core/abstractsqlstorage.h b/src/core/abstractsqlstorage.h index 624489fc..88ecfb5d 100644 --- a/src/core/abstractsqlstorage.h +++ b/src/core/abstractsqlstorage.h @@ -22,6 +22,7 @@ #include +#include #include #include #include @@ -42,6 +43,18 @@ public: virtual std::unique_ptr createMigrationReader() { return {}; } virtual std::unique_ptr createMigrationWriter() { return {}; } + /** + * An SQL query with associated resource filename + */ + struct SqlQueryResource { + QString queryString; ///< SQL query string + QString queryFilename; ///< Path to the resource file providing this query + + SqlQueryResource(const QString& queryString, const QString& queryFilename) + : queryString(std::move(queryString)), + queryFilename(std::move(queryFilename)) {} + }; + public slots: State init(const QVariantMap& settings = QVariantMap(), const QProcessEnvironment& environment = {}, @@ -76,16 +89,46 @@ protected: QStringList setupQueries(); - QStringList upgradeQueries(int ver); + /** + * Gets the collection of SQL upgrade queries and filenames for a given schema version + * + * @param ver SQL schema version + * @return List of SQL query strings and filenames + */ + QList upgradeQueries(int ver); bool upgradeDb(); bool watchQuery(QSqlQuery& query); int schemaVersion(); virtual int installedSchemaVersion() { return -1; }; - virtual bool updateSchemaVersion(int newVersion) = 0; + + /** + * Update the stored schema version number, optionally clearing the record of mid-schema steps + * + * @param newVersion New schema version number + * @param clearUpgradeStep If true, clear the record of any in-progress schema upgrades + * @return + */ + virtual bool updateSchemaVersion(int newVersion, bool clearUpgradeStep = true) = 0; + virtual bool setupSchemaVersion(int version) = 0; + /** + * Gets the last successful schema upgrade step, or an empty string if no upgrade is in progress + * + * @return Filename of last successful schema upgrade query, or empty string if not upgrading + */ + virtual QString schemaVersionUpgradeStep(); + + /** + * Sets the last successful schema upgrade step + * + * @param upgradeQuery The filename of the last successful schema upgrade query + * @return True if successfully set, otherwise false + */ + virtual bool setSchemaVersionUpgradeStep(QString upgradeQuery) = 0; + virtual void setConnectionProperties(const QVariantMap& properties, const QProcessEnvironment& environment, bool loadFromEnvironment) = 0; virtual QString driverName() = 0; inline virtual QString hostName() { return QString(); } diff --git a/src/core/postgresqlstorage.cpp b/src/core/postgresqlstorage.cpp index 6c2f07c9..40ff80cb 100644 --- a/src/core/postgresqlstorage.cpp +++ b/src/core/postgresqlstorage.cpp @@ -164,19 +164,39 @@ int PostgreSqlStorage::installedSchemaVersion() return AbstractSqlStorage::installedSchemaVersion(); } -bool PostgreSqlStorage::updateSchemaVersion(int newVersion) +bool PostgreSqlStorage::updateSchemaVersion(int newVersion, bool clearUpgradeStep) { - QSqlQuery query(logDb()); + // Atomically update the schema version and clear the upgrade step, if specified + // Note: This will need reworked if "updateSchemaVersion" is ever called within a transaction. + QSqlDatabase db = logDb(); + if (!beginTransaction(db)) { + qWarning() << "PostgreSqlStorage::updateSchemaVersion(int, bool): cannot start transaction!"; + qWarning() << " -" << qPrintable(db.lastError().text()); + return false; + } + + QSqlQuery query(db); query.prepare("UPDATE coreinfo SET value = :version WHERE key = 'schemaversion'"); query.bindValue(":version", newVersion); safeExec(query); - bool success = true; if (!watchQuery(query)) { - qCritical() << "PostgreSqlStorage::updateSchemaVersion(int): Updating schema version failed!"; - success = false; + qCritical() << "PostgreSqlStorage::updateSchemaVersion(int, bool): Updating schema version failed!"; + db.rollback(); + return false; } - return success; + + if (clearUpgradeStep) { + // Try clearing the upgrade step if requested + if (!setSchemaVersionUpgradeStep("")) { + db.rollback(); + return false; + } + } + + // Successful, commit and return true + db.commit(); + return true; } bool PostgreSqlStorage::setupSchemaVersion(int version) @@ -194,6 +214,50 @@ bool PostgreSqlStorage::setupSchemaVersion(int version) return success; } +QString PostgreSqlStorage::schemaVersionUpgradeStep() +{ + QSqlQuery query(logDb()); + query.prepare("SELECT value FROM coreinfo WHERE key = 'schemaupgradestep'"); + safeExec(query); + watchQuery(query); + if (query.first()) + return query.value(0).toString(); + + // Fall back to the default value + return AbstractSqlStorage::schemaVersionUpgradeStep(); +} + +bool PostgreSqlStorage::setSchemaVersionUpgradeStep(QString upgradeQuery) +{ + // Intentionally do not wrap in a transaction so other functions can include multiple operations + + QSqlQuery query(logDb()); + query.prepare("UPDATE coreinfo SET value = :upgradestep WHERE key = 'schemaupgradestep'"); + query.bindValue(":upgradestep", upgradeQuery); + safeExec(query); + + // Make sure that the query didn't fail (shouldn't ever happen), and that some non-zero number + // of rows were affected + bool success = watchQuery(query) && query.numRowsAffected() != 0; + + if (!success) { + // The key might not exist (Quassel 0.13.0 and older). Try inserting it... + query = QSqlQuery(logDb()); + query.prepare("INSERT INTO coreinfo (key, value) VALUES ('schemaupgradestep', :upgradestep)"); + query.bindValue(":upgradestep", upgradeQuery); + safeExec(query); + + if (!watchQuery(query)) { + qCritical() << Q_FUNC_INFO << "Setting schema upgrade step failed!"; + success = false; + } + else { + success = true; + } + } + return success; +} + UserId PostgreSqlStorage::addUser(const QString& user, const QString& password, const QString& authenticator) { QSqlQuery query(logDb()); diff --git a/src/core/postgresqlstorage.h b/src/core/postgresqlstorage.h index c97603c6..f93bf691 100644 --- a/src/core/postgresqlstorage.h +++ b/src/core/postgresqlstorage.h @@ -137,8 +137,24 @@ protected: QString userName() override { return _userName; } QString password() override { return _password; } int installedSchemaVersion() override; - bool updateSchemaVersion(int newVersion) override; + bool updateSchemaVersion(int newVersion, bool clearUpgradeStep) override; bool setupSchemaVersion(int version) override; + + /** + * Gets the last successful schema upgrade step, or an empty string if no upgrade is in progress + * + * @return Filename of last successful schema upgrade query, or empty string if not upgrading + */ + QString schemaVersionUpgradeStep() override; + + /** + * Sets the last successful schema upgrade step + * + * @param upgradeQuery The filename of the last successful schema upgrade query + * @return True if successfully set, otherwise false + */ + virtual bool setSchemaVersionUpgradeStep(QString upgradeQuery) override; + void safeExec(QSqlQuery& query); bool beginTransaction(QSqlDatabase& db); diff --git a/src/core/sqlitestorage.cpp b/src/core/sqlitestorage.cpp index 909c8177..58740136 100644 --- a/src/core/sqlitestorage.cpp +++ b/src/core/sqlitestorage.cpp @@ -75,21 +75,39 @@ int SqliteStorage::installedSchemaVersion() return AbstractSqlStorage::installedSchemaVersion(); } -bool SqliteStorage::updateSchemaVersion(int newVersion) +bool SqliteStorage::updateSchemaVersion(int newVersion, bool clearUpgradeStep) { // only used when there is a singlethread (during startup) // so we don't need locking here - QSqlQuery query(logDb()); + + QSqlDatabase db = logDb(); + + // Atomically update the schema version and clear the upgrade step, if specified + // Note: This will need reworked if "updateSchemaVersion" is ever called within a transaction. + db.transaction(); + + QSqlQuery query(db); query.prepare("UPDATE coreinfo SET value = :version WHERE key = 'schemaversion'"); query.bindValue(":version", newVersion); - query.exec(); + safeExec(query); - bool success = true; - if (query.lastError().isValid()) { - qCritical() << "SqliteStorage::updateSchemaVersion(int): Updating schema version failed!"; - success = false; + if (!watchQuery(query)) { + qCritical() << "SqliteStorage::updateSchemaVersion(int, bool): Updating schema version failed!"; + db.rollback(); + return false; } - return success; + + if (clearUpgradeStep) { + // Try clearing the upgrade step if requested + if (!setSchemaVersionUpgradeStep("")) { + db.rollback(); + return false; + } + } + + // Successful, commit and return true + db.commit(); + return true; } bool SqliteStorage::setupSchemaVersion(int version) @@ -109,6 +127,52 @@ bool SqliteStorage::setupSchemaVersion(int version) return success; } +QString SqliteStorage::schemaVersionUpgradeStep() +{ + // Only used when there is a singlethread (during startup), so we don't need locking here + QSqlQuery query(logDb()); + query.prepare("SELECT value FROM coreinfo WHERE key = 'schemaupgradestep'"); + safeExec(query); + watchQuery(query); + if (query.first()) + return query.value(0).toString(); + + // Fall back to the default value + return AbstractSqlStorage::schemaVersionUpgradeStep(); +} + +bool SqliteStorage::setSchemaVersionUpgradeStep(QString upgradeQuery) +{ + // Only used when there is a singlethread (during startup), so we don't need locking here + + // Intentionally do not wrap in a transaction so other functions can include multiple operations + QSqlQuery query(logDb()); + query.prepare("UPDATE coreinfo SET value = :upgradestep WHERE key = 'schemaupgradestep'"); + query.bindValue(":upgradestep", upgradeQuery); + safeExec(query); + + // Don't wrap with watchQuery to avoid an alarming message in the log when the key is missing + // Make sure that the query didn't fail, and that some non-zero number of rows were affected + bool success = !query.lastError().isValid() && query.numRowsAffected() != 0; + + if (!success) { + // The key might not exist (Quassel 0.13.0 and older). Try inserting it... + query = QSqlQuery(logDb()); + query.prepare("INSERT INTO coreinfo (key, value) VALUES ('schemaupgradestep', :upgradestep)"); + query.bindValue(":upgradestep", upgradeQuery); + safeExec(query); + + if (!watchQuery(query)) { + qCritical() << Q_FUNC_INFO << "Setting schema upgrade step failed!"; + success = false; + } + else { + success = true; + } + } + return success; +} + UserId SqliteStorage::addUser(const QString& user, const QString& password, const QString& authenticator) { QSqlDatabase db = logDb(); diff --git a/src/core/sqlitestorage.h b/src/core/sqlitestorage.h index d22ab07a..31f18d3e 100644 --- a/src/core/sqlitestorage.h +++ b/src/core/sqlitestorage.h @@ -139,8 +139,24 @@ protected: QString driverName() override { return "QSQLITE"; } QString databaseName() override { return backlogFile(); } int installedSchemaVersion() override; - bool updateSchemaVersion(int newVersion) override; + bool updateSchemaVersion(int newVersion, bool clearUpgradeStep) override; bool setupSchemaVersion(int version) override; + + /** + * Gets the last successful schema upgrade step, or an empty string if no upgrade is in progress + * + * @return Filename of last successful schema upgrade query, or empty string if not upgrading + */ + QString schemaVersionUpgradeStep() override; + + /** + * Sets the last successful schema upgrade step + * + * @param upgradeQuery The filename of the last successful schema upgrade query + * @return True if successfully set, otherwise false + */ + virtual bool setSchemaVersionUpgradeStep(QString upgradeQuery) override; + bool safeExec(QSqlQuery& query, int retryCount = 0); private: -- 2.20.1