From 86d463ba2465da0ae7393e895d8544f459c8f6b8 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 1 Apr 2021 14:00:46 +0100 Subject: [PATCH 1/3] Remove outdated constraint on remote_media_cache_thumbnails The `remote_media_cache_thumbnails_media_origin_media_id_thumbna_key` constraint is superceded by `remote_media_repository_thumbn_media_origin_id_width_height_met` (which adds `thumbnail_method` to the unique key). PR #7124 made an attempt to remove the old constraint, but got the name wrong, so it didn't work. Here we update the bg update and rerun it. Fixex #8649. --- changelog.d/9724.bugfix | 1 + .../databases/main/media_repository.py | 7 +++++- .../11drop_thumbnail_constraint.sql.postgres | 22 +++++++++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 changelog.d/9724.bugfix create mode 100644 synapse/storage/databases/main/schema/delta/59/11drop_thumbnail_constraint.sql.postgres diff --git a/changelog.d/9724.bugfix b/changelog.d/9724.bugfix new file mode 100644 index 000000000000..71283685c80d --- /dev/null +++ b/changelog.d/9724.bugfix @@ -0,0 +1 @@ +Fix longstanding bug which caused `duplicate key value violates unique constraint "remote_media_cache_thumbnails_media_origin_media_id_thumbna_key"` errors. diff --git a/synapse/storage/databases/main/media_repository.py b/synapse/storage/databases/main/media_repository.py index 4f3d19256233..5499bdbc688e 100644 --- a/synapse/storage/databases/main/media_repository.py +++ b/synapse/storage/databases/main/media_repository.py @@ -91,12 +91,17 @@ def __init__(self, database: DatabasePool, db_conn, hs): ) async def _drop_media_index_without_method(self, progress, batch_size): + """background update handler which removes the old constraints. + + Note that this is only run on postgres. + """ + def f(txn): txn.execute( "ALTER TABLE local_media_repository_thumbnails DROP CONSTRAINT IF EXISTS local_media_repository_thumbn_media_id_thumbnail_width_thum_key" ) txn.execute( - "ALTER TABLE remote_media_cache_thumbnails DROP CONSTRAINT IF EXISTS remote_media_repository_thumbn_media_id_thumbnail_width_thum_key" + "ALTER TABLE remote_media_cache_thumbnails DROP CONSTRAINT IF EXISTS remote_media_cache_thumbnails_media_origin_media_id_thumbna_key" ) await self.db_pool.runInteraction("drop_media_indices_without_method", f) diff --git a/synapse/storage/databases/main/schema/delta/59/11drop_thumbnail_constraint.sql.postgres b/synapse/storage/databases/main/schema/delta/59/11drop_thumbnail_constraint.sql.postgres new file mode 100644 index 000000000000..944f92d94266 --- /dev/null +++ b/synapse/storage/databases/main/schema/delta/59/11drop_thumbnail_constraint.sql.postgres @@ -0,0 +1,22 @@ +/* Copyright 2021 The Matrix.org Foundation C.I.C + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +-- drop old constraints on remote_media_cache_thumbnails +-- +-- This was originally part of 57.07, but it was done wrong, per +-- https://github.com/matrix-org/synapse/issues/8649, so we do it again. +INSERT INTO background_updates (ordering, update_name, progress_json, depends_on) VALUES + (5911, 'media_repository_drop_index_wo_method', '{}', 'remote_media_repository_thumbnails_method_idx'); + From 4dab842c44660a29125b7c6f5bac033cd446b39b Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 1 Apr 2021 14:13:36 +0100 Subject: [PATCH 2/3] fix newsfile name --- changelog.d/{9724.bugfix => 9725.bugfix} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{9724.bugfix => 9725.bugfix} (100%) diff --git a/changelog.d/9724.bugfix b/changelog.d/9725.bugfix similarity index 100% rename from changelog.d/9724.bugfix rename to changelog.d/9725.bugfix From 58a4d28b6553adaa933c4deacef2c7858ea21440 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 1 Apr 2021 17:37:27 +0100 Subject: [PATCH 3/3] I'm not allowed to re-run an old bg update :'( --- synapse/storage/databases/main/media_repository.py | 14 ++++++++++++-- .../59/11drop_thumbnail_constraint.sql.postgres | 2 +- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/synapse/storage/databases/main/media_repository.py b/synapse/storage/databases/main/media_repository.py index 5499bdbc688e..b7820ac7ff30 100644 --- a/synapse/storage/databases/main/media_repository.py +++ b/synapse/storage/databases/main/media_repository.py @@ -22,6 +22,9 @@ BG_UPDATE_REMOVE_MEDIA_REPO_INDEX_WITHOUT_METHOD = ( "media_repository_drop_index_wo_method" ) +BG_UPDATE_REMOVE_MEDIA_REPO_INDEX_WITHOUT_METHOD_2 = ( + "media_repository_drop_index_wo_method_2" +) class MediaSortOrder(Enum): @@ -85,8 +88,15 @@ def __init__(self, database: DatabasePool, db_conn, hs): unique=True, ) + # the original impl of _drop_media_index_without_method was broken (see + # https://github.com/matrix-org/synapse/issues/8649), so we replace the original + # impl with a no-op and run the fixed migration as + # media_repository_drop_index_wo_method_2. + self.db_pool.updates.register_noop_background_update( + BG_UPDATE_REMOVE_MEDIA_REPO_INDEX_WITHOUT_METHOD + ) self.db_pool.updates.register_background_update_handler( - BG_UPDATE_REMOVE_MEDIA_REPO_INDEX_WITHOUT_METHOD, + BG_UPDATE_REMOVE_MEDIA_REPO_INDEX_WITHOUT_METHOD_2, self._drop_media_index_without_method, ) @@ -106,7 +116,7 @@ def f(txn): await self.db_pool.runInteraction("drop_media_indices_without_method", f) await self.db_pool.updates._end_background_update( - BG_UPDATE_REMOVE_MEDIA_REPO_INDEX_WITHOUT_METHOD + BG_UPDATE_REMOVE_MEDIA_REPO_INDEX_WITHOUT_METHOD_2 ) return 1 diff --git a/synapse/storage/databases/main/schema/delta/59/11drop_thumbnail_constraint.sql.postgres b/synapse/storage/databases/main/schema/delta/59/11drop_thumbnail_constraint.sql.postgres index 944f92d94266..54c1bca3b1ee 100644 --- a/synapse/storage/databases/main/schema/delta/59/11drop_thumbnail_constraint.sql.postgres +++ b/synapse/storage/databases/main/schema/delta/59/11drop_thumbnail_constraint.sql.postgres @@ -18,5 +18,5 @@ -- This was originally part of 57.07, but it was done wrong, per -- https://github.com/matrix-org/synapse/issues/8649, so we do it again. INSERT INTO background_updates (ordering, update_name, progress_json, depends_on) VALUES - (5911, 'media_repository_drop_index_wo_method', '{}', 'remote_media_repository_thumbnails_method_idx'); + (5911, 'media_repository_drop_index_wo_method_2', '{}', 'remote_media_repository_thumbnails_method_idx');