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

Add index to local_group_updates.stream_id #11231

Merged
merged 6 commits into from
Nov 2, 2021
Merged
Show file tree
Hide file tree
Changes from 2 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/11231.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Minor speed up to start up times and getting updates for groups, by adding missing index to `local_group_updates.stream_id`.
erikjohnston marked this conversation as resolved.
Show resolved Hide resolved
17 changes: 16 additions & 1 deletion synapse/storage/databases/main/group_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,20 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from typing import Any, Dict, List, Optional, Tuple
from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple

from typing_extensions import TypedDict

from synapse.api.errors import SynapseError
from synapse.storage._base import SQLBaseStore, db_to_json
from synapse.storage.database import DatabasePool
from synapse.storage.types import Connection
from synapse.types import JsonDict
from synapse.util import json_encoder

if TYPE_CHECKING:
from synapse.server import HomeServer

# The category ID for the "default" category. We don't store as null in the
# database to avoid the fun of null != null
_DEFAULT_CATEGORY_ID = ""
Expand All @@ -35,6 +40,16 @@ class _RoomInGroup(TypedDict):


class GroupServerWorkerStore(SQLBaseStore):
def __init__(self, database: DatabasePool, db_conn: Connection, hs: "HomeServer"):
database.updates.register_background_index_update(
update_name="local_group_updates_index",
index_name="local_group_updates_stream_id_index",
table="local_group_updates",
columns=("stream_id"),
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this tuple is missing a comma.

unique=True,
)
super().__init__(database, db_conn, hs)

async def get_group(self, group_id: str) -> Optional[Dict[str, Any]]:
return await self.db_pool.simple_select_one(
table="groups",
Expand Down
18 changes: 18 additions & 0 deletions synapse/storage/schema/main/delta/65/03_local_group_updates.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/* 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.
*/

-- Check old events for thread relations.
Copy link
Member

Choose a reason for hiding this comment

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

Copy/pasted from elsewhere?

INSERT INTO background_updates (ordering, update_name, progress_json) VALUES
(6503, 'local_group_updates_index', '{}');
Copy link
Member

Choose a reason for hiding this comment

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

Looks like someone else sniped us and took 6503:

INSERT INTO background_updates (ordering, update_name, progress_json) VALUES
(6503, 'remove_hidden_devices_from_device_inbox', '{}');

Bump to 6504? 😇

Copy link
Member Author

Choose a reason for hiding this comment

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

It doesn't really matter, but sure

Copy link
Member

Choose a reason for hiding this comment

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

Oh! I suppose it doesn't matter as this query doesn't rely on being after remove_hidden_devices_from_device_inbox for any reason, correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, exactly!