From b4e28e1c27a9b2a31e208d8e845b2142d2e63e45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 11 Mar 2026 10:58:42 +0100 Subject: [PATCH 01/11] fix(catalog): make search FK migration safe for large databases MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The search FK migration previously ran all DDL and a bulk DELETE in a single transaction, holding AccessExclusiveLock for the entire duration. On large tables this blocks all reads for potentially minutes or hours. This restructures the migration per database engine: - PostgreSQL: batch-deletes orphans before DDL, uses NOT VALID to skip full table scan under AccessExclusiveLock, then VALIDATE CONSTRAINT under the weaker ShareUpdateExclusiveLock - MySQL: batch-deletes orphans with LIMIT before DDL - SQLite: unchanged simple approach (no locking concerns) Also sets transaction: false so the batched deletes run outside the DDL transaction. Co-Authored-By: Claude Opus 4.6 Signed-off-by: Fredrik Adelöw --- ...20260214000000_search_fk_final_entities.js | 187 +++++++++++++++--- 1 file changed, 155 insertions(+), 32 deletions(-) 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..917a4e36b2 100644 --- a/plugins/catalog-backend/migrations/20260214000000_search_fk_final_entities.js +++ b/plugins/catalog-backend/migrations/20260214000000_search_fk_final_entities.js @@ -21,49 +21,172 @@ * to final_entities(entity_id). This allows search entries to reference * final entities directly, with CASCADE delete when entities are removed. * + * For PostgreSQL and MySQL, the migration is structured to minimize lock + * time on large tables by batch-deleting orphaned rows before any DDL. + * PostgreSQL additionally uses NOT VALID / VALIDATE CONSTRAINT to keep + * the AccessExclusiveLock duration minimal. + * * @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')) { + // Batch-delete orphaned rows BEFORE touching the schema. + // This runs outside any DDL lock, so it doesn't block reads. + for (;;) { + const deleted = await knex.raw(` + DELETE FROM "search" + WHERE ctid IN ( + SELECT s.ctid FROM "search" s + LEFT JOIN "final_entities" fe ON s."entity_id" = fe."entity_id" + WHERE fe."entity_id" IS NULL + LIMIT 10000 + ) + `); + if (deleted.rowCount === 0) { + break; + } + } - // 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'); - }); + // Drop old FK and add new one with NOT VALID (minimal lock time). + // NOT VALID skips the full table scan — we already cleaned up orphans above. + await knex.raw( + `ALTER TABLE "search" DROP CONSTRAINT "search_entity_id_foreign"`, + ); + await knex.raw(` + ALTER TABLE "search" + ADD CONSTRAINT "search_entity_id_foreign" + FOREIGN KEY ("entity_id") REFERENCES "final_entities"("entity_id") + ON DELETE CASCADE + NOT VALID + `); + + // Validate the FK separately. This only takes a + // ShareUpdateExclusiveLock, which does NOT block reads/writes. + 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. + for (;;) { + const [result] = await knex.raw( + `DELETE FROM \`search\` WHERE \`entity_id\` NOT IN (SELECT \`entity_id\` FROM \`final_entities\`) LIMIT 10000`, + ); + if (result.affectedRows === 0) { + break; + } + } + + // Drop old FK and add new one. MySQL does not support NOT VALID, but + // the table is already clean so validation is fast. + await knex.schema.alterTable('search', table => { + table.dropForeign(['entity_id']); + }); + await knex.schema.alterTable('search', table => { + table + .foreign('entity_id') + .references('entity_id') + .inTable('final_entities') + .onDelete('CASCADE'); + }); + } else { + // SQLite: simple approach, locking is not a concern + await knex.schema.alterTable('search', table => { + table.dropForeign(['entity_id']); + }); + + await knex('search') + .whereNotIn('entity_id', knex('final_entities').select('entity_id')) + .delete(); + + await knex.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')) { + for (;;) { + const deleted = await knex.raw(` + DELETE FROM "search" + WHERE ctid IN ( + SELECT s.ctid FROM "search" s + LEFT JOIN "refresh_state" rs ON s."entity_id" = rs."entity_id" + WHERE rs."entity_id" IS NULL + LIMIT 10000 + ) + `); + if (deleted.rowCount === 0) { + break; + } + } - // 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 knex.raw( + `ALTER TABLE "search" DROP CONSTRAINT "search_entity_id_foreign"`, + ); + await knex.raw(` + ALTER TABLE "search" + ADD CONSTRAINT "search_entity_id_foreign" + FOREIGN KEY ("entity_id") REFERENCES "refresh_state"("entity_id") + ON DELETE CASCADE + NOT VALID + `); + + await knex.raw( + `ALTER TABLE "search" VALIDATE CONSTRAINT "search_entity_id_foreign"`, + ); + } else if (client.includes('mysql')) { + for (;;) { + const [result] = await knex.raw( + `DELETE FROM \`search\` WHERE \`entity_id\` NOT IN (SELECT \`entity_id\` FROM \`refresh_state\`) LIMIT 10000`, + ); + if (result.affectedRows === 0) { + break; + } + } + + await knex.schema.alterTable('search', table => { + table.dropForeign(['entity_id']); + }); + await knex.schema.alterTable('search', table => { + table + .foreign('entity_id') + .references('entity_id') + .inTable('refresh_state') + .onDelete('CASCADE'); + }); + } else { + await knex.schema.alterTable('search', table => { + table.dropForeign(['entity_id']); + }); + + await knex('search') + .whereNotIn('entity_id', knex('refresh_state').select('entity_id')) + .delete(); + + await knex.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, }; From ffe5febfd625e1f31fe7c4b1bc5da0c808f12356 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 11 Mar 2026 11:04:52 +0100 Subject: [PATCH 02/11] fix: use LEFT JOIN for MySQL batch deletes in search FK migration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review feedback: replace NOT IN subquery with LEFT JOIN for MySQL batch deletes. Since MySQL doesn't support LIMIT in multi-table DELETE, orphan entity_ids are found via SELECT first, then deleted in a separate statement. Co-Authored-By: Claude Opus 4.6 Signed-off-by: Fredrik Adelöw --- ...20260214000000_search_fk_final_entities.js | 29 ++++++++++++++----- 1 file changed, 21 insertions(+), 8 deletions(-) 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 917a4e36b2..1b483268b0 100644 --- a/plugins/catalog-backend/migrations/20260214000000_search_fk_final_entities.js +++ b/plugins/catalog-backend/migrations/20260214000000_search_fk_final_entities.js @@ -69,13 +69,21 @@ exports.up = async function up(knex) { ); } else if (client.includes('mysql')) { // Batch-delete orphaned rows before DDL to reduce lock time. + // Uses LEFT JOIN to find orphans efficiently, then deletes by entity_id. + // MySQL doesn't support LIMIT in multi-table DELETE, so we find the + // orphaned entity_ids first, then delete in a separate statement. for (;;) { - const [result] = await knex.raw( - `DELETE FROM \`search\` WHERE \`entity_id\` NOT IN (SELECT \`entity_id\` FROM \`final_entities\`) LIMIT 10000`, - ); - if (result.affectedRows === 0) { + const [orphanIds] = await knex.raw(` + SELECT DISTINCT s.\`entity_id\` FROM \`search\` s + LEFT JOIN \`final_entities\` fe ON s.\`entity_id\` = fe.\`entity_id\` + WHERE fe.\`entity_id\` IS NULL + LIMIT 10000 + `); + if (orphanIds.length === 0) { break; } + const ids = orphanIds.map(r => r.entity_id); + await knex('search').whereIn('entity_id', ids).delete(); } // Drop old FK and add new one. MySQL does not support NOT VALID, but @@ -148,12 +156,17 @@ exports.down = async function down(knex) { ); } else if (client.includes('mysql')) { for (;;) { - const [result] = await knex.raw( - `DELETE FROM \`search\` WHERE \`entity_id\` NOT IN (SELECT \`entity_id\` FROM \`refresh_state\`) LIMIT 10000`, - ); - if (result.affectedRows === 0) { + const [orphanIds] = await knex.raw(` + SELECT DISTINCT s.\`entity_id\` FROM \`search\` s + LEFT JOIN \`refresh_state\` rs ON s.\`entity_id\` = rs.\`entity_id\` + WHERE rs.\`entity_id\` IS NULL + LIMIT 10000 + `); + if (orphanIds.length === 0) { break; } + const ids = orphanIds.map(r => r.entity_id); + await knex('search').whereIn('entity_id', ids).delete(); } await knex.schema.alterTable('search', table => { From 42a42d56251bd95e6ca030754ebdb1b5cb01d0b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 11 Mar 2026 11:06:57 +0100 Subject: [PATCH 03/11] fix: add JSDoc type annotation for MySQL orphan ID mapping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.6 Signed-off-by: Fredrik Adelöw --- .../migrations/20260214000000_search_fk_final_entities.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) 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 1b483268b0..9c90567dfc 100644 --- a/plugins/catalog-backend/migrations/20260214000000_search_fk_final_entities.js +++ b/plugins/catalog-backend/migrations/20260214000000_search_fk_final_entities.js @@ -82,7 +82,9 @@ exports.up = async function up(knex) { if (orphanIds.length === 0) { break; } - const ids = orphanIds.map(r => r.entity_id); + const ids = orphanIds.map( + (/** @type {{ entity_id: string }} */ r) => r.entity_id, + ); await knex('search').whereIn('entity_id', ids).delete(); } @@ -165,7 +167,9 @@ exports.down = async function down(knex) { if (orphanIds.length === 0) { break; } - const ids = orphanIds.map(r => r.entity_id); + const ids = orphanIds.map( + (/** @type {{ entity_id: string }} */ r) => r.entity_id, + ); await knex('search').whereIn('entity_id', ids).delete(); } From 65364716da916c9724523d1eb5b87aa13277d178 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 11 Mar 2026 11:53:28 +0100 Subject: [PATCH 04/11] fix: add NULL safety and idempotent DROP CONSTRAINT to search FK migration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prevent orphan cleanup queries from incorrectly matching rows with NULL entity_id via LEFT JOIN, and use DROP CONSTRAINT IF EXISTS for safer partial re-runs given transaction: false. Co-Authored-By: Claude Opus 4.6 Signed-off-by: Fredrik Adelöw --- .../migrations/20260214000000_search_fk_final_entities.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) 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 9c90567dfc..6c2136de4d 100644 --- a/plugins/catalog-backend/migrations/20260214000000_search_fk_final_entities.js +++ b/plugins/catalog-backend/migrations/20260214000000_search_fk_final_entities.js @@ -41,6 +41,7 @@ exports.up = async function up(knex) { SELECT s.ctid FROM "search" s LEFT JOIN "final_entities" fe ON s."entity_id" = fe."entity_id" WHERE fe."entity_id" IS NULL + AND s."entity_id" IS NOT NULL LIMIT 10000 ) `); @@ -52,7 +53,7 @@ exports.up = async function up(knex) { // Drop old FK and add new one with NOT VALID (minimal lock time). // NOT VALID skips the full table scan — we already cleaned up orphans above. await knex.raw( - `ALTER TABLE "search" DROP CONSTRAINT "search_entity_id_foreign"`, + `ALTER TABLE "search" DROP CONSTRAINT IF EXISTS "search_entity_id_foreign"`, ); await knex.raw(` ALTER TABLE "search" @@ -77,6 +78,7 @@ exports.up = async function up(knex) { SELECT DISTINCT s.\`entity_id\` FROM \`search\` s LEFT JOIN \`final_entities\` fe ON s.\`entity_id\` = fe.\`entity_id\` WHERE fe.\`entity_id\` IS NULL + AND s.\`entity_id\` IS NOT NULL LIMIT 10000 `); if (orphanIds.length === 0) { @@ -134,6 +136,7 @@ exports.down = async function down(knex) { SELECT s.ctid FROM "search" s LEFT JOIN "refresh_state" rs ON s."entity_id" = rs."entity_id" WHERE rs."entity_id" IS NULL + AND s."entity_id" IS NOT NULL LIMIT 10000 ) `); @@ -143,7 +146,7 @@ exports.down = async function down(knex) { } await knex.raw( - `ALTER TABLE "search" DROP CONSTRAINT "search_entity_id_foreign"`, + `ALTER TABLE "search" DROP CONSTRAINT IF EXISTS "search_entity_id_foreign"`, ); await knex.raw(` ALTER TABLE "search" @@ -162,6 +165,7 @@ exports.down = async function down(knex) { SELECT DISTINCT s.\`entity_id\` FROM \`search\` s LEFT JOIN \`refresh_state\` rs ON s.\`entity_id\` = rs.\`entity_id\` WHERE rs.\`entity_id\` IS NULL + AND s.\`entity_id\` IS NOT NULL LIMIT 10000 `); if (orphanIds.length === 0) { From 3644b725f88912cf7045436cfa7142b001d9f6e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 11 Mar 2026 13:58:44 +0100 Subject: [PATCH 05/11] add changeset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/ready-ghosts-fail.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/ready-ghosts-fail.md diff --git a/.changeset/ready-ghosts-fail.md b/.changeset/ready-ghosts-fail.md new file mode 100644 index 0000000000..de3c600bc0 --- /dev/null +++ b/.changeset/ready-ghosts-fail.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend': patch +--- + +Optimize a catalog migration for big databases From 1ecf82a5c29baccae68adeda04994b0e8864ab6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 11 Mar 2026 14:00:52 +0100 Subject: [PATCH 06/11] catalog-backend: wrap MySQL/SQLite migration branches in explicit transactions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The search FK migration uses transaction: false for PostgreSQL's benefit, but this left MySQL and SQLite branches non-atomic. A failure between dropForeign and the new addForeign would leave the table with no FK constraint. Wrap those branches in explicit knex.transaction() calls. Co-Authored-By: Claude Opus 4.6 Signed-off-by: Fredrik Adelöw --- ...20260214000000_search_fk_final_entities.js | 101 ++++++++++-------- 1 file changed, 56 insertions(+), 45 deletions(-) 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 6c2136de4d..4f3885aa17 100644 --- a/plugins/catalog-backend/migrations/20260214000000_search_fk_final_entities.js +++ b/plugins/catalog-backend/migrations/20260214000000_search_fk_final_entities.js @@ -90,34 +90,41 @@ exports.up = async function up(knex) { await knex('search').whereIn('entity_id', ids).delete(); } - // Drop old FK and add new one. MySQL does not support NOT VALID, but - // the table is already clean so validation is fast. - await knex.schema.alterTable('search', table => { - table.dropForeign(['entity_id']); - }); - await knex.schema.alterTable('search', table => { - table - .foreign('entity_id') - .references('entity_id') - .inTable('final_entities') - .onDelete('CASCADE'); + // Drop old FK and add new one inside an explicit transaction, since the + // global transaction wrapper is disabled for this migration. MySQL does + // not support NOT VALID, but the table is already clean so validation + // is fast. + await knex.transaction(async trx => { + await trx.schema.alterTable('search', table => { + table.dropForeign(['entity_id']); + }); + await trx.schema.alterTable('search', table => { + table + .foreign('entity_id') + .references('entity_id') + .inTable('final_entities') + .onDelete('CASCADE'); + }); }); } else { - // SQLite: simple approach, locking is not a concern - await knex.schema.alterTable('search', table => { - table.dropForeign(['entity_id']); - }); + // 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 knex('search') - .whereNotIn('entity_id', knex('final_entities').select('entity_id')) - .delete(); + await trx('search') + .whereNotIn('entity_id', trx('final_entities').select('entity_id')) + .delete(); - await knex.schema.alterTable('search', table => { - table - .foreign('entity_id') - .references('entity_id') - .inTable('final_entities') - .onDelete('CASCADE'); + await trx.schema.alterTable('search', table => { + table + .foreign('entity_id') + .references('entity_id') + .inTable('final_entities') + .onDelete('CASCADE'); + }); }); } }; @@ -177,31 +184,35 @@ exports.down = async function down(knex) { await knex('search').whereIn('entity_id', ids).delete(); } - await knex.schema.alterTable('search', table => { - table.dropForeign(['entity_id']); - }); - await knex.schema.alterTable('search', table => { - table - .foreign('entity_id') - .references('entity_id') - .inTable('refresh_state') - .onDelete('CASCADE'); + await knex.transaction(async trx => { + await trx.schema.alterTable('search', table => { + table.dropForeign(['entity_id']); + }); + await trx.schema.alterTable('search', table => { + table + .foreign('entity_id') + .references('entity_id') + .inTable('refresh_state') + .onDelete('CASCADE'); + }); }); } else { - await knex.schema.alterTable('search', table => { - table.dropForeign(['entity_id']); - }); + await knex.transaction(async trx => { + await trx.schema.alterTable('search', table => { + table.dropForeign(['entity_id']); + }); - await knex('search') - .whereNotIn('entity_id', knex('refresh_state').select('entity_id')) - .delete(); + await trx('search') + .whereNotIn('entity_id', trx('refresh_state').select('entity_id')) + .delete(); - await knex.schema.alterTable('search', table => { - table - .foreign('entity_id') - .references('entity_id') - .inTable('refresh_state') - .onDelete('CASCADE'); + await trx.schema.alterTable('search', table => { + table + .foreign('entity_id') + .references('entity_id') + .inTable('refresh_state') + .onDelete('CASCADE'); + }); }); } }; From 89a1c313d0b65e295a2b803e74cb1d800a3f29c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 11 Mar 2026 14:16:25 +0100 Subject: [PATCH 07/11] catalog-backend: address review comments on search FK migration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Close PG race window: drop old FK and add NOT VALID FK before batch cleanup, so no new orphans can be inserted during cleanup - Extract batch-delete helpers (batchDeleteOrphansPg, batchDeleteOrphansMysql) to reduce duplication across up/down and dialects - Fix ShareUpdateExclusiveLock comment to be more precise - Make changeset message more descriptive Co-Authored-By: Claude Opus 4.6 Signed-off-by: Fredrik Adelöw --- .changeset/ready-ghosts-fail.md | 2 +- ...20260214000000_search_fk_final_entities.js | 135 ++++++++---------- 2 files changed, 64 insertions(+), 73 deletions(-) diff --git a/.changeset/ready-ghosts-fail.md b/.changeset/ready-ghosts-fail.md index de3c600bc0..a1e4a68602 100644 --- a/.changeset/ready-ghosts-fail.md +++ b/.changeset/ready-ghosts-fail.md @@ -2,4 +2,4 @@ '@backstage/plugin-catalog-backend': patch --- -Optimize a catalog migration for big databases +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 4f3885aa17..7048f2da75 100644 --- a/plugins/catalog-backend/migrations/20260214000000_search_fk_final_entities.js +++ b/plugins/catalog-backend/migrations/20260214000000_search_fk_final_entities.js @@ -16,6 +16,56 @@ // @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 @@ -32,26 +82,9 @@ exports.up = async function up(knex) { const client = knex.client.config.client; if (client.includes('pg')) { - // Batch-delete orphaned rows BEFORE touching the schema. - // This runs outside any DDL lock, so it doesn't block reads. - for (;;) { - const deleted = await knex.raw(` - DELETE FROM "search" - WHERE ctid IN ( - SELECT s.ctid FROM "search" s - LEFT JOIN "final_entities" fe ON s."entity_id" = fe."entity_id" - WHERE fe."entity_id" IS NULL - AND s."entity_id" IS NOT NULL - LIMIT 10000 - ) - `); - if (deleted.rowCount === 0) { - break; - } - } - - // Drop old FK and add new one with NOT VALID (minimal lock time). - // NOT VALID skips the full table scan — we already cleaned up orphans above. + // Drop old FK and immediately add the new one as NOT VALID. This + // prevents new orphan rows from being inserted while we clean up + // existing ones, closing the race window between cleanup and FK add. await knex.raw( `ALTER TABLE "search" DROP CONSTRAINT IF EXISTS "search_entity_id_foreign"`, ); @@ -63,32 +96,19 @@ exports.up = async function up(knex) { NOT VALID `); + // 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 reads/writes. + // 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. - // Uses LEFT JOIN to find orphans efficiently, then deletes by entity_id. - // MySQL doesn't support LIMIT in multi-table DELETE, so we find the - // orphaned entity_ids first, then delete in a separate statement. - for (;;) { - const [orphanIds] = await knex.raw(` - SELECT DISTINCT s.\`entity_id\` FROM \`search\` s - LEFT JOIN \`final_entities\` fe ON s.\`entity_id\` = fe.\`entity_id\` - WHERE fe.\`entity_id\` IS NULL - AND s.\`entity_id\` IS NOT NULL - LIMIT 10000 - `); - 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(); - } + await batchDeleteOrphansMysql(knex, 'final_entities'); // Drop old FK and add new one inside an explicit transaction, since the // global transaction wrapper is disabled for this migration. MySQL does @@ -136,22 +156,6 @@ exports.down = async function down(knex) { const client = knex.client.config.client; if (client.includes('pg')) { - for (;;) { - const deleted = await knex.raw(` - DELETE FROM "search" - WHERE ctid IN ( - SELECT s.ctid FROM "search" s - LEFT JOIN "refresh_state" rs ON s."entity_id" = rs."entity_id" - WHERE rs."entity_id" IS NULL - AND s."entity_id" IS NOT NULL - LIMIT 10000 - ) - `); - if (deleted.rowCount === 0) { - break; - } - } - await knex.raw( `ALTER TABLE "search" DROP CONSTRAINT IF EXISTS "search_entity_id_foreign"`, ); @@ -163,26 +167,13 @@ exports.down = async function down(knex) { NOT VALID `); + await batchDeleteOrphansPg(knex, 'refresh_state'); + await knex.raw( `ALTER TABLE "search" VALIDATE CONSTRAINT "search_entity_id_foreign"`, ); } else if (client.includes('mysql')) { - for (;;) { - const [orphanIds] = await knex.raw(` - SELECT DISTINCT s.\`entity_id\` FROM \`search\` s - LEFT JOIN \`refresh_state\` rs ON s.\`entity_id\` = rs.\`entity_id\` - WHERE rs.\`entity_id\` IS NULL - AND s.\`entity_id\` IS NOT NULL - LIMIT 10000 - `); - 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(); - } + await batchDeleteOrphansMysql(knex, 'refresh_state'); await knex.transaction(async trx => { await trx.schema.alterTable('search', table => { From 09c7e49f82e8d7a9d32ec8009db4fdcf63a4e6e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 11 Mar 2026 14:43:10 +0100 Subject: [PATCH 08/11] catalog-backend: combine PG DROP+ADD into single ALTER TABLE, fix MySQL comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Combine DROP CONSTRAINT and ADD CONSTRAINT into a single ALTER TABLE statement for PostgreSQL, eliminating the brief window where no FK exists - Reword MySQL transaction comment to clarify that ALTER TABLE causes implicit commits in InnoDB, so the wrapper doesn't provide full atomicity Co-Authored-By: Claude Opus 4.6 Signed-off-by: Fredrik Adelöw --- ...20260214000000_search_fk_final_entities.js | 23 ++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) 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 7048f2da75..71caeb600d 100644 --- a/plugins/catalog-backend/migrations/20260214000000_search_fk_final_entities.js +++ b/plugins/catalog-backend/migrations/20260214000000_search_fk_final_entities.js @@ -82,14 +82,13 @@ exports.up = async function up(knex) { const client = knex.client.config.client; if (client.includes('pg')) { - // Drop old FK and immediately add the new one as NOT VALID. This - // prevents new orphan rows from being inserted while we clean up - // existing ones, closing the race window between cleanup and FK add. - await knex.raw( - `ALTER TABLE "search" DROP CONSTRAINT IF EXISTS "search_entity_id_foreign"`, - ); + // 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 @@ -110,10 +109,10 @@ exports.up = async function up(knex) { // Batch-delete orphaned rows before DDL to reduce lock time. await batchDeleteOrphansMysql(knex, 'final_entities'); - // Drop old FK and add new one inside an explicit transaction, since the - // global transaction wrapper is disabled for this migration. MySQL does - // not support NOT VALID, but the table is already clean so validation - // is fast. + // Perform the FK changes. Note that in MySQL/InnoDB, ALTER TABLE + // statements cause implicit commits, so wrapping in a transaction does + // not provide full atomicity. However the table is already cleaned of + // orphans and validation remains fast. await knex.transaction(async trx => { await trx.schema.alterTable('search', table => { table.dropForeign(['entity_id']); @@ -156,11 +155,9 @@ exports.down = async function down(knex) { const client = knex.client.config.client; if (client.includes('pg')) { - await knex.raw( - `ALTER TABLE "search" DROP CONSTRAINT IF EXISTS "search_entity_id_foreign"`, - ); 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 From 1b69ff9c82f1553cdabcca458173060dd21a2224 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 11 Mar 2026 14:56:45 +0100 Subject: [PATCH 09/11] catalog-backend: make MySQL FK swap idempotent with retry loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the non-atomic MySQL transaction wrapper with a retry loop that checks information_schema before dropping the FK and retries the add if new orphan rows appear during the window between DROP and ADD. Co-Authored-By: Claude Opus 4.6 Signed-off-by: Fredrik Adelöw --- ...20260214000000_search_fk_final_entities.js | 93 +++++++++++++------ 1 file changed, 65 insertions(+), 28 deletions(-) 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 71caeb600d..4e1fc53d24 100644 --- a/plugins/catalog-backend/migrations/20260214000000_search_fk_final_entities.js +++ b/plugins/catalog-backend/migrations/20260214000000_search_fk_final_entities.js @@ -109,22 +109,41 @@ exports.up = async function up(knex) { // Batch-delete orphaned rows before DDL to reduce lock time. await batchDeleteOrphansMysql(knex, 'final_entities'); - // Perform the FK changes. Note that in MySQL/InnoDB, ALTER TABLE - // statements cause implicit commits, so wrapping in a transaction does - // not provide full atomicity. However the table is already cleaned of - // orphans and validation remains fast. - await knex.transaction(async trx => { - await trx.schema.alterTable('search', table => { - table.dropForeign(['entity_id']); - }); - await trx.schema.alterTable('search', table => { - table - .foreign('entity_id') - .references('entity_id') - .inTable('final_entities') - .onDelete('CASCADE'); - }); - }); + // 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. @@ -172,18 +191,36 @@ exports.down = async function down(knex) { } else if (client.includes('mysql')) { await batchDeleteOrphansMysql(knex, 'refresh_state'); - await knex.transaction(async trx => { - await trx.schema.alterTable('search', table => { - table.dropForeign(['entity_id']); - }); - await trx.schema.alterTable('search', table => { - table - .foreign('entity_id') - .references('entity_id') - .inTable('refresh_state') - .onDelete('CASCADE'); - }); - }); + 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 => { From 34af7b45510c30431d989521c01502f117d91a1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 11 Mar 2026 15:03:06 +0100 Subject: [PATCH 10/11] catalog-backend: improve search FK migration test coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add tests for NULL entity_id rows surviving migration, FK enforcement rejecting invalid inserts, and down migration orphan cleanup. Co-Authored-By: Claude Opus 4.6 Signed-off-by: Fredrik Adelöw --- .../src/tests/migrations.test.ts | 203 ++++++++++-------- 1 file changed, 114 insertions(+), 89 deletions(-) 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', }, ]); From 2b87d22ad415f26794a68022199bcd7dd5af1569 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Wed, 11 Mar 2026 16:19:19 +0100 Subject: [PATCH 11/11] Update plugins/catalog-backend/migrations/20260214000000_search_fk_final_entities.js MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Fredrik Adelöw --- .../20260214000000_search_fk_final_entities.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) 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 4e1fc53d24..4d43baf9a8 100644 --- a/plugins/catalog-backend/migrations/20260214000000_search_fk_final_entities.js +++ b/plugins/catalog-backend/migrations/20260214000000_search_fk_final_entities.js @@ -71,10 +71,14 @@ async function batchDeleteOrphansMysql(knex, refTable) { * to final_entities(entity_id). This allows search entries to reference * final entities directly, with CASCADE delete when entities are removed. * - * For PostgreSQL and MySQL, the migration is structured to minimize lock - * time on large tables by batch-deleting orphaned rows before any DDL. - * PostgreSQL additionally uses NOT VALID / VALIDATE CONSTRAINT to keep - * the AccessExclusiveLock duration minimal. + * 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 */