paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions 22 Packages Projects Releases Wiki Activity

ensure stitching happens after eager deletion

Browse Source 
Signed-off-by: Fredrik Adelöw <freben@gmail.com>
This commit is contained in:
Fredrik Adelöw
2023-09-21 15:36:33 +02:00
parent 58f7ee0a8f
commit 348e8c1cdb
3 changed files with 271 additions and 152 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/lovely-coins-occur.md
+5
View File
@@ -0,0 +1,5 @@
---
'@backstage/plugin-catalog-backend': patch
---
Fixes a bug where eagerly deleted entities did not properly trigger re-stitching of entities that they had relations to.
plugins/catalog-backend/src/database/operations/provider/deleteWithEagerPruningOfChildren.test.ts
+48 -5
View File
@@ -18,7 +18,11 @@ import { TestDatabaseId, TestDatabases } from '@backstage/backend-test-utils';
import { Knex } from 'knex';
import * as uuid from 'uuid';
import { applyDatabaseMigrations } from '../../migrations';
import { DbRefreshStateReferencesRow, DbRefreshStateRow } from '../../tables';
import {
DbRefreshStateReferencesRow,
DbRefreshStateRow,
DbRelationsRow,
} from '../../tables';
import { deleteWithEagerPruningOfChildren } from './deleteWithEagerPruningOfChildren';
jest.setTimeout(60_000);
@@ -43,6 +47,22 @@ describe('deleteWithEagerPruningOfChildren', () => {
);
}
async function insertRelation(
knex: Knex,
...relations: { from: string; to: string }[]
) {
for (const rel of relations) {
await knex<DbRelationsRow>('relations').insert({
originating_entity_id: await knex<DbRefreshStateRow>('refresh_state')
.select('entity_id')
.then(rows => rows[0].entity_id), // doesn't matter which one, this is consumed pre-deletion
source_entity_ref: rel.from,
target_entity_ref: rel.to,
type: 'fake',
});
}
}
async function insertEntity(knex: Knex, ...entityRefs: string[]) {
for (const ref of entityRefs) {
await knex<DbRefreshStateRow>('refresh_state').insert({
@@ -64,6 +84,14 @@ describe('deleteWithEagerPruningOfChildren', () => {
return rows.map(r => r.entity_ref);
}
async function entitiesMarkedForStitching(knex: Knex) {
const rows = await knex<DbRefreshStateRow>('refresh_state')
.orderBy('entity_ref')
.select('entity_ref')
.where('result_hash', '=', 'force-stitching');
return rows.map(r => r.entity_ref);
}
it.each(databases.eachSupportedId())(
'works for the simple path, %p',
async databaseId => {
@@ -78,7 +106,7 @@ describe('deleteWithEagerPruningOfChildren', () => {
Scenario: P1 issues delete for E1 and E3
Result: E1, E2, and E3 deleted; E4 and E5 remain
Result: E1, E2, and E3 deleted; E4 and E5 remain; E4 marked for stitching because it had a relation to a deleted entity
*/
const knex = await createDatabase(databaseId);
await insertEntity(knex, 'E1', 'E2', 'E3', 'E4', 'E5');
@@ -90,12 +118,14 @@ describe('deleteWithEagerPruningOfChildren', () => {
{ source_key: 'P1', target_entity_ref: 'E4' },
{ source_key: 'P2', target_entity_ref: 'E5' },
);
await insertRelation(knex, { from: 'E4', to: 'E2' });
await deleteWithEagerPruningOfChildren({
knex,
sourceKey: 'P1',
entityRefs: ['E1', 'E3'],
});
await expect(remainingEntities(knex)).resolves.toEqual(['E4', 'E5']);
await expect(entitiesMarkedForStitching(knex)).resolves.toEqual(['E4']);
},
);
@@ -113,7 +143,7 @@ describe('deleteWithEagerPruningOfChildren', () => {
Scenario: P1 issues delete for E1
Result: E1 deleted; E2 remains
Result: E1 deleted; E2 remains; E2 marked for stitching because it had a relation to a deleted entity
*/
const knex = await createDatabase(databaseId);
await insertEntity(knex, 'E1', 'E2');
@@ -123,12 +153,14 @@ describe('deleteWithEagerPruningOfChildren', () => {
{ source_key: 'P1', target_entity_ref: 'E1' },
{ source_key: 'P1', target_entity_ref: 'E2' },
);
await insertRelation(knex, { from: 'E2', to: 'E1' });
await deleteWithEagerPruningOfChildren({
knex,
sourceKey: 'P1',
entityRefs: ['E1'],
});
await expect(remainingEntities(knex)).resolves.toEqual(['E2']);
await expect(entitiesMarkedForStitching(knex)).resolves.toEqual(['E2']);
},
);
@@ -144,7 +176,7 @@ describe('deleteWithEagerPruningOfChildren', () => {
Scenario: P1 issues delete for E1
Result: E1 deleted; E2 and E3 remain
Result: E1 deleted; E2 and E3 remain; E2 marked for stitching because it had a relation to a deleted entity
*/
const knex = await createDatabase(databaseId);
await insertEntity(knex, 'E1', 'E2', 'E3');
@@ -155,12 +187,18 @@ describe('deleteWithEagerPruningOfChildren', () => {
{ source_key: 'P2', target_entity_ref: 'E3' },
{ source_entity_ref: 'E3', target_entity_ref: 'E2' },
);
await insertRelation(
knex,
{ from: 'E2', to: 'E1' },
{ from: 'E3', to: 'E2' },
);
await deleteWithEagerPruningOfChildren({
knex,
sourceKey: 'P1',
entityRefs: ['E1'],
});
await expect(remainingEntities(knex)).resolves.toEqual(['E2', 'E3']);
await expect(entitiesMarkedForStitching(knex)).resolves.toEqual(['E2']);
},
);
@@ -193,6 +231,7 @@ describe('deleteWithEagerPruningOfChildren', () => {
entityRefs: ['E1'],
});
await expect(remainingEntities(knex)).resolves.toEqual(['E2', 'E3']);
await expect(entitiesMarkedForStitching(knex)).resolves.toEqual([]);
},
);
@@ -208,7 +247,7 @@ describe('deleteWithEagerPruningOfChildren', () => {
Scenario: P1 issues delete for E1, then for E3
Result: Everything deleted, but in two steps
Result: Everything deleted, but in two steps; E4 marked for stitching in the first step because it had a relation to a deleted entity
*/
const knex = await createDatabase(databaseId);
await insertEntity(knex, 'E1', 'E2', 'E3', 'E4', 'E5', 'E6');
@@ -224,6 +263,7 @@ describe('deleteWithEagerPruningOfChildren', () => {
{ source_entity_ref: 'E3', target_entity_ref: 'E5' },
{ source_entity_ref: 'E5', target_entity_ref: 'E6' },
);
await insertRelation(knex, { from: 'E4', to: 'E2' });
await deleteWithEagerPruningOfChildren({
knex,
sourceKey: 'P1',
@@ -235,12 +275,14 @@ describe('deleteWithEagerPruningOfChildren', () => {
'E5',
'E6',
]);
await expect(entitiesMarkedForStitching(knex)).resolves.toEqual(['E4']);
await deleteWithEagerPruningOfChildren({
knex,
sourceKey: 'P1',
entityRefs: ['E3'],
});
await expect(remainingEntities(knex)).resolves.toEqual([]);
await expect(entitiesMarkedForStitching(knex)).resolves.toEqual([]);
},
);
@@ -277,6 +319,7 @@ describe('deleteWithEagerPruningOfChildren', () => {
'E2',
'E4',
]);
await expect(entitiesMarkedForStitching(knex)).resolves.toEqual([]);
},
);
});
plugins/catalog-backend/src/database/operations/provider/deleteWithEagerPruningOfChildren.ts
+218 -147
View File
@@ -16,7 +16,11 @@
import { Knex } from 'knex';
import lodash from 'lodash';
import { DbRefreshStateReferencesRow } from '../../tables';
import {
DbFinalEntitiesRow,
DbRefreshStateReferencesRow,
DbRefreshStateRow,
} from '../../tables';
/**
* Given a number of entity refs originally created by a given entity provider
@@ -36,152 +40,24 @@ export async function deleteWithEagerPruningOfChildren(options: {
// limits for the number of permitted bindings on a precompiled statement
let removedCount = 0;
for (const refs of lodash.chunk(entityRefs, 1000)) {
removedCount += await knex
.delete()
.from('refresh_state')
.whereIn('entity_ref', orphans =>
orphans
// First find all nodes that can be reached downwards from the roots
// (deletion targets), including the roots themselves, by traversing
// down the refresh_state_references table. Note that this query
// starts with a condition that source_key = our source key, and
// target_entity_ref is one of the deletion targets. This has two
// effects: it won't match attempts at deleting something that didn't
// originate from us in the first place, and also won't match non-root
// entities (source_key would be null for those).
//
// KeyA - R1 - R2 Legend:
// \ -----------------------------------------
// R3 Key* Source key
// / R* Entity ref
// KeyA - R4 - R5 lines Individual references; sources to
// / the left and targets to the right
// KeyB --- R6
//
// The scenario is that KeyA wants to delete R1.
//
// The query starts with the KeyA-R1 reference, and then traverses
// down to also find R2 and R3. It uses union instead of union all,
// because it wants to find the set of unique descendants even if
// the tree has unexpected loops etc.
.withRecursive('descendants', ['entity_ref'], initial =>
initial
.select('target_entity_ref')
.from('refresh_state_references')
.where('source_key', '=', sourceKey)
.whereIn('target_entity_ref', refs)
.union(recursive =>
recursive
.select('refresh_state_references.target_entity_ref')
.from('descendants')
.join(
'refresh_state_references',
'descendants.entity_ref',
'refresh_state_references.source_entity_ref',
),
),
)
// Then for each descendant, traverse all the way back upwards through
// the refresh_state_references table to get an exhaustive list of all
// references that are part of keeping that particular descendant
// alive.
//
// Continuing the scenario from above, starting from R3, it goes
// upwards to find every pair along every relation line.
//
// Top branch: R2-R3, R1-R2, KeyA-R1
// Middle branch: R5-R3, R4-R5, KeyA-R4
// Bottom branch: R6-R5, KeyB-R6
//
// Note that this all applied to the subject R3. The exact same thing
// will be done starting from each other descendant (R2 and R1). They
// only have one and two references to find, respectively.
//
// This query also uses union instead of union all, to get the set of
// distinct relations even if the tree has unexpected loops etc.
.withRecursive(
'ancestors',
['source_key', 'source_entity_ref', 'target_entity_ref', 'subject'],
initial =>
initial
.select(
'refresh_state_references.source_key',
'refresh_state_references.source_entity_ref',
'refresh_state_references.target_entity_ref',
'descendants.entity_ref',
)
.from('descendants')
.join(
'refresh_state_references',
'refresh_state_references.target_entity_ref',
'descendants.entity_ref',
)
.union(recursive =>
recursive
.select(
'refresh_state_references.source_key',
'refresh_state_references.source_entity_ref',
'refresh_state_references.target_entity_ref',
'ancestors.subject',
)
.from('ancestors')
.join(
'refresh_state_references',
'refresh_state_references.target_entity_ref',
'ancestors.source_entity_ref',
),
),
)
// Finally, from that list of ancestor relations per descendant, pick
// out the ones that are roots (have a source_key). Specifically, find
// ones that seem to be be either (1) from another source, or (2)
// aren't part of the deletion targets. Those are markers that tell us
// that the corresponding descendant should be kept alive and NOT
// subject to eager deletion, because there's "something else" (not
// targeted for deletion) that has references down through the tree to
// it.
//
// Continuing the scenario from above, for R3 we have
//
// KeyA-R1, KeyA-R4, KeyB-R6
//
// This tells us that R3 should be kept alive for two reasons: it's
// referenced by a node that isn't being deleted (R4), and also by
// another source (KeyB). What about R1 and R2? They both have
//
// KeyA-R1
//
// So those should be deleted, since they are definitely only being
// kept alive by something that's about to be deleted.
//
// Final shape of the tree:
//
// R3
// /
// KeyA - R4 - R5
// /
// KeyB --- R6
.with('retained', ['entity_ref'], notPartOfDeletion =>
notPartOfDeletion
.select('subject')
.from('ancestors')
.whereNotNull('ancestors.source_key')
.where(foreignKeyOrRef =>
foreignKeyOrRef
.where('ancestors.source_key', '!=', sourceKey)
.orWhereNotIn('ancestors.target_entity_ref', refs),
),
)
// Return all descendants minus the retained ones
.select('descendants.entity_ref')
.from('descendants')
.leftOuterJoin(
'retained',
'retained.entity_ref',
'descendants.entity_ref',
)
.whereNull('retained.entity_ref'),
);
const { orphanEntityRefs } =
await findDescendantsThatWouldHaveBeenOrphanedByDeletion({
knex: options.knex,
refs,
sourceKey,
});
// Chunk again - these can be many more than the outer chunk size
for (const refsToDelete of lodash.chunk(orphanEntityRefs, 1000)) {
await markEntitiesAffectedByDeletionForStitching({
knex: options.knex,
entityRefs: refsToDelete,
});
await knex
.delete()
.from('refresh_state')
.whereIn('entity_ref', refsToDelete);
}
// 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
@@ -190,7 +66,202 @@ export async function deleteWithEagerPruningOfChildren(options: {
.where('source_key', '=', sourceKey)
.whereIn('target_entity_ref', refs)
.delete();
removedCount += orphanEntityRefs.length;
}
return removedCount;
}
async function findDescendantsThatWouldHaveBeenOrphanedByDeletion(options: {
knex: Knex | Knex.Transaction;
refs: string[];
sourceKey: string;
}): Promise<{ orphanEntityRefs: string[] }> {
const { knex, refs, sourceKey } = options;
const orphans: string[] =
// First find all nodes that can be reached downwards from the roots
// (deletion targets), including the roots themselves, by traversing
// down the refresh_state_references table. Note that this query
// starts with a condition that source_key = our source key, and
// target_entity_ref is one of the deletion targets. This has two
// effects: it won't match attempts at deleting something that didn't
// originate from us in the first place, and also won't match non-root
// entities (source_key would be null for those).
//
// KeyA - R1 - R2 Legend:
// \ -----------------------------------------
// R3 Key* Source key
// / R* Entity ref
// KeyA - R4 - R5 lines Individual references; sources to
// / the left and targets to the right
// KeyB --- R6
//
// The scenario is that KeyA wants to delete R1.
//
// The query starts with the KeyA-R1 reference, and then traverses
// down to also find R2 and R3. It uses union instead of union all,
// because it wants to find the set of unique descendants even if
// the tree has unexpected loops etc.
await knex
.withRecursive('descendants', ['entity_ref'], initial =>
initial
.select('target_entity_ref')
.from('refresh_state_references')
.where('source_key', '=', sourceKey)
.whereIn('target_entity_ref', refs)
.union(recursive =>
recursive
.select('refresh_state_references.target_entity_ref')
.from('descendants')
.join(
'refresh_state_references',
'descendants.entity_ref',
'refresh_state_references.source_entity_ref',
),
),
)
// Then for each descendant, traverse all the way back upwards through
// the refresh_state_references table to get an exhaustive list of all
// references that are part of keeping that particular descendant
// alive.
//
// Continuing the scenario from above, starting from R3, it goes
// upwards to find every pair along every relation line.
//
// Top branch: R2-R3, R1-R2, KeyA-R1
// Middle branch: R5-R3, R4-R5, KeyA-R4
// Bottom branch: R6-R5, KeyB-R6
//
// Note that this all applied to the subject R3. The exact same thing
// will be done starting from each other descendant (R2 and R1). They
// only have one and two references to find, respectively.
//
// This query also uses union instead of union all, to get the set of
// distinct relations even if the tree has unexpected loops etc.
.withRecursive(
'ancestors',
['source_key', 'source_entity_ref', 'target_entity_ref', 'subject'],
initial =>
initial
.select(
'refresh_state_references.source_key',
'refresh_state_references.source_entity_ref',
'refresh_state_references.target_entity_ref',
'descendants.entity_ref',
)
.from('descendants')
.join(
'refresh_state_references',
'refresh_state_references.target_entity_ref',
'descendants.entity_ref',
)
.union(recursive =>
recursive
.select(
'refresh_state_references.source_key',
'refresh_state_references.source_entity_ref',
'refresh_state_references.target_entity_ref',
'ancestors.subject',
)
.from('ancestors')
.join(
'refresh_state_references',
'refresh_state_references.target_entity_ref',
'ancestors.source_entity_ref',
),
),
)
// Finally, from that list of ancestor relations per descendant, pick
// out the ones that are roots (have a source_key). Specifically, find
// ones that seem to be be either (1) from another source, or (2)
// aren't part of the deletion targets. Those are markers that tell us
// that the corresponding descendant should be kept alive and NOT
// subject to eager deletion, because there's "something else" (not
// targeted for deletion) that has references down through the tree to
// it.
//
// Continuing the scenario from above, for R3 we have
//
// KeyA-R1, KeyA-R4, KeyB-R6
//
// This tells us that R3 should be kept alive for two reasons: it's
// referenced by a node that isn't being deleted (R4), and also by
// another source (KeyB). What about R1 and R2? They both have
//
// KeyA-R1
//
// So those should be deleted, since they are definitely only being
// kept alive by something that's about to be deleted.
//
// Final shape of the tree:
//
// R3
// /
// KeyA - R4 - R5
// /
// KeyB --- R6
.with('retained', ['entity_ref'], notPartOfDeletion =>
notPartOfDeletion
.select('subject')
.from('ancestors')
.whereNotNull('ancestors.source_key')
.where(foreignKeyOrRef =>
foreignKeyOrRef
.where('ancestors.source_key', '!=', sourceKey)
.orWhereNotIn('ancestors.target_entity_ref', refs),
),
)
// Return all descendants minus the retained ones
.select('descendants.entity_ref AS entity_ref')
.from('descendants')
.leftOuterJoin(
'retained',
'retained.entity_ref',
'descendants.entity_ref',
)
.whereNull('retained.entity_ref')
.then(rows => rows.map(row => row.entity_ref));
return { orphanEntityRefs: orphans };
}
async function markEntitiesAffectedByDeletionForStitching(options: {
knex: Knex | Knex.Transaction;
entityRefs: string[];
}) {
const { knex, entityRefs } = options;
// We want to re-stitch anything that has a relation pointing to the
// soon-to-be-deleted entity. In many circumstances we also re-stitch children
// in the refresh_state_references graph because their orphan state might
// change, but not here - this code by its very definition is meant to not
// leave any orphans behind, so we can simplify away that.
const affectedIds = await knex
.select('refresh_state.entity_id AS entity_id')
.from('relations')
.join(
'refresh_state',
'relations.source_entity_ref',
'refresh_state.entity_ref',
)
.whereIn('relations.target_entity_ref', entityRefs)
.then(rows => rows.map(row => row.entity_id));
for (const ids of lodash.chunk(affectedIds, 1000)) {
await knex
.table<DbFinalEntitiesRow>('final_entities')
.update({
hash: 'force-stitching',
})
.whereIn('entity_id', ids);
await knex
.table<DbRefreshStateRow>('refresh_state')
.update({
result_hash: 'force-stitching',
next_update_at: knex.fn.now(),
})
.whereIn('entity_id', ids);
}
}