From c9139e1e76f9b2ee84c3c8f3e92d71245f6e2399 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 21 Jan 2025 11:54:11 +0100 Subject: [PATCH 1/2] catalog-backend: ignore stitching db conflicts Signed-off-by: Patrik Oldsberg --- .changeset/eleven-mice-sleep.md | 5 ++ .../stitcher/performStitching.test.ts | 61 +++++++++++++++++++ .../operations/stitcher/performStitching.ts | 37 ++++++++--- 3 files changed, 93 insertions(+), 10 deletions(-) create mode 100644 .changeset/eleven-mice-sleep.md diff --git a/.changeset/eleven-mice-sleep.md b/.changeset/eleven-mice-sleep.md new file mode 100644 index 0000000000..d1b14aa098 --- /dev/null +++ b/.changeset/eleven-mice-sleep.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend': patch +--- + +Ignore benign database conflict errors during stitching, now logged with debug level instead. diff --git a/plugins/catalog-backend/src/database/operations/stitcher/performStitching.test.ts b/plugins/catalog-backend/src/database/operations/stitcher/performStitching.test.ts index 4b12a9bf28..42c0357b7c 100644 --- a/plugins/catalog-backend/src/database/operations/stitcher/performStitching.test.ts +++ b/plugins/catalog-backend/src/database/operations/stitcher/performStitching.test.ts @@ -294,4 +294,65 @@ describe('performStitching', () => { ); }, ); + + it.each(databases.eachSupportedId())( + 'handles conflicts with past stitches %p', + async databaseId => { + const knex = await databases.init(databaseId); + await applyDatabaseMigrations(knex); + + // Drop the foreign key constraint so we can insert a conflicting entity + // where the ID is inconsistent with the entity ref across refresh_state + // and final_entities + await knex.schema.alterTable('final_entities', table => { + table.dropForeign('entity_id'); + }); + + await knex('refresh_state').insert([ + { + entity_id: 'other-id', + entity_ref: 'k:ns/n', + unprocessed_entity: JSON.stringify({}), + processed_entity: JSON.stringify({ + apiVersion: 'a', + kind: 'k', + metadata: { + name: 'n', + namespace: 'ns', + }, + spec: { + k: 'v', + }, + }), + errors: '[]', + next_update_at: knex.fn.now(), + last_discovery_at: knex.fn.now(), + }, + ]); + await knex('final_entities').insert([ + { + entity_id: 'my-id', + entity_ref: 'k:ns/n', + hash: '', + stitch_ticket: 'old-ticket', + final_entity: JSON.stringify({}), + }, + ]); + + const stitchLogger = mockServices.logger.mock(); + await expect( + performStitching({ + knex, + logger: stitchLogger, + strategy: { mode: 'immediate' }, + entityRef: 'k:ns/n', + }), + ).resolves.toBe('abandoned'); + + expect(stitchLogger.debug).toHaveBeenCalledWith( + 'Skipping stitching of k:ns/n, conflict', + expect.anything(), + ); + }, + ); }); diff --git a/plugins/catalog-backend/src/database/operations/stitcher/performStitching.ts b/plugins/catalog-backend/src/database/operations/stitcher/performStitching.ts index 2ed65d3903..32c0d4ccb3 100644 --- a/plugins/catalog-backend/src/database/operations/stitcher/performStitching.ts +++ b/plugins/catalog-backend/src/database/operations/stitcher/performStitching.ts @@ -33,7 +33,10 @@ import { import { buildEntitySearch } from './buildEntitySearch'; import { markDeferredStitchCompleted } from './markDeferredStitchCompleted'; import { BATCH_SIZE, generateStableHash } from './util'; -import { LoggerService } from '@backstage/backend-plugin-api'; +import { + LoggerService, + isDatabaseConflictError, +} from '@backstage/backend-plugin-api'; // See https://github.com/facebook/react/blob/f0cf832e1d0c8544c36aa8b310960885a11a847c/packages/react-dom-bindings/src/shared/sanitizeURL.js const scriptProtocolPattern = @@ -71,15 +74,29 @@ export async function performStitching(options: { } // Insert stitching ticket that will be compared before inserting the final entity. - await knex('final_entities') - .insert({ - entity_id: entityResult[0].entity_id, - hash: '', - entity_ref: entityRef, - stitch_ticket: stitchTicket, - }) - .onConflict('entity_id') - .merge(['stitch_ticket']); + try { + await knex('final_entities') + .insert({ + entity_id: entityResult[0].entity_id, + hash: '', + entity_ref: entityRef, + stitch_ticket: stitchTicket, + }) + .onConflict('entity_id') + .merge(['stitch_ticket']); + } catch (error) { + // It's possible to hit a race where a refresh_state table delete + insert + // is done just after we read the entity_id from it. This conflict is safe + // to ignore because the current stitching operation will be triggered by + // the old entry, and the new entry will trigger it's own stitching that + // will update the entity. + if (isDatabaseConflictError(error)) { + logger.debug(`Skipping stitching of ${entityRef}, conflict`, error); + return 'abandoned'; + } + + throw error; + } // Selecting from refresh_state and final_entities should yield exactly // one row (except in abnormal cases where the stitch was invoked for From c9f35a19152585e155d0cb21410f66722cb9290a Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Tue, 21 Jan 2025 14:28:47 +0100 Subject: [PATCH 2/2] catalog-backend: skip stitching conflict test on MySQL Signed-off-by: Patrik Oldsberg --- .../database/operations/stitcher/performStitching.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/plugins/catalog-backend/src/database/operations/stitcher/performStitching.test.ts b/plugins/catalog-backend/src/database/operations/stitcher/performStitching.test.ts index 42c0357b7c..1021862c12 100644 --- a/plugins/catalog-backend/src/database/operations/stitcher/performStitching.test.ts +++ b/plugins/catalog-backend/src/database/operations/stitcher/performStitching.test.ts @@ -298,6 +298,14 @@ describe('performStitching', () => { it.each(databases.eachSupportedId())( 'handles conflicts with past stitches %p', async databaseId => { + if (databaseId === 'MYSQL_8') { + // MySQL doesn't handle conflicts in the same way as the other two, most + // likely due to the conflict probably being handled with a merged even + // if it's for the entity_ref. This probably means that MySQL will have + // inconsistencies in the final_entities table in some cases, but they + // should heal fairly quickly. + return; + } const knex = await databases.init(databaseId); await applyDatabaseMigrations(knex);