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

Replace noop background updates with DELETE. #12954

Merged
merged 4 commits into from
Jun 13, 2022
Merged
Show file tree
Hide file tree
Changes from all 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/12954.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Replace noop background updates with `DELETE` delta.
2 changes: 0 additions & 2 deletions synapse/_scripts/synapse_port_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@
from synapse.storage.databases.main.events_bg_updates import (
EventsBackgroundUpdatesStore,
)
from synapse.storage.databases.main.group_server import GroupServerStore
from synapse.storage.databases.main.media_repository import (
MediaRepositoryBackgroundUpdateStore,
)
Expand Down Expand Up @@ -218,7 +217,6 @@ class Store(
PushRuleStore,
PusherWorkerStore,
PresenceBackgroundUpdateStore,
GroupServerStore,
):
def execute(self, f: Callable[..., R], *args: Any, **kwargs: Any) -> Awaitable[R]:
return self.db_pool.runInteraction(f.__name__, f, *args, **kwargs)
Expand Down
19 changes: 0 additions & 19 deletions synapse/storage/background_updates.py
Original file line number Diff line number Diff line change
Expand Up @@ -507,25 +507,6 @@ def register_background_update_handler(
update_handler
)

def register_noop_background_update(self, update_name: str) -> None:
"""Register a noop handler for a background update.

This is useful when we previously did a background update, but no
longer wish to do the update. In this case the background update should
be removed from the schema delta files, but there may still be some
users who have the background update queued, so this method should
also be called to clear the update.

Args:
update_name: Name of update
"""

async def noop_update(progress: JsonDict, batch_size: int) -> int:
await self._end_background_update(update_name)
return 1

self.register_background_update_handler(update_name, noop_update)

