Merge pull request #34001 from backstage/freben/facets-perf
catalog-backend: fix facets endpoint performance regression
This commit is contained in:
Fredrik Adelöw
@@ -0,0 +1,5 @@
|
||||
---
|
||||
'@backstage/plugin-catalog-backend': patch
|
||||
---
|
||||
|
||||
Fixed a performance regression in the `/entity-facets` endpoint when filters or permission conditions are applied, by routing the EXISTS-based filter through `final_entities` instead of correlating against the much larger `search` table.
|
||||
@@ -0,0 +1 @@
|
||||
Fix facets endpoint performance regression when filters or permissions are applied
|
||||
@@ -2430,9 +2430,297 @@ describe('DefaultEntitiesCatalog', () => {
|
||||
}),
|
||||
).resolves.toEqual({
|
||||
facets: {
|
||||
'metadata.name': expect.arrayContaining([
|
||||
'metadata.name': [{ value: 'one', count: 1 }],
|
||||
},
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
async function setupFacetsCatalog(
|
||||
databaseId: TestDatabaseId,
|
||||
entities: Entity[],
|
||||
) {
|
||||
await createDatabase(databaseId);
|
||||
for (const entity of entities) {
|
||||
await addEntityToSearch(entity);
|
||||
}
|
||||
return new DefaultEntitiesCatalog({
|
||||
database: knex,
|
||||
logger: mockServices.logger.mock(),
|
||||
stitcher,
|
||||
});
|
||||
}
|
||||
|
||||
it.each(databases.eachSupportedId())(
|
||||
'excludes not-yet-stitched entities from filtered facets, %p',
|
||||
async databaseId => {
|
||||
await createDatabase(databaseId);
|
||||
|
||||
await addEntityToSearch({
|
||||
apiVersion: 'a',
|
||||
kind: 'Component',
|
||||
metadata: { name: 'stitched' },
|
||||
spec: {},
|
||||
});
|
||||
|
||||
// Insert an unstitched entity: final_entity is NULL but search
|
||||
// rows exist. This simulates a race or future tombstone state.
|
||||
const unstitchedId = v4();
|
||||
await knex<DbRefreshStateRow>('refresh_state').insert({
|
||||
entity_id: unstitchedId,
|
||||
entity_ref: 'component:default/unstitched',
|
||||
unprocessed_entity: '{}',
|
||||
errors: '[]',
|
||||
next_update_at: '2031-01-01 23:00:00',
|
||||
last_discovery_at: '2021-04-01 13:37:00',
|
||||
});
|
||||
await knex<DbFinalEntitiesRow>('final_entities').insert({
|
||||
entity_id: unstitchedId,
|
||||
entity_ref: 'component:default/unstitched',
|
||||
hash: '',
|
||||
});
|
||||
await knex<DbSearchRow>('search').insert([
|
||||
{
|
||||
entity_id: unstitchedId,
|
||||
key: 'kind',
|
||||
value: 'component',
|
||||
original_value: 'Component',
|
||||
},
|
||||
{
|
||||
entity_id: unstitchedId,
|
||||
key: 'metadata.name',
|
||||
value: 'unstitched',
|
||||
original_value: 'unstitched',
|
||||
},
|
||||
]);
|
||||
|
||||
const catalog = new DefaultEntitiesCatalog({
|
||||
database: knex,
|
||||
logger: mockServices.logger.mock(),
|
||||
stitcher,
|
||||
});
|
||||
|
||||
// With filter: unstitched entity should be excluded because the
|
||||
// inner entityIdSubquery requires final_entity IS NOT NULL
|
||||
await expect(
|
||||
catalog.facets({
|
||||
facets: ['metadata.name'],
|
||||
filter: { key: 'kind', values: ['component'] },
|
||||
credentials: mockCredentials.none(),
|
||||
}),
|
||||
).resolves.toEqual({
|
||||
facets: {
|
||||
'metadata.name': [{ value: 'stitched', count: 1 }],
|
||||
},
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it.each(databases.eachSupportedId())(
|
||||
'filters with a predicate query, %p',
|
||||
async databaseId => {
|
||||
const catalog = await setupFacetsCatalog(databaseId, [
|
||||
{
|
||||
apiVersion: 'a',
|
||||
kind: 'Component',
|
||||
metadata: { name: 'one' },
|
||||
spec: { type: 'service' },
|
||||
},
|
||||
{
|
||||
apiVersion: 'a',
|
||||
kind: 'Component',
|
||||
metadata: { name: 'two' },
|
||||
spec: { type: 'library' },
|
||||
},
|
||||
{
|
||||
apiVersion: 'a',
|
||||
kind: 'API',
|
||||
metadata: { name: 'three' },
|
||||
spec: { type: 'openapi' },
|
||||
},
|
||||
]);
|
||||
|
||||
await expect(
|
||||
catalog.facets({
|
||||
facets: ['spec.type'],
|
||||
query: { kind: 'component' },
|
||||
credentials: mockCredentials.none(),
|
||||
}),
|
||||
).resolves.toEqual({
|
||||
facets: {
|
||||
'spec.type': [
|
||||
{ value: 'library', count: 1 },
|
||||
{ value: 'service', count: 1 },
|
||||
],
|
||||
},
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it.each(databases.eachSupportedId())(
|
||||
'filters with a predicate query using $in, %p',
|
||||
async databaseId => {
|
||||
const catalog = await setupFacetsCatalog(databaseId, [
|
||||
{
|
||||
apiVersion: 'a',
|
||||
kind: 'Component',
|
||||
metadata: { name: 'one' },
|
||||
spec: { type: 'service' },
|
||||
},
|
||||
{
|
||||
apiVersion: 'a',
|
||||
kind: 'API',
|
||||
metadata: { name: 'two' },
|
||||
spec: { type: 'openapi' },
|
||||
},
|
||||
{
|
||||
apiVersion: 'a',
|
||||
kind: 'System',
|
||||
metadata: { name: 'three' },
|
||||
spec: {},
|
||||
},
|
||||
]);
|
||||
|
||||
await expect(
|
||||
catalog.facets({
|
||||
facets: ['kind'],
|
||||
query: { kind: { $in: ['component', 'api'] } },
|
||||
credentials: mockCredentials.none(),
|
||||
}),
|
||||
).resolves.toEqual({
|
||||
facets: {
|
||||
kind: [
|
||||
{ value: 'API', count: 1 },
|
||||
{ value: 'Component', count: 1 },
|
||||
],
|
||||
},
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it.each(databases.eachSupportedId())(
|
||||
'filters with compound allOf filter, %p',
|
||||
async databaseId => {
|
||||
const catalog = await setupFacetsCatalog(databaseId, [
|
||||
{
|
||||
apiVersion: 'a',
|
||||
kind: 'Component',
|
||||
metadata: { name: 'one' },
|
||||
spec: { type: 'service' },
|
||||
},
|
||||
{
|
||||
apiVersion: 'a',
|
||||
kind: 'Component',
|
||||
metadata: { name: 'two' },
|
||||
spec: { type: 'library' },
|
||||
},
|
||||
{
|
||||
apiVersion: 'a',
|
||||
kind: 'API',
|
||||
metadata: { name: 'three' },
|
||||
spec: { type: 'openapi' },
|
||||
},
|
||||
]);
|
||||
|
||||
await expect(
|
||||
catalog.facets({
|
||||
facets: ['metadata.name'],
|
||||
filter: {
|
||||
allOf: [
|
||||
{ key: 'kind', values: ['component'] },
|
||||
{ key: 'spec.type', values: ['service'] },
|
||||
],
|
||||
},
|
||||
credentials: mockCredentials.none(),
|
||||
}),
|
||||
).resolves.toEqual({
|
||||
facets: {
|
||||
'metadata.name': [{ value: 'one', count: 1 }],
|
||||
},
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it.each(databases.eachSupportedId())(
|
||||
'filters with compound anyOf filter, %p',
|
||||
async databaseId => {
|
||||
const catalog = await setupFacetsCatalog(databaseId, [
|
||||
{
|
||||
apiVersion: 'a',
|
||||
kind: 'Component',
|
||||
metadata: { name: 'one' },
|
||||
spec: { type: 'service' },
|
||||
},
|
||||
{
|
||||
apiVersion: 'a',
|
||||
kind: 'API',
|
||||
metadata: { name: 'two' },
|
||||
spec: { type: 'openapi' },
|
||||
},
|
||||
{
|
||||
apiVersion: 'a',
|
||||
kind: 'System',
|
||||
metadata: { name: 'three' },
|
||||
spec: {},
|
||||
},
|
||||
]);
|
||||
|
||||
await expect(
|
||||
catalog.facets({
|
||||
facets: ['metadata.name'],
|
||||
filter: {
|
||||
anyOf: [
|
||||
{ key: 'kind', values: ['component'] },
|
||||
{ key: 'kind', values: ['api'] },
|
||||
],
|
||||
},
|
||||
credentials: mockCredentials.none(),
|
||||
}),
|
||||
).resolves.toEqual({
|
||||
facets: {
|
||||
'metadata.name': [
|
||||
{ value: 'one', count: 1 },
|
||||
]),
|
||||
{ value: 'two', count: 1 },
|
||||
],
|
||||
},
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
it.each(databases.eachSupportedId())(
|
||||
'filters with both filter and query combined, %p',
|
||||
async databaseId => {
|
||||
const catalog = await setupFacetsCatalog(databaseId, [
|
||||
{
|
||||
apiVersion: 'a',
|
||||
kind: 'Component',
|
||||
metadata: { name: 'one' },
|
||||
spec: { type: 'service' },
|
||||
},
|
||||
{
|
||||
apiVersion: 'a',
|
||||
kind: 'Component',
|
||||
metadata: { name: 'two' },
|
||||
spec: { type: 'library' },
|
||||
},
|
||||
{
|
||||
apiVersion: 'a',
|
||||
kind: 'API',
|
||||
metadata: { name: 'three' },
|
||||
spec: { type: 'openapi' },
|
||||
},
|
||||
]);
|
||||
|
||||
await expect(
|
||||
catalog.facets({
|
||||
facets: ['spec.type'],
|
||||
filter: { key: 'kind', values: ['component'] },
|
||||
query: { 'metadata.name': 'one' },
|
||||
credentials: mockCredentials.none(),
|
||||
}),
|
||||
).resolves.toEqual({
|
||||
facets: {
|
||||
'spec.type': [{ value: 'service', count: 1 }],
|
||||
},
|
||||
});
|
||||
},
|
||||
|
||||
@@ -690,13 +690,24 @@ export class DefaultEntitiesCatalog implements EntitiesCatalog {
|
||||
.groupBy(['search.key', 'search.original_value']);
|
||||
|
||||
if (request.filter || request.query) {
|
||||
// Build a subquery that finds matching entity IDs via
|
||||
// final_entities, so that the EXISTS-based filters correlate
|
||||
// against one-row-per-entity rather than the much larger search
|
||||
// table. The whereNotNull guard on final_entity excludes
|
||||
// not-yet-stitched (or future tombstoned) entities.
|
||||
const entityIdSubquery = this.database('final_entities')
|
||||
.select('final_entities.entity_id')
|
||||
.whereNotNull('final_entities.final_entity');
|
||||
|
||||
applyEntityFilterToQuery({
|
||||
filter: request.filter,
|
||||
query: request.query,
|
||||
targetQuery: query,
|
||||
onEntityIdField: 'search.entity_id',
|
||||
targetQuery: entityIdSubquery,
|
||||
onEntityIdField: 'final_entities.entity_id',
|
||||
knex: this.database,
|
||||
});
|
||||
|
||||
query.whereIn('search.entity_id', entityIdSubquery);
|
||||
}
|
||||
|
||||
const rows = await query;
|
||||
|
||||
Reference in New Issue
Block a user