Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Delete event_push_summary_unique_index again. #14669

Merged
merged 5 commits into from
Dec 14, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/14669.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a bug introduced in Synapse 1.70.0 which could cause spurious `UNIQUE constraint failed` errors in the `rotate_notifs` background job.
9 changes: 0 additions & 9 deletions synapse/storage/databases/main/event_push_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -274,15 +274,6 @@ def __init__(
self._clear_old_push_actions_staging, 30 * 60 * 1000
)

self.db_pool.updates.register_background_index_update(
"event_push_summary_unique_index",
index_name="event_push_summary_unique_index",
table="event_push_summary",
columns=["user_id", "room_id"],
unique=True,
replaces_index="event_push_summary_user_rm",
)

Comment on lines -277 to -285
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sanity check: what does the background update logic do if it tries to run an update but there's no handler for it?

Oh, the migration deletes the update in question, so it doesn't matter. 👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An error gets raised if it cannot find the background update, but as you've said we're deleting it so it doesn't matter.

self.db_pool.updates.register_background_index_update(
"event_push_summary_unique_index2",
index_name="event_push_summary_unique_index2",
Expand Down
33 changes: 33 additions & 0 deletions synapse/storage/schema/main/delta/73/23_fix_thread_index.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/* Copyright 2022 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.
*/

-- If a Synapse deployment made a large jump in versions (from < 1.62.0 to >= 1.70.0)
-- in a single upgrade then it might be possible for the event_push_summary_unique_index
-- to be created in the background from delta 71/02event_push_summary_unique.sql after
-- delta 73/06thread_notifications_thread_id_idx.sql is executed, causing it to
-- not drop the event_push_summary_unique_index index.
--
-- See https://github.com/matrix-org/synapse/issues/14641

-- Stop the index from being scheduled for creation in the background.
DELETE FROM background_updates WHERE update_name = 'event_push_summary_unique_index';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I sanity checked to see if we need to update UNIQUE_INDEX_BACKGROUND_UPDATES. It doesn't look like it: that only mentions the newest index, event_push_summary_unique_index2

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. Thanks for checking!


-- The above background job also replaces another index, if blah ensure that side-effect
clokep marked this conversation as resolved.
Show resolved Hide resolved
-- is applied.
DROP INDEX IF EXISTS event_push_summary_user_rm;

-- Fix deployments which ran the 73/06thread_notifications_thread_id_idx.sql delta
-- before the event_push_summary_unique_index background job was run.
DROP INDEX IF EXISTS event_push_summary_unique_index;