Skip to content

Conversation

@easyCZ
Copy link
Member

@easyCZ easyCZ commented Sep 6, 2022

Description

Adds missing indices to avoid full table scan.

Internal Context

Related Issue(s)

Relates to #13060

How to test

Release Notes

NONE

Documentation

Werft options:

  • /werft with-preview

/hold to run this manually as it is a long-running migration.

@easyCZ easyCZ requested a review from a team September 6, 2022 19:17
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Sep 6, 2022
@easyCZ
Copy link
Member Author

easyCZ commented Sep 7, 2022

/werft run

👍 started the job as gitpod-build-mp-db-workspace-instance-time-idx.1
(with .werft/ from main)


const startedTimeIndex = "IDX_workspace_instance__started_time";
if (!(await indexExists(queryRunner, TABLE_NAME, startedTimeIndex))) {
await queryRunner.query(`CREATE INDEX ${startedTimeIndex} ON ${TABLE_NAME} (startedTime)`);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the magical , ALGORITHM=INPLACE, LOCK=NONE statement here?

@geropl

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

magical

Link to MySQL docs: https://dev.mysql.com/doc/refman/5.7/en/innodb-online-ddl-operations.html

tl;dr: no, adding 2ndary indexes is a in-place operation that does not lock.

This is more relevant for adding primary keys, or re-naming (cmp. docs).

@easyCZ
Copy link
Member Author

easyCZ commented Sep 19, 2022

Removing auto-link to close #13060, just landing the PR doesn't actually fix the issue so we need to track manually.

@geropl
Copy link
Member

geropl commented Sep 29, 2022

@easyCZ We could merge this one now, right? 🤔

@easyCZ
Copy link
Member Author

easyCZ commented Sep 29, 2022

@geropl Yep! I'll just create a new migration and move the contents of this one to ensure the TS is most recent. I'm unsure exactly how the migrations handle cases where the migrations in the past is inserted.

@geropl
Copy link
Member

geropl commented Sep 29, 2022

I'm unsure exactly how the migrations handle cases where the migrations in the past is inserted.

It handles it well, e.g. does identity by name, and performs all which have not yet been executed. 👍

But maintaining order is nice as well 🙃

@easyCZ easyCZ force-pushed the mp/db-workspace-instance-time-idx branch from 155b312 to 3e92f33 Compare September 29, 2022 08:52
@easyCZ
Copy link
Member Author

easyCZ commented Sep 29, 2022

@geropl Thanks. I've moved the file anyway just to keep it clean.

/unhold

@roboquat roboquat merged commit cd47261 into main Sep 29, 2022
@roboquat roboquat deleted the mp/db-workspace-instance-time-idx branch September 29, 2022 09:01
@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Sep 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note-none size/S team: webapp Issue belongs to the WebApp team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants