diff --git a/.changeset/ready-ghosts-fail.md b/.changeset/ready-ghosts-fail.md new file mode 100644 index 0000000000..a1e4a68602 --- /dev/null +++ b/.changeset/ready-ghosts-fail.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend': patch +--- + +Make the `search` foreign key catalog migration non-blocking on large tables by using batch deletes and PostgreSQL `NOT VALID`/`VALIDATE` to reduce lock duration diff --git a/plugins/catalog-backend/migrations/20260214000000_search_fk_final_entities.js b/plugins/catalog-backend/migrations/20260214000000_search_fk_final_entities.js index ed26716ca9..4d43baf9a8 100644 --- a/plugins/catalog-backend/migrations/20260214000000_search_fk_final_entities.js +++ b/plugins/catalog-backend/migrations/20260214000000_search_fk_final_entities.js @@ -16,54 +16,238 @@ // @ts-check +const BATCH_SIZE = 10000; + +/** + * Batch-deletes orphaned search rows whose entity_id doesn't exist in the + * given reference table. Processes in chunks to avoid long locks. + * + * @param {import('knex').Knex} knex + * @param {string} refTable - The table to check entity_id against + */ +async function batchDeleteOrphansPg(knex, refTable) { + for (;;) { + const deleted = await knex.raw(` + DELETE FROM "search" + WHERE ctid IN ( + SELECT s.ctid FROM "search" s + LEFT JOIN "${refTable}" r ON s."entity_id" = r."entity_id" + WHERE r."entity_id" IS NULL + AND s."entity_id" IS NOT NULL + LIMIT ${BATCH_SIZE} + ) + `); + if (deleted.rowCount === 0) { + break; + } + } +} + +/** + * @param {import('knex').Knex} knex + * @param {string} refTable + */ +async function batchDeleteOrphansMysql(knex, refTable) { + for (;;) { + const [orphanIds] = await knex.raw(` + SELECT DISTINCT s.\`entity_id\` FROM \`search\` s + LEFT JOIN \`${refTable}\` r ON s.\`entity_id\` = r.\`entity_id\` + WHERE r.\`entity_id\` IS NULL + AND s.\`entity_id\` IS NOT NULL + LIMIT ${BATCH_SIZE} + `); + if (orphanIds.length === 0) { + break; + } + const ids = orphanIds.map( + (/** @type {{ entity_id: string }} */ r) => r.entity_id, + ); + await knex('search').whereIn('entity_id', ids).delete(); + } +} + /** * Changes the search table's foreign key from refresh_state(entity_id) * to final_entities(entity_id). This allows search entries to reference * final entities directly, with CASCADE delete when entities are removed. * + * On PostgreSQL, the migration first switches the foreign key to point at + * final_entities using a single ALTER TABLE with a NOT VALID constraint, + * then batch-deletes any pre-existing orphaned rows outside of DDL, and + * finally VALIDATEs the constraint to keep the AccessExclusiveLock window + * as short as possible. + * + * On MySQL, the migration batch-deletes orphaned rows in chunks around the + * foreign key change to reduce lock time on large tables. + * * @param {import('knex').Knex} knex */ exports.up = async function up(knex) { - // Step 1: Drop the old foreign key constraint (to refresh_state) - await knex.schema.alterTable('search', table => { - table.dropForeign(['entity_id']); - }); + const client = knex.client.config.client; - // Step 2: Delete orphaned rows where entity_id doesn't exist in final_entities - await knex('search') - .whereNotIn('entity_id', knex('final_entities').select('entity_id')) - .delete(); + if (client.includes('pg')) { + // Drop old FK and immediately add the new one as NOT VALID in a single + // ALTER TABLE statement. This prevents new orphan rows from being + // inserted while we clean up existing ones, and eliminates any window + // where no FK exists at all. + await knex.raw(` + ALTER TABLE "search" + DROP CONSTRAINT IF EXISTS "search_entity_id_foreign", + ADD CONSTRAINT "search_entity_id_foreign" + FOREIGN KEY ("entity_id") REFERENCES "final_entities"("entity_id") + ON DELETE CASCADE + NOT VALID + `); - // Step 3: Add new FK to final_entities(entity_id) with CASCADE - await knex.schema.alterTable('search', table => { - table - .foreign('entity_id') - .references('entity_id') - .inTable('final_entities') - .onDelete('CASCADE'); - }); + // Batch-delete orphaned rows that existed before the NOT VALID FK was + // added. This runs outside any DDL lock, so it doesn't block reads. + await batchDeleteOrphansPg(knex, 'final_entities'); + + // Validate the FK separately. This only takes a + // ShareUpdateExclusiveLock, which does not block normal reads/writes + // (DML) but can still conflict with some DDL or maintenance operations. + await knex.raw( + `ALTER TABLE "search" VALIDATE CONSTRAINT "search_entity_id_foreign"`, + ); + } else if (client.includes('mysql')) { + // Batch-delete orphaned rows before DDL to reduce lock time. + await batchDeleteOrphansMysql(knex, 'final_entities'); + + // Swap the FK with retry logic. MySQL DDL causes implicit commits so + // DROP and ADD are never truly atomic. If new orphan rows sneak in + // between the DROP and ADD, the ADD will fail — we clean up and retry. + // The information_schema check makes the DROP idempotent so that + // re-runs after a partial failure don't crash. + const MAX_ATTEMPTS = 5; + for (let attempt = 1; attempt <= MAX_ATTEMPTS; attempt++) { + const [fks] = await knex.raw(` + SELECT CONSTRAINT_NAME FROM information_schema.TABLE_CONSTRAINTS + WHERE TABLE_SCHEMA = DATABASE() + AND TABLE_NAME = 'search' + AND CONSTRAINT_NAME = 'search_entity_id_foreign' + AND CONSTRAINT_TYPE = 'FOREIGN KEY' + `); + if (fks.length > 0) { + await knex.schema.alterTable('search', table => { + table.dropForeign(['entity_id']); + }); + } + + await batchDeleteOrphansMysql(knex, 'final_entities'); + + try { + await knex.schema.alterTable('search', table => { + table + .foreign('entity_id') + .references('entity_id') + .inTable('final_entities') + .onDelete('CASCADE'); + }); + break; + } catch (e) { + if (attempt === MAX_ATTEMPTS) throw e; + } + } + } else { + // SQLite: wrap in an explicit transaction since the global transaction + // wrapper is disabled for this migration. + await knex.transaction(async trx => { + await trx.schema.alterTable('search', table => { + table.dropForeign(['entity_id']); + }); + + await trx('search') + .whereNotIn('entity_id', trx('final_entities').select('entity_id')) + .delete(); + + await trx.schema.alterTable('search', table => { + table + .foreign('entity_id') + .references('entity_id') + .inTable('final_entities') + .onDelete('CASCADE'); + }); + }); + } }; /** * @param {import('knex').Knex} knex */ exports.down = async function down(knex) { - // Step 1: Drop FK to final_entities - await knex.schema.alterTable('search', table => { - table.dropForeign(['entity_id']); - }); + const client = knex.client.config.client; - // Step 2: Delete orphaned rows where entity_id doesn't exist in refresh_state - await knex('search') - .whereNotIn('entity_id', knex('refresh_state').select('entity_id')) - .delete(); + if (client.includes('pg')) { + await knex.raw(` + ALTER TABLE "search" + DROP CONSTRAINT IF EXISTS "search_entity_id_foreign", + ADD CONSTRAINT "search_entity_id_foreign" + FOREIGN KEY ("entity_id") REFERENCES "refresh_state"("entity_id") + ON DELETE CASCADE + NOT VALID + `); - // Step 3: Add back FK to refresh_state(entity_id) with CASCADE - await knex.schema.alterTable('search', table => { - table - .foreign('entity_id') - .references('entity_id') - .inTable('refresh_state') - .onDelete('CASCADE'); - }); + await batchDeleteOrphansPg(knex, 'refresh_state'); + + await knex.raw( + `ALTER TABLE "search" VALIDATE CONSTRAINT "search_entity_id_foreign"`, + ); + } else if (client.includes('mysql')) { + await batchDeleteOrphansMysql(knex, 'refresh_state'); + + const MAX_ATTEMPTS = 5; + for (let attempt = 1; attempt <= MAX_ATTEMPTS; attempt++) { + const [fks] = await knex.raw(` + SELECT CONSTRAINT_NAME FROM information_schema.TABLE_CONSTRAINTS + WHERE TABLE_SCHEMA = DATABASE() + AND TABLE_NAME = 'search' + AND CONSTRAINT_NAME = 'search_entity_id_foreign' + AND CONSTRAINT_TYPE = 'FOREIGN KEY' + `); + if (fks.length > 0) { + await knex.schema.alterTable('search', table => { + table.dropForeign(['entity_id']); + }); + } + + await batchDeleteOrphansMysql(knex, 'refresh_state'); + + try { + await knex.schema.alterTable('search', table => { + table + .foreign('entity_id') + .references('entity_id') + .inTable('refresh_state') + .onDelete('CASCADE'); + }); + break; + } catch (e) { + if (attempt === MAX_ATTEMPTS) throw e; + } + } + } else { + await knex.transaction(async trx => { + await trx.schema.alterTable('search', table => { + table.dropForeign(['entity_id']); + }); + + await trx('search') + .whereNotIn('entity_id', trx('refresh_state').select('entity_id')) + .delete(); + + await trx.schema.alterTable('search', table => { + table + .foreign('entity_id') + .references('entity_id') + .inTable('refresh_state') + .onDelete('CASCADE'); + }); + }); + } +}; + +// Disable the default transaction wrapper so the batched deletes run +// outside of the DDL transaction that holds AccessExclusiveLock. +exports.config = { + transaction: false, }; diff --git a/plugins/catalog-backend/src/tests/migrations.test.ts b/plugins/catalog-backend/src/tests/migrations.test.ts index 4a469ab07e..24a23b2888 100644 --- a/plugins/catalog-backend/src/tests/migrations.test.ts +++ b/plugins/catalog-backend/src/tests/migrations.test.ts @@ -784,67 +784,96 @@ describe('migrations', () => { value: 'my-api', original_value: 'my-api', }, + // NULL entity_id row should survive the migration + { + entity_id: null, + key: 'global', + value: 'setting', + original_value: 'setting', + }, ]); // Verify initial state const preMigrationCount = await knex('search').count('* as count'); - expect(Number(preMigrationCount[0].count)).toBe(6); + expect(Number(preMigrationCount[0].count)).toBe(7); // Run the migration await migrateUpOnce(knex); - // Verify orphaned rows (id3) were deleted since id3 is not in final_entities - const postMigrationRows = await knex('search').orderBy([ - 'entity_id', - 'key', - ]); - expect(postMigrationRows).toEqual([ - { - entity_id: 'id1', + // Verify orphaned rows (id3) were deleted, but NULL entity_id row survived + const postMigrationRows = await knex('search'); + expect(postMigrationRows).toHaveLength(5); + expect(postMigrationRows).toEqual( + expect.arrayContaining([ + { + entity_id: null, + key: 'global', + value: 'setting', + original_value: 'setting', + }, + { + entity_id: 'id1', + key: 'kind', + value: 'component', + original_value: 'Component', + }, + { + entity_id: 'id1', + key: 'metadata.name', + value: 'service-a', + original_value: 'service-a', + }, + { + entity_id: 'id2', + key: 'kind', + value: 'component', + original_value: 'Component', + }, + { + entity_id: 'id2', + key: 'metadata.name', + value: 'service-b', + original_value: 'service-b', + }, + ]), + ); + + // Verify FK is enforced: inserting with a nonexistent entity_id should be rejected + await expect( + knex('search').insert({ + entity_id: 'nonexistent', key: 'kind', - value: 'component', - original_value: 'Component', - }, - { - entity_id: 'id1', - key: 'metadata.name', - value: 'service-a', - original_value: 'service-a', - }, - { - entity_id: 'id2', - key: 'kind', - value: 'component', - original_value: 'Component', - }, - { - entity_id: 'id2', - key: 'metadata.name', - value: 'service-b', - original_value: 'service-b', - }, - ]); + value: 'test', + original_value: 'test', + }), + ).rejects.toEqual(expect.anything()); // Verify FK cascade works: deleting from final_entities should cascade to search await knex('final_entities').where({ entity_id: 'id1' }).delete(); - const searchAfterDelete = await knex('search').orderBy([ - 'entity_id', - 'key', - ]); - expect(searchAfterDelete).toEqual([ - { - entity_id: 'id2', - key: 'kind', - value: 'component', - original_value: 'Component', - }, - { - entity_id: 'id2', - key: 'metadata.name', - value: 'service-b', - original_value: 'service-b', - }, - ]); + const searchAfterDelete = await knex('search'); + expect(searchAfterDelete).toHaveLength(3); + expect(searchAfterDelete).toEqual( + expect.arrayContaining([ + { + entity_id: null, + key: 'global', + value: 'setting', + original_value: 'setting', + }, + { + entity_id: 'id2', + key: 'kind', + value: 'component', + original_value: 'Component', + }, + { + entity_id: 'id2', + key: 'metadata.name', + value: 'service-b', + original_value: 'service-b', + }, + ]), + ); // Restore id1 for down migration test await knex('final_entities').insert({ @@ -863,53 +892,49 @@ describe('migrations', () => { }, ]); + // Delete id1 from refresh_state so it becomes an orphan for the down + // migration (exists in final_entities but not in refresh_state) + await knex('refresh_state').where({ entity_id: 'id1' }).delete(); + // Run the down migration await migrateDownOnce(knex); - // Verify data is still intact after down migration - const revertedSearchRows = await knex('search').orderBy([ - 'entity_id', - 'key', - ]); - expect(revertedSearchRows).toEqual([ - { - entity_id: 'id1', - key: 'kind', - value: 'component', - original_value: 'Component', - }, - { - entity_id: 'id2', - key: 'kind', - value: 'component', - original_value: 'Component', - }, - { - entity_id: 'id2', - key: 'metadata.name', - value: 'service-b', - original_value: 'service-b', - }, - ]); + // Verify id1 search rows were cleaned up (orphan for refresh_state FK), + // while id2 and the NULL entity_id row survived + const revertedSearchRows = await knex('search'); + expect(revertedSearchRows).toHaveLength(3); + expect(revertedSearchRows).toEqual( + expect.arrayContaining([ + { + entity_id: null, + key: 'global', + value: 'setting', + original_value: 'setting', + }, + { + entity_id: 'id2', + key: 'kind', + value: 'component', + original_value: 'Component', + }, + { + entity_id: 'id2', + key: 'metadata.name', + value: 'service-b', + original_value: 'service-b', + }, + ]), + ); // Verify FK is back to refresh_state: deleting from refresh_state should cascade - await knex('refresh_state').where({ entity_id: 'id1' }).delete(); - const afterRefreshStateDelete = await knex('search').orderBy([ - 'entity_id', - 'key', - ]); + await knex('refresh_state').where({ entity_id: 'id2' }).delete(); + const afterRefreshStateDelete = await knex('search'); expect(afterRefreshStateDelete).toEqual([ { - entity_id: 'id2', - key: 'kind', - value: 'component', - original_value: 'Component', - }, - { - entity_id: 'id2', - key: 'metadata.name', - value: 'service-b', - original_value: 'service-b', + entity_id: null, + key: 'global', + value: 'setting', + original_value: 'setting', }, ]);