Merge pull request #9766 from matrix-org/rav/drop_py35

Require py36, Postgres 9.6, and sqlite 3.22

.buildkite/scripts/test_old_deps.sh

@@ -1,6 +1,6 @@
 #!/usr/bin/env bash
 
-# this script is run by buildkite in a plain `xenial` container; it installs the
+# this script is run by buildkite in a plain `bionic` container; it installs the
 # minimal requirements for tox and hands over to the py3-old tox environment.
 
 set -ex

changelog.d/9766.feature

@@ -0,0 +1 @@
+Synapse now requires Python 3.6 or later. It also requires Postgres 9.6 or later or SQLite 3.22 or later.

scripts-dev/build_debian_packages

@@ -18,11 +18,9 @@ import threading
 from concurrent.futures import ThreadPoolExecutor
 
 DISTS = (
-    "debian:stretch",
     "debian:buster",
     "debian:bullseye",
     "debian:sid",
-    "ubuntu:xenial",
     "ubuntu:bionic",
     "ubuntu:focal",
     "ubuntu:groovy",

setup.py

@@ -123,7 +123,7 @@ setup(
     zip_safe=False,
     long_description=long_description,
     long_description_content_type="text/x-rst",
-    python_requires="~=3.5",
+    python_requires="~=3.6",
     classifiers=[
         "Development Status :: 5 - Production/Stable",
         "Topic :: Communications :: Chat",

synapse/storage/database.py

@@ -2059,69 +2059,18 @@ def make_in_list_sql_clause(
 KV = TypeVar("KV")
 
 
-def make_tuple_comparison_clause(
+def make_tuple_comparison_clause(keys: List[Tuple[str, KV]]) -> Tuple[str, List[KV]]:
-    database_engine: BaseDatabaseEngine, keys: List[Tuple[str, KV]]
-) -> Tuple[str, List[KV]]:
     """Returns a tuple comparison SQL clause
 
-    Depending what the SQL engine supports, builds a SQL clause that looks like either
+    Builds a SQL clause that looks like "(a, b) > (?, ?)"
-    "(a, b) > (?, ?)", or "(a > ?) OR (a == ? AND b > ?)".
 
     Args:
-        database_engine
         keys: A set of (column, value) pairs to be compared.
 
     Returns:
         A tuple of SQL query and the args
     """
-    if database_engine.supports_tuple_comparison:
+    return (
-        return (
+        "(%s) > (%s)" % (",".join(k[0] for k in keys), ",".join("?" for _ in keys)),
-            "(%s) > (%s)" % (",".join(k[0] for k in keys), ",".join("?" for _ in keys)),
+        [k[1] for k in keys],
-            [k[1] for k in keys],
+    )
-        )
-
-    # we want to build a clause
-    #    (a > ?) OR
-    #    (a == ? AND b > ?) OR
-    #    (a == ? AND b == ? AND c > ?)
-    #    ...
-    #    (a == ? AND b == ? AND ... AND z > ?)
-    #
-    # or, equivalently:
-    #
-    #  (a > ? OR (a == ? AND
-    #    (b > ? OR (b == ? AND
-    #      ...
-    #        (y > ? OR (y == ? AND
-    #          z > ?
-    #        ))
-    #      ...
-    #    ))
-    #  ))
-    #
-    # which itself is equivalent to (and apparently easier for the query optimiser):
-    #
-    #  (a >= ? AND (a > ? OR
-    #    (b >= ? AND (b > ? OR
-    #      ...
-    #        (y >= ? AND (y > ? OR
-    #          z > ?
-    #        ))
-    #      ...
-    #    ))
-    #  ))
-    #
-    #
-
-    clause = ""
-    args = []  # type: List[KV]
-    for k, v in keys[:-1]:
-        clause = clause + "(%s >= ? AND (%s > ? OR " % (k, k)
-        args.extend([v, v])
-
-    (k, v) = keys[-1]
-    clause += "%s > ?" % (k,)
-    args.append(v)
-
-    clause += "))" * (len(keys) - 1)
-    return clause, args

synapse/storage/databases/main/client_ips.py

@@ -298,7 +298,6 @@ class ClientIpBackgroundUpdateStore(SQLBaseStore):
             #      times, which is fine.
 
             where_clause, where_args = make_tuple_comparison_clause(
-                self.database_engine,
                 [("user_id", last_user_id), ("device_id", last_device_id)],
             )
 

synapse/storage/databases/main/devices.py

@@ -985,7 +985,7 @@ class DeviceBackgroundUpdateStore(SQLBaseStore):
 
         def _txn(txn):
             clause, args = make_tuple_comparison_clause(
-                self.db_pool.engine, [(x, last_row[x]) for x in KEY_COLS]
+                [(x, last_row[x]) for x in KEY_COLS]
             )
             sql = """
                 SELECT stream_id, destination, user_id, device_id, MAX(ts) AS ts

synapse/storage/databases/main/events_bg_updates.py

@@ -838,7 +838,6 @@ class EventsBackgroundUpdatesStore(SQLBaseStore):
         # We want to do a `(topological_ordering, stream_ordering) > (?,?)`
         # comparison, but that is not supported on older SQLite versions
         tuple_clause, tuple_args = make_tuple_comparison_clause(
-            self.database_engine,
             [
                 ("events.room_id", last_room_id),
                 ("topological_ordering", last_depth),

synapse/storage/engines/_base.py

@@ -42,14 +42,6 @@ class BaseDatabaseEngine(Generic[ConnectionType], metaclass=abc.ABCMeta):
         """
         ...
 
-    @property
-    @abc.abstractmethod
-    def supports_tuple_comparison(self) -> bool:
-        """
-        Do we support comparing tuples, i.e. `(a, b) > (c, d)`?
-        """
-        ...
-
     @property
     @abc.abstractmethod
     def supports_using_any_list(self) -> bool:

synapse/storage/engines/postgres.py

@@ -47,8 +47,8 @@ class PostgresEngine(BaseDatabaseEngine):
         self._version = db_conn.server_version
 
         # Are we on a supported PostgreSQL version?
-        if not allow_outdated_version and self._version < 90500:
+        if not allow_outdated_version and self._version < 90600:
-            raise RuntimeError("Synapse requires PostgreSQL 9.5+ or above.")
+            raise RuntimeError("Synapse requires PostgreSQL 9.6 or above.")
 
         with db_conn.cursor() as txn:
             txn.execute("SHOW SERVER_ENCODING")
@@ -129,13 +129,6 @@ class PostgresEngine(BaseDatabaseEngine):
         """
         return True
 
-    @property
-    def supports_tuple_comparison(self):
-        """
-        Do we support comparing tuples, i.e. `(a, b) > (c, d)`?
-        """
-        return True
-
     @property
     def supports_using_any_list(self):
         """Do we support using `a = ANY(?)` and passing a list"""

synapse/storage/engines/sqlite.py

@@ -56,14 +56,6 @@ class Sqlite3Engine(BaseDatabaseEngine["sqlite3.Connection"]):
         """
         return self.module.sqlite_version_info >= (3, 24, 0)
 
-    @property
-    def supports_tuple_comparison(self):
-        """
-        Do we support comparing tuples, i.e. `(a, b) > (c, d)`? This requires
-        SQLite 3.15+.
-        """
-        return self.module.sqlite_version_info >= (3, 15, 0)
-
     @property
     def supports_using_any_list(self):
         """Do we support using `a = ANY(?)` and passing a list"""
@@ -72,8 +64,11 @@ class Sqlite3Engine(BaseDatabaseEngine["sqlite3.Connection"]):
     def check_database(self, db_conn, allow_outdated_version: bool = False):
         if not allow_outdated_version:
             version = self.module.sqlite_version_info
-            if version < (3, 11, 0):
+            # Synapse is untested against older SQLite versions, and we don't want
-                raise RuntimeError("Synapse requires sqlite 3.11 or above.")
+            # to let users upgrade to a version of Synapse with broken support for their
+            # sqlite version, because it risks leaving them with a half-upgraded db.
+            if version < (3, 22, 0):
+                raise RuntimeError("Synapse requires sqlite 3.22 or above.")
 
     def check_new_database(self, txn):
         """Gets called when setting up a brand new database. This allows us to

tests/storage/test_database.py

@@ -36,17 +36,6 @@ def _stub_db_engine(**kwargs) -> BaseDatabaseEngine:
 
 class TupleComparisonClauseTestCase(unittest.TestCase):
     def test_native_tuple_comparison(self):
-        db_engine = _stub_db_engine(supports_tuple_comparison=True)
+        clause, args = make_tuple_comparison_clause([("a", 1), ("b", 2)])
-        clause, args = make_tuple_comparison_clause(db_engine, [("a", 1), ("b", 2)])
         self.assertEqual(clause, "(a,b) > (?,?)")
         self.assertEqual(args, [1, 2])
-
-    def test_emulated_tuple_comparison(self):
-        db_engine = _stub_db_engine(supports_tuple_comparison=False)
-        clause, args = make_tuple_comparison_clause(
-            db_engine, [("a", 1), ("b", 2), ("c", 3)]
-        )
-        self.assertEqual(
-            clause, "(a >= ? AND (a > ? OR (b >= ? AND (b > ? OR c > ?))))"
-        )
-        self.assertEqual(args, [1, 1, 2, 2, 3])

tox.ini

@@ -1,5 +1,8 @@
 [tox]
-envlist = packaging, py35, py36, py37, py38, py39, check_codestyle, check_isort
+envlist = packaging, py36, py37, py38, py39, check_codestyle, check_isort
+
+# we require tox>=2.3.2 for the fix to https://github.com/tox-dev/tox/issues/208
+minversion = 2.3.2
 
 [base]
 deps =
@@ -48,6 +51,7 @@ deps =
 extras =
     # install the optional dependendencies for tox environments without
     # '-noextras' in their name
+    # (this requires tox 3)
     !noextras: all
     test
 
@@ -74,8 +78,6 @@ commands =
     # we use "env" rather than putting a value in `setenv` so that it is not
     # inherited by other tox environments.
     #
-    # keep this in sync with the copy in `testenv:py3-old`.
-    #
     /usr/bin/env COVERAGE_PROCESS_START={toxinidir}/.coveragerc "{envbindir}/trial" {env:TRIAL_FLAGS:} {posargs:tests} {env:TOXSUFFIX:}
 
 # As of twisted 16.4, trial tries to import the tests as a package (previously
@@ -121,11 +123,7 @@ commands =
     # Install Synapse itself. This won't update any libraries.
     pip install -e ".[test]"
 
-    # we have to duplicate the command from `testenv` rather than refer to it
+    {[testenv]commands}
-    # as `{[testenv]commands}`, because we run on ubuntu xenial, which has
-    # tox 2.3.1, and https://github.com/tox-dev/tox/issues/208.
-    #
-    /usr/bin/env COVERAGE_PROCESS_START={toxinidir}/.coveragerc "{envbindir}/trial" {env:TRIAL_FLAGS:} {posargs:tests} {env:TOXSUFFIX:}
 
 [testenv:benchmark]
 deps =