mirror of
https://github.com/element-hq/synapse.git
synced 2026-08-21 16:50:21 +00:00
Unblock the stream_ordering bigint migration on PostgreSQL.
A Postgres database older than schema version 60 could not be upgraded,
because the `stream_ordering` -> bigint migration deadlocked with the foreign
keys that delta 74/03 added on `events.stream_ordering`:
* delta 79/04 tried to repoint those foreign keys at `stream_ordering2`, but
a foreign key needs a unique index on the referenced column and that index
is built by the `index_stream_ordering2` background update — which cannot
have run, since every delta is applied before any background update. It
failed with `InvalidForeignKey: there is no unique constraint matching
given keys for referenced table "events"`.
* without delta 79/04, the `replace_stream_ordering_column` background update
failed instead, because Postgres will not drop a column that a foreign key
depends on.
Move the responsibility into `replace_stream_ordering_column`, which now drops
the foreign keys before the column swap and recreates them from their saved
definitions afterwards, by which point `stream_ordering2` has been renamed into
place. The foreign keys are looked up rather than hardcoded: delta 79/04 only
knew about the three on the membership tables, and the sliding sync tables have
since added two more.
Delta 79/04 becomes a no-op, as it can no longer have any work to do.
Signed-off-by: Olivier 'reivilibre <oliverw@matrix.org>
This commit is contained in:
committed by
Olivier 'reivilibre
parent
5bc1195e6f
commit
471ab8b9bc
@@ -0,0 +1 @@
|
||||
Fix the `stream_ordering` bigint migration being unable to complete on PostgreSQL databases created before schema version 60.
|
||||
@@ -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(
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user