From b48317cfc67bcf51e280bebf76a9486b75f8a5f1 Mon Sep 17 00:00:00 2001 From: Jussi Hallila Date: Fri, 13 Jan 2023 17:24:33 +0100 Subject: [PATCH 1/2] Modify tech insights lifecycle queries performance Modifies database cleanup to remove all facts for entities instead of hand-picked ones only. Improves query execution a lot in large datasets. Changes semantics of the lifecycle deletion logic slightly for cases were historical entities/facts, that are , not present in the application anymore, were kept forever instead of being cleaned up. The new implementation is more along the expected lines. Signed-off-by: Jussi Hallila --- .changeset/unlucky-zebras-scream.md | 6 ++++++ .../persistence/TechInsightsDatabase.ts | 20 +++++++++---------- 2 files changed, 16 insertions(+), 10 deletions(-) create mode 100644 .changeset/unlucky-zebras-scream.md diff --git a/.changeset/unlucky-zebras-scream.md b/.changeset/unlucky-zebras-scream.md new file mode 100644 index 0000000000..7f50fe7fa3 --- /dev/null +++ b/.changeset/unlucky-zebras-scream.md @@ -0,0 +1,6 @@ +--- +'@backstage/plugin-tech-insights-backend': patch +--- + +Modifies database cleanup to remove all facts for entities instead of hand-picked ones only. Improves query execution a lot in large datasets. +Changes semantics of the lifecycle deletion logic slightly for cases were historical entities/facts, that are , not present in the application anymore, were kept forever instead of being cleaned up. The new implementation is more along the expected lines. diff --git a/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.ts b/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.ts index c6751e864a..1454fe8b1e 100644 --- a/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.ts +++ b/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.ts @@ -114,10 +114,10 @@ export class TechInsightsDatabase implements TechInsightsStore { if (lifecycle && isTtl(lifecycle)) { const expiration = DateTime.now().minus(lifecycle.timeToLive); - await this.deleteExpiredFactsByDate(tx, factRows, expiration); + await this.deleteExpiredFactsByDate(tx, id, expiration); } if (lifecycle && isMaxItems(lifecycle)) { - await this.deleteExpiredFactsByNumber(tx, factRows, lifecycle.maxItems); + await this.deleteExpiredFactsByNumber(tx, id, lifecycle.maxItems); } }); } @@ -195,13 +195,13 @@ export class TechInsightsDatabase implements TechInsightsStore { private async deleteExpiredFactsByDate( tx: Transaction, - factRows: { id: string; entity: string }[], + factRetrieverId: string, timestamp: DateTime, ) { await tx('facts') - .whereIn( - ['id', 'entity'], - factRows.map(it => [it.id, it.entity]), + .where({ id: factRetrieverId }) + .and.whereIn('entity', db => + db.distinct('entity').where({ id: factRetrieverId }), ) .and.where('timestamp', '<', timestamp.toISO()) .delete(); @@ -209,13 +209,13 @@ export class TechInsightsDatabase implements TechInsightsStore { private async deleteExpiredFactsByNumber( tx: Transaction, - factRows: { id: string; entity: string }[], + factRetrieverId: string, maxItems: number, ) { const deletables = await tx('facts') - .whereIn( - ['id', 'entity'], - factRows.map(it => [it.id, it.entity]), + .where({ id: factRetrieverId }) + .and.whereIn('entity', db => + db.distinct('entity').where({ id: factRetrieverId }), ) .and.leftJoin( joinTable => From 3262f0ec831ffc3dafa2073bca3ad6a0993fa13c Mon Sep 17 00:00:00 2001 From: Jussi Hallila Date: Mon, 16 Jan 2023 12:01:07 +0100 Subject: [PATCH 2/2] Bake subquery into the delete clause Signed-off-by: Jussi Hallila --- .../persistence/TechInsightsDatabase.ts | 74 ++++++++++--------- 1 file changed, 38 insertions(+), 36 deletions(-) diff --git a/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.ts b/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.ts index 1454fe8b1e..2e1c6ccabc 100644 --- a/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.ts +++ b/plugins/tech-insights-backend/src/service/persistence/TechInsightsDatabase.ts @@ -212,43 +212,45 @@ export class TechInsightsDatabase implements TechInsightsStore { factRetrieverId: string, maxItems: number, ) { - const deletables = await tx('facts') - .where({ id: factRetrieverId }) - .and.whereIn('entity', db => - db.distinct('entity').where({ id: factRetrieverId }), - ) - .and.leftJoin( - joinTable => - joinTable - .select('*') - .from( - this.db('facts') - .column( - { fid: 'id' }, - { fentity: 'entity' }, - { ftimestamp: 'timestamp' }, - ) - .column( - this.db.raw( - 'row_number() over (partition by id, entity order by timestamp desc) as fact_rank', - ), - ) - .as('ranks'), - ) - .where('fact_rank', '<=', maxItems) - .as('filterjoin'), - joinClause => { - joinClause - .on('filterjoin.fid', 'facts.id') - .on('filterjoin.fentity', 'facts.entity') - .on('filterjoin.ftimestamp', 'facts.timestamp'); - }, - ) - .whereNull('filterjoin.fid'); + const deletionFilterQuery = (subTx: Knex.QueryBuilder) => + subTx + .select(['id', 'entity', 'timestamp']) + .from('facts') + .where({ id: factRetrieverId }) + .and.whereIn('entity', db => + db.distinct('entity').where({ id: factRetrieverId }), + ) + .and.leftJoin( + joinTable => + joinTable + .select('*') + .from( + this.db('facts') + .column( + { fid: 'id' }, + { fentity: 'entity' }, + { ftimestamp: 'timestamp' }, + ) + .column( + this.db.raw( + 'row_number() over (partition by id, entity order by timestamp desc) as fact_rank', + ), + ) + .as('ranks'), + ) + .where('fact_rank', '<=', maxItems) + .as('filterjoin'), + joinClause => { + joinClause + .on('filterjoin.fid', 'facts.id') + .on('filterjoin.fentity', 'facts.entity') + .on('filterjoin.ftimestamp', 'facts.timestamp'); + }, + ) + .whereNull('filterjoin.fid'); await tx('facts') - .whereIn( - ['id', 'entity', 'timestamp'], - deletables.map(it => [it.id, it.entity, it.timestamp]), + .whereIn(['id', 'entity', 'timestamp'], database => + deletionFilterQuery(database), ) .delete(); }