Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

slow schema migrations #10226

Closed
richvdh opened this issue Jun 22, 2021 · 7 comments · Fixed by #10227
Closed

slow schema migrations #10226

richvdh opened this issue Jun 22, 2021 · 7 comments · Fixed by #10227
Assignees
Labels
X-Release-Blocker Must be resolved before making a release

Comments

@richvdh
Copy link
Member

richvdh commented Jun 22, 2021

The following migrations were very slow when we rolled them out to matrix.org recently:

2021-06-16 15:05:21.834 UTC [matrix main] LOG:  statement: ALTER TABLE room_stats_current ADD COLUMN knocked_members INT NOT NULL DEFAULT '0'
2021-06-16 15:06:53.980 UTC [matrix main] LOG:  duration: 92145.243 ms

2021-06-16 15:07:05.682 UTC [matrix main] LOG:  statement: ALTER TABLE room_stats_historical ADD COLUMN knocked_members BIGINT NOT NULL DEFAULT '0'
2021-06-16 15:08:22.587 UTC [matrix main] LOG:  duration: 76904.199 ms

2021-06-16 15:08:22.685 UTC [matrix main] LOG:  statement: ALTER TABLE room_stats_current ADD COLUMN knocked_members INT NOT NULL DEFAULT '0'
2021-06-16 15:08:33.102 UTC [matrix main] LOG:  duration: 10416.733 ms

2021-06-16 15:08:33.103 UTC [matrix main] LOG:  statement: ALTER TABLE room_stats_historical ADD COLUMN knocked_members BIGINT NOT NULL DEFAULT '0'
2021-06-16 15:09:41.457 UTC [matrix main] LOG:  duration: 68354.724 ms

(yes they ran twice. We aborted synapse and they got rolled back the first time.)

@richvdh
Copy link
Member Author

richvdh commented Jun 22, 2021

These were added in #6739

@richvdh richvdh added the X-Release-Blocker Must be resolved before making a release label Jun 22, 2021
@richvdh
Copy link
Member Author

richvdh commented Jun 22, 2021

columns with DEFAULT need a table rewrite.

@erikjohnston
Copy link
Member

I guess the fix here is to make it nullable and either a) add a bg update to rewrite it with zeros, or b) have the code handle null meaning 0?

@babolivier
Copy link
Contributor

babolivier commented Jun 22, 2021

If I remember correctly situations like this one happened in the past and option b) was generally what we've gone for.

@richvdh
Copy link
Member Author

richvdh commented Jun 22, 2021

given that you have to do (b) anyway even if you're doing (a) (because you have to cope with the data before the bg update runs), yes, we should do (b).

@erikjohnston
Copy link
Member

I'll quickly throw up a PR to fix this.

@richvdh
Copy link
Member Author

richvdh commented Jun 22, 2021

I'm going to reopen this again to remind us to fix up matrix.org

@richvdh richvdh reopened this Jun 22, 2021
@richvdh richvdh closed this as completed Jun 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
X-Release-Blocker Must be resolved before making a release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants