Merge pull request #33281 from backstage/freben/search-fk-safe-migration

fix(catalog): make search FK migration safe for large databases

.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

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,
 };

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',
         },
       ]);