restarting multiple workers at once risks applying database migrations multiple times

[richvdh]

each worker independently checks if the schema is up-to-date and applies the migrations if not. At best this fails with exceptions; at worst it could result in data corruption

[richvdh]

I thought we used to have a thing that stopped migrations from running on anything other than the main process. In any case empirically it doesn't work any more.

[richvdh]

if the solution to this ends up being any sort of database-level locking, it would be nice to consider #6467 at the same time.

[erikjohnston]

I think we should be able to just not run the prepare database step if we're on not-master?

[clokep]

I think we should be able to just not run the prepare database step if we're on not-master?

Is there a risk of those workers starting and then erroring because the migrations haven't finished yet?

[erikjohnston]

Hmm, true. We (matrix.org) do try and ensure master starts up first, but I'm not sure that's documented anywhere

[richvdh]

Is there a risk of those workers starting and then erroring because the migrations haven't finished yet?

that would be preferable to the current situation imho. But I also wouldn't object to the other workers going into a sleep/check loop until the db got upgraded.

Hmm, true. We (matrix.org) do try and ensure master starts up first, but I'm not sure that's documented anywhere

I'm also not convinced it's true that we wait for the master to restart before we restart the workers on the other server.

[erikjohnston]

Hmm, true. We (matrix.org) do try and ensure master starts up first, but I'm not sure that's documented anywhere

I'm also not convinced it's true that we wait for the master to restart before we restart the workers on the other server.

We have historically said that we restart the server with master on before the other servers when upgrading. That advice has probably gotten a bit lost over time too

[richvdh]

@reivilibre suggests maybe we could lock the schema version table while we do the migration

[erikjohnston]

applied_schema_deltas has a unique index so applying the same delta concurrently should cause one of the transactions to fail (and thus rollback the delta changes).

[richvdh]

applied_schema_deltas has a unique index so applying the same delta concurrently should cause one of the transactions to fail (and thus rollback the delta changes).

well this might be true if we actually made the upgrades respect transactions correctly (cf #6467)

[erikjohnston]

Well that feels like a prerequisite to this if we're not correctly wrapping things in transactions?

[richvdh]

depends, but sure it would be a good thing to fix :)

[richvdh]

Looking at this more closely, because psycopg starts a new transaction for every statement, concurrent attempts to upgrade the database will probably fail for the reason Erik gave previously.

I still think it's confusing, though.

[richvdh]

fixed by #8266