def register_background_index_update(
self,
update_name: str,
Expand Down
2 changes: 0 additions & 2 deletions synapse/storage/databases/main/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
from .events_bg_updates import EventsBackgroundUpdatesStore
from .events_forward_extremities import EventForwardExtremitiesStore
from .filtering import FilteringStore
from .group_server import GroupServerStore
from .keys import KeyStore
from .lock import LockStore
from .media_repository import MediaRepositoryStore
Expand Down Expand Up @@ -117,7 +116,6 @@ class DataStore(
DeviceStore,
DeviceInboxStore,
UserDirectoryStore,
GroupServerStore,
UserErasureStore,
MonthlyActiveUsersWorkerStore,
StatsStore,
Expand Down
11 changes: 0 additions & 11 deletions synapse/storage/databases/main/deviceinbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -834,8 +834,6 @@ def _add_messages_to_local_device_inbox_txn(

class DeviceInboxBackgroundUpdateStore(SQLBaseStore):
DEVICE_INBOX_STREAM_ID = "device_inbox_stream_drop"
REMOVE_DELETED_DEVICES = "remove_deleted_devices_from_device_inbox"
REMOVE_HIDDEN_DEVICES = "remove_hidden_devices_from_device_inbox"
REMOVE_DEAD_DEVICES_FROM_INBOX = "remove_dead_devices_from_device_inbox"

def __init__(
Expand All @@ -857,15 +855,6 @@ def __init__(
self.DEVICE_INBOX_STREAM_ID, self._background_drop_index_device_inbox
)

# Used to be a background update that deletes all device_inboxes for deleted
# devices.
self.db_pool.updates.register_noop_background_update(
self.REMOVE_DELETED_DEVICES
)
# Used to be a background update that deletes all device_inboxes for hidden
# devices.
self.db_pool.updates.register_noop_background_update(self.REMOVE_HIDDEN_DEVICES)

self.db_pool.updates.register_background_update_handler(
self.REMOVE_DEAD_DEVICES_FROM_INBOX,
self._remove_dead_devices_from_device_inbox,
Expand Down
9 changes: 0 additions & 9 deletions synapse/storage/databases/main/devices.py
Original file line number Diff line number Diff line change
Expand Up @@ -1240,15 +1240,6 @@ def __init__(
self._remove_duplicate_outbound_pokes,
)

# a pair of background updates that were added during the 1.14 release cycle,
# but replaced with 58/06dlols_unique_idx.py
self.db_pool.updates.register_noop_background_update(
"device_lists_outbound_last_success_unique_idx",
)
self.db_pool.updates.register_noop_background_update(
"drop_device_lists_outbound_last_success_non_unique_idx",
)

async def _drop_device_list_streams_non_unique_indexes(
self, progress: JsonDict, batch_size: int
) -> int:
Expand Down
5 changes: 0 additions & 5 deletions synapse/storage/databases/main/events_bg_updates.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,11 +177,6 @@ def __init__(
self._purged_chain_cover_index,
)

# The event_thread_relation background update was replaced with the
# event_arbitrary_relations one, which handles any relation to avoid
# needed to potentially crawl the entire events table in the future.
self.db_pool.updates.register_noop_background_update("event_thread_relation")

self.db_pool.updates.register_background_update_handler(
"event_arbitrary_relations",
self._event_arbitrary_relations,
Expand Down
34 changes: 0 additions & 34 deletions synapse/storage/databases/main/group_server.py

This file was deleted.

10 changes: 0 additions & 10 deletions synapse/storage/databases/main/media_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,6 @@
if TYPE_CHECKING:
from synapse.server import HomeServer

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"
)
Expand Down Expand Up @@ -111,13 +108,6 @@ def __init__(
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_2,
self._drop_media_index_without_method,
Expand Down
11 changes: 0 additions & 11 deletions synapse/storage/databases/main/registration.py
Original file line number Diff line number Diff line change
Expand Up @@ -1805,21 +1805,10 @@ def __init__(
columns=["creation_ts"],
)

# we no longer use refresh tokens, but it's possible that some people
# might have a background update queued to build this index. Just
# clear the background update.
self.db_pool.updates.register_noop_background_update(
"refresh_tokens_device_index"
)

self.db_pool.updates.register_background_update_handler(
"users_set_deactivated_flag", self._background_update_set_deactivated_flag
)

self.db_pool.updates.register_noop_background_update(
"user_threepids_grandfather"
)

self.db_pool.updates.register_background_index_update(
"user_external_ids_user_id_idx",
index_name="user_external_ids_user_id_idx",
Expand Down
10 changes: 0 additions & 10 deletions synapse/storage/databases/main/search.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ class SearchBackgroundUpdateStore(SearchWorkerStore):

EVENT_SEARCH_UPDATE_NAME = "event_search"
EVENT_SEARCH_ORDER_UPDATE_NAME = "event_search_order"
EVENT_SEARCH_USE_GIST_POSTGRES_NAME = "event_search_postgres_gist"
EVENT_SEARCH_USE_GIN_POSTGRES_NAME = "event_search_postgres_gin"
EVENT_SEARCH_DELETE_NON_STRINGS = "event_search_sqlite_delete_non_strings"

Expand All @@ -132,15 +131,6 @@ def __init__(
self.EVENT_SEARCH_ORDER_UPDATE_NAME, self._background_reindex_search_order
)

# we used to have a background update to turn the GIN index into a
# GIST one; we no longer do that (obviously) because we actually want
# a GIN index. However, it's possible that some people might still have
# the background update queued, so we register a handler to clear the
# background update.
self.db_pool.updates.register_noop_background_update(
self.EVENT_SEARCH_USE_GIST_POSTGRES_NAME
)

self.db_pool.updates.register_background_update_handler(
self.EVENT_SEARCH_USE_GIN_POSTGRES_NAME, self._background_reindex_gin_search
)
Expand Down
5 changes: 0 additions & 5 deletions synapse/storage/databases/main/stats.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,6 @@ def __init__(
self.db_pool.updates.register_background_update_handler(
"populate_stats_process_users", self._populate_stats_process_users
)
# we no longer need to perform clean-up, but we will give ourselves
# the potential to reintroduce it in the future – so documentation
# will still encourage the use of this no-op handler.
self.db_pool.updates.register_noop_background_update("populate_stats_cleanup")
self.db_pool.updates.register_noop_background_update("populate_stats_prepare")

async def _populate_stats_process_users(
self, progress: JsonDict, batch_size: int
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/* 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.
*/

-- Clean-up background updates which should no longer be run. Previously these
-- used the (now removed) register_noop_background_update method.

-- Used to be a background update that deletes all device_inboxes for deleted
-- devices.
DELETE FROM background_updates WHERE update_name = 'remove_deleted_devices_from_device_inbox';
-- Used to be a background update that deletes all device_inboxes for hidden
-- devices.
DELETE FROM background_updates WHERE update_name = 'remove_hidden_devices_from_device_inbox';

-- A pair of background updates that were added during the 1.14 release cycle,
-- but replaced with 58/06dlols_unique_idx.py
DELETE FROM background_updates WHERE update_name = 'device_lists_outbound_last_success_unique_idx';
DELETE FROM background_updates WHERE update_name = 'drop_device_lists_outbound_last_success_non_unique_idx';

-- The event_thread_relation background update was replaced with the
-- event_arbitrary_relations one, which handles any relation to avoid
-- needed to potentially crawl the entire events table in the future.
DELETE FROM background_updates WHERE update_name = 'event_thread_relation';

-- A legacy groups background update.
DELETE FROM background_updates WHERE update_name = 'local_group_updates_index';

-- 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.
DELETE FROM background_updates WHERE update_name = 'media_repository_drop_index_wo_method';

-- We no longer use refresh tokens, but it's possible that some people
-- might have a background update queued to build this index. Just
-- clear the background update.
DELETE FROM background_updates WHERE update_name = 'refresh_tokens_device_index';

DELETE FROM background_updates WHERE update_name = 'user_threepids_grandfather';

-- We used to have a background update to turn the GIN index into a
-- GIST one; we no longer do that (obviously) because we actually want
-- a GIN index. However, it's possible that some people might still have
-- the background update queued, so we register a handler to clear the
-- background update.
DELETE FROM background_updates WHERE update_name = 'event_search_postgres_gist';

-- We no longer need to perform clean-up.
DELETE FROM background_updates WHERE update_name = 'populate_stats_cleanup';
DELETE FROM background_updates WHERE update_name = 'populate_stats_prepare';
28 changes: 0 additions & 28 deletions tests/handlers/test_stats.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,19 +43,12 @@ def _add_background_updates(self):
# Ugh, have to reset this flag
self.store.db_pool.updates._all_done = False

self.get_success(
self.store.db_pool.simple_insert(
"background_updates",
{"update_name": "populate_stats_prepare", "progress_json": "{}"},
)
)
self.get_success(
self.store.db_pool.simple_insert(
"background_updates",
{
"update_name": "populate_stats_process_rooms",
"progress_json": "{}",
"depends_on": "populate_stats_prepare",
},
)
)
Expand All @@ -69,16 +62,6 @@ def _add_background_updates(self):
},
)
)
self.get_success(
self.store.db_pool.simple_insert(
"background_updates",
{
"update_name": "populate_stats_cleanup",
"progress_json": "{}",
"depends_on": "populate_stats_process_users",
},
)
)

async def get_all_room_state(self):
return await self.store.db_pool.simple_select_list(
Expand Down Expand Up @@ -533,7 +516,6 @@ def test_incomplete_stats(self):
{
"update_name": "populate_stats_process_rooms",
"progress_json": "{}",
"depends_on": "populate_stats_prepare",
},
)
)
Expand All @@ -547,16 +529,6 @@ def test_incomplete_stats(self):
},
)
)
self.get_success(
self.store.db_pool.simple_insert(
"background_updates",
{
"update_name": "populate_stats_cleanup",
"progress_json": "{}",
"depends_on": "populate_stats_process_users",
},
)
)

self.wait_for_background_updates()

Expand Down