Initial impl of capping MAU #3630

neilisfragile · 2018-07-31T11:06:11Z

Tracks Mau naiving by count(*) on user_ips.
Blocks registration and sign in if value exceeded
Publishes metrics for consumption by Prometheus

The idea here is to cap the monthly active users (MAU) on a server. This is an initial cut that simply stops registration and login for if MAU is above configured threshold - @erikjohnston

krombel · 2018-07-31T11:33:02Z

synapse/handlers/auth.py

+        log in.
+        """
+        if self.hs.config.limit_usage_by_mau is True:
+            current_mau = self.store.count_monthly_users()


This method will be hit very often. So you might want to either add some cache on store.count_monthly_users() or use a value which might be generated by generate_monthly_active_users

agreed though deliberately omitted since count_monthly_users() is really only a stepping stone method to get the rest of the infra out. In current form only called regularly in the case where the config is enabled.

ok. If that is on your radar I am fine with it for now

krombel · 2018-07-31T11:34:33Z

synapse/handlers/register.py

@@ -531,3 +533,15 @@ def _join_user_to_room(self, requester, room_identifier):
            remote_room_hosts=remote_room_hosts,
            action="join",
        )
+
+    def _check_mau_limits(self):


you might want to deduplicate this somehow

same as above, and also the calculating the total count is duped with the daily count - but likley to change structure

erikjohnston · 2018-07-31T15:20:04Z

synapse/app/homeserver.py

+max_mau_value_gauge = Gauge("synapse_admin_max_mau_value", "MAU Limit")
+limit_usage_by_mau_gauge = Gauge(
+    "synapse_admin_limit_usage_by_mau", "MAU Limiting enabled"
+)


I would probably not bother having limit_usage_by_mau_gauge, and simply set it to 0 or null if limit isn't enabled

erikjohnston · 2018-07-31T15:21:19Z

synapse/app/homeserver.py

+        limit_usage_by_mau_gauge.set(float(hs.config.limit_usage_by_mau))
+
+    generate_monthly_active_users()
+    clock.looping_call(generate_monthly_active_users, 5 * 60 * 1000)


Does it make sense for this to be done if hs.config.limit_usage_by_mau is false?

right yes, need to be called once but the looping is a waste

erikjohnston · 2018-07-31T15:22:59Z

synapse/storage/__init__.py

+            count, = txn.fetchone()
+            return count
+        finally:
+            txn.close()


This should either accept a txn argument or use runInteraction, otherwise its going to be too easy to call this on the main thread (which is bad as this method blocks)

I was originally trying to keep things simple without deferring - will make the change

erikjohnston · 2018-07-31T15:23:43Z

synapse/app/homeserver.py

+    def generate_monthly_active_users():
+        count = 0
+        if hs.config.limit_usage_by_mau:
+            count = hs.get_datastore().count_monthly_users()


count_monthly_users will need to be asynchronous and return a deferred, which we'll need to yield

agreed as above

This still needs a yield

neilisfragile · 2018-08-01T11:05:14Z

review items implemented

…u_sign_in_log_in_limits

erikjohnston

Just needs to have the yield added, and then its good to go

erikjohnston · 2018-08-01T14:45:06Z

changelog.d/3630.feature

@@ -0,0 +1 @@
+Blocks registration and sign in if max mau value exceeded


Sorry, can we quickly change this to "Add ability to limit number of monthly active users on the server", or some such. Otherwise its going to be confusing for people to read the changelog who haven't had the full context.

erikjohnston · 2018-08-01T14:45:29Z

synapse/app/homeserver.py

+    def generate_monthly_active_users():
+        count = 0
+        if hs.config.limit_usage_by_mau:
+            count = hs.get_datastore().count_monthly_users()


This still needs a yield

erikjohnston · 2018-08-01T14:47:39Z

synapse/storage/__init__.py

+        This method should be refactored with count_daily_users - the only
+        reason not to is waiting on definition of mau
+        returns:
+            defered: resolves to int


FWIW, we generally start the docstring on the sameline of the quotes, so something like:

"""Counts the number of users who used this homeserver in the last 30 days. This method should be refactored with count_daily_users - the only reason not to is waiting on definition of MAU. Returns: Deferred[int] """

And that's also how you annotate return types (such that IDEs correctly interpret it)

neilisfragile and others added 6 commits July 30, 2018 15:55

limit register and sign in on number of monthly users

251e6c1

factor out metrics from __init__ to app/homeserver

9b13817

monthly_active_tests

cefac79

actually close conn

fef7e58

remove errant logging

21276ff

fix user_ips counting

7d05406

neilisfragile assigned erikjohnston Jul 31, 2018

krombel reviewed Jul 31, 2018

View reviewed changes

neilisfragile added 2 commits July 31, 2018 13:16

coding style

df2235e

mau limts

5bb39b1

erikjohnston changed the title ~~Neilj/mau sign in log in limits~~ Initial impl of capping MAU Jul 31, 2018

neilisfragile mentioned this pull request Jul 31, 2018

API for monthly_active_users table #3633

Merged

erikjohnston suggested changes Jul 31, 2018

View reviewed changes

neilisfragile added 3 commits August 1, 2018 10:20

only need to loop if mau limiting is enabled

c507fa1

make count_monthly_users async synapse/handlers/auth.py

7931393

remove errant print

6023cdd

erikjohnston assigned neilisfragile and unassigned erikjohnston Aug 1, 2018

neilisfragile added 3 commits August 1, 2018 11:46

remove need to plot limit_usage_by_mau

2c54f1c

count_monthly_users() async

0aba3d3

fix known broken test

4e6e001

neilisfragile assigned erikjohnston and unassigned neilisfragile Aug 1, 2018

neilisfragile added 3 commits August 1, 2018 13:42

Merge branch 'develop' of github.com:matrix-org/synapse into neilj/ma…

303f1c8

…u_sign_in_log_in_limits

fix test for py3

6eed16d

fix isort

c480c4c

This was referenced Aug 1, 2018

Neilj/mau limits2 #3529

Closed

Neilj/mau limits #3510

Closed

erikjohnston suggested changes Aug 1, 2018

View reviewed changes

erikjohnston assigned neilisfragile and unassigned erikjohnston Aug 1, 2018

neilisfragile added 2 commits August 1, 2018 16:17

improve clarity

7ff44d9

count_monthly_users is now async

b7f203a

erikjohnston approved these changes Aug 1, 2018

View reviewed changes

neilisfragile merged commit 085435e into develop Aug 1, 2018

neilisfragile deleted the neilj/mau_sign_in_log_in_limits branch August 1, 2018 15:58

richvdh mentioned this pull request Aug 28, 2018

Some Postgres database queries are long running / blocking #3682

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial impl of capping MAU #3630

Initial impl of capping MAU #3630

neilisfragile commented Jul 31, 2018 •

edited by erikjohnston

Loading

krombel Jul 31, 2018

neilisfragile Jul 31, 2018

krombel Jul 31, 2018

krombel Jul 31, 2018 •

edited

Loading

neilisfragile Jul 31, 2018

erikjohnston Jul 31, 2018

erikjohnston Jul 31, 2018

neilisfragile Jul 31, 2018

erikjohnston Jul 31, 2018

neilisfragile Jul 31, 2018

erikjohnston Jul 31, 2018

neilisfragile Jul 31, 2018

erikjohnston Aug 1, 2018

neilisfragile commented Aug 1, 2018

erikjohnston left a comment

erikjohnston Aug 1, 2018

erikjohnston Aug 1, 2018

erikjohnston Aug 1, 2018 •

edited

Loading

		@@ -0,0 +1 @@
		Blocks registration and sign in if max mau value exceeded

Initial impl of capping MAU #3630

Initial impl of capping MAU #3630

Conversation

neilisfragile commented Jul 31, 2018 • edited by erikjohnston Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krombel Jul 31, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neilisfragile commented Aug 1, 2018

erikjohnston left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erikjohnston Aug 1, 2018 • edited Loading

Choose a reason for hiding this comment

neilisfragile commented Jul 31, 2018 •

edited by erikjohnston

Loading

krombel Jul 31, 2018 •

edited

Loading

erikjohnston Aug 1, 2018 •

edited

Loading