From 1d2875f357f369a8b02ecece9b61df5cd628bd68 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 6 Jul 2021 16:32:02 +0100 Subject: [PATCH 1/8] ANALYZE new stream ordering column --- synapse/storage/databases/main/events_bg_updates.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index 1c95c66648ca..fcab4be03afc 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -32,6 +32,8 @@ _REPLACE_STREAM_ORDERING_SQL_COMMANDS = ( # there should be no leftover rows without a stream_ordering2, but just in case... "UPDATE events SET stream_ordering2 = stream_ordering WHERE stream_ordering2 IS NULL", + # Build stats on the new column + "ANALYZE events(stream_ordering2)" # now we can drop the rule and switch the columns "DROP RULE populate_stream_ordering2 ON events", "ALTER TABLE events DROP COLUMN stream_ordering", From f6965f44c67ad1fe19ecef607f0de6d27f555a96 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 6 Jul 2021 16:34:28 +0100 Subject: [PATCH 2/8] Changelog --- changelog.d/10327.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/10327.bugfix diff --git a/changelog.d/10327.bugfix b/changelog.d/10327.bugfix new file mode 100644 index 000000000000..7ebda7cdc29c --- /dev/null +++ b/changelog.d/10327.bugfix @@ -0,0 +1 @@ +Fix a long-standing bug where Synapse would return errors after 231 events were handled by the server. From 3d9d3bc32f716482317406c248452776305ecf6a Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 6 Jul 2021 16:36:27 +0100 Subject: [PATCH 3/8] Fix changelog name --- changelog.d/{10327.bugfix => 10326.bugfix} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{10327.bugfix => 10326.bugfix} (100%) diff --git a/changelog.d/10327.bugfix b/changelog.d/10326.bugfix similarity index 100% rename from changelog.d/10327.bugfix rename to changelog.d/10326.bugfix From a817acdf929faff926d87526c027de85ff6b7b4b Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 6 Jul 2021 16:42:26 +0100 Subject: [PATCH 4/8] Run in a separate transaction --- synapse/storage/databases/main/events_bg_updates.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index fcab4be03afc..59347ee91335 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -32,8 +32,6 @@ _REPLACE_STREAM_ORDERING_SQL_COMMANDS = ( # there should be no leftover rows without a stream_ordering2, but just in case... "UPDATE events SET stream_ordering2 = stream_ordering WHERE stream_ordering2 IS NULL", - # Build stats on the new column - "ANALYZE events(stream_ordering2)" # now we can drop the rule and switch the columns "DROP RULE populate_stream_ordering2 ON events", "ALTER TABLE events DROP COLUMN stream_ordering", @@ -1143,11 +1141,20 @@ async def _background_replace_stream_ordering_column( ) -> int: """Drop the old 'stream_ordering' column and rename 'stream_ordering2' into its place.""" + def analyze_new_column(txn: Cursor) -> None: + txn.execute("ANALYZE events(stream_ordering2)") + def process(txn: Cursor) -> None: for sql in _REPLACE_STREAM_ORDERING_SQL_COMMANDS: logger.info("completing stream_ordering migration: %s", sql) txn.execute(sql) + # ANALYZE the new column to build stats on it, to encourage PostgreSQL to use the + # indexes on it. + await self.db_pool.runInteraction( + "_background_analyze_new_stream_ordering_column", analyze_new_column + ) + await self.db_pool.runInteraction( "_background_replace_stream_ordering_column", process ) From 731778f4bcc6d3d4a4e1074200d2d84866551c36 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 6 Jul 2021 18:05:34 +0200 Subject: [PATCH 5/8] Update synapse/storage/databases/main/events_bg_updates.py Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- synapse/storage/databases/main/events_bg_updates.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index 59347ee91335..55d2298223ce 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -1151,9 +1151,7 @@ def process(txn: Cursor) -> None: # ANALYZE the new column to build stats on it, to encourage PostgreSQL to use the # indexes on it. - await self.db_pool.runInteraction( - "_background_analyze_new_stream_ordering_column", analyze_new_column - ) + await self.db_pool.execute("background_analyze_new_stream_ordering_column", None, "ANALYZE events(stream_ordering2)") await self.db_pool.runInteraction( "_background_replace_stream_ordering_column", process From a6fafb94064f1d5024b595a6358426a75bf63df0 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 6 Jul 2021 17:13:47 +0100 Subject: [PATCH 6/8] Lint --- synapse/storage/databases/main/events_bg_updates.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index 55d2298223ce..7672ac1c0f9f 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -1151,7 +1151,11 @@ def process(txn: Cursor) -> None: # ANALYZE the new column to build stats on it, to encourage PostgreSQL to use the # indexes on it. - await self.db_pool.execute("background_analyze_new_stream_ordering_column", None, "ANALYZE events(stream_ordering2)") + await self.db_pool.execute( + "background_analyze_new_stream_ordering_column", + None, + "ANALYZE events(stream_ordering2)", + ) await self.db_pool.runInteraction( "_background_replace_stream_ordering_column", process From c8ac366cebf60ad70b57c35a995dbf399a12317f Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 6 Jul 2021 18:26:20 +0100 Subject: [PATCH 7/8] remove useless function --- synapse/storage/databases/main/events_bg_updates.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index 7672ac1c0f9f..25ca5d2d883d 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -1141,9 +1141,6 @@ async def _background_replace_stream_ordering_column( ) -> int: """Drop the old 'stream_ordering' column and rename 'stream_ordering2' into its place.""" - def analyze_new_column(txn: Cursor) -> None: - txn.execute("ANALYZE events(stream_ordering2)") - def process(txn: Cursor) -> None: for sql in _REPLACE_STREAM_ORDERING_SQL_COMMANDS: logger.info("completing stream_ordering migration: %s", sql) From 849e7fd03bf5547eba9a621354ecba4c79b17841 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 7 Jul 2021 10:26:54 +0100 Subject: [PATCH 8/8] Prevent execute from calling fetchall --- synapse/storage/databases/main/events_bg_updates.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index 25ca5d2d883d..29f33bac55bc 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/synapse/storage/databases/main/events_bg_updates.py @@ -1148,9 +1148,11 @@ def process(txn: Cursor) -> None: # ANALYZE the new column to build stats on it, to encourage PostgreSQL to use the # indexes on it. + # We need to pass execute a dummy function to handle the txn's result otherwise + # it tries to call fetchall() on it and fails because there's no result to fetch. await self.db_pool.execute( "background_analyze_new_stream_ordering_column", - None, + lambda txn: None, "ANALYZE events(stream_ordering2)", )