From e5ac8656615da97ffc5a4bb4f41387fa95869be4 Mon Sep 17 00:00:00 2001 From: Junker der Provinz Date: Fri, 13 Nov 2026 13:57:50 +0100 Subject: [PATCH] Fix largest rooms Admin API failing when pg_stats has no common room IDs (#20390) Fixes #19561 `GET /_synapse/admin/v1/statistics/database/rooms` returns a 500 when `pg_stats` has no common `room_id` values for one of the tables it checks. This happens in two cases: - The table is empty or not yet analyzed, so `pg_stats` has no row for it. - No value is more common than the others, so `most_common_vals` is `NULL`. `event_edges.room_id` hits this on servers where all rows were written after schema 72 stopped filling the column. The change skips the table in both cases, since it adds nothing to the estimate. Thanks to @christianarielli for finding the cause and suggesting the fix. They're credited as co-author on the commit. The new test runs only on Postgres, as the endpoint doesn't support SQLite. It creates a room, runs `ANALYZE` while `event_push_actions` is empty and `event_edges.room_id` is all `NULL`, and checks for a 200 with the room listed. I ran it against Postgres 17. `event_edges.room_id` isn't written any more, so dropping that table from the list might make sense. I left that out to keep this PR small. ### Pull Request Checklist * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: - Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from `EventStore` to `EventWorkerStore`.". - Use markdown where necessary, mostly for `code blocks`. - End with either a period (.) or an exclamation mark (!). - Start with a capital letter. - Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) --------- Co-authored-by: Christian Arielli <903609+christianarielli@users.noreply.github.com> Co-authored-by: Eric Eastwood --- changelog.d/20390.bugfix | 1 + synapse/storage/controllers/stats.py | 10 ++++- tests/rest/admin/test_statistics.py | 57 +++++++++++++++++++++++++++- 3 files changed, 65 insertions(+), 3 deletions(-) create mode 100644 changelog.d/20390.bugfix 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"]])