From 3e0864ad6d8c05904aa97e185bb2ec43326274a2 Mon Sep 17 00:00:00 2001 From: Steve Larson <9larsons@gmail.com> Date: Tue, 3 Dec 2024 08:03:12 -0600 Subject: [PATCH] Added index for redirects lookup (#21783) ref https://linear.app/ghost/issue/ENG-1811/ - added first migration for truncating column length; required in order to be able to add an index - added second migration for creating the index After sending a newsletter, Ghost can struggle with the amount of incoming traffic (particularly link scanners) with particularly large pools of recipients. Part of this is link lookup, which an index has shown to help with when added manually on the db. Given that, this change is to make it native. I ran a stats service query to gather data on usage, as well as did code review, and I've not seen any use of this table that isn't using the 8 digit slug, i.e. /r/12345678, or 11 characters total. I made this 191 in case that use case changes, while still allowing us the value of an index. Note: SQLite does not support altering columns to my knowledge, so we have to create a temp table with the new column data to push back in. This should be ok because use of SQLite should be limited. In the past, I've seen migrations skipped for SQLite and I'd rather not do that here if able. --- ...02-17-32-40-alter-length-redirects-from.js | 19 ++++++++++++ ...12-02-17-48-40-add-index-redirects-from.js | 31 +++++++++++++++++++ ghost/core/core/server/data/schema/schema.js | 2 +- .../unit/server/data/schema/integrity.test.js | 2 +- 4 files changed, 52 insertions(+), 2 deletions(-) create mode 100644 ghost/core/core/server/data/migrations/versions/5.102/2024-12-02-17-32-40-alter-length-redirects-from.js create mode 100644 ghost/core/core/server/data/migrations/versions/5.102/2024-12-02-17-48-40-add-index-redirects-from.js diff --git a/ghost/core/core/server/data/migrations/versions/5.102/2024-12-02-17-32-40-alter-length-redirects-from.js b/ghost/core/core/server/data/migrations/versions/5.102/2024-12-02-17-32-40-alter-length-redirects-from.js new file mode 100644 index 0000000000..fccd444193 --- /dev/null +++ b/ghost/core/core/server/data/migrations/versions/5.102/2024-12-02-17-32-40-alter-length-redirects-from.js @@ -0,0 +1,19 @@ +// For information on writing migrations, see https://www.notion.so/ghost/Database-migrations-eb5b78c435d741d2b34a582d57c24253 + +const logging = require('@tryghost/logging'); +const {createNonTransactionalMigration} = require('../../utils'); + +module.exports = createNonTransactionalMigration( + async function up(knex) { + logging.info('Altering length of redirects.from'); + await knex.schema.alterTable('redirects', function (table) { + table.string('from', 191).notNullable().alter(); + }); + }, + async function down(knex) { + logging.info('Reverting length of redirects.from'); + await knex.schema.alterTable('redirects', function (table) { + table.string('from', 191).notNullable().alter(); + }); + } +); diff --git a/ghost/core/core/server/data/migrations/versions/5.102/2024-12-02-17-48-40-add-index-redirects-from.js b/ghost/core/core/server/data/migrations/versions/5.102/2024-12-02-17-48-40-add-index-redirects-from.js new file mode 100644 index 0000000000..fc4eb62b28 --- /dev/null +++ b/ghost/core/core/server/data/migrations/versions/5.102/2024-12-02-17-48-40-add-index-redirects-from.js @@ -0,0 +1,31 @@ +// For information on writing migrations, see https://www.notion.so/ghost/Database-migrations-eb5b78c435d741d2b34a582d57c24253 + +const logging = require('@tryghost/logging'); +const {createNonTransactionalMigration} = require('../../utils'); +const {addIndex, dropIndex} = require('../../../schema/commands'); +const errors = require('@tryghost/errors'); + +module.exports = createNonTransactionalMigration( + async function up(knex) { + logging.info('Adding index to redirects.from'); + + const columnInfo = await knex('redirects').columnInfo('from'); + // knex is wrong; it's returning a string not a number so we re-cast it to satisfy the type checker + if (columnInfo.maxLength.toString() !== '191') { + logging.error(`Column length is not 191. Ensure the previous migration has been applied successfully. Column info: ${JSON.stringify(columnInfo)}`); + throw new errors.MigrationError({ + message: 'Column length is not 191. Ensure the previous migration has been applied successfully.' + }); + } + + await addIndex('redirects', ['from'], knex); + }, + async function down(knex) { + logging.info('Removing index from redirects.from'); + try { + await dropIndex('redirects', ['from'], knex); + } catch (error) { + logging.error(`Error removing index from redirects.from: ${error}`); + } + } +); diff --git a/ghost/core/core/server/data/schema/schema.js b/ghost/core/core/server/data/schema/schema.js index 0b1dab15a3..96afdb911d 100644 --- a/ghost/core/core/server/data/schema/schema.js +++ b/ghost/core/core/server/data/schema/schema.js @@ -993,7 +993,7 @@ module.exports = { }, redirects: { id: {type: 'string', maxlength: 24, nullable: false, primary: true}, - from: {type: 'string', maxlength: 2000, nullable: false}, + from: {type: 'string', maxlength: 191, nullable: false, index: true}, to: {type: 'string', maxlength: 2000, nullable: false}, post_id: {type: 'string', maxlength: 24, nullable: true, unique: false, references: 'posts.id', setNullDelete: true}, created_at: {type: 'dateTime', nullable: false}, diff --git a/ghost/core/test/unit/server/data/schema/integrity.test.js b/ghost/core/test/unit/server/data/schema/integrity.test.js index 8b1f114df9..195ad20cc2 100644 --- a/ghost/core/test/unit/server/data/schema/integrity.test.js +++ b/ghost/core/test/unit/server/data/schema/integrity.test.js @@ -35,7 +35,7 @@ const validateRouteSettings = require('../../../../../core/server/services/route */ describe('DB version integrity', function () { // Only these variables should need updating - const currentSchemaHash = 'f12341a0c74998eeb4628322fd0982fb'; + const currentSchemaHash = 'b26690fb57ffd0edbddb4cd9e02b17d6'; const currentFixturesHash = '80e79d1efd5da275e19cb375afb4ad04'; const currentSettingsHash = '47a75e8898fab270174a0c905cb3e914'; const currentRoutesHash = '3d180d52c663d173a6be791ef411ed01';