remove needless trial param

...and only promote them to non-trial if we have MAU headroom

changelog.d/3739.feature

@@ -0,0 +1 @@
+add `mau_trial_days` config option to specify threshold beyond which users are counted as MAU

synapse/config/server.py

@@ -80,6 +80,9 @@ class ServerConfig(Config):
         self.mau_limits_reserved_threepids = config.get(
             "mau_limit_reserved_threepids", []
         )
+        self.mau_trial_days = config.get(
+            "mau_trial_days", 0,
+        )
 
         # Options to disable HS
         self.hs_disabled = config.get("hs_disabled", False)
@@ -365,6 +368,7 @@ class ServerConfig(Config):
           # Enables monthly active user checking
           # limit_usage_by_mau: False
           # max_mau_value: 50
+          # mau_trial_days: 2
           #
           # Sometimes the server admin will want to ensure certain accounts are
           # never blocked by mau checking. These accounts are specified here.

synapse/storage/monthly_active_users.py

@@ -46,7 +46,7 @@ class MonthlyActiveUsersStore(SQLBaseStore):
                 tp["medium"], tp["address"]
             )
             if user_id:
-                yield self.upsert_monthly_active_user(user_id)
+                yield self.upsert_monthly_active_user(user_id, False)
                 reserved_user_list.append(user_id)
             else:
                 logger.warning(
@@ -88,14 +88,43 @@ class MonthlyActiveUsersStore(SQLBaseStore):
 
             txn.execute(sql, query_args)
 
-            # If MAU user count still exceeds the MAU threshold, then delete on
-            # a least recently active basis.
+            # Promote trial users to non-trial users, oldest first, assuming we
+            # have MAU headroom available.  Otherwise we leave them stuck in trial
+            # purgatory until their 30 days is up.
+            #
+            # We don't need to worry about reserved users, as they are already non-trial.
+
+            mau_trial_ms = self.hs.config.mau_trial_days * 24 * 60 * 60 * 1000
+
+            sql = "SELECT count(*) FROM monthly_active_users WHERE NOT trial"
+            txn.execute(sql)
+            non_trial_users, = txn.fetchone()
+
+            sql = """
+                UPDATE monthly_active_users SET trial=? WHERE user_id IN (
+                    SELECT user_id FROM monthly_active_users
+                    WHERE trial
+                    ORDER BY (timestamp - first_active) DESC
+                    LIMIT ?
+                ) AND timestamp - first_active >= ?
+                """
+
+            limit = self.hs.config.max_mau_value - non_trial_users
+            if limit < 0:
+                limit = 0
+
+            txn.execute(sql, (False, limit, mau_trial_ms))
+
+            # If non-trial MAU user count still exceeds the MAU threshold, then
+            # delete on a least recently active basis.
+            #
             # Note it is not possible to write this query using OFFSET due to
             # incompatibilities in how sqlite and postgres support the feature.
             # sqlite requires 'LIMIT -1 OFFSET ?', the LIMIT must be present
             # 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
@@ -105,9 +134,10 @@ class MonthlyActiveUsersStore(SQLBaseStore):
                 DELETE FROM monthly_active_users
                 WHERE user_id NOT IN (
                     SELECT user_id FROM monthly_active_users
+                    WHERE NOT trial
                     ORDER BY timestamp DESC
                     LIMIT ?
-                    )
+                    ) AND NOT trial
                 """
             # Need if/else since 'AND user_id NOT IN ({})' fails on Postgres
             # when len(reserved_users) == 0. Works fine on sqlite.
@@ -140,21 +170,27 @@ class MonthlyActiveUsersStore(SQLBaseStore):
         """
 
         def _count_users(txn):
-            sql = "SELECT COALESCE(count(*), 0) FROM monthly_active_users"
+            sql = """
+                SELECT COALESCE(count(*), 0)
+                FROM monthly_active_users
+                WHERE NOT trial
+            """
 
             txn.execute(sql)
             count, = txn.fetchone()
             return count
         return self.runInteraction("count_users", _count_users)
 
-    def upsert_monthly_active_user(self, user_id):
+    def upsert_monthly_active_user(self, user_id, trial=False):
         """
             Updates or inserts monthly active user member
             Arguments:
                 user_id (str): user to add/update
+                trial (bool): whether the user is entering a trial or not
             Deferred[bool]: True if a new entry was created, False if an
                 existing one was updated.
         """
+        now = int(self._clock.time_msec())
         is_insert = self._simple_upsert(
             desc="upsert_monthly_active_user",
             table="monthly_active_users",
@@ -162,7 +198,11 @@ class MonthlyActiveUsersStore(SQLBaseStore):
                 "user_id": user_id,
             },
             values={
-                "timestamp": int(self._clock.time_msec()),
+                "timestamp": now,
+            },
+            insertion_values={
+                "first_active": now,
+                "trial": trial,
             },
             lock=False,
         )
@@ -174,6 +214,7 @@ class MonthlyActiveUsersStore(SQLBaseStore):
     def user_last_seen_monthly_active(self, user_id):
         """
             Checks if a given user is part of the monthly active user group
+            or a trial user.
             Arguments:
                 user_id (str): user to add/update
             Return:
@@ -181,15 +222,32 @@ class MonthlyActiveUsersStore(SQLBaseStore):
 
         """
 
