From 0b07f02e19e5ed1eba0e74c3863def9f849b5f9a Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 28 Aug 2018 13:39:49 +0100 Subject: Make sure that we close db connections opened during init We should explicitly close any db connections we open, because failing to do so can block other transactions as per https://github.com/matrix-org/synapse/issues/3682. Let's also try to factor out some of the boilerplate by having server classes define their datastore class rather than duplicating the whole of `setup`. --- synapse/app/appservice.py | 5 +---- synapse/app/client_reader.py | 5 +---- synapse/app/event_creator.py | 5 +---- synapse/app/federation_reader.py | 5 +---- synapse/app/federation_sender.py | 5 +---- synapse/app/frontend_proxy.py | 5 +---- synapse/app/homeserver.py | 14 ++++++++------ synapse/app/media_repository.py | 5 +---- synapse/app/pusher.py | 5 +---- synapse/app/synchrotron.py | 5 +---- synapse/app/user_dir.py | 5 +---- 11 files changed, 18 insertions(+), 46 deletions(-) (limited to 'synapse/app') diff --git a/synapse/app/appservice.py b/synapse/app/appservice.py index 3348a8ec6d..86b5067400 100644 --- a/synapse/app/appservice.py +++ b/synapse/app/appservice.py @@ -51,10 +51,7 @@ class AppserviceSlaveStore( class AppserviceServer(HomeServer): - def setup(self): - logger.info("Setting up.") - self.datastore = AppserviceSlaveStore(self.get_db_conn(), self) - logger.info("Finished setting up.") + DATASTORE_CLASS = AppserviceSlaveStore def _listen_http(self, listener_config): port = listener_config["port"] diff --git a/synapse/app/client_reader.py b/synapse/app/client_reader.py index ab79a45646..ce2b113dbb 100644 --- a/synapse/app/client_reader.py +++ b/synapse/app/client_reader.py @@ -74,10 +74,7 @@ class ClientReaderSlavedStore( class ClientReaderServer(HomeServer): - def setup(self): - logger.info("Setting up.") - self.datastore = ClientReaderSlavedStore(self.get_db_conn(), self) - logger.info("Finished setting up.") + DATASTORE_CLASS = ClientReaderSlavedStore def _listen_http(self, listener_config): port = listener_config["port"] diff --git a/synapse/app/event_creator.py b/synapse/app/event_creator.py index a34c89fa99..f98e456ea0 100644 --- a/synapse/app/event_creator.py +++ b/synapse/app/event_creator.py @@ -90,10 +90,7 @@ class EventCreatorSlavedStore( class EventCreatorServer(HomeServer): - def setup(self): - logger.info("Setting up.") - self.datastore = EventCreatorSlavedStore(self.get_db_conn(), self) - logger.info("Finished setting up.") + DATASTORE_CLASS = EventCreatorSlavedStore def _listen_http(self, listener_config): port = listener_config["port"] diff --git a/synapse/app/federation_reader.py b/synapse/app/federation_reader.py index 7d8105778d..60f5973505 100644 --- a/synapse/app/federation_reader.py +++ b/synapse/app/federation_reader.py @@ -72,10 +72,7 @@ class FederationReaderSlavedStore( class FederationReaderServer(HomeServer): - def setup(self): - logger.info("Setting up.") - self.datastore = FederationReaderSlavedStore(self.get_db_conn(), self) - logger.info("Finished setting up.") + DATASTORE_CLASS = FederationReaderSlavedStore def _listen_http(self, listener_config): port = listener_config["port"] diff --git a/synapse/app/federation_sender.py b/synapse/app/federation_sender.py index d59007099b..60dd09aac3 100644 --- a/synapse/app/federation_sender.py +++ b/synapse/app/federation_sender.py @@ -78,10 +78,7 @@ class FederationSenderSlaveStore( class FederationSenderServer(HomeServer): - def setup(self): - logger.info("Setting up.") - self.datastore = FederationSenderSlaveStore(self.get_db_conn(), self) - logger.info("Finished setting up.") + DATASTORE_CLASS = FederationSenderSlaveStore def _listen_http(self, listener_config): port = listener_config["port"] diff --git a/synapse/app/frontend_proxy.py b/synapse/app/frontend_proxy.py index 8d484c1cd4..8c0b9c67b0 100644 --- a/synapse/app/frontend_proxy.py +++ b/synapse/app/frontend_proxy.py @@ -148,10 +148,7 @@ class FrontendProxySlavedStore( class FrontendProxyServer(HomeServer): - def setup(self): - logger.info("Setting up.") - self.datastore = FrontendProxySlavedStore(self.get_db_conn(), self) - logger.info("Finished setting up.") + DATASTORE_CLASS = FrontendProxySlavedStore def _listen_http(self, listener_config): port = listener_config["port"] diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 005921dcf7..3eb5b663de 100755 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -62,7 +62,7 @@ from synapse.rest.key.v1.server_key_resource import LocalKey from synapse.rest.key.v2 import KeyApiV2Resource from synapse.rest.media.v0.content_repository import ContentRepoResource from synapse.server import HomeServer -from synapse.storage import are_all_users_on_domain +from synapse.storage import DataStore, are_all_users_on_domain from synapse.storage.engines import IncorrectDatabaseSetup, create_engine from synapse.storage.prepare_database import UpgradeDatabaseException, prepare_database from synapse.util.caches import CACHE_SIZE_FACTOR @@ -111,6 +111,8 @@ def build_resource_for_web_client(hs): class SynapseHomeServer(HomeServer): + DATASTORE_CLASS = DataStore + def _listener_http(self, config, listener_config): port = listener_config["port"] bind_addresses = listener_config["bind_addresses"] @@ -356,13 +358,13 @@ def setup(config_options): logger.info("Preparing database: %s...", config.database_config['name']) try: - db_conn = hs.get_db_conn(run_new_connection=False) - prepare_database(db_conn, database_engine, config=config) - database_engine.on_new_connection(db_conn) + with hs.get_db_conn(run_new_connection=False) as db_conn: + prepare_database(db_conn, database_engine, config=config) + database_engine.on_new_connection(db_conn) - hs.run_startup_checks(db_conn, database_engine) + hs.run_startup_checks(db_conn, database_engine) - db_conn.commit() + db_conn.commit() except UpgradeDatabaseException: sys.stderr.write( "\nFailed to upgrade database.\n" diff --git a/synapse/app/media_repository.py b/synapse/app/media_repository.py index fd1f6cbf7e..e3dbb3b4e6 100644 --- a/synapse/app/media_repository.py +++ b/synapse/app/media_repository.py @@ -60,10 +60,7 @@ class MediaRepositorySlavedStore( class MediaRepositoryServer(HomeServer): - def setup(self): - logger.info("Setting up.") - self.datastore = MediaRepositorySlavedStore(self.get_db_conn(), self) - logger.info("Finished setting up.") + DATASTORE_CLASS = MediaRepositorySlavedStore def _listen_http(self, listener_config): port = listener_config["port"] diff --git a/synapse/app/pusher.py b/synapse/app/pusher.py index a4fc7e91fa..244c604de9 100644 --- a/synapse/app/pusher.py +++ b/synapse/app/pusher.py @@ -78,10 +78,7 @@ class PusherSlaveStore( class PusherServer(HomeServer): - def setup(self): - logger.info("Setting up.") - self.datastore = PusherSlaveStore(self.get_db_conn(), self) - logger.info("Finished setting up.") + DATASTORE_CLASS = PusherSlaveStore def remove_pusher(self, app_id, push_key, user_id): self.get_tcp_replication().send_remove_pusher(app_id, push_key, user_id) diff --git a/synapse/app/synchrotron.py b/synapse/app/synchrotron.py index 27e1998660..6662340797 100644 --- a/synapse/app/synchrotron.py +++ b/synapse/app/synchrotron.py @@ -249,10 +249,7 @@ class SynchrotronApplicationService(object): class SynchrotronServer(HomeServer): - def setup(self): - logger.info("Setting up.") - self.datastore = SynchrotronSlavedStore(self.get_db_conn(), self) - logger.info("Finished setting up.") + DATASTORE_CLASS = SynchrotronSlavedStore def _listen_http(self, listener_config): port = listener_config["port"] diff --git a/synapse/app/user_dir.py b/synapse/app/user_dir.py index 1388a42b59..96ffcaf073 100644 --- a/synapse/app/user_dir.py +++ b/synapse/app/user_dir.py @@ -94,10 +94,7 @@ class UserDirectorySlaveStore( class UserDirectoryServer(HomeServer): - def setup(self): - logger.info("Setting up.") - self.datastore = UserDirectorySlaveStore(self.get_db_conn(), self) - logger.info("Finished setting up.") + DATASTORE_CLASS = UserDirectorySlaveStore def _listen_http(self, listener_config): port = listener_config["port"] -- cgit 1.5.1 From 0ddf48672421aa64920342b959dc77432c869889 Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Wed, 12 Sep 2018 11:58:52 +0100 Subject: expose number of real reserved users --- synapse/app/homeserver.py | 10 +++++++--- synapse/storage/monthly_active_users.py | 17 ++++++++++++++++ tests/storage/test_monthly_active_users.py | 31 ++++++++++++++++++++++++++++++ 3 files changed, 55 insertions(+), 3 deletions(-) (limited to 'synapse/app') diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 3eb5b663de..e6372bcec2 100755 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -307,6 +307,7 @@ class SynapseHomeServer(HomeServer): # Gauges to expose monthly active user control metrics current_mau_gauge = Gauge("synapse_admin_mau:current", "Current MAU") max_mau_gauge = Gauge("synapse_admin_mau:max", "MAU Limit") +reserved_mau_gauge = Gauge("synapse_admin_mau:reserved", "Reserved real MAU users") def setup(config_options): @@ -531,10 +532,13 @@ def run(hs): @defer.inlineCallbacks def generate_monthly_active_users(): - count = 0 + current_mau_count = 0 + reserved_mau_count = 0 if hs.config.limit_usage_by_mau: - count = yield hs.get_datastore().get_monthly_active_count() - current_mau_gauge.set(float(count)) + current_mau_count = yield hs.get_datastore().get_monthly_active_count() + reserved_mau_count = yield hs.get_datastore().get_reserved_real_user_account() + current_mau_gauge.set(float(current_mau_count)) + reserved_mau_gauge.set(float(reserved_mau_count)) max_mau_gauge.set(float(hs.config.max_mau_value)) hs.get_datastore().initialise_reserved_users( diff --git a/synapse/storage/monthly_active_users.py b/synapse/storage/monthly_active_users.py index b890c152db..53d125d305 100644 --- a/synapse/storage/monthly_active_users.py +++ b/synapse/storage/monthly_active_users.py @@ -146,6 +146,23 @@ class MonthlyActiveUsersStore(SQLBaseStore): return count return self.runInteraction("count_users", _count_users) + @defer.inlineCallbacks + def get_reserved_real_user_account(self): + """Of the reserved threepids defined in config, how many are associated + with registered users? + + Returns: + Defered[int]: Number of real reserved users + """ + count = 0 + for tp in self.hs.config.mau_limits_reserved_threepids: + user_id = yield self.hs.get_datastore().get_user_id_by_threepid( + tp["medium"], tp["address"] + ) + if user_id: + count = count + 1 + defer.returnValue(count) + @defer.inlineCallbacks def upsert_monthly_active_user(self, user_id): """ diff --git a/tests/storage/test_monthly_active_users.py b/tests/storage/test_monthly_active_users.py index ccfc21b7fc..662f2ed845 100644 --- a/tests/storage/test_monthly_active_users.py +++ b/tests/storage/test_monthly_active_users.py @@ -183,3 +183,34 @@ class MonthlyActiveUsersTestCase(HomeserverTestCase): self.store.populate_monthly_active_users('user_id') self.pump() self.store.upsert_monthly_active_user.assert_not_called() + + def test_get_reserved_real_user_account(self): + # Test no reserved users, or reserved threepids + count = self.store.get_reserved_real_user_account() + self.assertEquals(self.get_success(count), 0) + # Test reserved users but no registered users + + user1 = '@user1:example.com' + user2 = '@user2:example.com' + user1_email = 'user1@example.com' + user2_email = 'user2@example.com' + threepids = [ + {'medium': 'email', 'address': user1_email}, + {'medium': 'email', 'address': user2_email}, + ] + self.hs.config.mau_limits_reserved_threepids = threepids + self.store.initialise_reserved_users(threepids) + self.pump() + count = self.store.get_reserved_real_user_account() + self.assertEquals(self.get_success(count), 0) + + # Test reserved registed users + self.store.register(user_id=user1, token="123", password_hash=None) + self.store.register(user_id=user2, token="456", password_hash=None) + self.pump() + + now = int(self.hs.get_clock().time_msec()) + self.store.user_add_threepid(user1, "email", user1_email, now, now) + self.store.user_add_threepid(user2, "email", user2_email, now, now) + count = self.store.get_reserved_real_user_account() + self.assertEquals(self.get_success(count), len(threepids)) -- cgit 1.5.1 From 8decd6233dabd87160794949ffd95282e60ab01e Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Wed, 12 Sep 2018 16:22:15 +0100 Subject: improve naming --- changelog.d/3846.feature | 2 +- synapse/app/homeserver.py | 14 +++++++++----- synapse/storage/monthly_active_users.py | 2 +- tests/storage/test_monthly_active_users.py | 6 +++--- 4 files changed, 14 insertions(+), 10 deletions(-) (limited to 'synapse/app') diff --git a/changelog.d/3846.feature b/changelog.d/3846.feature index dc9e9d86cc..453c11d3f8 100644 --- a/changelog.d/3846.feature +++ b/changelog.d/3846.feature @@ -1 +1 @@ -create synapse_admin_mau:reserved metric to expose number of real reaserved users +Add synapse_admin_mau:registered_reserved_users metric to expose number of real reaserved users diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index e6372bcec2..ac97e19649 100755 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -307,7 +307,10 @@ class SynapseHomeServer(HomeServer): # Gauges to expose monthly active user control metrics current_mau_gauge = Gauge("synapse_admin_mau:current", "Current MAU") max_mau_gauge = Gauge("synapse_admin_mau:max", "MAU Limit") -reserved_mau_gauge = Gauge("synapse_admin_mau:reserved", "Reserved real MAU users") +registered_reserved_users_mau_gauge = Gauge( + "synapse_admin_mau:registered_reserved_users", + "Registered users with reserved threepids" +) def setup(config_options): @@ -533,12 +536,13 @@ def run(hs): @defer.inlineCallbacks def generate_monthly_active_users(): current_mau_count = 0 - reserved_mau_count = 0 + reserved_count = 0 + store = hs.get_datastore() if hs.config.limit_usage_by_mau: - current_mau_count = yield hs.get_datastore().get_monthly_active_count() - reserved_mau_count = yield hs.get_datastore().get_reserved_real_user_account() + current_mau_count = yield store.get_monthly_active_count() + reserved_count = yield store.get_registered_reserved_users_count() current_mau_gauge.set(float(current_mau_count)) - reserved_mau_gauge.set(float(reserved_mau_count)) + registered_reserved_users_mau_gauge.set(float(reserved_count)) max_mau_gauge.set(float(hs.config.max_mau_value)) hs.get_datastore().initialise_reserved_users( diff --git a/synapse/storage/monthly_active_users.py b/synapse/storage/monthly_active_users.py index 53d125d305..59580949f1 100644 --- a/synapse/storage/monthly_active_users.py +++ b/synapse/storage/monthly_active_users.py @@ -147,7 +147,7 @@ class MonthlyActiveUsersStore(SQLBaseStore): return self.runInteraction("count_users", _count_users) @defer.inlineCallbacks - def get_reserved_real_user_account(self): + def get_registered_reserved_users_count(self): """Of the reserved threepids defined in config, how many are associated with registered users? diff --git a/tests/storage/test_monthly_active_users.py b/tests/storage/test_monthly_active_users.py index 662f2ed845..686f12a0dc 100644 --- a/tests/storage/test_monthly_active_users.py +++ b/tests/storage/test_monthly_active_users.py @@ -186,7 +186,7 @@ class MonthlyActiveUsersTestCase(HomeserverTestCase): def test_get_reserved_real_user_account(self): # Test no reserved users, or reserved threepids - count = self.store.get_reserved_real_user_account() + count = self.store.get_registered_reserved_users_count() self.assertEquals(self.get_success(count), 0) # Test reserved users but no registered users @@ -201,7 +201,7 @@ class MonthlyActiveUsersTestCase(HomeserverTestCase): self.hs.config.mau_limits_reserved_threepids = threepids self.store.initialise_reserved_users(threepids) self.pump() - count = self.store.get_reserved_real_user_account() + count = self.store.get_registered_reserved_users_count() self.assertEquals(self.get_success(count), 0) # Test reserved registed users @@ -212,5 +212,5 @@ class MonthlyActiveUsersTestCase(HomeserverTestCase): now = int(self.hs.get_clock().time_msec()) self.store.user_add_threepid(user1, "email", user1_email, now, now) self.store.user_add_threepid(user2, "email", user2_email, now, now) - count = self.store.get_reserved_real_user_account() + count = self.store.get_registered_reserved_users_count() self.assertEquals(self.get_success(count), len(threepids)) -- cgit 1.5.1