From 47e375111e63680c0f0bc5fa1bf101a7cfd9d6f9 Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Thu, 5 Sep 2019 21:13:45 +1000 Subject: [PATCH 01/18] update tests, add metric --- synapse/config/metrics.py | 18 +++++ synapse/storage/roommember.py | 31 +++++++++ tests/storage/test_roommember.py | 113 ++++++++++++++++++++++--------- 3 files changed, 130 insertions(+), 32 deletions(-) diff --git a/synapse/config/metrics.py b/synapse/config/metrics.py index 3698441963b1..ef3fc4f8e236 100644 --- a/synapse/config/metrics.py +++ b/synapse/config/metrics.py @@ -13,6 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import attr from ._base import Config, ConfigError MISSING_SENTRY = """Missing sentry-sdk library. This is required to enable sentry @@ -20,6 +21,18 @@ """ +@attr.s +class MetricsFlags(object): + known_servers = attr.ib(default=False, validator=attr.validators.instance_of(bool)) + + @classmethod + def all_off(cls): + """ + Instantiate the flags with all options set to off. + """ + return cls(**{x.name: False for x in attr.fields(cls)}) + + class MetricsConfig(Config): def read_config(self, config, **kwargs): self.enable_metrics = config.get("enable_metrics", False) @@ -27,6 +40,11 @@ def read_config(self, config, **kwargs): self.metrics_port = config.get("metrics_port") self.metrics_bind_host = config.get("metrics_bind_host", "127.0.0.1") + if self.enable_metrics: + self.metrics_flags = MetricsFlags(**config.get("metrics_flags", {})) + else: + self.metrics_flags = MetricsFlags.all_off() + self.sentry_enabled = "sentry" in config if self.sentry_enabled: try: diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index f8b682ebd9e5..1493d621c2b6 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -24,6 +24,7 @@ from twisted.internet import defer from synapse.api.constants import EventTypes, Membership +from synapse.metrics import LaterGauge from synapse.metrics.background_process_metrics import run_as_background_process from synapse.storage._base import LoggingTransaction from synapse.storage.events_worker import EventsWorkerStore @@ -74,6 +75,36 @@ def __init__(self, db_conn, hs): self._check_safe_current_state_events_membership_updated_txn(txn) txn.close() + if self.hs.config.metrics_flags.known_servers: + self._known_servers_count = 1 + self.hs.get_clock().looping_call(self._count_known_servers, 60 * 1000) + self.hs.get_clock().call_later(1000, self._count_known_servers) + LaterGauge( + "synapse_servers_known_about", "", [], lambda: self._known_servers_count + ) + + @defer.inlineCallbacks + def _count_known_servers(self): + """ + Count the servers that this server knows about. + """ + + def _transact(txn): + query = """ + SELECT COUNT(DISTINCT substr(user_id, pos+1)) + FROM + (SELECT user_id, instr(user_id, ':') AS pos FROM room_memberships) + """ + txn.execute(query) + return list(txn)[0][0] + + count = yield self.runInteraction("get_known_servers", _transact) + + # We always know about ourselves, even if we have nothing in + # room_memberships (for example, the server is new). + self._known_servers_count = max([count, 1]) + return self._known_servers_count + def _check_safe_current_state_events_membership_updated_txn(self, txn): """Checks if it is safe to assume the new current_state_events membership column is up to date diff --git a/tests/storage/test_roommember.py b/tests/storage/test_roommember.py index 64cb294c37ec..331338d2e11d 100644 --- a/tests/storage/test_roommember.py +++ b/tests/storage/test_roommember.py @@ -18,73 +18,122 @@ from twisted.internet import defer +from synapse.rest.admin import register_servlets_for_client_rest_resource from synapse.api.constants import EventTypes, Membership from synapse.api.room_versions import RoomVersions from synapse.types import Requester, RoomID, UserID +from synapse.rest.client.v1 import login, room from tests import unittest from tests.utils import create_room, setup_test_homeserver -class RoomMemberStoreTestCase(unittest.TestCase): - @defer.inlineCallbacks - def setUp(self): - hs = yield setup_test_homeserver( - self.addCleanup, resource_for_federation=Mock(), http_client=None - ) +class RoomMemberStoreTestCase(unittest.HomeserverTestCase): + + servlets = [ + login.register_servlets, + register_servlets_for_client_rest_resource, + room.register_servlets, + ] + + def prepare(self, reactor, clock, hs): + # We can't test the RoomMemberStore on its own without the other event # storage logic self.store = hs.get_datastore() self.event_builder_factory = hs.get_event_builder_factory() self.event_creation_handler = hs.get_event_creation_handler() - self.u_alice = UserID.from_string("@alice:test") - self.u_bob = UserID.from_string("@bob:test") + self.u_alice = self.register_user("alice", "pass") + self.t_alice = self.login("alice", "pass") + self.u_bob = self.register_user("bob", "pass") # User elsewhere on another host self.u_charlie = UserID.from_string("@charlie:elsewhere") - self.room = RoomID.from_string("!abc123:test") - - yield create_room(hs, self.room.to_string(), self.u_alice.to_string()) - - @defer.inlineCallbacks def inject_room_member(self, room, user, membership, replaces_state=None): builder = self.event_builder_factory.for_room_version( RoomVersions.V1, { "type": EventTypes.Member, - "sender": user.to_string(), - "state_key": user.to_string(), - "room_id": room.to_string(), + "sender": user, + "state_key": user, + "room_id": room, "content": {"membership": membership}, }, ) - event, context = yield self.event_creation_handler.create_new_client_event( - builder + event, context = self.get_success( + self.event_creation_handler.create_new_client_event(builder) ) - yield self.store.persist_event(event, context) + self.get_success(self.store.persist_event(event, context)) return event - @defer.inlineCallbacks def test_one_member(self): - yield self.inject_room_member(self.room, self.u_alice, Membership.JOIN) - - self.assertEquals( - [self.room.to_string()], - [ - m.room_id - for m in ( - yield self.store.get_rooms_for_user_where_membership_is( - self.u_alice.to_string(), [Membership.JOIN] - ) - ) - ], + + # Alice creates the room, and is automatically joined + self.room = self.helper.create_room_as(self.u_alice, tok=self.t_alice) + + rooms_for_user = self.get_success( + self.store.get_rooms_for_user_where_membership_is( + self.u_alice, [Membership.JOIN] + ) ) + self.assertEquals([self.room], [m.room_id for m in rooms_for_user]) + + def test_count_known_servers(self): + """ + _count_known_servers will calculate how many servers are in a room. + """ + self.room = self.helper.create_room_as(self.u_alice, tok=self.t_alice) + self.inject_room_member(self.room, self.u_bob, Membership.JOIN) + self.inject_room_member(self.room, self.u_charlie.to_string(), Membership.JOIN) + + servers = self.get_success(self.store._count_known_servers()) + self.assertEqual(servers, 2) + + def test_count_known_servers_stat_counter_disabled(self): + """ + If enabled, the metrics for how many servers are known will be counted. + """ + self.assertTrue("_known_servers_count" not in self.store.__dict__.keys()) + + self.room = self.helper.create_room_as(self.u_alice, tok=self.t_alice) + self.inject_room_member(self.room, self.u_bob, Membership.JOIN) + self.inject_room_member(self.room, self.u_charlie.to_string(), Membership.JOIN) + + self.pump(20) + + self.assertTrue("_known_servers_count" not in self.store.__dict__.keys()) + + @unittest.override_config( + {"enable_metrics": True, "metrics_flags": {"known_servers": True}} + ) + def test_count_known_servers_stat_counter_enabled(self): + """ + If enabled, the metrics for how many servers are known will be counted. + """ + # Initialises to 1 -- itself + self.assertEqual(self.store._known_servers_count, 1) + + self.pump(20) + + # No rooms have been joined, so technically the SQL returns 0, but it + # will still say it knows about itself. + self.assertEqual(self.store._known_servers_count, 1) + + self.room = self.helper.create_room_as(self.u_alice, tok=self.t_alice) + self.inject_room_member(self.room, self.u_bob, Membership.JOIN) + self.inject_room_member(self.room, self.u_charlie.to_string(), Membership.JOIN) + + self.pump(20) + + # It now knows about Charlie's server. + self.assertEqual(self.store._known_servers_count, 2) + class CurrentStateMembershipUpdateTestCase(unittest.HomeserverTestCase): def prepare(self, reactor, clock, homeserver): From 7d8c0500225c68d7167713fd76d357fa73652112 Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Thu, 5 Sep 2019 21:16:35 +1000 Subject: [PATCH 02/18] changelog --- changelog.d/5981.feature | 1 + synapse/storage/roommember.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 changelog.d/5981.feature diff --git a/changelog.d/5981.feature b/changelog.d/5981.feature new file mode 100644 index 000000000000..e39514273d6e --- /dev/null +++ b/changelog.d/5981.feature @@ -0,0 +1 @@ +Setting metrics_flags.known_servers to True in the configuration will publish the synapse_federation_known_servers metric over Prometheus. This represents the total number of servers your server knows about (i.e. is in rooms with), including itself. diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index 1493d621c2b6..b9ac57101167 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -80,7 +80,7 @@ def __init__(self, db_conn, hs): self.hs.get_clock().looping_call(self._count_known_servers, 60 * 1000) self.hs.get_clock().call_later(1000, self._count_known_servers) LaterGauge( - "synapse_servers_known_about", "", [], lambda: self._known_servers_count + "synapse_federation_known_servers", "", [], lambda: self._known_servers_count ) @defer.inlineCallbacks From 6a3de9f00f3d17c23d2a98073e350222103b5ec8 Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Thu, 5 Sep 2019 21:20:24 +1000 Subject: [PATCH 03/18] changelog --- synapse/config/metrics.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/synapse/config/metrics.py b/synapse/config/metrics.py index ef3fc4f8e236..b884c61e538a 100644 --- a/synapse/config/metrics.py +++ b/synapse/config/metrics.py @@ -76,6 +76,16 @@ def generate_config_section(self, report_stats=None, **kwargs): #sentry: # dsn: "..." + # Flags to enable metrics which are not suitable to be enabled by + # default, either for performance reasons or limited use. + # + # known_servers: Publish synapse_federation_known_servers, a gauge of + # the number of servers this homeserver knows about, including itself. + # May cause performance problems on large homeservers. + # + #metrics_flags: + # known_servers: true + # Whether or not to report anonymized homeserver usage statistics. """ From 871ff26da3d06cf65e4acb28c626c46f214ff1b0 Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Thu, 5 Sep 2019 21:20:37 +1000 Subject: [PATCH 04/18] add flags to config --- docs/sample_config.yaml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 43969bbb7032..939048f3f8df 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -926,6 +926,16 @@ uploads_path: "DATADIR/uploads" #sentry: # dsn: "..." +# Flags to enable metrics which are not suitable to be enabled by +# default, either for performance reasons or limited use. +# +# known_servers: Publish synapse_federation_known_servers, a gauge of +# the number of servers this homeserver knows about, including itself. +# May cause performance problems on large homeservers. +# +#metrics_flags: +# known_servers: true + # Whether or not to report anonymized homeserver usage statistics. # report_stats: true|false From 94d7f2f5d33d1c5f8f7d86b2b3a57c34ce7a8116 Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Thu, 5 Sep 2019 21:25:53 +1000 Subject: [PATCH 05/18] add flags to config --- synapse/config/metrics.py | 1 + synapse/storage/roommember.py | 5 ++++- tests/storage/test_roommember.py | 4 ++-- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/synapse/config/metrics.py b/synapse/config/metrics.py index b884c61e538a..93ccb36428b2 100644 --- a/synapse/config/metrics.py +++ b/synapse/config/metrics.py @@ -14,6 +14,7 @@ # limitations under the License. import attr + from ._base import Config, ConfigError MISSING_SENTRY = """Missing sentry-sdk library. This is required to enable sentry diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index b9ac57101167..66dd174ceb28 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -80,7 +80,10 @@ def __init__(self, db_conn, hs): self.hs.get_clock().looping_call(self._count_known_servers, 60 * 1000) self.hs.get_clock().call_later(1000, self._count_known_servers) LaterGauge( - "synapse_federation_known_servers", "", [], lambda: self._known_servers_count + "synapse_federation_known_servers", + "", + [], + lambda: self._known_servers_count, ) @defer.inlineCallbacks diff --git a/tests/storage/test_roommember.py b/tests/storage/test_roommember.py index 331338d2e11d..d5da114d458a 100644 --- a/tests/storage/test_roommember.py +++ b/tests/storage/test_roommember.py @@ -18,11 +18,11 @@ from twisted.internet import defer -from synapse.rest.admin import register_servlets_for_client_rest_resource from synapse.api.constants import EventTypes, Membership from synapse.api.room_versions import RoomVersions -from synapse.types import Requester, RoomID, UserID +from synapse.rest.admin import register_servlets_for_client_rest_resource from synapse.rest.client.v1 import login, room +from synapse.types import Requester, RoomID, UserID from tests import unittest from tests.utils import create_room, setup_test_homeserver From 0c123caad40dc0f1af104d5207dd45768160c486 Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Thu, 5 Sep 2019 21:29:33 +1000 Subject: [PATCH 06/18] clean up --- synapse/config/metrics.py | 1 + tests/storage/test_roommember.py | 9 ++------- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/synapse/config/metrics.py b/synapse/config/metrics.py index 93ccb36428b2..acfdc7a0644c 100644 --- a/synapse/config/metrics.py +++ b/synapse/config/metrics.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- # Copyright 2015, 2016 OpenMarket Ltd +# Copyright 2019 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. diff --git a/tests/storage/test_roommember.py b/tests/storage/test_roommember.py index d5da114d458a..02bbfa95695f 100644 --- a/tests/storage/test_roommember.py +++ b/tests/storage/test_roommember.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- # Copyright 2014-2016 OpenMarket Ltd +# Copyright 2019 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. @@ -13,19 +14,13 @@ # See the License for the specific language governing permissions and # limitations under the License. - -from mock import Mock - -from twisted.internet import defer - from synapse.api.constants import EventTypes, Membership from synapse.api.room_versions import RoomVersions from synapse.rest.admin import register_servlets_for_client_rest_resource from synapse.rest.client.v1 import login, room -from synapse.types import Requester, RoomID, UserID +from synapse.types import Requester, UserID from tests import unittest -from tests.utils import create_room, setup_test_homeserver class RoomMemberStoreTestCase(unittest.HomeserverTestCase): From 21b811af9d33c078739d25077eaa3cc66dcf38b3 Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Thu, 5 Sep 2019 21:49:04 +1000 Subject: [PATCH 07/18] clean up the sql maybe? --- synapse/storage/roommember.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index 66dd174ceb28..b3d9f5326235 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -94,9 +94,11 @@ def _count_known_servers(self): def _transact(txn): query = """ - SELECT COUNT(DISTINCT substr(user_id, pos+1)) - FROM - (SELECT user_id, instr(user_id, ':') AS pos FROM room_memberships) + SELECT COUNT(DISTINCT substr(rm.user_id, pos+1)) + FROM ( + SELECT rm.user_id, instr(rm.user_id, ':') AS pos FROM room_memberships as rm + INNER JOIN current_state_events as c WHERE c.type = 'm.room.member' + ) """ txn.execute(query) return list(txn)[0][0] From 4415459d6e098a559619b12b073f2fa0c3f0beef Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Thu, 5 Sep 2019 21:50:16 +1000 Subject: [PATCH 08/18] clean up the sql maybe? --- synapse/storage/roommember.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index b3d9f5326235..d04876e6950f 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -94,9 +94,10 @@ def _count_known_servers(self): def _transact(txn): query = """ - SELECT COUNT(DISTINCT substr(rm.user_id, pos+1)) + SELECT COUNT(DISTINCT substr(user_id, pos+1)) FROM ( - SELECT rm.user_id, instr(rm.user_id, ':') AS pos FROM room_memberships as rm + SELECT rm.user_id as user_id, instr(rm.user_id, ':') + AS pos FROM room_memberships as rm INNER JOIN current_state_events as c WHERE c.type = 'm.room.member' ) """ From 911cc7d7dde8d60c528b4f99902b1e3e7739c2a6 Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Thu, 5 Sep 2019 21:53:05 +1000 Subject: [PATCH 09/18] update config --- docs/sample_config.yaml | 16 ++++++++-------- synapse/config/metrics.py | 16 ++++++++-------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 939048f3f8df..5485e9b28312 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -926,15 +926,15 @@ uploads_path: "DATADIR/uploads" #sentry: # dsn: "..." -# Flags to enable metrics which are not suitable to be enabled by -# default, either for performance reasons or limited use. +# Flags to enable Prometheus metrics which are not suitable to be +# enabled by default, either for performance reasons or limited use. # -# known_servers: Publish synapse_federation_known_servers, a gauge of -# the number of servers this homeserver knows about, including itself. -# May cause performance problems on large homeservers. -# -#metrics_flags: -# known_servers: true +metrics_flags: + # Publish synapse_federation_known_servers, a g auge of the number of + # servers this homeserver knows about, including itself. May cause + # performance problems on large homeservers. + # + #known_servers: true # Whether or not to report anonymized homeserver usage statistics. # report_stats: true|false diff --git a/synapse/config/metrics.py b/synapse/config/metrics.py index acfdc7a0644c..73f169cceaa6 100644 --- a/synapse/config/metrics.py +++ b/synapse/config/metrics.py @@ -78,15 +78,15 @@ def generate_config_section(self, report_stats=None, **kwargs): #sentry: # dsn: "..." - # Flags to enable metrics which are not suitable to be enabled by - # default, either for performance reasons or limited use. + # Flags to enable Prometheus metrics which are not suitable to be + # enabled by default, either for performance reasons or limited use. # - # known_servers: Publish synapse_federation_known_servers, a gauge of - # the number of servers this homeserver knows about, including itself. - # May cause performance problems on large homeservers. - # - #metrics_flags: - # known_servers: true + metrics_flags: + # Publish synapse_federation_known_servers, a g auge of the number of + # servers this homeserver knows about, including itself. May cause + # performance problems on large homeservers. + # + #known_servers: true # Whether or not to report anonymized homeserver usage statistics. """ From 130510cc608609ab2343838ff183cbc5d37c2329 Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Thu, 5 Sep 2019 22:10:59 +1000 Subject: [PATCH 10/18] update config --- synapse/storage/roommember.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index d04876e6950f..a40717f9ef25 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -98,7 +98,8 @@ def _transact(txn): FROM ( SELECT rm.user_id as user_id, instr(rm.user_id, ':') AS pos FROM room_memberships as rm - INNER JOIN current_state_events as c WHERE c.type = 'm.room.member' + INNER JOIN current_state_events as c ON rm.event_id = c.event_id + WHERE c.type = 'm.room.member' ) """ txn.execute(query) From 5967d872a35b7d308b2d3f53365ff893b2d73135 Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Thu, 5 Sep 2019 22:26:23 +1000 Subject: [PATCH 11/18] update config --- synapse/storage/roommember.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index a40717f9ef25..b1b36f29b1f6 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -94,13 +94,13 @@ def _count_known_servers(self): def _transact(txn): query = """ - SELECT COUNT(DISTINCT substr(user_id, pos+1)) + SELECT COUNT(DISTINCT substr(out.user_id, pos+1)) FROM ( SELECT rm.user_id as user_id, instr(rm.user_id, ':') AS pos FROM room_memberships as rm INNER JOIN current_state_events as c ON rm.event_id = c.event_id WHERE c.type = 'm.room.member' - ) + ) as out """ txn.execute(query) return list(txn)[0][0] From f5f750361e72e0ea8b460802bf56b2381a374263 Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Thu, 5 Sep 2019 22:44:57 +1000 Subject: [PATCH 12/18] update config --- synapse/storage/roommember.py | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index b1b36f29b1f6..a343f1864380 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -27,6 +27,7 @@ from synapse.metrics import LaterGauge from synapse.metrics.background_process_metrics import run_as_background_process from synapse.storage._base import LoggingTransaction +from synapse.storage.engines import Sqlite3Engine from synapse.storage.events_worker import EventsWorkerStore from synapse.types import get_domain_from_id from synapse.util.async_helpers import Linearizer @@ -93,15 +94,26 @@ def _count_known_servers(self): """ def _transact(txn): - query = """ - SELECT COUNT(DISTINCT substr(out.user_id, pos+1)) - FROM ( - SELECT rm.user_id as user_id, instr(rm.user_id, ':') - AS pos FROM room_memberships as rm - INNER JOIN current_state_events as c ON rm.event_id = c.event_id - WHERE c.type = 'm.room.member' - ) as out - """ + if isinstance(self.database_engine, Sqlite3Engine): + query = """ + SELECT COUNT(DISTINCT substr(out.user_id, pos+1)) + FROM ( + SELECT rm.user_id as user_id, instr(rm.user_id, ':') + AS pos FROM room_memberships as rm + INNER JOIN current_state_events as c ON rm.event_id = c.event_id + WHERE c.type = 'm.room.member' + ) as out + """ + else: + query = """ + SELECT COUNT(DISTINCT substring(out.user_id from pos+1)) + FROM ( + SELECT rm.user_id as user_id, position(rm.user_id, ':') + AS pos FROM room_memberships as rm + INNER JOIN current_state_events as c ON rm.event_id = c.event_id + WHERE c.type = 'm.room.member' + ) as out + """ txn.execute(query) return list(txn)[0][0] From fe328a843eebc13705cda88cb0ce06d5008f76c4 Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Fri, 6 Sep 2019 00:35:44 +1000 Subject: [PATCH 13/18] try erik's sql --- synapse/storage/roommember.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index a343f1864380..63bf874cec2a 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -106,13 +106,10 @@ def _transact(txn): """ else: query = """ - SELECT COUNT(DISTINCT substring(out.user_id from pos+1)) - FROM ( - SELECT rm.user_id as user_id, position(rm.user_id, ':') - AS pos FROM room_memberships as rm - INNER JOIN current_state_events as c ON rm.event_id = c.event_id - WHERE c.type = 'm.room.member' - ) as out + SELECT COUNT( + DISTINCT array_to_string((regexp_split_to_array(state_key, ':'))[2:], ':')) + FROM current_state_events + WHERE type = 'm.room.member' AND membership = 'join'; """ txn.execute(query) return list(txn)[0][0] From 1c609c2ec8238af6823c2eb8a063ed163c608d10 Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Fri, 6 Sep 2019 01:18:17 +1000 Subject: [PATCH 14/18] fix --- synapse/storage/roommember.py | 3 +-- tests/storage/test_roommember.py | 8 ++++++++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index 63bf874cec2a..4af393cfe89e 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -106,8 +106,7 @@ def _transact(txn): """ else: query = """ - SELECT COUNT( - DISTINCT array_to_string((regexp_split_to_array(state_key, ':'))[2:], ':')) + SELECT COUNT(DISTINCT array_to_string((regexp_split_to_array(state_key, ':'))[2:], ':')) FROM current_state_events WHERE type = 'm.room.member' AND membership = 'join'; """ diff --git a/tests/storage/test_roommember.py b/tests/storage/test_roommember.py index 02bbfa95695f..447a3c6ffb32 100644 --- a/tests/storage/test_roommember.py +++ b/tests/storage/test_roommember.py @@ -14,6 +14,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +from unittest.mock import Mock + from synapse.api.constants import EventTypes, Membership from synapse.api.room_versions import RoomVersions from synapse.rest.admin import register_servlets_for_client_rest_resource @@ -31,6 +33,12 @@ class RoomMemberStoreTestCase(unittest.HomeserverTestCase): room.register_servlets, ] + def make_homeserver(self, reactor, clock): + hs = self.setup_test_homeserver( + resource_for_federation=Mock(), http_client=None + ) + return hs + def prepare(self, reactor, clock, hs): # We can't test the RoomMemberStore on its own without the other event From 93a6e281de219e15162836487ed2faa65f00c577 Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Fri, 6 Sep 2019 01:46:13 +1000 Subject: [PATCH 15/18] dammit pg9.5 --- synapse/storage/roommember.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index 4af393cfe89e..5a3e19481803 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -106,7 +106,7 @@ def _transact(txn): """ else: query = """ - SELECT COUNT(DISTINCT array_to_string((regexp_split_to_array(state_key, ':'))[2:], ':')) + SELECT COUNT(DISTINCT split_part(state_key, ':', 2)) FROM current_state_events WHERE type = 'm.room.member' AND membership = 'join'; """ From dc90650b02df0dae59805ea3e5802c4ca696f91f Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Fri, 6 Sep 2019 20:28:47 +1000 Subject: [PATCH 16/18] run in background process --- synapse/storage/roommember.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index 5a3e19481803..134b81fbe39d 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -78,8 +78,18 @@ def __init__(self, db_conn, hs): if self.hs.config.metrics_flags.known_servers: self._known_servers_count = 1 - self.hs.get_clock().looping_call(self._count_known_servers, 60 * 1000) - self.hs.get_clock().call_later(1000, self._count_known_servers) + self.hs.get_clock().looping_call( + run_as_background_process, + 60 * 1000, + "_count_known_servers", + self._count_known_servers, + ) + self.hs.get_clock().call_later( + 1000, + run_as_background_process, + "_count_known_servers", + self._count_known_servers, + ) LaterGauge( "synapse_federation_known_servers", "", From d1b3715c2f6b283314da6bcbb5757ff84be79516 Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Fri, 6 Sep 2019 20:34:32 +1000 Subject: [PATCH 17/18] run in background process --- synapse/storage/roommember.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index 134b81fbe39d..4df8ebdacdbd 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -101,6 +101,9 @@ def __init__(self, db_conn, hs): def _count_known_servers(self): """ Count the servers that this server knows about. + + The statistic is stored on the class for the + `synapse_federation_known_servers` LaterGauge to collect. """ def _transact(txn): From d701fe4a5ae009f06c2d9b8320676db9381e352b Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Fri, 6 Sep 2019 21:27:56 +1000 Subject: [PATCH 18/18] test for loading None --- synapse/config/metrics.py | 3 ++- tests/config/test_generate.py | 25 ++++++++++++++----------- tests/config/test_load.py | 34 ++++++++++++++++++++++------------ 3 files changed, 38 insertions(+), 24 deletions(-) diff --git a/synapse/config/metrics.py b/synapse/config/metrics.py index 73f169cceaa6..653b990e67d8 100644 --- a/synapse/config/metrics.py +++ b/synapse/config/metrics.py @@ -43,7 +43,8 @@ def read_config(self, config, **kwargs): self.metrics_bind_host = config.get("metrics_bind_host", "127.0.0.1") if self.enable_metrics: - self.metrics_flags = MetricsFlags(**config.get("metrics_flags", {})) + _metrics_config = config.get("metrics_flags") or {} + self.metrics_flags = MetricsFlags(**_metrics_config) else: self.metrics_flags = MetricsFlags.all_off() diff --git a/tests/config/test_generate.py b/tests/config/test_generate.py index 5017cbce853b..2684e662de32 100644 --- a/tests/config/test_generate.py +++ b/tests/config/test_generate.py @@ -17,6 +17,8 @@ import re import shutil import tempfile +from contextlib import redirect_stdout +from io import StringIO from synapse.config.homeserver import HomeServerConfig @@ -32,17 +34,18 @@ def tearDown(self): shutil.rmtree(self.dir) def test_generate_config_generates_files(self): - HomeServerConfig.load_or_generate_config( - "", - [ - "--generate-config", - "-c", - self.file, - "--report-stats=yes", - "-H", - "lemurs.win", - ], - ) + with redirect_stdout(StringIO()): + HomeServerConfig.load_or_generate_config( + "", + [ + "--generate-config", + "-c", + self.file, + "--report-stats=yes", + "-H", + "lemurs.win", + ], + ) self.assertSetEqual( set(["homeserver.yaml", "lemurs.win.log.config", "lemurs.win.signing.key"]), diff --git a/tests/config/test_load.py b/tests/config/test_load.py index 6bfc1970ad57..b3e557bd6ab1 100644 --- a/tests/config/test_load.py +++ b/tests/config/test_load.py @@ -15,6 +15,8 @@ import os.path import shutil import tempfile +from contextlib import redirect_stdout +from io import StringIO import yaml @@ -26,7 +28,6 @@ class ConfigLoadingTestCase(unittest.TestCase): def setUp(self): self.dir = tempfile.mkdtemp() - print(self.dir) self.file = os.path.join(self.dir, "homeserver.yaml") def tearDown(self): @@ -94,18 +95,27 @@ def test_disable_registration(self): ) self.assertTrue(config.enable_registration) + def test_stats_enabled(self): + self.generate_config_and_remove_lines_containing("enable_metrics") + self.add_lines_to_config(["enable_metrics: true"]) + + # The default Metrics Flags are off by default. + config = HomeServerConfig.load_config("", ["-c", self.file]) + self.assertFalse(config.metrics_flags.known_servers) + def generate_config(self): - HomeServerConfig.load_or_generate_config( - "", - [ - "--generate-config", - "-c", - self.file, - "--report-stats=yes", - "-H", - "lemurs.win", - ], - ) + with redirect_stdout(StringIO()): + HomeServerConfig.load_or_generate_config( + "", + [ + "--generate-config", + "-c", + self.file, + "--report-stats=yes", + "-H", + "lemurs.win", + ], + ) def generate_config_and_remove_lines_containing(self, needle): self.generate_config()