diff --git a/changelog.d/20101.bugfix b/changelog.d/20101.bugfix new file mode 100644 index 0000000000..63c2c22bda --- /dev/null +++ b/changelog.d/20101.bugfix @@ -0,0 +1 @@ +Fix the `stream_ordering` bigint migration being unable to complete on PostgreSQL databases created before schema version 60. diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index 9a11f9b9bb..9489233280 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -90,6 +90,28 @@ _REPLACE_STREAM_ORDERING_SQL_COMMANDS = ( "ALTER INDEX events_ts2 RENAME TO events_ts", ) +# Postgres refuses to drop a column that a foreign key depends on, so any foreign key +# referencing `events.stream_ordering` has to be dropped before the swap above and +# recreated afterwards (by which point `stream_ordering2` has been renamed into place, +# so the saved definitions still name the right column). +# +# We look these up rather than hardcoding them, because the set has grown over time: +# delta 74/03 added three on the membership tables, and the sliding sync tables later +# added two more. +# +# Foreign keys which already reference `stream_ordering2` — because delta +# 79/04 repointed them — are deliberately not matched here: the rename carries +# those over on its own. +_SELECT_STREAM_ORDERING_FOREIGN_KEYS_SQL = """ + SELECT c.conrelid::regclass::text, quote_ident(c.conname), pg_get_constraintdef(c.oid) + FROM pg_constraint c + JOIN pg_attribute a + ON a.attrelid = c.confrelid AND a.attnum = ANY (c.confkey) + WHERE c.contype = 'f' + AND c.confrelid = 'events'::regclass + AND a.attname = 'stream_ordering' +""" + @attr.s(slots=True, frozen=True, auto_attribs=True) class _CalculateChainCover: @@ -1476,10 +1498,31 @@ class EventsBackgroundUpdatesStore( """Drop the old 'stream_ordering' column and rename 'stream_ordering2' into its place.""" def process(txn: Cursor) -> None: + txn.execute(_SELECT_STREAM_ORDERING_FOREIGN_KEYS_SQL) + foreign_keys = txn.fetchall() + + for table, constraint, _definition in foreign_keys: + logger.info( + "dropping %s on %s so that stream_ordering can be replaced", + constraint, + table, + ) + txn.execute(f"ALTER TABLE {table} DROP CONSTRAINT {constraint}") + for sql in _REPLACE_STREAM_ORDERING_SQL_COMMANDS: logger.info("completing stream_ordering migration: %s", sql) txn.execute(sql) + for table, constraint, definition in foreign_keys: + logger.info("restoring %s on %s", constraint, table) + # Constraints which were not `NOT VALID` are validated as they are + # added, which scans the table. That is acceptable here: a database + # still running this migration predates every table that holds such a + # constraint, so those tables are empty at this point. + txn.execute( + f"ALTER TABLE {table} ADD CONSTRAINT {constraint} {definition}" + ) + # ANALYZE the new column to build stats on it, to encourage PostgreSQL to use the # indexes on it. await self.db_pool.runInteraction( diff --git a/synapse/storage/schema/main/delta/79/04_mitigate_stream_ordering_update_race.py b/synapse/storage/schema/main/delta/79/04_mitigate_stream_ordering_update_race.py index ae63585847..6cd555124a 100644 --- a/synapse/storage/schema/main/delta/79/04_mitigate_stream_ordering_update_race.py +++ b/synapse/storage/schema/main/delta/79/04_mitigate_stream_ordering_update_race.py @@ -8,7 +8,7 @@ from synapse.storage.database import LoggingTransaction -from synapse.storage.engines import BaseDatabaseEngine, PostgresEngine +from synapse.storage.engines import BaseDatabaseEngine def run_create( @@ -16,55 +16,24 @@ def run_create( database_engine: BaseDatabaseEngine, ) -> None: """ - An attempt to mitigate a painful race between foreground and background updates - touching the `stream_ordering` column of the events table. More info can be found + This delta used to repoint the `event_stream_ordering_fkey` foreign keys added by + delta 74/03 at `events.stream_ordering2`, so that they would survive the column + swap performed by the `replace_stream_ordering_column` background update. It was an + attempt to mitigate a painful race between foreground and background updates + touching the `stream_ordering` column of the events table; more info can be found at https://github.com/matrix-org/synapse/issues/15677. + + It could never do so successfully, because the unique index that a foreign key on + `stream_ordering2` requires is itself built by a background update + (`index_stream_ordering2`), and every delta runs before any background update does. + Whenever this delta had work to do it therefore failed with + + psycopg2.errors.InvalidForeignKey: there is no unique constraint matching + given keys for referenced table "events" + + which left any Postgres database older than schema version 60 unable to upgrade. + + `replace_stream_ordering_column` now drops and recreates these foreign keys itself, + which works no matter which column they currently reference, so there is nothing + left for this delta to do. """ - - # technically the bg update we're concerned with below should only have been added in - # postgres but it doesn't hurt to be extra careful - if isinstance(database_engine, PostgresEngine): - select_sql = """ - SELECT 1 FROM background_updates - WHERE update_name = 'replace_stream_ordering_column' - """ - cur.execute(select_sql) - res = cur.fetchone() - - # if the background update `replace_stream_ordering_column` is still pending, we need - # to drop the indexes added in 7403, and re-add them to the column `stream_ordering2` - # with the idea that they will be preserved when the column is renamed `stream_ordering` - # after the background update has finished - if res: - drop_cse_sql = """ - ALTER TABLE current_state_events DROP CONSTRAINT IF EXISTS event_stream_ordering_fkey - """ - cur.execute(drop_cse_sql) - - drop_lcm_sql = """ - ALTER TABLE local_current_membership DROP CONSTRAINT IF EXISTS event_stream_ordering_fkey - """ - cur.execute(drop_lcm_sql) - - drop_rm_sql = """ - ALTER TABLE room_memberships DROP CONSTRAINT IF EXISTS event_stream_ordering_fkey - """ - cur.execute(drop_rm_sql) - - add_cse_sql = """ - ALTER TABLE current_state_events ADD CONSTRAINT event_stream_ordering_fkey - FOREIGN KEY (event_stream_ordering) REFERENCES events(stream_ordering2) NOT VALID; - """ - cur.execute(add_cse_sql) - - add_lcm_sql = """ - ALTER TABLE local_current_membership ADD CONSTRAINT event_stream_ordering_fkey - FOREIGN KEY (event_stream_ordering) REFERENCES events(stream_ordering2) NOT VALID; - """ - cur.execute(add_lcm_sql) - - add_rm_sql = """ - ALTER TABLE room_memberships ADD CONSTRAINT event_stream_ordering_fkey - FOREIGN KEY (event_stream_ordering) REFERENCES events(stream_ordering2) NOT VALID; - """ - cur.execute(add_rm_sql)