chore(database): indices for columns used in 'where' and 'order by' clauses #3880

iainsproat · 2025-01-23T18:34:26Z

Description & motivation

We analyzed a running database and identified significant tables where indices were involved in a low proportion of queries. This PR adds indices to select table columns to aim to improve the index hit ratio and overall performance.

Existing indices can be found with:

select *
from pg_indexes
WHERE schemaname='public'
AND tablename='automation_triggers';

Changes:

migration to add indices for columns in select tables

To-do before merge:

Screenshots:

Validation of changes:

Checklist:

My pull request follows the guidelines in the Contributing guide?
My pull request does not duplicate any other open Pull Requests for the same update/change?
My commits are related to the pull request and do not amend unrelated code or documentation.
My code follows a similar style to existing code.
I have added appropriate tests.
I have updated or added relevant documentation.

References

…lauses

linear · 2025-01-23T18:34:29Z

WEB-2499 Postgres indices should be used >95% of the time for any table > 10000 rows

iainsproat · 2025-01-24T09:33:03Z

knex doesn't seem to add Indices concurrently knex/knex#3913
Need to amend creation of index to use a raw sql, as so: https://github.com/backstage/backstage/pull/22594/files

fabis94 · 2025-01-24T09:51:48Z

packages/server/modules/core/migrations/20250123162109_required_indices.ts

