Merge pull request #6152 from matrix-org/erikj/fix_room_list

Fix public room list pagination.
pull/6154/head
Erik Johnston 2019-10-02 15:38:05 +01:00 committed by GitHub
commit 824df3ead4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 60 additions and 27 deletions

1
changelog.d/6152.misc Normal file
View File

@ -0,0 +1 @@
Improve performance of the public room list directory.

View File

@ -142,12 +142,12 @@ class RoomListHandler(BaseHandler):
if since_token: if since_token:
batch_token = RoomListNextBatch.from_token(since_token) batch_token = RoomListNextBatch.from_token(since_token)
last_room_id = batch_token.last_room_id bounds = (batch_token.last_joined_members, batch_token.last_room_id)
forwards = batch_token.direction_is_forward forwards = batch_token.direction_is_forward
else: else:
batch_token = None batch_token = None
bounds = None
last_room_id = None
forwards = True forwards = True
# we request one more than wanted to see if there are more pages to come # we request one more than wanted to see if there are more pages to come
@ -157,7 +157,7 @@ class RoomListHandler(BaseHandler):
network_tuple, network_tuple,
search_filter, search_filter,
probing_limit, probing_limit,
last_room_id=last_room_id, bounds=bounds,
forwards=forwards, forwards=forwards,
ignore_non_federatable=from_federation, ignore_non_federatable=from_federation,
) )
@ -193,30 +193,38 @@ class RoomListHandler(BaseHandler):
more_to_come = False more_to_come = False
if num_results > 0: if num_results > 0:
final_room_id = results[-1]["room_id"] final_entry = results[-1]
initial_room_id = results[0]["room_id"] initial_entry = results[0]
if forwards: if forwards:
if batch_token: if batch_token:
# If there was a token given then we assume that there # If there was a token given then we assume that there
# must be previous results. # must be previous results.
response["prev_batch"] = RoomListNextBatch( response["prev_batch"] = RoomListNextBatch(
last_room_id=initial_room_id, direction_is_forward=False last_joined_members=initial_entry["num_joined_members"],
last_room_id=initial_entry["room_id"],
direction_is_forward=False,
).to_token() ).to_token()
if more_to_come: if more_to_come:
response["next_batch"] = RoomListNextBatch( response["next_batch"] = RoomListNextBatch(
last_room_id=final_room_id, direction_is_forward=True last_joined_members=final_entry["num_joined_members"],
last_room_id=final_entry["room_id"],
direction_is_forward=True,
).to_token() ).to_token()
else: else:
if batch_token: if batch_token:
response["next_batch"] = RoomListNextBatch( response["next_batch"] = RoomListNextBatch(
last_room_id=final_room_id, direction_is_forward=True last_joined_members=final_entry["num_joined_members"],
last_room_id=final_entry["room_id"],
direction_is_forward=True,
).to_token() ).to_token()
if more_to_come: if more_to_come:
response["prev_batch"] = RoomListNextBatch( response["prev_batch"] = RoomListNextBatch(
last_room_id=initial_room_id, direction_is_forward=False last_joined_members=initial_entry["num_joined_members"],
last_room_id=initial_entry["room_id"],
direction_is_forward=False,
).to_token() ).to_token()
for room in results: for room in results:
@ -449,12 +457,17 @@ class RoomListNextBatch(
namedtuple( namedtuple(
"RoomListNextBatch", "RoomListNextBatch",
( (
"last_joined_members", # The count to get rooms after/before
"last_room_id", # The room_id to get rooms after/before "last_room_id", # The room_id to get rooms after/before
"direction_is_forward", # Bool if this is a next_batch, false if prev_batch "direction_is_forward", # Bool if this is a next_batch, false if prev_batch
), ),
) )
): ):
KEY_DICT = {"last_room_id": "r", "direction_is_forward": "d"} KEY_DICT = {
"last_joined_members": "m",
"last_room_id": "r",
"direction_is_forward": "d",
}
REVERSE_KEY_DICT = {v: k for k, v in KEY_DICT.items()} REVERSE_KEY_DICT = {v: k for k, v in KEY_DICT.items()}

View File

@ -17,6 +17,7 @@
import collections import collections
import logging import logging
import re import re
from typing import Optional, Tuple
from canonicaljson import json from canonicaljson import json
@ -25,6 +26,7 @@ from twisted.internet import defer
from synapse.api.errors import StoreError from synapse.api.errors import StoreError
from synapse.storage._base import SQLBaseStore from synapse.storage._base import SQLBaseStore
from synapse.storage.search import SearchStore from synapse.storage.search import SearchStore
from synapse.types import ThirdPartyInstanceID
from synapse.util.caches.descriptors import cached, cachedInlineCallbacks from synapse.util.caches.descriptors import cached, cachedInlineCallbacks
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
@ -119,24 +121,25 @@ class RoomWorkerStore(SQLBaseStore):
@defer.inlineCallbacks @defer.inlineCallbacks
def get_largest_public_rooms( def get_largest_public_rooms(
self, self,
network_tuple, network_tuple: Optional[ThirdPartyInstanceID],
search_filter, search_filter: Optional[dict],
limit, limit: Optional[int],
last_room_id, bounds: Optional[Tuple[int, str]],
forwards, forwards: bool,
ignore_non_federatable=False, ignore_non_federatable: bool = False,
): ):
"""Gets the largest public rooms (where largest is in terms of joined """Gets the largest public rooms (where largest is in terms of joined
members, as tracked in the statistics table). members, as tracked in the statistics table).
Args: Args:
network_tuple (ThirdPartyInstanceID|None): network_tuple
search_filter (dict|None): search_filter
limit (int|None): Maxmimum number of rows to return, unlimited otherwise. limit: Maxmimum number of rows to return, unlimited otherwise.
last_room_id (str|None): if present, a room ID which bounds the bounds: An uppoer or lower bound to apply to result set if given,
result set, and is always *excluded* from the result set. consists of a joined member count and room_id (these are
forwards (bool): true iff going forwards, going backwards otherwise excluded from result set).
ignore_non_federatable (bool): If true filters out non-federatable rooms. forwards: true iff going forwards, going backwards otherwise
ignore_non_federatable: If true filters out non-federatable rooms.
Returns: Returns:
Rooms in order: biggest number of joined users first. Rooms in order: biggest number of joined users first.
@ -147,13 +150,29 @@ class RoomWorkerStore(SQLBaseStore):
where_clauses = [] where_clauses = []
query_args = [] query_args = []
if last_room_id: # Work out the bounds if we're given them, these bounds look slightly
# odd, but are designed to help query planner use indices by pulling
# out a common bound.
if bounds:
last_joined_members, last_room_id = bounds
if forwards: if forwards:
where_clauses.append("room_id < ?") where_clauses.append(
"""
joined_members <= ? AND (
joined_members < ? OR room_id < ?
)
"""
)
else: else:
where_clauses.append("? < room_id") where_clauses.append(
"""
joined_members >= ? AND (
joined_members > ? OR room_id > ?
)
"""
)
query_args += [last_room_id] query_args += [last_joined_members, last_joined_members, last_room_id]
if search_filter and search_filter.get("generic_search_term", None): if search_filter and search_filter.get("generic_search_term", None):
search_term = "%" + search_filter["generic_search_term"] + "%" search_term = "%" + search_filter["generic_search_term"] + "%"