From fbf382f1f80a4a44f7baca12dba3f1f0e32f7aaa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Sun, 22 Feb 2026 22:37:18 +0100 Subject: [PATCH] catalog: minor internal optimisation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/metal-humans-move.md | 5 + .../operations/stitcher/markForStitching.ts | 52 +-------- .../catalog-backend/src/database/util.test.ts | 110 ++++++++++++++++++ plugins/catalog-backend/src/database/util.ts | 45 ++++++- 4 files changed, 161 insertions(+), 51 deletions(-) create mode 100644 .changeset/metal-humans-move.md create mode 100644 plugins/catalog-backend/src/database/util.test.ts diff --git a/.changeset/metal-humans-move.md b/.changeset/metal-humans-move.md new file mode 100644 index 0000000000..21881246fc --- /dev/null +++ b/.changeset/metal-humans-move.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend': patch +--- + +Minor internal optimisation diff --git a/plugins/catalog-backend/src/database/operations/stitcher/markForStitching.ts b/plugins/catalog-backend/src/database/operations/stitcher/markForStitching.ts index d26fb8d6d4..e4172e8cdf 100644 --- a/plugins/catalog-backend/src/database/operations/stitcher/markForStitching.ts +++ b/plugins/catalog-backend/src/database/operations/stitcher/markForStitching.ts @@ -17,33 +17,11 @@ import { Knex } from 'knex'; import splitToChunks from 'lodash/chunk'; import { v4 as uuid } from 'uuid'; -import { ErrorLike, isError } from '@backstage/errors'; import { StitchingStrategy } from '../../../stitching/types'; -import { setTimeout as sleep } from 'node:timers/promises'; import { DbFinalEntitiesRow, DbRefreshStateRow } from '../../tables'; +import { retryOnDeadlock } from '../../util'; const UPDATE_CHUNK_SIZE = 100; // Smaller chunks reduce contention -const DEADLOCK_RETRY_ATTEMPTS = 3; -const DEADLOCK_BASE_DELAY_MS = 25; - -// PostgreSQL deadlock error code -const POSTGRES_DEADLOCK_SQLSTATE = '40P01'; - -/** - * Checks if the given error is a deadlock error for the database engine in use. - */ -function isDeadlockError( - knex: Knex | Knex.Transaction, - e: unknown, -): e is ErrorLike { - if (knex.client.config.client.includes('pg')) { - // PostgreSQL deadlock detection - return isError(e) && e.code === POSTGRES_DEADLOCK_SQLSTATE; - } - - // Add more database engine checks here as needed - return false; -} /** * Marks a number of entities for stitching some time in the near @@ -69,12 +47,7 @@ export async function markForStitching(options: { .update({ hash: 'force-stitching', }) - .whereIn( - 'entity_id', - knex('refresh_state') - .select('entity_id') - .whereIn('entity_ref', chunk), - ); + .whereIn('entity_ref', chunk); await retryOnDeadlock(async () => { await knex .table('refresh_state') @@ -143,24 +116,3 @@ function sortSplit(input: Iterable | undefined): string[][] { array.sort(); return splitToChunks(array, UPDATE_CHUNK_SIZE); } - -async function retryOnDeadlock( - fn: () => Promise, - knex: Knex | Knex.Transaction, - retries = DEADLOCK_RETRY_ATTEMPTS, - baseMs = DEADLOCK_BASE_DELAY_MS, -): Promise { - let attempt = 0; - for (;;) { - try { - return await fn(); - } catch (e: unknown) { - if (isDeadlockError(knex, e) && attempt < retries) { - await sleep(baseMs * Math.pow(2, attempt)); - attempt++; - continue; - } - throw e; - } - } -} diff --git a/plugins/catalog-backend/src/database/util.test.ts b/plugins/catalog-backend/src/database/util.test.ts new file mode 100644 index 0000000000..c69c2b362e --- /dev/null +++ b/plugins/catalog-backend/src/database/util.test.ts @@ -0,0 +1,110 @@ +/* + * Copyright 2026 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Knex } from 'knex'; +import { retryOnDeadlock } from './util'; + +function mockKnex(client: string): Knex { + return { client: { config: { client } } } as unknown as Knex; +} + +function pgDeadlockError(): Error & { code: string } { + const err = new Error('deadlock detected') as Error & { code: string }; + err.code = '40P01'; + return err; +} + +describe('retryOnDeadlock', () => { + it('returns the result on success', async () => { + const fn = jest.fn().mockResolvedValue('ok'); + const result = await retryOnDeadlock(fn, mockKnex('pg'), 3, 1); + expect(result).toBe('ok'); + expect(fn).toHaveBeenCalledTimes(1); + }); + + it('retries on PostgreSQL deadlock errors', async () => { + const fn = jest + .fn() + .mockRejectedValueOnce(pgDeadlockError()) + .mockRejectedValueOnce(pgDeadlockError()) + .mockResolvedValue('recovered'); + + const result = await retryOnDeadlock(fn, mockKnex('pg'), 3, 1); + expect(result).toBe('recovered'); + expect(fn).toHaveBeenCalledTimes(3); + }); + + it('throws after exhausting all retries', async () => { + const fn = jest.fn().mockRejectedValue(pgDeadlockError()); + + await expect(retryOnDeadlock(fn, mockKnex('pg'), 3, 1)).rejects.toThrow( + 'deadlock detected', + ); + // 1 initial + 3 retries = 4 calls + expect(fn).toHaveBeenCalledTimes(4); + }); + + it('does not retry non-deadlock errors on PostgreSQL', async () => { + const err = new Error('something else'); + const fn = jest.fn().mockRejectedValue(err); + + await expect(retryOnDeadlock(fn, mockKnex('pg'), 3, 1)).rejects.toThrow( + 'something else', + ); + expect(fn).toHaveBeenCalledTimes(1); + }); + + it('does not retry deadlock-like errors on non-PostgreSQL engines', async () => { + const fn = jest.fn().mockRejectedValue(pgDeadlockError()); + + await expect( + retryOnDeadlock(fn, mockKnex('better-sqlite3'), 3, 1), + ).rejects.toThrow('deadlock detected'); + expect(fn).toHaveBeenCalledTimes(1); + }); + + it('applies exponential backoff between retries', async () => { + const timestamps: number[] = []; + const fn = jest.fn().mockImplementation(async () => { + timestamps.push(Date.now()); + if (timestamps.length <= 3) { + throw pgDeadlockError(); + } + return 'done'; + }); + + const baseMs = 50; + await retryOnDeadlock(fn, mockKnex('pg'), 3, baseMs); + + expect(fn).toHaveBeenCalledTimes(4); + // Verify delays increase (with some tolerance for timing) + const delay1 = timestamps[1] - timestamps[0]; + const delay2 = timestamps[2] - timestamps[1]; + const delay3 = timestamps[3] - timestamps[2]; + expect(delay1).toBeGreaterThanOrEqual(baseMs * 0.8); + expect(delay2).toBeGreaterThanOrEqual(baseMs * 2 * 0.8); + expect(delay3).toBeGreaterThanOrEqual(baseMs * 4 * 0.8); + }); + + it('defaults to 3 retries when not specified', async () => { + const fn = jest.fn().mockRejectedValue(pgDeadlockError()); + + await expect(retryOnDeadlock(fn, mockKnex('pg'))).rejects.toThrow( + 'deadlock detected', + ); + expect(fn).toHaveBeenCalledTimes(4); + }); +}); diff --git a/plugins/catalog-backend/src/database/util.ts b/plugins/catalog-backend/src/database/util.ts index 3734670a41..39dc69c025 100644 --- a/plugins/catalog-backend/src/database/util.ts +++ b/plugins/catalog-backend/src/database/util.ts @@ -15,8 +15,11 @@ */ import { Entity } from '@backstage/catalog-model'; -import { createHash } from 'node:crypto'; +import { ErrorLike, isError } from '@backstage/errors'; import stableStringify from 'fast-json-stable-stringify'; +import { Knex } from 'knex'; +import { createHash } from 'node:crypto'; +import { setTimeout as sleep } from 'node:timers/promises'; export function generateStableHash(entity: Entity) { return createHash('sha1') @@ -31,3 +34,43 @@ export function generateTargetKey(target: string) { .digest('hex')}` : target; } + +/** + * Retries an operation on database deadlock errors. + */ +export async function retryOnDeadlock( + fn: () => Promise, + knex: Knex | Knex.Transaction, + retries = 3, + baseMs = 25, +): Promise { + let attempt = 0; + for (;;) { + try { + return await fn(); + } catch (e: unknown) { + if (isDeadlockError(knex, e) && attempt < retries) { + await sleep(baseMs * Math.pow(2, attempt)); + attempt++; + continue; + } + throw e; + } + } +} + +/** + * Checks if the given error is a deadlock error for the database engine in use. + */ +function isDeadlockError( + knex: Knex | Knex.Transaction, + e: unknown, +): e is ErrorLike { + if (knex.client.config.client.includes('pg')) { + // PostgreSQL deadlock detection via error code + return isError(e) && e.code === '40P01'; + } + + // Add more database engine checks here as needed + return false; +}