Switch to upserting bg update name, rather than renaming

synapse/storage/databases/main/deviceinbox.py

@@ -560,11 +560,8 @@ def _add_messages_to_local_device_inbox_txn(
 
 class DeviceInboxBackgroundUpdateStore(SQLBaseStore):
     DEVICE_INBOX_STREAM_ID = "device_inbox_stream_drop"
-    REMOVE_HIDDEN_DEVICES = "remove_hidden_devices_from_device_inbox"
     REMOVE_DELETED_DEVICES = "remove_deleted_devices_from_device_inbox"
-    # See '05remove_deleted_devices_from_device_inbox.sql' for an explanation on
-    # why this is needed.
-    REMOVE_DELETED_DEVICES_V2 = "remove_deleted_devices_from_device_inbox_v2"
+    REMOVE_HIDDEN_DEVICES = "remove_hidden_devices_from_device_inbox"
 
     def __init__(self, database: DatabasePool, db_conn, hs: "HomeServer"):
         super().__init__(database, db_conn, hs)
@@ -585,11 +582,6 @@ def __init__(self, database: DatabasePool, db_conn, hs: "HomeServer"):
             self._remove_deleted_devices_from_device_inbox,
         )
 
-        self.db_pool.updates.register_background_update_handler(
-            self.REMOVE_DELETED_DEVICES_V2,
-            self._remove_deleted_devices_from_device_inbox_v2,
-        )
-
         self.db_pool.updates.register_background_update_handler(
             self.REMOVE_HIDDEN_DEVICES,
             self._remove_hidden_devices_from_device_inbox,
@@ -607,15 +599,12 @@ def reindex_txn(conn):
 
         return 1
 
-    async def _remove_deleted_devices_from_device_inbox_v2(
+    async def _remove_deleted_devices_from_device_inbox(
         self, progress: JsonDict, batch_size: int
     ) -> int:
         """A background update that deletes all device_inboxes for deleted devices.
 
-        This should only need to be run once (when users upgrade to v1.47.0).
-
-        See '05remove_deleted_devices_from_device_inbox.sql' for an explanation on
-        why this is needed.
+        This should only need to be run once (when users upgrade to v1.47.0)
 
         Args:
             progress: JsonDict used to store progress of this background update
@@ -670,7 +659,7 @@ def _remove_deleted_devices_from_device_inbox_txn(
                 # it may be that the stream_id does not change in several runs
                 self.db_pool.updates._background_update_progress_txn(
                     txn,
-                    self.REMOVE_DELETED_DEVICES_V2,
+                    self.REMOVE_DELETED_DEVICES,
                     {
                         "device_id": rows[-1][0],
                         "user_id": rows[-1][1],
@@ -688,21 +677,11 @@ def _remove_deleted_devices_from_device_inbox_txn(
         # The task is finished when no more lines are deleted.
         if not number_deleted:
             await self.db_pool.updates._end_background_update(
-                self.REMOVE_DELETED_DEVICES_V2
+                self.REMOVE_DELETED_DEVICES
             )
 
         return number_deleted
 
-    async def _remove_deleted_devices_from_device_inbox(
-        self, progress: JsonDict, batch_size: int
-    ) -> int:
-        """
-        This background update has been converted to a no-op. See
-        _remove_deleted_devices_from_device_inbox_v2 instead.
-        """
-        await self.db_pool.updates._end_background_update(self.REMOVE_DELETED_DEVICES)
-        return 0
-
     async def _remove_hidden_devices_from_device_inbox(
         self, progress: JsonDict, batch_size: int
     ) -> int:

synapse/storage/schema/main/delta/65/06remove_deleted_devices_from_device_inbox.sql

@@ -18,11 +18,17 @@
 -- when a device was deleted using Synapse earlier than 1.47.0.
 -- This runs as background task, but may take a bit to finish.
 
--- The name of this update ending is '_v2' is due to it accidentally
+-- Remove any existing instances of this job running. It's OK to stop and restart this job,
+-- as it's just deleting entries from a table - no progress will be lost.
+--
+-- This is necessary due a similar migration running the job accidentally
 -- being included in schema version 64 during v1.47.0rc1,rc2. If a
 -- homeserver had updated from Synapse <=v1.45.0 (schema version <=64),
--- then they would have run the original version of this background update
--- already. So we rename it here, to ensure it is run regardless of upgrade path.
+-- then they would have started running this background update already.
+-- If that update was still running, then simply inserting it again would
+-- cause an SQL failure. So we effectively do an "upsert" here instead.
+
+DELETE FROM background_updates WHERE update_name = 'remove_deleted_devices_from_device_inbox';
 
 INSERT INTO background_updates (ordering, update_name, progress_json) VALUES
-  (6506, 'remove_deleted_devices_from_device_inbox_v2', '{}');
+  (6506, 'remove_deleted_devices_from_device_inbox', '{}');

tests/storage/databases/main/test_deviceinbox.py

@@ -66,7 +66,7 @@ def test_background_remove_deleted_devices_from_device_inbox(self):
             self.store.db_pool.simple_insert(
                 "background_updates",
                 {
-                    "update_name": "remove_deleted_devices_from_device_inbox_v2",
+                    "update_name": "remove_deleted_devices_from_device_inbox",
                     "progress_json": "{}",
                 },
             )