From feba9ee69d9db39b3b4e1bd03cbcab9787458b04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Sat, 7 Dec 2024 15:18:56 +0100 Subject: [PATCH] Use a join based strategy for filtering, when having small page sizes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/empty-colts-promise.md | 5 + .../src/service/DefaultEntitiesCatalog.ts | 174 ++++--------- .../request/applyEntityFilterToQuery.test.ts | 196 ++++++++++++++ .../request/applyEntityFilterToQuery.ts | 242 ++++++++++++++++++ 4 files changed, 488 insertions(+), 129 deletions(-) create mode 100644 .changeset/empty-colts-promise.md create mode 100644 plugins/catalog-backend/src/service/request/applyEntityFilterToQuery.test.ts create mode 100644 plugins/catalog-backend/src/service/request/applyEntityFilterToQuery.ts diff --git a/.changeset/empty-colts-promise.md b/.changeset/empty-colts-promise.md new file mode 100644 index 0000000000..9f7f979b98 --- /dev/null +++ b/.changeset/empty-colts-promise.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend': patch +--- + +Use a join based strategy for filtering, when having small page sizes diff --git a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts index fa43eb79d2..633b485604 100644 --- a/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts +++ b/plugins/catalog-backend/src/service/DefaultEntitiesCatalog.ts @@ -49,13 +49,11 @@ import { isQueryEntitiesCursorRequest, isQueryEntitiesInitialRequest, } from './util'; -import { - EntitiesSearchFilter, - EntityFilter, -} from '@backstage/plugin-catalog-node'; +import { EntityFilter } from '@backstage/plugin-catalog-node'; import { LoggerService } from '@backstage/backend-plugin-api'; +import { applyEntityFilterToQuery } from './request/applyEntityFilterToQuery'; -const DEFAULT_LIMIT = 20; +const DEFAULT_LIMIT = 200; function parsePagination(input?: EntityPagination): EntityPagination { if (!input) { @@ -102,84 +100,6 @@ function stringifyPagination( return base64; } -function addCondition( - queryBuilder: Knex.QueryBuilder, - db: Knex, - filter: EntitiesSearchFilter, - negate: boolean = false, - entityIdField = 'entity_id', -): void { - const key = filter.key.toLowerCase(); - const values = filter.values?.map(v => v.toLowerCase()); - - // NOTE(freben): This used to be a set of OUTER JOIN, which may seem to - // make a lot of sense. However, it had abysmal performance on sqlite - // when datasets grew large, so we're using IN instead. - const matchQuery = db('search') - .select('search.entity_id') - .where({ key }) - .andWhere(function keyFilter() { - if (values?.length === 1) { - this.where({ value: values.at(0) }); - } else if (values) { - this.andWhere('value', 'in', values); - } - }); - queryBuilder.andWhere(entityIdField, negate ? 'not in' : 'in', matchQuery); -} - -function isEntitiesSearchFilter( - filter: EntitiesSearchFilter | EntityFilter, -): filter is EntitiesSearchFilter { - return filter.hasOwnProperty('key'); -} - -function isOrEntityFilter( - filter: { anyOf: EntityFilter[] } | EntityFilter, -): filter is { anyOf: EntityFilter[] } { - return filter.hasOwnProperty('anyOf'); -} - -function isNegationEntityFilter( - filter: { not: EntityFilter } | EntityFilter, -): filter is { not: EntityFilter } { - return filter.hasOwnProperty('not'); -} - -function parseFilter( - filter: EntityFilter, - query: Knex.QueryBuilder, - db: Knex, - negate: boolean = false, - entityIdField = 'entity_id', -): Knex.QueryBuilder { - if (isNegationEntityFilter(filter)) { - return parseFilter(filter.not, query, db, !negate, entityIdField); - } - - if (isEntitiesSearchFilter(filter)) { - return query.andWhere(function filterFunction() { - addCondition(this, db, filter, negate, entityIdField); - }); - } - - return query[negate ? 'andWhereNot' : 'andWhere'](function filterFunction() { - if (isOrEntityFilter(filter)) { - for (const subFilter of filter.anyOf ?? []) { - this.orWhere(subQuery => - parseFilter(subFilter, subQuery, db, false, entityIdField), - ); - } - } else { - for (const subFilter of filter.allOf ?? []) { - this.andWhere(subQuery => - parseFilter(subFilter, subQuery, db, false, entityIdField), - ); - } - } - }); -} - export class DefaultEntitiesCatalog implements EntitiesCatalog { private readonly database: Knex; private readonly logger: LoggerService; @@ -197,6 +117,7 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog { async entities(request?: EntitiesRequest): Promise { const db = this.database; + const { limit, offset } = parsePagination(request?.pagination); let entitiesQuery = db('final_entities').select('final_entities.*'); @@ -216,13 +137,13 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog { entitiesQuery = entitiesQuery.whereNotNull('final_entities.final_entity'); if (request?.filter) { - entitiesQuery = parseFilter( - request.filter, - entitiesQuery, - db, - false, - 'final_entities.entity_id', - ); + entitiesQuery = applyEntityFilterToQuery({ + filter: request.filter, + targetQuery: entitiesQuery, + onEntityIdField: 'final_entities.entity_id', + knex: db, + strategy: limit !== undefined && limit <= 500 ? 'join' : 'in', + }); } request?.order?.forEach(({ order }, index) => { @@ -248,7 +169,6 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog { entitiesQuery.orderBy('final_entities.entity_id', 'asc'); // stable sort } - const { limit, offset } = parsePagination(request?.pagination); if (limit !== undefined) { entitiesQuery = entitiesQuery.limit(limit + 1); } @@ -299,13 +219,12 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog { .whereIn('final_entities.entity_ref', chunk); if (request?.filter) { - query = parseFilter( - request.filter, - query, - this.database, - false, - 'final_entities.entity_id', - ); + query = applyEntityFilterToQuery({ + filter: request.filter, + targetQuery: query, + onEntityIdField: 'final_entities.entity_id', + knex: this.database, + }); } for (const row of await query) { @@ -325,8 +244,6 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog { async queryEntities( request: QueryEntitiesRequest, ): Promise { - const db = this.database; - const limit = request.limit ?? DEFAULT_LIMIT; const cursor: Omit & { @@ -354,7 +271,7 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog { // The first part of the query builder is a subquery that applies all of the // filtering. - const dbQuery = db.with( + const dbQuery = this.database.with( 'filtered', ['entity_id', 'final_entity', ...(sortField ? ['value'] : [])], inner => { @@ -383,13 +300,13 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog { // Add regular filters, if given if (cursor.filter) { - parseFilter( - cursor.filter, - inner, - db, - false, - 'final_entities.entity_id', - ); + applyEntityFilterToQuery({ + filter: cursor.filter, + targetQuery: inner, + onEntityIdField: 'final_entities.entity_id', + knex: this.database, + strategy: limit <= 500 ? 'join' : 'in', + }); } // Add full text search filters, if given @@ -406,20 +323,20 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog { // If there is one item, apply the like query to the top level query which is already // filtered based on the singular sortField. inner.andWhereRaw( - 'value like ?', + 'search.value like ?', `%${normalizedFullTextFilterTerm.toLocaleLowerCase('en-US')}%`, ); } else { - const matchQuery = db('search') - .select('entity_id') + const matchQuery = this.database('search') + .select('search.entity_id') // textFilterFields must be lowercased to match searchable keys in database, i.e. spec.profile.displayName -> spec.profile.displayname .whereIn( - 'key', + 'search.key', textFilterFields.map(field => field.toLocaleLowerCase('en-US')), ) .andWhere(function keyFilter() { this.andWhereRaw( - 'value like ?', + 'search.value like ?', `%${normalizedFullTextFilterTerm.toLocaleLowerCase( 'en-US', )}%`, @@ -457,13 +374,13 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog { const [first, second] = cursor.orderFieldValues; dbQuery.andWhere(function nested() { this.where( - 'value', + 'filtered.value', isFetchingBackwards !== isOrderingDescending ? '<' : '>', first, ) - .orWhere('value', '=', first) + .orWhere('filtered.value', '=', first) .andWhere( - 'entity_id', + 'filtered.entity_id', isFetchingBackwards !== isOrderingDescending ? '<' : '>', second, ); @@ -480,20 +397,20 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog { if (isFetchingBackwards) { order = invertOrder(order); } - if (db.client.config.client === 'pg') { + if (this.database.client.config.client === 'pg') { // pg correctly orders by the column value and handling nulls in one go dbQuery.orderBy([ ...(sortField ? [ { - column: 'value', + column: 'filtered.value', order, nulls: 'last', }, ] : []), { - column: 'entity_id', + column: 'filtered.entity_id', order, }, ]); @@ -505,18 +422,18 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog { ...(sortField ? [ { - column: 'value', + column: 'filtered.value', order: undefined, nulls: 'last', }, { - column: 'value', + column: 'filtered.value', order, }, ] : []), { - column: 'entity_id', + column: 'filtered.entity_id', order, }, ]); @@ -768,13 +685,12 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog { .groupBy(['search.key', 'search.original_value']); if (request.filter) { - parseFilter( - request.filter, - query, - this.database, - false, - 'search.entity_id', - ); + applyEntityFilterToQuery({ + filter: request.filter, + targetQuery: query, + onEntityIdField: 'search.entity_id', + knex: this.database, + }); } const rows = await query; diff --git a/plugins/catalog-backend/src/service/request/applyEntityFilterToQuery.test.ts b/plugins/catalog-backend/src/service/request/applyEntityFilterToQuery.test.ts new file mode 100644 index 0000000000..ea64f5b400 --- /dev/null +++ b/plugins/catalog-backend/src/service/request/applyEntityFilterToQuery.test.ts @@ -0,0 +1,196 @@ +/* + * Copyright 2024 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 { TestDatabases } from '@backstage/backend-test-utils'; +import { applyEntityFilterToQuery } from './applyEntityFilterToQuery'; +import { + DbFinalEntitiesRow, + DbRefreshStateRow, + DbSearchRow, +} from '../../database/tables'; +import { Knex } from 'knex'; +import { applyDatabaseMigrations } from '../../database/migrations'; +import { EntityFilter } from '@backstage/plugin-catalog-node'; +import { Entity, stringifyEntityRef } from '@backstage/catalog-model'; +import { v4 as uuid } from 'uuid'; +import { buildEntitySearch } from '../../database/operations/stitcher/buildEntitySearch'; + +jest.setTimeout(60_000); + +const databases = TestDatabases.create(); +const strategies = ['in', 'join'] as const; + +describe.each(databases.eachSupportedId())( + 'applyEntityFilterToQuery, %p', + databaseId => { + // #region setup + let knex: Knex; + + beforeAll(async () => { + knex = await databases.init(databaseId); + await applyDatabaseMigrations(knex); + await addEntity({ + apiVersion: 'a', + kind: 'k', + metadata: { name: '1', namespace: 'default' }, + spec: { + foo: 'a', + unique: true, + }, + }); + await addEntity({ + apiVersion: 'a', + kind: 'k', + metadata: { name: '2', namespace: 'default' }, + spec: { + foo: 'a', + }, + }); + await addEntity({ + apiVersion: 'a', + kind: 'k', + metadata: { name: '3', namespace: 'default' }, + spec: { + foo: 'b', + }, + }); + await addEntity({ + apiVersion: 'a', + kind: 'k', + metadata: { name: '4', namespace: 'default' }, + spec: {}, + }); + }); + + afterAll(async () => { + knex.destroy(); + }); + + async function addEntity(entity: Entity) { + const id = uuid(); + const entityRef = stringifyEntityRef(entity); + const entityJson = JSON.stringify(entity); + + await knex('refresh_state').insert({ + entity_id: id, + entity_ref: entityRef, + unprocessed_entity: entityJson, + errors: '[]', + next_update_at: '2031-01-01 23:00:00', + last_discovery_at: '2021-04-01 13:37:00', + }); + + await knex('final_entities').insert({ + entity_id: id, + entity_ref: entityRef, + final_entity: entityJson, + hash: 'h', + stitch_ticket: '', + }); + + const search = await buildEntitySearch(id, entity); + await knex('search').insert(search); + + return id; + } + // #endregion + + describe.each(strategies)('with strategy %p', strategy => { + async function query(filter: EntityFilter): Promise { + const q = + knex('final_entities').whereNotNull( + 'final_entity', + ); + applyEntityFilterToQuery({ + filter: filter, + targetQuery: q, + onEntityIdField: 'final_entities.entity_id', + knex, + strategy, + }); + return await q.then(rows => + rows + .map(row => JSON.parse(row.final_entity!).metadata.name) + .toSorted(), + ); + } + + it('filters correctly', async () => { + await expect(query({ key: 'spec.foo' })).resolves.toEqual([ + '1', + '2', + '3', + ]); + + await expect( + query({ key: 'spec.foo', values: ['a'] }), + ).resolves.toEqual(['1', '2']); + + await expect( + query({ key: 'spec.foo', values: ['b'] }), + ).resolves.toEqual(['3']); + + await expect( + query({ key: 'spec.foo', values: ['a', 'b'] }), + ).resolves.toEqual(['1', '2', '3']); + + await expect( + query({ + anyOf: [ + { key: 'spec.foo', values: ['a'] }, + { key: 'spec.foo', values: ['b'] }, + ], + }), + ).resolves.toEqual(['1', '2', '3']); + + await expect( + query({ not: { key: 'spec.foo', values: ['a'] } }), + ).resolves.toEqual(['3', '4']); + + await expect( + query({ + not: { + anyOf: [ + { key: 'spec.foo', values: ['a'] }, + { key: 'spec.foo', values: ['b'] }, + ], + }, + }), + ).resolves.toEqual(['4']); + + await expect( + query({ + allOf: [ + { key: 'spec.foo' }, + { not: { key: 'spec.foo', values: ['a'] } }, + ], + }), + ).resolves.toEqual(['3']); + + await expect(query({ key: 'spec.unique' })).resolves.toEqual(['1']); + + await expect( + query({ + allOf: [ + { key: 'spec.foo', values: ['a'] }, + { not: { key: 'spec.unique' } }, + ], + }), + ).resolves.toEqual(['2']); + }); + }); + }, +); diff --git a/plugins/catalog-backend/src/service/request/applyEntityFilterToQuery.ts b/plugins/catalog-backend/src/service/request/applyEntityFilterToQuery.ts new file mode 100644 index 0000000000..bf5c69d6b3 --- /dev/null +++ b/plugins/catalog-backend/src/service/request/applyEntityFilterToQuery.ts @@ -0,0 +1,242 @@ +/* + * Copyright 2024 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 { + EntitiesSearchFilter, + EntityFilter, +} from '@backstage/plugin-catalog-node'; +import { Knex } from 'knex'; +import { DbSearchRow } from '../../database/tables'; + +function isEntitiesSearchFilter( + filter: EntitiesSearchFilter | EntityFilter, +): filter is EntitiesSearchFilter { + return filter.hasOwnProperty('key'); +} + +function isOrEntityFilter( + filter: EntityFilter, +): filter is { anyOf: EntityFilter[] } { + return filter.hasOwnProperty('anyOf'); +} + +function isAndEntityFilter( + filter: EntityFilter, +): filter is { allOf: EntityFilter[] } { + return filter.hasOwnProperty('allOf'); +} + +function isNegationEntityFilter( + filter: EntityFilter, +): filter is { not: EntityFilter } { + return filter.hasOwnProperty('not'); +} + +/** + * Applies filtering through a number of WHERE IN subqueries. Example: + * + * ``` + * SELECT * FROM final_entities + * WHERE + * entity_id IN ( + * SELECT entity_id FROM search + * WHERE key = 'kind' AND value = 'component' + * ) + * AND entity_id IN ( + * SELECT entity_id FROM search + * WHERE key = 'spec.lifecycle' AND value = 'production' + * ) + * AND final_entities.final_entity IS NOT NULL + * ``` + * + * This strategy is a good all-rounder, in the sense that it has medium-good + * performance on most queries on all database engines. However, it does not + * scale well down to very short runtimes as well as the JOIN strategy. + */ +function applyInStrategy( + filter: EntityFilter, + targetQuery: Knex.QueryBuilder, + onEntityIdField: string, + knex: Knex, + negate: boolean, +): Knex.QueryBuilder { + if (isNegationEntityFilter(filter)) { + return applyInStrategy( + filter.not, + targetQuery, + onEntityIdField, + knex, + !negate, + ); + } + + if (isEntitiesSearchFilter(filter)) { + const key = filter.key.toLowerCase(); + const values = filter.values?.map(v => v.toLowerCase()); + const matchQuery = knex('search') + .select('search.entity_id') + .where({ key }) + .andWhere(function keyFilter() { + if (values?.length === 1) { + this.where({ value: values.at(0) }); + } else if (values) { + this.andWhere('value', 'in', values); + } + }); + return targetQuery.andWhere( + onEntityIdField, + negate ? 'not in' : 'in', + matchQuery, + ); + } + + return targetQuery[negate ? 'andWhereNot' : 'andWhere']( + function filterFunction() { + if (isOrEntityFilter(filter)) { + for (const subFilter of filter.anyOf ?? []) { + this.orWhere(subQuery => + applyInStrategy(subFilter, subQuery, onEntityIdField, knex, false), + ); + } + } else { + for (const subFilter of filter.allOf ?? []) { + this.andWhere(subQuery => + applyInStrategy(subFilter, subQuery, onEntityIdField, knex, false), + ); + } + } + }, + ); +} + +/** + * Applies filtering through a number of JOINs with the search table. Example: + * + * ``` + * SELECT * FROM final_entities + * LEFT OUTER JOIN search AS filter_0 + * ON filter_0.entity_id = final_entities.entity_id + * AND filter_0.key = 'kind' + * LEFT OUTER JOIN search AS filter_1 + * ON filter_1.entity_id = final_entities.entity_id + * AND filter_1.key = 'spec.lifecycle' + * WHERE (filter_0.value = 'component' AND filter_1.value = 'production') + * AND final_entities.final_entity IS NOT NULL + * ``` + * + * This strategy has very good performance on nested medium complexity queries + * on pg, but can be slow on sqlite. It also has much larger variance than the + * IN strategy: for small page sizes (< 500 or so, depending on circumstances) + * it generates a fast plan, but then at some threshold switches over to scans + * which suddenly lead to much worse performance than IN. Therefore it can be + * important to pick carefully between the strategies. + */ +function applyJoinStrategy( + filter: EntityFilter, + targetQuery: Knex.QueryBuilder, + onEntityIdField: string, +): Knex.QueryBuilder { + // First we traverse the entire query tree to gather up all of the unique keys + // that are tested against, and make sure to make an outer join on the search + // table for each of them. As we do so, collect the table aliases made along + // the way. In the end, this map may contain for example + // `{ 'kind': 'filter_0', 'spec.lifecycle': 'filter_1' }` + const keyToSearchTableAlias = new Map(); + function recursiveMakeJoinAliases(filterNode: EntityFilter) { + if (isNegationEntityFilter(filterNode)) { + recursiveMakeJoinAliases(filterNode.not); + } else if (isOrEntityFilter(filterNode)) { + filterNode.anyOf.forEach(recursiveMakeJoinAliases); + } else if (isAndEntityFilter(filterNode)) { + filterNode.allOf.forEach(recursiveMakeJoinAliases); + } else { + const key = filterNode.key.toLowerCase(); + if (!keyToSearchTableAlias.has(key)) { + const alias = `filter_${keyToSearchTableAlias.size}`; + keyToSearchTableAlias.set(key, alias); + targetQuery.leftOuterJoin({ [alias]: 'search' }, inner => + inner + .on(`${alias}.entity_id`, onEntityIdField) + .andOnVal(`${alias}.key`, key), + ); + } + } + } + recursiveMakeJoinAliases(filter); + + // Then we traverse the query tree again, this time building up the actual + // WHERE query based on values from the aliases above + function recursiveBuildQuery( + queryBuilder: Knex.QueryBuilder, + filterNode: EntityFilter, + ) { + if (isNegationEntityFilter(filterNode)) { + queryBuilder.whereNot(inner => + recursiveBuildQuery(inner, filterNode.not), + ); + } else if (isOrEntityFilter(filterNode)) { + // This extra nesting is needed to make sure that the ORs are grouped + // separately and not "leak" next to ANDs in the caller's query. + queryBuilder.andWhere(inner => { + for (const subFilter of filterNode.anyOf) { + inner.orWhere(inner2 => recursiveBuildQuery(inner2, subFilter)); + } + }); + } else if (isAndEntityFilter(filterNode)) { + for (const subFilter of filterNode.allOf) { + queryBuilder.andWhere(inner => recursiveBuildQuery(inner, subFilter)); + } + } else { + const key = filterNode.key.toLowerCase(); + const values = filterNode.values?.map(v => v.toLowerCase()); + const column = `${keyToSearchTableAlias.get(key)}.value`; + if (!values) { + queryBuilder.whereNotNull(column); + } else if (values.length === 1) { + // Null check needed since NULL = 'string' evaluates to NULL, not FALSE + queryBuilder.whereNotNull(column).andWhere(column, values[0]); + } else { + queryBuilder.whereIn(column, values); + } + } + } + recursiveBuildQuery(targetQuery, filter); + + return targetQuery; +} + +// The actual exported function +export function applyEntityFilterToQuery(options: { + filter: EntityFilter; + targetQuery: Knex.QueryBuilder; + onEntityIdField: string; + knex: Knex; + strategy?: 'in' | 'join'; +}): Knex.QueryBuilder { + const { + filter, + targetQuery, + onEntityIdField, + knex, + strategy = 'in', + } = options; + if (strategy === 'in') { + return applyInStrategy(filter, targetQuery, onEntityIdField, knex, false); + } else if (strategy === 'join') { + return applyJoinStrategy(filter, targetQuery, onEntityIdField); + } + throw new Error(`Unsupported filtering strategy ${strategy}`); +}