diff --git a/changelog.d/20390.bugfix b/changelog.d/20390.bugfix new file mode 100644 index 0000000000..23e845a77f --- /dev/null +++ b/changelog.d/20390.bugfix @@ -0,0 +1 @@ +Fix the `/_synapse/admin/v1/statistics/database/rooms` Admin API failing with a 500 error when Postgres has no statistics, or no most common `room_id` values, for one of the tables it checks. diff --git a/synapse/storage/controllers/stats.py b/synapse/storage/controllers/stats.py index 5378797f04..0cfad96e6a 100644 --- a/synapse/storage/controllers/stats.py +++ b/synapse/storage/controllers/stats.py @@ -92,9 +92,15 @@ class StatsController: txn.execute(column_sql, (table,)) row = txn.fetchone() - assert row is not None - vals, freqs = row + # There are no stats for a table that is empty or hasn't been + # analyzed yet, and Postgres leaves `most_common_vals` NULL when + # no value is more common than the others, e.g. if every + # `room_id` in the table is NULL. + if row is None or row[0] is None: + continue + + vals, freqs = row for room_id, freq in zip(vals, freqs): room_estimates[room_id] += int(freq * table_size) diff --git a/tests/rest/admin/test_statistics.py b/tests/rest/admin/test_statistics.py index 3dc7e5dc97..4cc29d0897 100644 --- a/tests/rest/admin/test_statistics.py +++ b/tests/rest/admin/test_statistics.py @@ -25,13 +25,15 @@ from twisted.web.resource import Resource import synapse.rest.admin from synapse.api.errors import Codes -from synapse.rest.client import login +from synapse.rest.client import login, room from synapse.server import HomeServer +from synapse.storage.database import LoggingTransaction from synapse.types import JsonDict from synapse.util.clock import Clock from tests import unittest from tests.test_utils import SMALL_PNG +from tests.utils import USE_POSTGRES_FOR_TESTS class UserMediaStatisticsTestCase(unittest.HomeserverTestCase): @@ -521,3 +523,56 @@ class UserMediaStatisticsTestCase(unittest.HomeserverTestCase): returned_order = [row["user_id"] for row in channel.json_body["users"]] self.assertListEqual(expected_user_list, returned_order) self._check_fields(channel.json_body["users"]) + + +class LargestRoomsStatisticsTestCase(unittest.HomeserverTestCase): + if not USE_POSTGRES_FOR_TESTS: + skip = "Requires Postgres" + + servlets = [ + synapse.rest.admin.register_servlets, + login.register_servlets, + room.register_servlets, + ] + + def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: + self.admin_user = self.register_user("admin", "pass", admin=True) + self.admin_user_tok = self.login("admin", "pass") + + self.url = "/_synapse/admin/v1/statistics/database/rooms" + + def test_tables_without_common_room_ids(self) -> None: + """ + Tables without stats in `pg_stats`, or whose `room_id` column is entirely + NULL, have no most common values and must not break the estimate. + """ + room_id = self.helper.create_room_as(self.admin_user, tok=self.admin_user_tok) + + # Nothing has been analyzed yet, so there is nothing to estimate from. + channel = self.make_request( + "GET", + self.url, + access_token=self.admin_user_tok, + ) + self.assertEqual(200, channel.code, msg=channel.json_body) + self.assertNotIn(room_id, [r["room_id"] for r in channel.json_body["rooms"]]) + + for i in range(5): + self.helper.send(room_id, f"message {i}", tok=self.admin_user_tok) + + # `event_push_actions` is still empty and `event_edges.room_id` is never + # written, so neither gets a list of most common values. + def analyze_txn(txn: LoggingTransaction) -> None: + txn.execute("ANALYZE") + + self.get_success( + self.hs.get_datastores().main.db_pool.runInteraction("analyze", analyze_txn) + ) + + channel = self.make_request( + "GET", + self.url, + access_token=self.admin_user_tok, + ) + self.assertEqual(200, channel.code, msg=channel.json_body) + self.assertIn(room_id, [r["room_id"] for r in channel.json_body["rooms"]])