mirror of
https://github.com/element-hq/synapse.git
synced 2024-12-22 20:50:23 +03:00
respond to review comments
This commit is contained in:
parent
2ffa761c5e
commit
df2833fadf
3 changed files with 53 additions and 41 deletions
|
@ -605,13 +605,13 @@ def run(hs):
|
||||||
@defer.inlineCallbacks
|
@defer.inlineCallbacks
|
||||||
def generate_monthly_active_users():
|
def generate_monthly_active_users():
|
||||||
current_mau_count = 0
|
current_mau_count = 0
|
||||||
reserved_count = 0
|
reserved_users = ()
|
||||||
store = hs.get_datastore()
|
store = hs.get_datastore()
|
||||||
if hs.config.limit_usage_by_mau or hs.config.mau_stats_only:
|
if hs.config.limit_usage_by_mau or hs.config.mau_stats_only:
|
||||||
current_mau_count = yield store.get_monthly_active_count()
|
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))
|
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))
|
max_mau_gauge.set(float(hs.config.max_mau_value))
|
||||||
|
|
||||||
def start_generate_monthly_active_users():
|
def start_generate_monthly_active_users():
|
||||||
|
|
|
@ -32,7 +32,6 @@ class MonthlyActiveUsersStore(SQLBaseStore):
|
||||||
super(MonthlyActiveUsersStore, self).__init__(None, hs)
|
super(MonthlyActiveUsersStore, self).__init__(None, hs)
|
||||||
self._clock = hs.get_clock()
|
self._clock = hs.get_clock()
|
||||||
self.hs = hs
|
self.hs = hs
|
||||||
self.reserved_users = ()
|
|
||||||
# Do not add more reserved users than the total allowable number
|
# Do not add more reserved users than the total allowable number
|
||||||
self._new_transaction(
|
self._new_transaction(
|
||||||
dbconn,
|
dbconn,
|
||||||
|
@ -51,7 +50,6 @@ class MonthlyActiveUsersStore(SQLBaseStore):
|
||||||
txn (cursor):
|
txn (cursor):
|
||||||
threepids (list[dict]): List of threepid dicts to reserve
|
threepids (list[dict]): List of threepid dicts to reserve
|
||||||
"""
|
"""
|
||||||
reserved_user_list = []
|
|
||||||
|
|
||||||
for tp in threepids:
|
for tp in threepids:
|
||||||
user_id = self.get_user_id_by_threepid_txn(txn, tp["medium"], tp["address"])
|
user_id = self.get_user_id_by_threepid_txn(txn, tp["medium"], tp["address"])
|
||||||
|
@ -60,10 +58,8 @@ class MonthlyActiveUsersStore(SQLBaseStore):
|
||||||
is_support = self.is_support_user_txn(txn, user_id)
|
is_support = self.is_support_user_txn(txn, user_id)
|
||||||
if not is_support:
|
if not is_support:
|
||||||
self.upsert_monthly_active_user_txn(txn, user_id)
|
self.upsert_monthly_active_user_txn(txn, user_id)
|
||||||
reserved_user_list.append(user_id)
|
|
||||||
else:
|
else:
|
||||||
logger.warning("mau limit reserved threepid %s not found in db" % tp)
|
logger.warning("mau limit reserved threepid %s not found in db" % tp)
|
||||||
self.reserved_users = tuple(reserved_user_list)
|
|
||||||
|
|
||||||
@defer.inlineCallbacks
|
@defer.inlineCallbacks
|
||||||
def reap_monthly_active_users(self):
|
def reap_monthly_active_users(self):
|
||||||
|
@ -74,8 +70,11 @@ class MonthlyActiveUsersStore(SQLBaseStore):
|
||||||
Deferred[]
|
Deferred[]
|
||||||
"""
|
"""
|
||||||
|
|
||||||
def _reap_users(txn):
|
def _reap_users(txn, reserved_users):
|
||||||
# Purge stale users
|
"""
|
||||||
|
Args:
|
||||||
|
reserved_users (tuple): reserved users to preserve
|
||||||
|
"""
|
||||||
|
|
||||||
thirty_days_ago = int(self._clock.time_msec()) - (1000 * 60 * 60 * 24 * 30)
|
thirty_days_ago = int(self._clock.time_msec()) - (1000 * 60 * 60 * 24 * 30)
|
||||||
query_args = [thirty_days_ago]
|
query_args = [thirty_days_ago]
|
||||||
|
@ -83,12 +82,12 @@ class MonthlyActiveUsersStore(SQLBaseStore):
|
||||||
|
|
||||||
# Need if/else since 'AND user_id NOT IN ({})' fails on Postgres
|
# Need if/else since 'AND user_id NOT IN ({})' fails on Postgres
|
||||||
# when len(reserved_users) == 0. Works fine on sqlite.
|
# 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
|
# questionmarks is a hack to overcome sqlite not supporting
|
||||||
# tuples in 'WHERE IN %s'
|
# 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)
|
sql = base_sql + """ AND user_id NOT IN ({})""".format(question_marks)
|
||||||
else:
|
else:
|
||||||
sql = base_sql
|
sql = base_sql
|
||||||
|
@ -105,7 +104,7 @@ class MonthlyActiveUsersStore(SQLBaseStore):
|
||||||
# While Postgres does not require 'LIMIT', but also does not support
|
# 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
|
# negative LIMIT values. So there is no way to write it that both can
|
||||||
# support
|
# support
|
||||||
if len(self.reserved_users) == 0:
|
if len(reserved_users) == 0:
|
||||||
sql = """
|
sql = """
|
||||||
DELETE FROM monthly_active_users
|
DELETE FROM monthly_active_users
|
||||||
WHERE user_id NOT IN (
|
WHERE user_id NOT IN (
|
||||||
|
@ -119,7 +118,9 @@ class MonthlyActiveUsersStore(SQLBaseStore):
|
||||||
# when len(reserved_users) == 0. Works fine on sqlite.
|
# when len(reserved_users) == 0. Works fine on sqlite.
|
||||||
else:
|
else:
|
||||||
# Must be >= 0 for postgres
|
# 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
|
# It is important to filter reserved users twice to guard
|
||||||
# against the case where the reserved user is present in the
|
# against the case where the reserved user is present in the
|
||||||
|
@ -132,13 +133,22 @@ class MonthlyActiveUsersStore(SQLBaseStore):
|
||||||
ORDER BY timestamp DESC
|
ORDER BY timestamp DESC
|
||||||
LIMIT ?
|
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)
|
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
|
# It seems poor to invalidate the whole cache, Postgres supports
|
||||||
# 'Returning' which would allow me to invalidate only the
|
# 'Returning' which would allow me to invalidate only the
|
||||||
# specific users, but sqlite has no way to do this and instead
|
# specific users, but sqlite has no way to do this and instead
|
||||||
|
@ -167,27 +177,25 @@ class MonthlyActiveUsersStore(SQLBaseStore):
|
||||||
return self.runInteraction("count_users", _count_users)
|
return self.runInteraction("count_users", _count_users)
|
||||||
|
|
||||||
@defer.inlineCallbacks
|
@defer.inlineCallbacks
|
||||||
def get_registered_reserved_users_count(self):
|
def get_registered_reserved_users(self):
|
||||||
"""Of the reserved threepids defined in config, how many are associated
|
"""Of the reserved threepids defined in config, which are associated
|
||||||
with registered users?
|
with registered users?
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
Defered[int]: Number of real reserved users
|
Defered[tuple]: Real reserved users
|
||||||
"""
|
"""
|
||||||
count = 0
|
|
||||||
users = ()
|
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(
|
user_id = yield self.hs.get_datastore().get_user_id_by_threepid(
|
||||||
tp["medium"], tp["address"]
|
tp["medium"], tp["address"]
|
||||||
)
|
)
|
||||||
if user_id:
|
if user_id:
|
||||||
count = count + 1
|
|
||||||
users = users + (user_id,)
|
users = users + (user_id,)
|
||||||
|
|
||||||
# Update reserved_users to ensure it stays in sync, this is important
|
return users
|
||||||
# for reaping.
|
|
||||||
self.reserved_users = users
|
|
||||||
return count
|
|
||||||
|
|
||||||
@defer.inlineCallbacks
|
@defer.inlineCallbacks
|
||||||
def upsert_monthly_active_user(self, user_id):
|
def upsert_monthly_active_user(self, user_id):
|
||||||
|
|
|
@ -50,6 +50,7 @@ class MonthlyActiveUsersTestCase(unittest.HomeserverTestCase):
|
||||||
{"medium": "email", "address": user2_email},
|
{"medium": "email", "address": user2_email},
|
||||||
{"medium": "email", "address": user3_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
|
# -1 because user3 is a support user and does not count
|
||||||
user_num = len(threepids) - 1
|
user_num = len(threepids) - 1
|
||||||
|
|
||||||
|
@ -84,6 +85,7 @@ class MonthlyActiveUsersTestCase(unittest.HomeserverTestCase):
|
||||||
self.hs.config.max_mau_value = 0
|
self.hs.config.max_mau_value = 0
|
||||||
|
|
||||||
self.reactor.advance(FORTY_DAYS)
|
self.reactor.advance(FORTY_DAYS)
|
||||||
|
self.hs.config.max_mau_value = 5
|
||||||
|
|
||||||
self.store.reap_monthly_active_users()
|
self.store.reap_monthly_active_users()
|
||||||
self.pump()
|
self.pump()
|
||||||
|
@ -173,23 +175,24 @@ class MonthlyActiveUsersTestCase(unittest.HomeserverTestCase):
|
||||||
# monthly_active_users table are reserved
|
# monthly_active_users table are reserved
|
||||||
now = int(self.hs.get_clock().time_msec())
|
now = int(self.hs.get_clock().time_msec())
|
||||||
if i != 0:
|
if i != 0:
|
||||||
self.get_success(self.store.register_user(user_id=user, password_hash=None))
|
self.get_success(
|
||||||
#self.pump()
|
self.store.register_user(user_id=user, password_hash=None)
|
||||||
self.get_success(self.store.user_add_threepid(user, "email", email, now, now))
|
)
|
||||||
|
self.get_success(
|
||||||
|
self.store.user_add_threepid(user, "email", email, now, now)
|
||||||
|
)
|
||||||
|
|
||||||
self.hs.config.mau_limits_reserved_threepids = threepids
|
self.hs.config.mau_limits_reserved_threepids = threepids
|
||||||
self.store.runInteraction(
|
self.store.runInteraction(
|
||||||
"initialise", self.store._initialise_reserved_users, threepids
|
"initialise", self.store._initialise_reserved_users, threepids
|
||||||
)
|
)
|
||||||
#self.pump()
|
|
||||||
count = self.store.get_monthly_active_count()
|
count = self.store.get_monthly_active_count()
|
||||||
self.assertTrue(self.get_success(count), initial_users)
|
self.assertTrue(self.get_success(count), initial_users)
|
||||||
|
|
||||||
count = self.store.get_registered_reserved_users_count()
|
users = self.store.get_registered_reserved_users()
|
||||||
self.assertEquals(self.get_success(count), reserved_user_number)
|
self.assertEquals(len(self.get_success(users)), reserved_user_number)
|
||||||
|
|
||||||
self.get_success(self.store.reap_monthly_active_users())
|
self.get_success(self.store.reap_monthly_active_users())
|
||||||
#self.pump()
|
|
||||||
count = self.store.get_monthly_active_count()
|
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)
|
||||||
|
|
||||||
|
@ -227,8 +230,8 @@ class MonthlyActiveUsersTestCase(unittest.HomeserverTestCase):
|
||||||
|
|
||||||
def test_get_reserved_real_user_account(self):
|
def test_get_reserved_real_user_account(self):
|
||||||
# Test no reserved users, or reserved threepids
|
# Test no reserved users, or reserved threepids
|
||||||
count = self.store.get_registered_reserved_users_count()
|
users = self.get_success(self.store.get_registered_reserved_users())
|
||||||
self.assertEquals(self.get_success(count), 0)
|
self.assertEquals(len(users), 0)
|
||||||
# Test reserved users but no registered users
|
# Test reserved users but no registered users
|
||||||
|
|
||||||
user1 = "@user1:example.com"
|
user1 = "@user1:example.com"
|
||||||
|
@ -246,8 +249,8 @@ class MonthlyActiveUsersTestCase(unittest.HomeserverTestCase):
|
||||||
)
|
)
|
||||||
|
|
||||||
self.pump()
|
self.pump()
|
||||||
count = self.store.get_registered_reserved_users_count()
|
users = self.get_success(self.store.get_registered_reserved_users())
|
||||||
self.assertEquals(self.get_success(count), 0)
|
self.assertEquals(len(users), 0)
|
||||||
|
|
||||||
# Test reserved registed users
|
# Test reserved registed users
|
||||||
self.store.register_user(user_id=user1, password_hash=None)
|
self.store.register_user(user_id=user1, password_hash=None)
|
||||||
|
@ -257,8 +260,9 @@ class MonthlyActiveUsersTestCase(unittest.HomeserverTestCase):
|
||||||
now = int(self.hs.get_clock().time_msec())
|
now = int(self.hs.get_clock().time_msec())
|
||||||
self.store.user_add_threepid(user1, "email", user1_email, now, now)
|
self.store.user_add_threepid(user1, "email", user1_email, now, now)
|
||||||
self.store.user_add_threepid(user2, "email", user2_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):
|
def test_support_user_not_add_to_mau_limits(self):
|
||||||
support_user_id = "@support:test"
|
support_user_id = "@support:test"
|
||||||
|
|
Loading…
Reference in a new issue