mirror of
https://github.com/element-hq/synapse.git
synced 2026-11-14 16:09:34 +00:00
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 <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [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 <erice@element.io>
This commit is contained in:
3 files changed
+65
-3
No files matched your search
@@ -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.
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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"]])
|
||||
Reference in new issue
Block a user