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

Handle eager deletion even in the face of duplicate references

Browse Source 
Signed-off-by: Fredrik Adelöw <freben@gmail.com>
This commit is contained in:
Fredrik Adelöw
2023-02-10 19:31:53 +01:00
parent c010a30fe9
commit 1aec041c34
5 changed files with 419 additions and 77 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/itchy-rings-rule.md
+5
View File
@@ -0,0 +1,5 @@
---
'@backstage/plugin-catalog-backend': patch
---
Fixed an issue where entities sometimes were not properly deleted during a full mutation.
plugins/catalog-backend/src/database/DefaultCatalogDatabase.test.ts
+1
View File
@@ -105,6 +105,7 @@ describe('DefaultCatalogDatabase', () => {
'location:default/root-2',
]);
},
60_000,
);
});
});
plugins/catalog-backend/src/database/DefaultProviderDatabase.test.ts
+34
View File
@@ -745,5 +745,39 @@ describe('DefaultProviderDatabase', () => {
},
60_000,
);
it.each(databases.eachSupportedId())(
'should gracefully handle accidental duplicate refresh state references when deletion happens during a full sync, %p',
async databaseId => {
const fakeLogger = { debug: jest.fn() };
const { knex, db } = await createDatabase(
databaseId,
fakeLogger as any,
);
await createLocations(knex, ['component:default/a']);
await insertRefRow(knex, {
source_key: 'a',
target_entity_ref: 'component:default/a',
});
await insertRefRow(knex, {
source_key: 'a',
target_entity_ref: 'component:default/a',
});
await db.transaction(async tx => {
await db.replaceUnprocessedEntities(tx, {
type: 'full',
sourceKey: 'a',
items: [],
});
});
const state = await knex<DbRefreshStateRow>('refresh_state').select();
expect(state).toEqual([]);
},
60_000,
);
});
});
plugins/catalog-backend/src/database/operations/provider/deleteWithEagerPruningOfChildren.test.ts
+278
View File
@@ -0,0 +1,278 @@
/*
* Copyright 2023 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 { 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 { deleteWithEagerPruningOfChildren } from './deleteWithEagerPruningOfChildren';
describe('deleteWithEagerPruningOfChildren', () => {
const databases = TestDatabases.create({
ids: ['MYSQL_8', 'POSTGRES_13', 'POSTGRES_9', 'SQLITE_3'],
});
async function createDatabase(databaseId: TestDatabaseId) {
const knex = await databases.init(databaseId);
await applyDatabaseMigrations(knex);
return knex;
}
async function run(
knex: Knex,
options: { entityRefs: string[]; sourceKey: string },
): Promise<number> {
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[]
) {
return knex<DbRefreshStateReferencesRow>('refresh_state_references').insert(
refs,
);
}
async function insertEntity(knex: Knex, ...entityRefs: string[]) {
for (const ref of entityRefs) {
await knex<DbRefreshStateRow>('refresh_state').insert({
entity_id: uuid.v4(),
entity_ref: ref,
unprocessed_entity: '{}',
processed_entity: '{}',
errors: '[]',
next_update_at: '2021-04-01 13:37:00',
last_discovery_at: '2021-04-01 13:37:00',
});
}
}
async function remainingEntities(knex: Knex) {
const rows = await knex<DbRefreshStateRow>('refresh_state')
.orderBy('entity_ref')
.select('entity_ref');
return rows.map(r => r.entity_ref);
}
it.each(databases.eachSupportedId())(
'works for the simple path, %p',
async databaseId => {
/*
P1 - E1 - E2
P1 - E3
P1 - E4
P2 - E5
Scenario: P1 issues delete for E1 and E3
Result: E1, E2, and E3 deleted; E4 and E5 remain
*/
const knex = await createDatabase(databaseId);
await insertEntity(knex, 'E1', 'E2', 'E3', 'E4', 'E5');
await insertReference(
knex,
{ source_key: 'P1', target_entity_ref: 'E1' },
{ source_entity_ref: 'E1', target_entity_ref: 'E2' },
{ source_key: 'P1', target_entity_ref: 'E3' },
{ source_key: 'P1', target_entity_ref: 'E4' },
{ source_key: 'P2', target_entity_ref: 'E5' },
);
await run(knex, { sourceKey: 'P1', entityRefs: ['E1', 'E3'] });
await expect(remainingEntities(knex)).resolves.toEqual(['E4', 'E5']);
},
60_000,
);
it.each(databases.eachSupportedId())(
'works when there are multiple identical references, %p',
async databaseId => {
/*
P1
\
E1
/
P1
P1 - E2
Scenario: P1 issues delete for E1
Result: E1 deleted; E2 remains
*/
const knex = await createDatabase(databaseId);
await insertEntity(knex, 'E1', 'E2');
await insertReference(
knex,
{ source_key: 'P1', target_entity_ref: 'E1' },
{ source_key: 'P1', target_entity_ref: 'E1' },
{ source_key: 'P1', target_entity_ref: 'E2' },
);
await run(knex, { sourceKey: 'P1', entityRefs: ['E1'] });
await expect(remainingEntities(knex)).resolves.toEqual(['E2']);
},
60_000,
);
it.each(databases.eachSupportedId())(
'leaves out things that have roots in other source keys, %p',
async databaseId => {
/*
P1 - E1
\
E2
/
P2 - E3
Scenario: P1 issues delete for E1
Result: E1 deleted; E2 and E3 remain
*/
const knex = await createDatabase(databaseId);
await insertEntity(knex, 'E1', 'E2', 'E3');
await insertReference(
knex,
{ source_key: 'P1', target_entity_ref: 'E1' },
{ source_entity_ref: 'E1', target_entity_ref: 'E2' },
{ source_key: 'P2', target_entity_ref: 'E3' },
{ source_entity_ref: 'E3', target_entity_ref: 'E2' },
);
await run(knex, { sourceKey: 'P1', entityRefs: ['E1'] });
await expect(remainingEntities(knex)).resolves.toEqual(['E2', 'E3']);
},
60_000,
);
it.each(databases.eachSupportedId())(
'leaves out things that have several different roots for the same source key, %p',
async databaseId => {
/*
P1 - E1
\
E2
/
P1 - E3
Scenario: P1 issues delete for E1
Result: E1 deleted; E2 and E3 remain
*/
const knex = await createDatabase(databaseId);
await insertEntity(knex, 'E1', 'E2', 'E3');
await insertReference(
knex,
{ source_key: 'P1', target_entity_ref: 'E1' },
{ source_entity_ref: 'E1', target_entity_ref: 'E2' },
{ source_key: 'P1', target_entity_ref: 'E3' },
{ source_entity_ref: 'E3', target_entity_ref: 'E2' },
);
await run(knex, { sourceKey: 'P1', entityRefs: ['E1'] });
await expect(remainingEntities(knex)).resolves.toEqual(['E2', 'E3']);
},
60_000,
);
it.each(databases.eachSupportedId())(
'handles cycles and diamonds gracefully, %p',
async databaseId => {
/*
P1 - E1 <-> E2
\
E4 E6
/ \ /
P1 -- E3 -- E5
Scenario: P1 issues delete for E1, then for E3
Result: Everything deleted, but in two steps
*/
const knex = await createDatabase(databaseId);
await insertEntity(knex, 'E1', 'E2', 'E3', 'E4', 'E5', 'E6');
await insertReference(
knex,
{ source_key: 'P1', target_entity_ref: 'E1' },
{ source_entity_ref: 'E1', target_entity_ref: 'E2' },
{ source_entity_ref: 'E2', target_entity_ref: 'E1' },
{ source_entity_ref: 'E2', target_entity_ref: 'E6' },
{ source_key: 'P1', target_entity_ref: 'E3' },
{ source_entity_ref: 'E3', target_entity_ref: 'E4' },
{ source_entity_ref: 'E4', target_entity_ref: 'E5' },
{ source_entity_ref: 'E3', target_entity_ref: 'E5' },
{ source_entity_ref: 'E5', target_entity_ref: 'E6' },
);
await run(knex, { sourceKey: 'P1', entityRefs: ['E1'] });
await expect(remainingEntities(knex)).resolves.toEqual([
'E3',
'E4',
'E5',
'E6',
]);
await run(knex, { sourceKey: 'P1', entityRefs: ['E3'] });
await expect(remainingEntities(knex)).resolves.toEqual([]);
},
60_000,
);
it.each(databases.eachSupportedId())(
'silently ignores attempts to delete things that are not your own and/or are not roots, %p',
async databaseId => {
/*
P1 - E1 - E2
P1 - E3
P2 - E4
Scenario: P1 issues delete for E2, E3, and E4
Result: E3 is deleted; E1, E2 and E4 remain
*/
const knex = await createDatabase(databaseId);
await insertEntity(knex, 'E1', 'E2', 'E3', 'E4');
await insertReference(
knex,
{ source_key: 'P1', target_entity_ref: 'E1' },
{ source_entity_ref: 'E1', target_entity_ref: 'E2' },
{ source_key: 'P1', target_entity_ref: 'E3' },
{ source_key: 'P2', target_entity_ref: 'E4' },
);
await run(knex, { sourceKey: 'P1', entityRefs: ['E2', 'E3', 'E4'] });
await expect(remainingEntities(knex)).resolves.toEqual([
'E1',
'E2',
'E4',
]);
},
60_000,
);
});
plugins/catalog-backend/src/database/operations/provider/deleteWithEagerPruningOfChildren.ts
+101 -77
View File
@@ -31,113 +31,137 @@ export async function deleteWithEagerPruningOfChildren(options: {
sourceKey: string;
}): Promise<number> {
const { tx, entityRefs, sourceKey } = options;
let removedCount = 0;
const rootId = () =>
tx.raw(
tx.client.config.client.includes('mysql')
? 'CAST(NULL as UNSIGNED INT)'
: 'CAST(NULL as INT)',
[],
);
// 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)) {
/*
WITH RECURSIVE
-- All the nodes that can be reached downwards from our root
descendants(root_id, entity_ref) AS (
SELECT id, target_entity_ref
descendants(entity_ref) AS (
SELECT target_entity_ref
FROM refresh_state_references
WHERE source_key = "R1" AND target_entity_ref = "A"
WHERE source_key = "R1" AND target_entity_ref IN [...refs]
UNION
SELECT descendants.root_id, target_entity_ref
SELECT target_entity_ref
FROM descendants
JOIN refresh_state_references ON source_entity_ref = descendants.entity_ref
),
-- All the nodes that can be reached upwards from the descendants
ancestors(root_id, via_entity_ref, to_entity_ref) AS (
SELECT CAST(NULL as INT), entity_ref, entity_ref
-- All the individual relations that can be reached upwards from each descendant
ancestors(source_key, source_entity_ref, target_entity_ref, subject) AS (
SELECT source_key, source_entity_ref, target_entity_ref, descendants.entity_ref
FROM descendants
JOIN refresh_state_references ON refresh_state_references.target_entity_ref = descendants.entity_ref
UNION
SELECT
CASE WHEN source_key IS NOT NULL THEN id ELSE NULL END,
source_entity_ref,
ancestors.to_entity_ref
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 ON target_entity_ref = ancestors.via_entity_ref
JOIN refresh_state_references ON refresh_state_references.target_entity_ref = ancestors.source_entity_ref
)
-- Start out with all of the descendants
SELECT descendants.entity_ref
FROM descendants
-- Expand with all ancestors that point to those, but aren't the current root
LEFT OUTER JOIN ancestors
ON ancestors.to_entity_ref = descendants.entity_ref
AND ancestors.root_id IS NOT NULL
AND ancestors.root_id != descendants.root_id
-- Exclude all lines that had such a foreign ancestor
WHERE ancestors.root_id IS NULL;
-- Exclude those who seem to have a root relation somewhere upwards that's not part of our own deletion
WHERE NOT EXISTS (
SELECT * FROM ancestors
WHERE ancestors.subject = descendants.entity_ref
AND ancestors.source_key IS NOT NULL
AND (ancestors.source_key != "R1" OR ancestors.target_entity_ref NOT IN [...refs])
)
*/
removedCount += await tx<DbRefreshStateRow>('refresh_state')
.whereIn('entity_ref', function orphanedEntityRefs(orphans) {
return (
orphans
// All the nodes that can be reached downwards from our root
.withRecursive('descendants', function descendants(outer) {
return outer
.select({ root_id: 'id', entity_ref: 'target_entity_ref' })
.from('refresh_state_references')
.where('source_key', sourceKey)
.whereIn('target_entity_ref', refs)
.union(function recursive(inner) {
return inner
.select({
root_id: 'descendants.root_id',
entity_ref: 'refresh_state_references.target_entity_ref',
})
.from('descendants')
.join('refresh_state_references', {
'descendants.entity_ref':
'refresh_state_references.source_entity_ref',
});
});
})
// All the nodes that can be reached upwards from the descendants
.withRecursive('ancestors', function ancestors(outer) {
return outer
.select({
root_id: rootId(),
via_entity_ref: 'entity_ref',
to_entity_ref: 'entity_ref',
})
.from('descendants')
.union(function recursive(inner) {
return inner
.select({
root_id: tx.raw(
'CASE WHEN source_key IS NOT NULL THEN id ELSE NULL END',
[],
),
via_entity_ref: 'source_entity_ref',
to_entity_ref: 'ancestors.to_entity_ref',
})
.from('ancestors')
.join('refresh_state_references', {
target_entity_ref: 'ancestors.via_entity_ref',
});
});
})
.withRecursive(
'descendants',
['entity_ref'],
function descendants(outer) {
return outer
.select({ entity_ref: 'target_entity_ref' })
.from('refresh_state_references')
.where('source_key', '=', sourceKey)
.whereIn('target_entity_ref', refs)
.union(function recursive(inner) {
return inner
.select({
entity_ref:
'refresh_state_references.target_entity_ref',
})
.from('descendants')
.join('refresh_state_references', {
'descendants.entity_ref':
'refresh_state_references.source_entity_ref',
});
});
},
)
// All the relations that can be reached upwards from each descendant
.withRecursive(
'ancestors',
[
'source_key',
'source_entity_ref',
'target_entity_ref',
'subject',
],
function ancestors(outer) {
return outer
.select({
source_key: 'refresh_state_references.source_key',
source_entity_ref:
'refresh_state_references.source_entity_ref',
target_entity_ref:
'refresh_state_references.target_entity_ref',
subject: 'descendants.entity_ref',
})
.from('descendants')
.join('refresh_state_references', {
'refresh_state_references.target_entity_ref':
'descendants.entity_ref',
})
.union(function recursive(inner) {
return inner
.select({
source_key: 'refresh_state_references.source_key',
source_entity_ref:
'refresh_state_references.source_entity_ref',
target_entity_ref:
'refresh_state_references.target_entity_ref',
subject: 'ancestors.subject',
})
.from('ancestors')
.join('refresh_state_references', {
'refresh_state_references.target_entity_ref':
'ancestors.source_entity_ref',
});
});
},
)
// Start out with all of the descendants
.select('descendants.entity_ref')
.from('descendants')
// Expand with all ancestors that point to those, but aren't the current root
.leftOuterJoin('ancestors', function keepaliveRoots() {
this.on('ancestors.to_entity_ref', '=', 'descendants.entity_ref');
this.andOnNotNull('ancestors.root_id');
this.andOn('ancestors.root_id', '!=', 'descendants.root_id');
// Exclude those who seem to have a root relation somewhere upwards that's not part of our own deletion
.whereNotExists(function otherAncestors(outer) {
outer
.from('ancestors')
.where(
'ancestors.subject',
'=',
tx.ref('descendants.entity_ref'),
)
.whereNotNull('ancestors.source_key')
.andWhere(function differentRoot(inner) {
inner
.where('ancestors.source_key', '!=', sourceKey)
.orWhereNotIn('ancestors.target_entity_ref', refs);
});
})
.whereNull('ancestors.root_id')
);
})
.delete();