+  })
+  await knex.schema.alterTable('commits', (table) => {
+    // table.index('id') // index should already be available due to this being the primary key
+    table.index('author')


do these really need to be separate indices, instead of composite ones? it was at least the case in MySQL that if you had an index like "x, y, z", then it would be used even if you filter by only "x" or "x" and "y", as long as the order is the same (so it wouldn't work when just filtering by "z")

important to think about this, cause they do take up space

This was the best explanation I found on the subject: https://stackoverflow.com/questions/11352056/postgresql-composite-primary-key#comment113601692_11352543

That's even better than what I expected then, I didn't think there would be a speedup on just filtering "y" at all. But my point is that we need to look at what the actual filter conditions are, and if "y" is usually used together with "x", then there's no need to make "y" have its own index

Just having X and Y as separate indices could even be worse than having a "X, Y" index, if both are filtered together. It's just going to use the X or the Y index, where "X, Y" would be the better one

getPaginatedProjectModelsBaseQueryFactory, getModelTreeItemsFilteredBaseQueryFactory, getUserAuthoredCommitCountsFactory query on the author column alone

fabis94 · 2025-01-24T09:52:37Z

surprising that so many tables don't have appropriate indices. we need to be more attentive to new table creating migrations, indices should be planned out from the very beginning, if possible

iainsproat · 2025-01-24T09:56:51Z

surprising that so many tables don't have appropriate indices. we need to be more attentive to new table creating migrations, indices should be planned out from the very beginning, if possible

There's a lot more in the list linked in the Linear issue, but those were much smaller tables (at the moment). I'll do the work of sweeping them up in some follow up PRs.

…d-95-of-the-time-for-any-table

iainsproat

Reviewed all the indices, and there are indeed some candidates for being composite indices instead. Will revise.

iainsproat · 2025-01-24T11:51:27Z

packages/server/modules/core/migrations/20250123162109_required_indices.ts

+export async function up(knex: Knex): Promise<void> {
+  await knex.schema.alterTable('stream_commits', (table) => {
+    // table.index('streamId') // index should already be available due to this being the first item in the composite primary key
+    table.index('commitId')


getCommitStreamsFactory & markCommitStreamUpdatedFactory has a 'where' statement on this column alone

iainsproat · 2025-01-24T11:51:45Z

packages/server/modules/core/migrations/20250123162109_required_indices.ts

+  })
+  await knex.schema.alterTable('branch_commits', (table) => {
+    // table.index('branchId') // index should already be available due to this being the first item in the composite primary key
+    table.index('commitId')


markCommitBranchUpdatedFactory, moveCommitsToBranchFactory, getCommitBranchesFactory, switchCommitBranchFactory has a 'where' statement on this column alone

iainsproat · 2025-01-24T11:57:09Z

packages/server/modules/core/migrations/20250123162109_required_indices.ts

+  })
+  await knex.schema.alterTable('refresh_tokens', (table) => {
+    // table.index('id') // index should already be available due to this being the primary key
+    table.index('userId')


deleteExistingAuthTokensFactory has a 'where' statement on this column alone. (Though I'm confused as to what the statement in that factory is actually doing, it appears to query and then not use the result)

iainsproat · 2025-01-24T12:00:13Z

packages/server/modules/core/migrations/20250123162109_required_indices.ts

+  await knex.schema.alterTable('commits', (table) => {
+    // table.index('id') // index should already be available due to this being the primary key
+    table.index('author')
+    table.index('sourceApplication')


getPaginatedProjectModelsBaseQueryFactory, getModelTreeItemsFilteredBaseQueryFactory, getStreamsSourceAppsFactory query on the sourceApplication column alone

iainsproat · 2025-01-24T12:00:16Z

packages/server/modules/core/migrations/20250123162109_required_indices.ts

+  })
+  await knex.schema.alterTable('commits', (table) => {
+    // table.index('id') // index should already be available due to this being the primary key
+    table.index('author')


getPaginatedProjectModelsBaseQueryFactory, getModelTreeItemsFilteredBaseQueryFactory, getUserAuthoredCommitCountsFactory query on the author column alone

iainsproat · 2025-01-24T16:14:00Z

packages/server/modules/core/migrations/20250123162109_required_indices.ts

+    //table.index('id') // index should already be available due to this being the primary key
+    // table.index('gendoGenerationId') // already explicitly indexed
+    table.index('versionId')
+    table.index('createdAt') // used in an ORDER BY clause. https://www.postgresql.org/docs/current/indexes-ordering.html


getLatestVersionRenderRequestsFactory and getVersionRenderRequestFactory include this column in an ORDER BY. It is always used alongside versionId, so could be a composite index.

iainsproat · 2025-01-24T16:14:54Z

packages/server/modules/core/migrations/20250123162109_required_indices.ts

+  })
+  await knex.schema.alterTable('pwdreset_tokens', (table) => {
+    // table.index('id') // index should already be available due to this being the primary key
+    table.index('email')


baseQueryFactory includes this column, and is used alone in deleteTokensFactory

iainsproat · 2025-01-24T16:15:31Z

packages/server/modules/core/migrations/20250123162109_required_indices.ts

+  await knex.schema.alterTable('pwdreset_tokens', (table) => {
+    // table.index('id') // index should already be available due to this being the primary key
+    table.index('email')
+    table.index('createdAt')


getPendingTokenFactory includes this column, but alongside email so could be the secondary in a composite index.

iainsproat · 2025-01-24T16:18:11Z

packages/server/modules/core/migrations/20250123162109_required_indices.ts

+  })
+  await knex.schema.alterTable('automation_triggers', (table) => {
+    // table.index('automationRevisionId') // index should already be available due to this being the first item in the composite primary key
+    table.index('triggerType')


getActiveTriggerDefinitionsFactory includes this column, alongside triggeringId
getAutomationTriggerDefinitionsFactory includes this column, alongside automationRevisionId.

iainsproat · 2025-01-24T16:18:27Z

packages/server/modules/core/migrations/20250123162109_required_indices.ts

+  await knex.schema.alterTable('automation_triggers', (table) => {
+    // table.index('automationRevisionId') // index should already be available due to this being the first item in the composite primary key
+    table.index('triggerType')
+    table.index('triggeringId')


getActiveTriggerDefinitionsFactory includes this column, but only alongside triggerType

…d-95-of-the-time-for-any-table

chore(database): indices for columns used in 'where' and 'order by' c…

872b524

…lauses

iainsproat changed the title ~~chore(database): indices for columns used in 'where' and 'order by' c…~~ chore(database): indices for columns used in 'where' and 'order by' clauses Jan 23, 2025

iainsproat marked this pull request as ready for review January 24, 2025 09:30

iainsproat requested review from alemagio, gjedlicska and fabis94 January 24, 2025 09:30

iainsproat marked this pull request as draft January 24, 2025 09:35

iainsproat removed request for fabis94, alemagio and gjedlicska January 24, 2025 09:35

fabis94 reviewed Jan 24, 2025

View reviewed changes

Merge branch 'main' into iain/web-2499-postgres-indices-should-be-use…

4eb88eb

…d-95-of-the-time-for-any-table

iainsproat commented Jan 24, 2025

View reviewed changes

iainsproat added 4 commits January 24, 2025 17:38

create indices concurrently, and create composite indices

54a0acb

Merge branch 'main' into iain/web-2499-postgres-indices-should-be-use…

89efb5d

…d-95-of-the-time-for-any-table

fix: migration cannot be run within a transaction

10602a0

Merge branch 'main' into iain/web-2499-postgres-indices-should-be-use…

885df00

…d-95-of-the-time-for-any-table

iainsproat marked this pull request as ready for review February 5, 2025 09:39

iainsproat requested review from fabis94 and gjedlicska February 5, 2025 12:41

iainsproat marked this pull request as draft February 6, 2025 11:06

iainsproat added 3 commits February 6, 2025 11:06

Merge branch 'main' into iain/web-2499-postgres-indices-should-be-use…

4bf140d

…d-95-of-the-time-for-any-table

revert irrelevant and unnecessary change

f033a79

use knex helpers even if index is built sequentially

7ccbcf1

iainsproat marked this pull request as ready for review February 6, 2025 16:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(database): indices for columns used in 'where' and 'order by' clauses #3880

chore(database): indices for columns used in 'where' and 'order by' clauses #3880

iainsproat commented Jan 23, 2025 •

edited

Loading

linear bot commented Jan 23, 2025

iainsproat commented Jan 24, 2025 •

edited

Loading

fabis94 Jan 24, 2025

fabis94 Jan 24, 2025

iainsproat Jan 24, 2025

fabis94 Jan 24, 2025 •

edited

Loading

fabis94 Jan 24, 2025

iainsproat Jan 24, 2025

fabis94 commented Jan 24, 2025

iainsproat commented Jan 24, 2025

iainsproat left a comment

iainsproat Jan 24, 2025

iainsproat Jan 24, 2025

iainsproat Jan 24, 2025

iainsproat Jan 24, 2025

iainsproat Jan 24, 2025

iainsproat Jan 24, 2025

iainsproat Jan 24, 2025

iainsproat Jan 24, 2025

iainsproat Jan 24, 2025

iainsproat Jan 24, 2025

chore(database): indices for columns used in 'where' and 'order by' clauses #3880

Are you sure you want to change the base?

chore(database): indices for columns used in 'where' and 'order by' clauses #3880

Conversation

iainsproat commented Jan 23, 2025 • edited Loading

Description & motivation

Changes:

To-do before merge:

Screenshots:

Validation of changes:

Checklist:

References

linear bot commented Jan 23, 2025

iainsproat commented Jan 24, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabis94 Jan 24, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabis94 commented Jan 24, 2025

iainsproat commented Jan 24, 2025

iainsproat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iainsproat commented Jan 23, 2025 •

edited

Loading

iainsproat commented Jan 24, 2025 •

edited

Loading

fabis94 Jan 24, 2025 •

edited

Loading