From abade34633da2002014a1dd7e4da3f558e610582 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 8 Apr 2021 12:00:47 +0100 Subject: [PATCH 1/6] Require py36 and Postgres 9.6 --- changelog.d/9766.feature | 1 + setup.py | 2 +- synapse/storage/engines/postgres.py | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-) create mode 100644 changelog.d/9766.feature diff --git a/changelog.d/9766.feature b/changelog.d/9766.feature new file mode 100644 index 0000000000..030bdb4561 --- /dev/null +++ b/changelog.d/9766.feature @@ -0,0 +1 @@ +Synapse now requires Python 3.6 or later and Postgres 9.6 or later. diff --git a/setup.py b/setup.py index 29e9971dc1..4e9e333c60 100755 --- a/setup.py +++ b/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", diff --git a/synapse/storage/engines/postgres.py b/synapse/storage/engines/postgres.py index 80a3558aec..d95f88b3e9 100644 --- a/synapse/storage/engines/postgres.py +++ b/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: - raise RuntimeError("Synapse requires PostgreSQL 9.5+ or above.") + if not allow_outdated_version and self._version < 90600: + raise RuntimeError("Synapse requires PostgreSQL 9.6 or above.") with db_conn.cursor() as txn: txn.execute("SHOW SERVER_ENCODING") From 3ada9b42640137d908fa2db64e78f3f79d11dd3a Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 8 Apr 2021 13:45:19 +0100 Subject: [PATCH 2/6] Drop support for sqlite<3.22 as well --- changelog.d/9766.feature | 2 +- synapse/storage/database.py | 62 ++++------------------------- synapse/storage/engines/_base.py | 8 ---- synapse/storage/engines/postgres.py | 7 ---- synapse/storage/engines/sqlite.py | 15 +++---- tests/storage/test_database.py | 12 +----- 6 files changed, 14 insertions(+), 92 deletions(-) diff --git a/changelog.d/9766.feature b/changelog.d/9766.feature index 030bdb4561..ecf49cfee1 100644 --- a/changelog.d/9766.feature +++ b/changelog.d/9766.feature @@ -1 +1 @@ -Synapse now requires Python 3.6 or later and Postgres 9.6 or later. +Synapse now requires Python 3.6 or later. It also requires Postgres 9.6 or later or SQLite 3.22 or later. diff --git a/synapse/storage/database.py b/synapse/storage/database.py index 94590e7b45..a2f016a7ef 100644 --- a/synapse/storage/database.py +++ b/synapse/storage/database.py @@ -2060,68 +2060,20 @@ KV = TypeVar("KV") def make_tuple_comparison_clause( - database_engine: BaseDatabaseEngine, keys: List[Tuple[str, 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 - "(a, b) > (?, ?)", or "(a > ?) OR (a == ? AND b > ?)". + Builds a SQL clause that looks like "(a, b) > (?, ?)" Args: - database_engine + _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 ( - "(%s) > (%s)" % (",".join(k[0] for k in keys), ",".join("?" for _ 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 + return ( + "(%s) > (%s)" % (",".join(k[0] for k in keys), ",".join("?" for _ in keys)), + [k[1] for k in keys], + ) diff --git a/synapse/storage/engines/_base.py b/synapse/storage/engines/_base.py index cca839c70f..21db1645d3 100644 --- a/synapse/storage/engines/_base.py +++ b/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: diff --git a/synapse/storage/engines/postgres.py b/synapse/storage/engines/postgres.py index d95f88b3e9..dba8cc51d3 100644 --- a/synapse/storage/engines/postgres.py +++ b/synapse/storage/engines/postgres.py @@ -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""" diff --git a/synapse/storage/engines/sqlite.py b/synapse/storage/engines/sqlite.py index b87e7798da..f4f16456f2 100644 --- a/synapse/storage/engines/sqlite.py +++ b/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): - raise RuntimeError("Synapse requires sqlite 3.11 or above.") + # Synapse is untested against older SQLite versions, and we don't want + # 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 diff --git a/tests/storage/test_database.py b/tests/storage/test_database.py index 5a77c84962..1bba58fc03 100644 --- a/tests/storage/test_database.py +++ b/tests/storage/test_database.py @@ -36,17 +36,7 @@ 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) + db_engine = _stub_db_engine() 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]) From 9278eb701e7f5358f80e416f3330e6bc380d100f Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 8 Apr 2021 12:03:12 +0100 Subject: [PATCH 3/6] drop support for stretch and xenial --- scripts-dev/build_debian_packages | 2 -- 1 file changed, 2 deletions(-) diff --git a/scripts-dev/build_debian_packages b/scripts-dev/build_debian_packages index d0685c8b35..bddc441df2 100755 --- a/scripts-dev/build_debian_packages +++ b/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", From 04ff88139a70bcc852480229672626de7e620b9f Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 8 Apr 2021 12:09:48 +0100 Subject: [PATCH 4/6] Update tox.ini to remove py35 --- tox.ini | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/tox.ini b/tox.ini index b2bc6f23ef..998b04b224 100644 --- a/tox.ini +++ b/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 - # 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]commands} [testenv:benchmark] deps = From 77e56deffcf685ca5d0a264059f0c326e53e99af Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 8 Apr 2021 15:26:49 +0100 Subject: [PATCH 5/6] update test_old_deps script --- .buildkite/scripts/test_old_deps.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.buildkite/scripts/test_old_deps.sh b/.buildkite/scripts/test_old_deps.sh index 3753f41a40..9270d55f04 100755 --- a/.buildkite/scripts/test_old_deps.sh +++ b/.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 From 24c58ebfc992eec4b37c5aead8d9eef8e7afdd16 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 8 Apr 2021 18:29:57 +0100 Subject: [PATCH 6/6] remove unused param on `make_tuple_comparison_clause` --- synapse/storage/database.py | 5 +---- synapse/storage/databases/main/client_ips.py | 1 - synapse/storage/databases/main/devices.py | 2 +- synapse/storage/databases/main/events_bg_updates.py | 1 - tests/storage/test_database.py | 3 +-- 5 files changed, 3 insertions(+), 9 deletions(-) diff --git a/synapse/storage/database.py b/synapse/storage/database.py index a2f016a7ef..b302cd5786 100644 --- a/synapse/storage/database.py +++ b/synapse/storage/database.py @@ -2059,15 +2059,12 @@ def make_in_list_sql_clause( KV = TypeVar("KV") -def make_tuple_comparison_clause( - _database_engine: BaseDatabaseEngine, keys: List[Tuple[str, KV]] -) -> Tuple[str, List[KV]]: +def make_tuple_comparison_clause(keys: List[Tuple[str, KV]]) -> Tuple[str, List[KV]]: """Returns a tuple comparison SQL clause Builds a SQL clause that looks like "(a, b) > (?, ?)" Args: - _database_engine keys: A set of (column, value) pairs to be compared. Returns: diff --git a/synapse/storage/databases/main/client_ips.py b/synapse/storage/databases/main/client_ips.py index 6d18e692b0..ea3c15fd0e 100644 --- a/synapse/storage/databases/main/client_ips.py +++ b/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)], ) diff --git a/synapse/storage/databases/main/devices.py b/synapse/storage/databases/main/devices.py index d327e9aa0b..9bf8ba888f 100644 --- a/synapse/storage/databases/main/devices.py +++ b/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 diff --git a/synapse/storage/databases/main/events_bg_updates.py b/synapse/storage/databases/main/events_bg_updates.py index 78367ea58d..79e7df6ca9 100644 --- a/synapse/storage/databases/main/events_bg_updates.py +++ b/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), diff --git a/tests/storage/test_database.py b/tests/storage/test_database.py index 1bba58fc03..a906d30e73 100644 --- a/tests/storage/test_database.py +++ b/tests/storage/test_database.py @@ -36,7 +36,6 @@ def _stub_db_engine(**kwargs) -> BaseDatabaseEngine: class TupleComparisonClauseTestCase(unittest.TestCase): def test_native_tuple_comparison(self): - db_engine = _stub_db_engine() - clause, args = make_tuple_comparison_clause(db_engine, [("a", 1), ("b", 2)]) + clause, args = make_tuple_comparison_clause([("a", 1), ("b", 2)]) self.assertEqual(clause, "(a,b) > (?,?)") self.assertEqual(args, [1, 2])