From 985953f5287fb791c976ab16bee481c8e3280881 Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Fri, 4 Oct 2019 12:05:24 +0100 Subject: [PATCH 1/7] fix misshandling mau reaping where reserved users are present --- synapse/storage/monthly_active_users.py | 58 +++++++++++++++------- tests/storage/test_monthly_active_users.py | 49 ++++++++++++++++++ 2 files changed, 89 insertions(+), 18 deletions(-) diff --git a/synapse/storage/monthly_active_users.py b/synapse/storage/monthly_active_users.py index 752e9788a29f..0df211925897 100644 --- a/synapse/storage/monthly_active_users.py +++ b/synapse/storage/monthly_active_users.py @@ -97,6 +97,7 @@ def _reap_users(txn): txn.execute(sql, query_args) + max_mau_value = self.hs.config.max_mau_value if self.hs.config.limit_usage_by_mau: # If MAU user count still exceeds the MAU threshold, then delete on # a least recently active basis. @@ -106,29 +107,44 @@ def _reap_users(txn): # While Postgres does not require 'LIMIT', but also does not support # negative LIMIT values. So there is no way to write it that both can # support - safe_guard = self.hs.config.max_mau_value - len(self.reserved_users) - # Must be greater than zero for postgres - safe_guard = safe_guard if safe_guard > 0 else 0 - query_args = [safe_guard] - - base_sql = """ - DELETE FROM monthly_active_users - WHERE user_id NOT IN ( - SELECT user_id FROM monthly_active_users - ORDER BY timestamp DESC - LIMIT ? + if len(self.reserved_users) == 0: + sql = """ + DELETE FROM monthly_active_users + WHERE user_id NOT IN ( + SELECT user_id FROM monthly_active_users + ORDER BY timestamp DESC + LIMIT ? ) - """ + """ + txn.execute(sql, (max_mau_value,)) # Need if/else since 'AND user_id NOT IN ({})' fails on Postgres # when len(reserved_users) == 0. Works fine on sqlite. - if len(self.reserved_users) > 0: - query_args.extend(self.reserved_users) - sql = base_sql + """ AND user_id NOT IN ({})""".format( + else: + safe_guard = max_mau_value - len(self.reserved_users) + # Must be greater than zero for postgres + safe_guard = safe_guard if safe_guard > 0 else 0 + # It is important to filter reserved users twice to guard + # against the case where the reserved user is present in the + # SELECT, meaning that a legitmate mau is deleted. + sql = """ + DELETE FROM monthly_active_users + WHERE user_id NOT IN ( + SELECT user_id FROM monthly_active_users + WHERE user_id NOT IN ({})""".format( + ",".join(questionmarks) + ) + """ + ORDER BY timestamp DESC + LIMIT ? + ) + AND user_id NOT IN ({})""".format( ",".join(questionmarks) ) - else: - sql = base_sql - txn.execute(sql, query_args) + query_args = [] + query_args.extend(self.reserved_users) + query_args.append(safe_guard) + query_args.extend(self.reserved_users) + + txn.execute(sql, query_args) yield self.runInteraction("reap_monthly_active_users", _reap_users) # It seems poor to invalidate the whole cache, Postgres supports @@ -167,12 +183,18 @@ def get_registered_reserved_users_count(self): Defered[int]: Number of real reserved users """ count = 0 + users = () 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 + users = users + (user_id,) + + # Update reserved_users to ensure it stays in sync, this is important + # for reaping. + self.reserved_users = users return count @defer.inlineCallbacks diff --git a/tests/storage/test_monthly_active_users.py b/tests/storage/test_monthly_active_users.py index 1494650d10be..62aa2bfed786 100644 --- a/tests/storage/test_monthly_active_users.py +++ b/tests/storage/test_monthly_active_users.py @@ -158,6 +158,54 @@ def test_reap_monthly_active_users(self): count = self.store.get_monthly_active_count() self.assertEquals(self.get_success(count), 0) + def test_reap_monthly_active_users_reserved_users(self): + """ Tests that reaping correctly handles reaping where reserved users are + present""" + + self.hs.config.max_mau_value = 5 + initial_users = 5 + reserved_user_number = initial_users - 1 + threepids = [] + for i in range(initial_users): + user = "@user%d:server" % i + email = "user%d@example.com" % i + self.store.upsert_monthly_active_user(user) + threepids.append({"medium": "email", "address": email},) + # Need to ensure that the most recent entries in the + # monthly_active_users table are reserved + now = int(self.hs.get_clock().time_msec()) + if i != 0: + self.store.register_user(user_id=user, password_hash=None) + self.pump() + self.store.user_add_threepid(user, "email", email, now, now) + + self.hs.config.mau_limits_reserved_threepids = threepids + self.store.runInteraction( + "initialise", self.store._initialise_reserved_users, threepids + ) + self.pump() + count = self.store.get_monthly_active_count() + self.assertTrue(self.get_success(count), initial_users) + + count = self.store.get_registered_reserved_users_count() + self.assertEquals(self.get_success(count), reserved_user_number) + + self.store.reap_monthly_active_users() + self.pump() + count = self.store.get_monthly_active_count() + self.assertEquals( + self.get_success(count), self.hs.config.max_mau_value + ) + + def _populate_reserved_users(self, user, email): + """Helper function that registers userpopulates reserved users""" + # Test reserved registed users + self.store.register_user(user_id=user, password_hash=None) + self.pump() + + now = int(self.hs.get_clock().time_msec()) + self.store.user_add_threepid(user, "email", email, now, now) + def test_populate_monthly_users_is_guest(self): # Test that guest users are not added to mau list user_id = "@user_id:host" @@ -198,6 +246,7 @@ def test_get_reserved_real_user_account(self): user1 = "@user1:example.com" user2 = "@user2:example.com" + user1_email = "user1@example.com" user2_email = "user2@example.com" threepids = [ From 0d146cb0ee161da3d3b89c7dc4f2580e7b334c60 Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Fri, 4 Oct 2019 12:07:02 +0100 Subject: [PATCH 2/7] fix logic and remove unused code --- tests/storage/test_monthly_active_users.py | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/tests/storage/test_monthly_active_users.py b/tests/storage/test_monthly_active_users.py index 62aa2bfed786..f2cca0f5c2eb 100644 --- a/tests/storage/test_monthly_active_users.py +++ b/tests/storage/test_monthly_active_users.py @@ -148,7 +148,7 @@ def test_reap_monthly_active_users(self): self.pump() count = self.store.get_monthly_active_count() self.assertEquals( - self.get_success(count), initial_users - self.hs.config.max_mau_value + self.get_success(count), self.hs.config.max_mau_value ) self.reactor.advance(FORTY_DAYS) @@ -197,15 +197,6 @@ def test_reap_monthly_active_users_reserved_users(self): self.get_success(count), self.hs.config.max_mau_value ) - def _populate_reserved_users(self, user, email): - """Helper function that registers userpopulates reserved users""" - # Test reserved registed users - self.store.register_user(user_id=user, password_hash=None) - self.pump() - - now = int(self.hs.get_clock().time_msec()) - self.store.user_add_threepid(user, "email", email, now, now) - def test_populate_monthly_users_is_guest(self): # Test that guest users are not added to mau list user_id = "@user_id:host" From 760688e667b09cbf9ad5a1fe59743bb07f628136 Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Fri, 4 Oct 2019 12:27:27 +0100 Subject: [PATCH 3/7] towncrier --- changelog.d/6168.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/6168.bugfix diff --git a/changelog.d/6168.bugfix b/changelog.d/6168.bugfix new file mode 100644 index 000000000000..39e8e9d019e2 --- /dev/null +++ b/changelog.d/6168.bugfix @@ -0,0 +1 @@ +Fix monthly active user reaping where reserved users are specified. From 4b0cf830aa3ffec4fe17dd992dd860c864767491 Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Fri, 4 Oct 2019 13:43:23 +0100 Subject: [PATCH 4/7] black --- tests/storage/test_monthly_active_users.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/tests/storage/test_monthly_active_users.py b/tests/storage/test_monthly_active_users.py index f2cca0f5c2eb..b7a90cbbf60a 100644 --- a/tests/storage/test_monthly_active_users.py +++ b/tests/storage/test_monthly_active_users.py @@ -147,9 +147,7 @@ def test_reap_monthly_active_users(self): self.store.reap_monthly_active_users() self.pump() count = self.store.get_monthly_active_count() - self.assertEquals( - self.get_success(count), self.hs.config.max_mau_value - ) + self.assertEquals(self.get_success(count), self.hs.config.max_mau_value) self.reactor.advance(FORTY_DAYS) self.store.reap_monthly_active_users() @@ -170,7 +168,7 @@ def test_reap_monthly_active_users_reserved_users(self): user = "@user%d:server" % i email = "user%d@example.com" % i self.store.upsert_monthly_active_user(user) - threepids.append({"medium": "email", "address": email},) + threepids.append({"medium": "email", "address": email}) # Need to ensure that the most recent entries in the # monthly_active_users table are reserved now = int(self.hs.get_clock().time_msec()) @@ -193,9 +191,7 @@ def test_reap_monthly_active_users_reserved_users(self): self.store.reap_monthly_active_users() self.pump() count = self.store.get_monthly_active_count() - self.assertEquals( - self.get_success(count), self.hs.config.max_mau_value - ) + self.assertEquals(self.get_success(count), self.hs.config.max_mau_value) def test_populate_monthly_users_is_guest(self): # Test that guest users are not added to mau list From 2ffa761c5effae808a9ed1171f5c9764f094e03f Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Wed, 9 Oct 2019 22:15:02 +0100 Subject: [PATCH 5/7] respond to review comments --- synapse/storage/monthly_active_users.py | 26 ++++++++-------------- tests/storage/test_monthly_active_users.py | 14 ++++++------ 2 files changed, 16 insertions(+), 24 deletions(-) diff --git a/synapse/storage/monthly_active_users.py b/synapse/storage/monthly_active_users.py index 0df211925897..9ca7d9b5d38c 100644 --- a/synapse/storage/monthly_active_users.py +++ b/synapse/storage/monthly_active_users.py @@ -86,12 +86,10 @@ def _reap_users(txn): if len(self.reserved_users) > 0: # questionmarks is a hack to overcome sqlite not supporting # tuples in 'WHERE IN %s' - questionmarks = "?" * len(self.reserved_users) + question_marks = ",".join("?" * len(self.reserved_users)) query_args.extend(self.reserved_users) - sql = base_sql + """ AND user_id NOT IN ({})""".format( - ",".join(questionmarks) - ) + sql = base_sql + """ AND user_id NOT IN ({})""".format(question_marks) else: sql = base_sql @@ -120,9 +118,9 @@ def _reap_users(txn): # Need if/else since 'AND user_id NOT IN ({})' fails on Postgres # when len(reserved_users) == 0. Works fine on sqlite. else: - safe_guard = max_mau_value - len(self.reserved_users) - # Must be greater than zero for postgres - safe_guard = safe_guard if safe_guard > 0 else 0 + # Must be >= 0 for postgres + num_of_non_reserved_users_to_remove = max(max_mau_value - len(self.reserved_users), 0) + # It is important to filter reserved users twice to guard # against the case where the reserved user is present in the # SELECT, meaning that a legitmate mau is deleted. @@ -130,19 +128,13 @@ def _reap_users(txn): DELETE FROM monthly_active_users WHERE user_id NOT IN ( SELECT user_id FROM monthly_active_users - WHERE user_id NOT IN ({})""".format( - ",".join(questionmarks) - ) + """ + WHERE user_id NOT IN ({}) ORDER BY timestamp DESC LIMIT ? ) - AND user_id NOT IN ({})""".format( - ",".join(questionmarks) - ) - query_args = [] - query_args.extend(self.reserved_users) - query_args.append(safe_guard) - query_args.extend(self.reserved_users) + AND user_id NOT IN ({})""".format(question_marks, question_marks) + + query_args = [*self.reserved_users, num_of_non_reserved_users_to_remove, *self.reserved_users] txn.execute(sql, query_args) diff --git a/tests/storage/test_monthly_active_users.py b/tests/storage/test_monthly_active_users.py index b7a90cbbf60a..93a5be594eca 100644 --- a/tests/storage/test_monthly_active_users.py +++ b/tests/storage/test_monthly_active_users.py @@ -167,29 +167,29 @@ def test_reap_monthly_active_users_reserved_users(self): for i in range(initial_users): user = "@user%d:server" % i email = "user%d@example.com" % i - self.store.upsert_monthly_active_user(user) + self.get_success(self.store.upsert_monthly_active_user(user)) threepids.append({"medium": "email", "address": email}) # Need to ensure that the most recent entries in the # monthly_active_users table are reserved now = int(self.hs.get_clock().time_msec()) if i != 0: - self.store.register_user(user_id=user, password_hash=None) - self.pump() - self.store.user_add_threepid(user, "email", email, now, now) + self.get_success(self.store.register_user(user_id=user, password_hash=None)) + #self.pump() + self.get_success(self.store.user_add_threepid(user, "email", email, now, now)) self.hs.config.mau_limits_reserved_threepids = threepids self.store.runInteraction( "initialise", self.store._initialise_reserved_users, threepids ) - self.pump() + #self.pump() count = self.store.get_monthly_active_count() self.assertTrue(self.get_success(count), initial_users) count = self.store.get_registered_reserved_users_count() self.assertEquals(self.get_success(count), reserved_user_number) - self.store.reap_monthly_active_users() - self.pump() + self.get_success(self.store.reap_monthly_active_users()) + #self.pump() count = self.store.get_monthly_active_count() self.assertEquals(self.get_success(count), self.hs.config.max_mau_value) From df2833fadf356e63ab9b5c2bb292640de1ac1f2c Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Wed, 9 Oct 2019 23:13:46 +0100 Subject: [PATCH 6/7] respond to review comments --- synapse/app/homeserver.py | 6 +-- synapse/storage/monthly_active_users.py | 58 ++++++++++++---------- tests/storage/test_monthly_active_users.py | 30 ++++++----- 3 files changed, 53 insertions(+), 41 deletions(-) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 774326dff9ec..eb54f5685347 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -605,13 +605,13 @@ def reap_monthly_active_users(): @defer.inlineCallbacks def generate_monthly_active_users(): current_mau_count = 0 - reserved_count = 0 + reserved_users = () store = hs.get_datastore() if hs.config.limit_usage_by_mau or hs.config.mau_stats_only: current_mau_count = yield store.get_monthly_active_count() - reserved_count = yield store.get_registered_reserved_users_count() + reserved_users = yield store.get_registered_reserved_users() current_mau_gauge.set(float(current_mau_count)) - registered_reserved_users_mau_gauge.set(float(reserved_count)) + registered_reserved_users_mau_gauge.set(float(len(reserved_users))) max_mau_gauge.set(float(hs.config.max_mau_value)) def start_generate_monthly_active_users(): diff --git a/synapse/storage/monthly_active_users.py b/synapse/storage/monthly_active_users.py index 9ca7d9b5d38c..584cb8878982 100644 --- a/synapse/storage/monthly_active_users.py +++ b/synapse/storage/monthly_active_users.py @@ -32,7 +32,6 @@ def __init__(self, dbconn, hs): super(MonthlyActiveUsersStore, self).__init__(None, hs) self._clock = hs.get_clock() self.hs = hs - self.reserved_users = () # Do not add more reserved users than the total allowable number self._new_transaction( dbconn, @@ -51,7 +50,6 @@ def _initialise_reserved_users(self, txn, threepids): txn (cursor): threepids (list[dict]): List of threepid dicts to reserve """ - reserved_user_list = [] for tp in threepids: user_id = self.get_user_id_by_threepid_txn(txn, tp["medium"], tp["address"]) @@ -60,10 +58,8 @@ def _initialise_reserved_users(self, txn, threepids): is_support = self.is_support_user_txn(txn, user_id) if not is_support: self.upsert_monthly_active_user_txn(txn, user_id) - reserved_user_list.append(user_id) else: logger.warning("mau limit reserved threepid %s not found in db" % tp) - self.reserved_users = tuple(reserved_user_list) @defer.inlineCallbacks def reap_monthly_active_users(self): @@ -74,8 +70,11 @@ def reap_monthly_active_users(self): Deferred[] """ - def _reap_users(txn): - # Purge stale users + def _reap_users(txn, reserved_users): + """ + Args: + reserved_users (tuple): reserved users to preserve + """ thirty_days_ago = int(self._clock.time_msec()) - (1000 * 60 * 60 * 24 * 30) query_args = [thirty_days_ago] @@ -83,12 +82,12 @@ def _reap_users(txn): # Need if/else since 'AND user_id NOT IN ({})' fails on Postgres # when len(reserved_users) == 0. Works fine on sqlite. - if len(self.reserved_users) > 0: + if len(reserved_users) > 0: # questionmarks is a hack to overcome sqlite not supporting # tuples in 'WHERE IN %s' - question_marks = ",".join("?" * len(self.reserved_users)) + question_marks = ",".join("?" * len(reserved_users)) - query_args.extend(self.reserved_users) + query_args.extend(reserved_users) sql = base_sql + """ AND user_id NOT IN ({})""".format(question_marks) else: sql = base_sql @@ -105,7 +104,7 @@ def _reap_users(txn): # While Postgres does not require 'LIMIT', but also does not support # negative LIMIT values. So there is no way to write it that both can # support - if len(self.reserved_users) == 0: + if len(reserved_users) == 0: sql = """ DELETE FROM monthly_active_users WHERE user_id NOT IN ( @@ -119,8 +118,10 @@ def _reap_users(txn): # when len(reserved_users) == 0. Works fine on sqlite. else: # Must be >= 0 for postgres - num_of_non_reserved_users_to_remove = max(max_mau_value - len(self.reserved_users), 0) - + num_of_non_reserved_users_to_remove = max( + max_mau_value - len(reserved_users), 0 + ) + # It is important to filter reserved users twice to guard # against the case where the reserved user is present in the # SELECT, meaning that a legitmate mau is deleted. @@ -132,13 +133,22 @@ def _reap_users(txn): ORDER BY timestamp DESC LIMIT ? ) - AND user_id NOT IN ({})""".format(question_marks, question_marks) + AND user_id NOT IN ({})""".format( + question_marks, question_marks + ) - query_args = [*self.reserved_users, num_of_non_reserved_users_to_remove, *self.reserved_users] + query_args = [ + *reserved_users, + num_of_non_reserved_users_to_remove, + *reserved_users, + ] txn.execute(sql, query_args) - yield self.runInteraction("reap_monthly_active_users", _reap_users) + reserved_users = yield self.get_registered_reserved_users() + yield self.runInteraction( + "reap_monthly_active_users", _reap_users, reserved_users + ) # It seems poor to invalidate the whole cache, Postgres supports # 'Returning' which would allow me to invalidate only the # specific users, but sqlite has no way to do this and instead @@ -167,27 +177,25 @@ def _count_users(txn): return self.runInteraction("count_users", _count_users) @defer.inlineCallbacks - def get_registered_reserved_users_count(self): - """Of the reserved threepids defined in config, how many are associated + def get_registered_reserved_users(self): + """Of the reserved threepids defined in config, which are associated with registered users? Returns: - Defered[int]: Number of real reserved users + Defered[tuple]: Real reserved users """ - count = 0 users = () - for tp in self.hs.config.mau_limits_reserved_threepids: + + for tp in self.hs.config.mau_limits_reserved_threepids[ + : self.hs.config.max_mau_value + ]: user_id = yield self.hs.get_datastore().get_user_id_by_threepid( tp["medium"], tp["address"] ) if user_id: - count = count + 1 users = users + (user_id,) - # Update reserved_users to ensure it stays in sync, this is important - # for reaping. - self.reserved_users = users - return count + return users @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 93a5be594eca..90a63dc47784 100644 --- a/tests/storage/test_monthly_active_users.py +++ b/tests/storage/test_monthly_active_users.py @@ -50,6 +50,7 @@ def test_initialise_reserved_users(self): {"medium": "email", "address": user2_email}, {"medium": "email", "address": user3_email}, ] + self.hs.config.mau_limits_reserved_threepids = threepids # -1 because user3 is a support user and does not count user_num = len(threepids) - 1 @@ -84,6 +85,7 @@ def test_initialise_reserved_users(self): self.hs.config.max_mau_value = 0 self.reactor.advance(FORTY_DAYS) + self.hs.config.max_mau_value = 5 self.store.reap_monthly_active_users() self.pump() @@ -173,23 +175,24 @@ def test_reap_monthly_active_users_reserved_users(self): # monthly_active_users table are reserved now = int(self.hs.get_clock().time_msec()) if i != 0: - self.get_success(self.store.register_user(user_id=user, password_hash=None)) - #self.pump() - self.get_success(self.store.user_add_threepid(user, "email", email, now, now)) + self.get_success( + self.store.register_user(user_id=user, password_hash=None) + ) + self.get_success( + self.store.user_add_threepid(user, "email", email, now, now) + ) self.hs.config.mau_limits_reserved_threepids = threepids self.store.runInteraction( "initialise", self.store._initialise_reserved_users, threepids ) - #self.pump() count = self.store.get_monthly_active_count() self.assertTrue(self.get_success(count), initial_users) - count = self.store.get_registered_reserved_users_count() - self.assertEquals(self.get_success(count), reserved_user_number) + users = self.store.get_registered_reserved_users() + self.assertEquals(len(self.get_success(users)), reserved_user_number) self.get_success(self.store.reap_monthly_active_users()) - #self.pump() count = self.store.get_monthly_active_count() self.assertEquals(self.get_success(count), self.hs.config.max_mau_value) @@ -227,8 +230,8 @@ def test_populate_monthly_users_should_not_update(self): def test_get_reserved_real_user_account(self): # Test no reserved users, or reserved threepids - count = self.store.get_registered_reserved_users_count() - self.assertEquals(self.get_success(count), 0) + users = self.get_success(self.store.get_registered_reserved_users()) + self.assertEquals(len(users), 0) # Test reserved users but no registered users user1 = "@user1:example.com" @@ -246,8 +249,8 @@ def test_get_reserved_real_user_account(self): ) self.pump() - count = self.store.get_registered_reserved_users_count() - self.assertEquals(self.get_success(count), 0) + users = self.get_success(self.store.get_registered_reserved_users()) + self.assertEquals(len(users), 0) # Test reserved registed users self.store.register_user(user_id=user1, password_hash=None) @@ -257,8 +260,9 @@ def test_get_reserved_real_user_account(self): 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_registered_reserved_users_count() - self.assertEquals(self.get_success(count), len(threepids)) + + users = self.get_success(self.store.get_registered_reserved_users()) + self.assertEquals(len(users), len(threepids)) def test_support_user_not_add_to_mau_limits(self): support_user_id = "@support:test" From f9e74ebfbd2c4f6c189f0d3d49892679f673c8e2 Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Thu, 10 Oct 2019 21:51:27 +0100 Subject: [PATCH 7/7] respond to review comments --- synapse/storage/monthly_active_users.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/synapse/storage/monthly_active_users.py b/synapse/storage/monthly_active_users.py index 584cb8878982..3803604be763 100644 --- a/synapse/storage/monthly_active_users.py +++ b/synapse/storage/monthly_active_users.py @@ -88,7 +88,7 @@ def _reap_users(txn, reserved_users): question_marks = ",".join("?" * len(reserved_users)) query_args.extend(reserved_users) - sql = base_sql + """ AND user_id NOT IN ({})""".format(question_marks) + sql = base_sql + " AND user_id NOT IN ({})".format(question_marks) else: sql = base_sql @@ -133,7 +133,8 @@ def _reap_users(txn, reserved_users): ORDER BY timestamp DESC LIMIT ? ) - AND user_id NOT IN ({})""".format( + AND user_id NOT IN ({}) + """.format( question_marks, question_marks ) @@ -182,9 +183,9 @@ def get_registered_reserved_users(self): with registered users? Returns: - Defered[tuple]: Real reserved users + Defered[list]: Real reserved users """ - users = () + users = [] for tp in self.hs.config.mau_limits_reserved_threepids[ : self.hs.config.max_mau_value @@ -193,7 +194,7 @@ def get_registered_reserved_users(self): tp["medium"], tp["address"] ) if user_id: - users = users + (user_id,) + users.append(user_id) return users