diff options
| -rw-r--r-- | synapse/storage/monthly_active_users.py | 26 | ||||
| -rw-r--r-- | 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 0df2119258..9ca7d9b5d3 100644 --- a/synapse/storage/monthly_active_users.py +++ b/synapse/storage/monthly_active_users.py @@ -86,12 +86,10 @@ class MonthlyActiveUsersStore(SQLBaseStore): 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 @@ class MonthlyActiveUsersStore(SQLBaseStore): # 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 @@ class MonthlyActiveUsersStore(SQLBaseStore): 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 b7a90cbbf6..93a5be594e 100644 --- a/tests/storage/test_monthly_active_users.py +++ b/tests/storage/test_monthly_active_users.py @@ -167,29 +167,29 @@ class MonthlyActiveUsersTestCase(unittest.HomeserverTestCase): 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) |