Skip to content

Commit

Permalink
Added index for redirects lookup (#21783)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
9larsons authored Dec 3, 2024
1 parent 6677214 commit 3e0864a
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -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();
});
}
);
Original file line number Diff line number Diff line change
@@ -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}`);
}
}
);
2 changes: 1 addition & 1 deletion ghost/core/core/server/data/schema/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
2 changes: 1 addition & 1 deletion ghost/core/test/unit/server/data/schema/integrity.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down

0 comments on commit 3e0864a

Please sign in to comment.