-        return(self._simple_select_one_onecol(
-            table="monthly_active_users",
-            keyvalues={
-                "user_id": user_id,
-            },
-            retcol="timestamp",
-            allow_none=True,
-            desc="user_last_seen_monthly_active",
-        ))
+        # FIXME: we should probably return whether this is a trial user or not.
+
+        mau_trial_ms = self.hs.config.mau_trial_days * 24 * 60 * 60 * 1000
+
+        def _user_last_seen_monthly_active(txn):
+            sql = """
+                SELECT timestamp
+                FROM monthly_active_users
+                WHERE user_id = ? AND (
+                    (NOT trial) OR
+                    (timestamp - first_active < ?)
+                )
+            """
+
+            txn.execute(sql, (user_id, mau_trial_ms, ))
+            rows = txn.fetchall()
+            if rows:
+                timestamp = rows[0][0]
+                return timestamp
+            else:
+                return None
+
+        return self.runInteraction(
+            "user_last_seen_monthly_active",
+            _user_last_seen_monthly_active,
+        )
 
     @defer.inlineCallbacks
     def populate_monthly_active_users(self, user_id):
@@ -203,6 +261,8 @@ class MonthlyActiveUsersStore(SQLBaseStore):
             last_seen_timestamp = yield self.user_last_seen_monthly_active(user_id)
             now = self.hs.get_clock().time_msec()
 
+            create_as_trial = self.hs.config.mau_trial_days > 0
+
             # We want to reduce to the total number of db writes, and are happy
             # to trade accuracy of timestamp in order to lighten load. This means
             # We always insert new users (where MAU threshold has not been reached),
@@ -211,6 +271,6 @@ class MonthlyActiveUsersStore(SQLBaseStore):
             if last_seen_timestamp is None:
                 count = yield self.get_monthly_active_count()
                 if count < self.hs.config.max_mau_value:
-                    yield self.upsert_monthly_active_user(user_id)
+                    yield self.upsert_monthly_active_user(user_id, create_as_trial)
             elif now - last_seen_timestamp > LAST_SEEN_GRANULARITY:
                 yield self.upsert_monthly_active_user(user_id)

synapse/storage/prepare_database.py

@@ -25,7 +25,7 @@ logger = logging.getLogger(__name__)
 
 # Remember to update this number every time a change is made to database
 # schema files, so the users will be informed on server restarts.
-SCHEMA_VERSION = 51
+SCHEMA_VERSION = 52
 
 dir_path = os.path.abspath(os.path.dirname(__file__))
 

synapse/storage/schema/delta/52/free_mau.sql

@@ -0,0 +1,19 @@
+/* Copyright 2018 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+ALTER TABLE monthly_active_users ADD COLUMN first_active BIGINT DEFAULT 0 NOT NULL;
+ALTER TABLE monthly_active_users ADD COLUMN trial BOOLEAN;
+
+CREATE INDEX monthly_active_users_first_active ON monthly_active_users(first_active);

tests/handlers/test_auth.py

@@ -41,6 +41,7 @@ class AuthTestCase(unittest.TestCase):
         self.macaroon_generator = self.hs.get_macaroon_generator()
         # MAU tests
         self.hs.config.max_mau_value = 50
+        self.hs.config.mau_trial_days = 0
         self.small_number_of_users = 1
         self.large_number_of_users = 100
 

tests/handlers/test_register.py

@@ -54,6 +54,7 @@ class RegistrationTestCase(unittest.TestCase):
         self.handler = self.hs.get_handlers().registration_handler
         self.store = self.hs.get_datastore()
         self.hs.config.max_mau_value = 50
+        self.hs.config.mau_trial_days = 0
         self.lots_of_users = 100
         self.small_number_of_users = 1
 

tests/handlers/test_sync.py

@@ -42,6 +42,7 @@ class SyncTestCase(tests.unittest.TestCase):
 
         self.hs.config.limit_usage_by_mau = True
         self.hs.config.max_mau_value = 1
+        self.hs.config.mau_trial_days = 0
 
         # Check that the happy case does not throw errors
         yield self.store.upsert_monthly_active_user(user_id1)

tests/storage/test_client_ips.py

@@ -59,6 +59,7 @@ class ClientIpStoreTestCase(tests.unittest.TestCase):
     def test_disabled_monthly_active_user(self):
         self.hs.config.limit_usage_by_mau = False
         self.hs.config.max_mau_value = 50
+        self.hs.config.mau_trial_days = 0
         user_id = "@user:server"
         yield self.store.insert_client_ip(
             user_id, "access_token", "ip", "user_agent", "device_id"
@@ -70,6 +71,7 @@ class ClientIpStoreTestCase(tests.unittest.TestCase):
     def test_adding_monthly_active_user_when_full(self):
         self.hs.config.limit_usage_by_mau = True
         self.hs.config.max_mau_value = 50
+        self.hs.config.mau_trial_days = 0
         lots_of_users = 100
         user_id = "@user:server"
 
@@ -86,6 +88,7 @@ class ClientIpStoreTestCase(tests.unittest.TestCase):
     def test_adding_monthly_active_user_when_space(self):
         self.hs.config.limit_usage_by_mau = True
         self.hs.config.max_mau_value = 50
+        self.hs.config.mau_trial_days = 0
         user_id = "@user:server"
         active = yield self.store.user_last_seen_monthly_active(user_id)
         self.assertFalse(active)
@@ -100,6 +103,7 @@ class ClientIpStoreTestCase(tests.unittest.TestCase):
     def test_updating_monthly_active_user_when_space(self):
         self.hs.config.limit_usage_by_mau = True
         self.hs.config.max_mau_value = 50
+        self.hs.config.mau_trial_days = 0
         user_id = "@user:server"
 
         active = yield self.store.user_last_seen_monthly_active(user_id)

tests/storage/test_monthly_active_users.py

@@ -30,6 +30,7 @@ class MonthlyActiveUsersTestCase(tests.unittest.TestCase):
     def setUp(self):
         self.hs = yield setup_test_homeserver(self.addCleanup)
         self.store = self.hs.get_datastore()
+        self.hs.config.mau_trial_days = 0
 
     @defer.inlineCallbacks
     def test_initialise_reserved_users(self):