From eebcb8753f96af2ea621813d71f44f7273d898a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Tue, 20 Jun 2023 20:48:27 +0200 Subject: [PATCH 1/2] run deleteWithEagerPruningOfChildren test without transactions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/forty-onions-kick.md | 5 ++ .../src/database/DefaultProviderDatabase.ts | 2 +- .../deleteWithEagerPruningOfChildren.test.ts | 62 +++++++++++-------- .../deleteWithEagerPruningOfChildren.ts | 8 +-- 4 files changed, 45 insertions(+), 32 deletions(-) create mode 100644 .changeset/forty-onions-kick.md diff --git a/.changeset/forty-onions-kick.md b/.changeset/forty-onions-kick.md new file mode 100644 index 0000000000..74be2c142e --- /dev/null +++ b/.changeset/forty-onions-kick.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend': patch +--- + +Run the `deleteWithEagerPruningOfChildren` test without transactions, to try to get rid of CI flakiness diff --git a/plugins/catalog-backend/src/database/DefaultProviderDatabase.ts b/plugins/catalog-backend/src/database/DefaultProviderDatabase.ts index a0d376327b..2a97d1f2a9 100644 --- a/plugins/catalog-backend/src/database/DefaultProviderDatabase.ts +++ b/plugins/catalog-backend/src/database/DefaultProviderDatabase.ts @@ -81,7 +81,7 @@ export class DefaultProviderDatabase implements ProviderDatabase { if (toRemove.length) { const removedCount = await deleteWithEagerPruningOfChildren({ - tx, + knex: tx, entityRefs: toRemove, sourceKey: options.sourceKey, }); diff --git a/plugins/catalog-backend/src/database/operations/provider/deleteWithEagerPruningOfChildren.test.ts b/plugins/catalog-backend/src/database/operations/provider/deleteWithEagerPruningOfChildren.test.ts index e3cc813c96..dcd87613de 100644 --- a/plugins/catalog-backend/src/database/operations/provider/deleteWithEagerPruningOfChildren.test.ts +++ b/plugins/catalog-backend/src/database/operations/provider/deleteWithEagerPruningOfChildren.test.ts @@ -34,26 +34,6 @@ describe('deleteWithEagerPruningOfChildren', () => { return knex; } - async function run( - knex: Knex, - options: { entityRefs: string[]; sourceKey: string }, - ): Promise { - let result: number; - await knex.transaction( - async tx => { - // We can't return here, as knex swallows the return type in case the - // transaction is rolled back: - // https://github.com/knex/knex/blob/e37aeaa31c8ef9c1b07d2e4d3ec6607e557d800d/lib/transaction.js#L136 - result = await deleteWithEagerPruningOfChildren({ tx, ...options }); - }, - { - // If we explicitly trigger a rollback, don't fail. - doNotRejectOnRollback: true, - }, - ); - return result!; - } - async function insertReference( knex: Knex, ...refs: DbRefreshStateReferencesRow[] @@ -110,7 +90,11 @@ describe('deleteWithEagerPruningOfChildren', () => { { source_key: 'P1', target_entity_ref: 'E4' }, { source_key: 'P2', target_entity_ref: 'E5' }, ); - await run(knex, { sourceKey: 'P1', entityRefs: ['E1', 'E3'] }); + await deleteWithEagerPruningOfChildren({ + knex, + sourceKey: 'P1', + entityRefs: ['E1', 'E3'], + }); await expect(remainingEntities(knex)).resolves.toEqual(['E4', 'E5']); }, ); @@ -139,7 +123,11 @@ describe('deleteWithEagerPruningOfChildren', () => { { source_key: 'P1', target_entity_ref: 'E1' }, { source_key: 'P1', target_entity_ref: 'E2' }, ); - await run(knex, { sourceKey: 'P1', entityRefs: ['E1'] }); + await deleteWithEagerPruningOfChildren({ + knex, + sourceKey: 'P1', + entityRefs: ['E1'], + }); await expect(remainingEntities(knex)).resolves.toEqual(['E2']); }, ); @@ -167,7 +155,11 @@ describe('deleteWithEagerPruningOfChildren', () => { { source_key: 'P2', target_entity_ref: 'E3' }, { source_entity_ref: 'E3', target_entity_ref: 'E2' }, ); - await run(knex, { sourceKey: 'P1', entityRefs: ['E1'] }); + await deleteWithEagerPruningOfChildren({ + knex, + sourceKey: 'P1', + entityRefs: ['E1'], + }); await expect(remainingEntities(knex)).resolves.toEqual(['E2', 'E3']); }, ); @@ -195,7 +187,11 @@ describe('deleteWithEagerPruningOfChildren', () => { { source_key: 'P1', target_entity_ref: 'E3' }, { source_entity_ref: 'E3', target_entity_ref: 'E2' }, ); - await run(knex, { sourceKey: 'P1', entityRefs: ['E1'] }); + await deleteWithEagerPruningOfChildren({ + knex, + sourceKey: 'P1', + entityRefs: ['E1'], + }); await expect(remainingEntities(knex)).resolves.toEqual(['E2', 'E3']); }, ); @@ -228,14 +224,22 @@ describe('deleteWithEagerPruningOfChildren', () => { { source_entity_ref: 'E3', target_entity_ref: 'E5' }, { source_entity_ref: 'E5', target_entity_ref: 'E6' }, ); - await run(knex, { sourceKey: 'P1', entityRefs: ['E1'] }); + await deleteWithEagerPruningOfChildren({ + knex, + sourceKey: 'P1', + entityRefs: ['E1'], + }); await expect(remainingEntities(knex)).resolves.toEqual([ 'E3', 'E4', 'E5', 'E6', ]); - await run(knex, { sourceKey: 'P1', entityRefs: ['E3'] }); + await deleteWithEagerPruningOfChildren({ + knex, + sourceKey: 'P1', + entityRefs: ['E3'], + }); await expect(remainingEntities(knex)).resolves.toEqual([]); }, ); @@ -263,7 +267,11 @@ describe('deleteWithEagerPruningOfChildren', () => { { source_key: 'P1', target_entity_ref: 'E3' }, { source_key: 'P2', target_entity_ref: 'E4' }, ); - await run(knex, { sourceKey: 'P1', entityRefs: ['E2', 'E3', 'E4'] }); + await deleteWithEagerPruningOfChildren({ + knex, + sourceKey: 'P1', + entityRefs: ['E2', 'E3', 'E4'], + }); await expect(remainingEntities(knex)).resolves.toEqual([ 'E1', 'E2', diff --git a/plugins/catalog-backend/src/database/operations/provider/deleteWithEagerPruningOfChildren.ts b/plugins/catalog-backend/src/database/operations/provider/deleteWithEagerPruningOfChildren.ts index 33da37bc55..027860fcc6 100644 --- a/plugins/catalog-backend/src/database/operations/provider/deleteWithEagerPruningOfChildren.ts +++ b/plugins/catalog-backend/src/database/operations/provider/deleteWithEagerPruningOfChildren.ts @@ -26,17 +26,17 @@ import { DbRefreshStateReferencesRow } from '../../tables'; * the removal of their parents. */ export async function deleteWithEagerPruningOfChildren(options: { - tx: Knex.Transaction; + knex: Knex | Knex.Transaction; entityRefs: string[]; sourceKey: string; }): Promise { - const { tx, entityRefs, sourceKey } = options; + const { knex, entityRefs, sourceKey } = options; // Split up the operation by (large) chunks, so that we do not hit database // limits for the number of permitted bindings on a precompiled statement let removedCount = 0; for (const refs of lodash.chunk(entityRefs, 1000)) { - removedCount += await tx + removedCount += await knex .delete() .from('refresh_state') .whereIn('entity_ref', orphans => @@ -186,7 +186,7 @@ export async function deleteWithEagerPruningOfChildren(options: { // Delete the references that originate only from this entity provider. Note // that there may be more than one entity provider making a "claim" for a // given root entity, if they emit with the same location key. - await tx('refresh_state_references') + await knex('refresh_state_references') .where('source_key', '=', sourceKey) .whereIn('target_entity_ref', refs) .delete(); From 0aa1daf1e374a9618fa76f6df0c193df81984908 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Mon, 3 Jul 2023 12:37:08 +0200 Subject: [PATCH 2/2] remove changeset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/forty-onions-kick.md | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 .changeset/forty-onions-kick.md diff --git a/.changeset/forty-onions-kick.md b/.changeset/forty-onions-kick.md deleted file mode 100644 index 74be2c142e..0000000000 --- a/.changeset/forty-onions-kick.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@backstage/plugin-catalog-backend': patch ---- - -Run the `deleteWithEagerPruningOfChildren` test without transactions, to try to get rid of CI flakiness