From ec46a40e395b505c2da7b47b06ce3bdd0324b549 Mon Sep 17 00:00:00 2001 From: Daniel Lockyer Date: Wed, 21 Jul 2021 14:16:49 +0100 Subject: [PATCH] =?UTF-8?q?=E2=9A=A1=20Optimized=20performance=20of=20inte?= =?UTF-8?q?grity=20check=20DB=20calls?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit refs https://github.com/TryGhost/knex-migrator/issues/252 - the referenced issue describes performance problems with the integrity check code in knex-migrator - this commit solves half of the problem - 1 DB call per migration minor version folder - this results in 70+ (and growing!) DB calls at boot, with each one going back and forth to the DB... not efficient - what the code actually needed was a count of how many migrations are currently in the DB, group by minor version - this is very easy to do in SQL and therefore knex - this commit switches the code around to group all the DB migrations first, and then compares against the folders locally - this results in a drop of SQL queries from 70+ to 1 🚀 --- lib/index.js | 118 ++++++++++++++++++++++++++------------------------- 1 file changed, 60 insertions(+), 58 deletions(-) diff --git a/lib/index.js b/lib/index.js index 5434c01f..75712b6e 100644 --- a/lib/index.js +++ b/lib/index.js @@ -1132,7 +1132,6 @@ KnexMigrator.prototype._integrityCheck = function _integrityCheck(options) { subfolder = this.subfolder, force = options.force, folders = [], - operations = {}, toReturn = {}, futureVersions = []; @@ -1148,29 +1147,69 @@ KnexMigrator.prototype._integrityCheck = function _integrityCheck(options) { // ignore } - _.each(folders, function (folder) { - // CASE: versions/1.1-members or versions/2.0-payments - if (folder !== 'init') { - try { - folder = folder.match(/([\d._]+)/)[0]; - } catch (err) { - logging.warn('Cannot parse folder name.'); - logging.warn('Ignore Folder: ' + folder); - return; - } - } + return this + .connection('migrations') + .select('version') + .count('version', {as: 'c'}) + .groupBy('version') + .then((dbMigrations) => { + _.each(folders, function (folder) { + // CASE: versions/1.1-members or versions/2.0-payments + if (folder !== 'init') { + try { + folder = folder.match(/([\d._]+)/)[0]; + } catch (err) { + logging.warn('Cannot parse folder name.'); + logging.warn('Ignore Folder: ' + folder); + return; + } + } + + // CASE: + // if your current version is 1.0 and you add migration scripts for the next version 1.1 + // we won't execute them until your current version changes to 1.1 or until you force KM to migrate to it + if (self.currentVersion && !force) { + if (utils.isGreaterThanVersion({smallerVersion: self.currentVersion, greaterVersion: folder})) { + futureVersions.push(folder); + } + } + + let actual = 0; + let expected; + + const migrationCount = dbMigrations.find(m => m.version === folder); + if (migrationCount) { + actual = migrationCount.c; + } + + if (folder !== 'init') { + expected = utils.listFiles(path.join(self.migrationPath, subfolder, folder)).length; + } else { + expected = utils.listFiles(path.join(self.migrationPath, folder)).length; + } - // CASE: - // if your current version is 1.0 and you add migration scripts for the next version 1.1 - // we won't execute them until your current version changes to 1.1 or until you force KM to migrate to it - if (self.currentVersion && !force) { - if (utils.isGreaterThanVersion({smallerVersion: self.currentVersion, greaterVersion: folder})) { - futureVersions.push(folder); + debug('Version ' + folder + ' expected: ' + expected); + debug('Version ' + folder + ' actual: ' + actual); + + toReturn[folder] = { + expected: expected, + actual: actual + }; + }); + + // CASE: ensure that either you have to run `migrate --force` or they ran already + if (futureVersions.length) { + _.each(futureVersions, function (futureVersion) { + if (toReturn[futureVersion].actual !== toReturn[futureVersion].expected) { + logging.warn('knex-migrator is skipping ' + futureVersion); + logging.warn('Current version in MigratorConfig.js is smaller then requested version, use --force to proceed!'); + logging.warn('Please run `knex-migrator migrate --v ' + futureVersion + ' --force` to proceed!'); + delete toReturn[futureVersion]; + } + }); } - } - operations[folder] = self.connection('migrations').where({ - version: folder + return toReturn; }).catch(function onMigrationsLookupError(err) { // CASE: no database selected (database.connection.database="") if (err.errno === 1046) { @@ -1199,43 +1238,6 @@ KnexMigrator.prototype._integrityCheck = function _integrityCheck(options) { throw err; }); - }); - - return Promise.props(operations) - .then(function (result) { - _.each(result, function (value, version) { - let actual = value.length, - expected = actual; - - if (version !== 'init') { - expected = utils.listFiles(path.join(self.migrationPath, subfolder, version)).length; - } else { - expected = utils.listFiles(path.join(self.migrationPath, version)).length; - } - - debug('Version ' + version + ' expected: ' + expected); - debug('Version ' + version + ' actual: ' + actual); - - toReturn[version] = { - expected: expected, - actual: actual - }; - }); - - // CASE: ensure that either you have to run `migrate --force` or they ran already - if (futureVersions.length) { - _.each(futureVersions, function (futureVersion) { - if (toReturn[futureVersion].actual !== toReturn[futureVersion].expected) { - logging.warn('knex-migrator is skipping ' + futureVersion); - logging.warn('Current version in MigratorConfig.js is smaller then requested version, use --force to proceed!'); - logging.warn('Please run `knex-migrator migrate --v ' + futureVersion + ' --force` to proceed!'); - delete toReturn[futureVersion]; - } - }); - } - - return toReturn; - }); }; module.exports = KnexMigrator;