From 50b93722d48b46583a719498cb72c0020de084fa Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 4 Nov 2021 17:36:03 +0000 Subject: [PATCH 1/4] Fix rolling back when using workers Fixes #11252 --- synapse/storage/prepare_database.py | 20 ++++----- tests/storage/test_rollback_worker.py | 65 +++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 11 deletions(-) create mode 100644 tests/storage/test_rollback_worker.py diff --git a/synapse/storage/prepare_database.py b/synapse/storage/prepare_database.py index 1629d2a53c2c..f90dd5de1ba2 100644 --- a/synapse/storage/prepare_database.py +++ b/synapse/storage/prepare_database.py @@ -133,22 +133,20 @@ def prepare_database( # if it's a worker app, refuse to upgrade the database, to avoid multiple # workers doing it at once. - if ( - config.worker.worker_app is not None - and version_info.current_version != SCHEMA_VERSION - ): + if config.worker.worker_app is None: + _upgrade_existing_database( + cur, + version_info, + database_engine, + config, + databases=databases, + ) + elif version_info.current_version < SCHEMA_VERSION: raise UpgradeDatabaseException( OUTDATED_SCHEMA_ON_WORKER_ERROR % (SCHEMA_VERSION, version_info.current_version) ) - _upgrade_existing_database( - cur, - version_info, - database_engine, - config, - databases=databases, - ) else: logger.info("%r: Initialising new database", databases) diff --git a/tests/storage/test_rollback_worker.py b/tests/storage/test_rollback_worker.py new file mode 100644 index 000000000000..98d837e715e9 --- /dev/null +++ b/tests/storage/test_rollback_worker.py @@ -0,0 +1,65 @@ +# Copyright 2021 The Matrix.org Foundation C.I.C. +# +# 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. +from synapse.app.generic_worker import GenericWorkerServer +from synapse.storage.database import LoggingDatabaseConnection +from synapse.storage.prepare_database import PrepareDatabaseException, prepare_database +from synapse.storage.schema import SCHEMA_VERSION + +from tests.unittest import HomeserverTestCase + + +class WorkerSchemaTests(HomeserverTestCase): + def make_homeserver(self, reactor, clock): + hs = self.setup_test_homeserver( + federation_http_client=None, homeserver_to_use=GenericWorkerServer + ) + return hs + + def default_config(self): + conf = super().default_config() + + # Mark this as a worker app. + conf["worker_app"] = "yes" + + return conf + + def test_rolling_back(self): + """Test that workers can start if the DB is a newer schema version""" + + db_pool = self.hs.get_datastore().db_pool + db_conn = LoggingDatabaseConnection( + db_pool._db_pool.connect(), + db_pool.engine, + "tests", + ) + + db_conn.execute("UPDATE schema_version SET version = ?", (SCHEMA_VERSION + 1,)) + db_conn.commit() + + prepare_database(db_conn, db_pool.engine, self.hs.config) + + def test_not_upgraded(self): + """Test that workers don't start if the DB has an older schema version""" + db_pool = self.hs.get_datastore().db_pool + db_conn = LoggingDatabaseConnection( + db_pool._db_pool.connect(), + db_pool.engine, + "tests", + ) + + db_conn.execute("UPDATE schema_version SET version = ?", (SCHEMA_VERSION - 1,)) + db_conn.commit() + + with self.assertRaises(PrepareDatabaseException): + prepare_database(db_conn, db_pool.engine, self.hs.config) From ab8d3f00dc5d8ae5c30f7738766db08902d56402 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 4 Nov 2021 17:42:07 +0000 Subject: [PATCH 2/4] Newsfile --- changelog.d/11255.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/11255.bugfix diff --git a/changelog.d/11255.bugfix b/changelog.d/11255.bugfix new file mode 100644 index 000000000000..ce7259262439 --- /dev/null +++ b/changelog.d/11255.bugfix @@ -0,0 +1 @@ +Fix rolling back Synapse version when using workers. From e10cd3683e92a5e29e1dd5a221d44023e4f8b902 Mon Sep 17 00:00:00 2001 From: Dan Callahan Date: Thu, 4 Nov 2021 22:40:25 +0000 Subject: [PATCH 3/4] Fix tests Signed-off-by: Dan Callahan --- tests/storage/test_rollback_worker.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/storage/test_rollback_worker.py b/tests/storage/test_rollback_worker.py index 98d837e715e9..a6be9a1bb184 100644 --- a/tests/storage/test_rollback_worker.py +++ b/tests/storage/test_rollback_worker.py @@ -44,7 +44,9 @@ def test_rolling_back(self): "tests", ) - db_conn.execute("UPDATE schema_version SET version = ?", (SCHEMA_VERSION + 1,)) + cur = db_conn.cursor() + cur.execute("UPDATE schema_version SET version = ?", (SCHEMA_VERSION + 1,)) + db_conn.commit() prepare_database(db_conn, db_pool.engine, self.hs.config) @@ -58,7 +60,9 @@ def test_not_upgraded(self): "tests", ) - db_conn.execute("UPDATE schema_version SET version = ?", (SCHEMA_VERSION - 1,)) + cur = db_conn.cursor() + cur.execute("UPDATE schema_version SET version = ?", (SCHEMA_VERSION - 1,)) + db_conn.commit() with self.assertRaises(PrepareDatabaseException): From 3e0db26d6cefc5f2575471d66f2a40f0b46f264c Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 5 Nov 2021 10:42:52 +0000 Subject: [PATCH 4/4] Add comment --- synapse/storage/prepare_database.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/synapse/storage/prepare_database.py b/synapse/storage/prepare_database.py index f90dd5de1ba2..b5c1c14ee3d1 100644 --- a/synapse/storage/prepare_database.py +++ b/synapse/storage/prepare_database.py @@ -142,6 +142,9 @@ def prepare_database( databases=databases, ) elif version_info.current_version < SCHEMA_VERSION: + # If the DB is on an older version than we expect the we refuse + # to start the worker (as the main process needs to run first to + # update the schema). raise UpgradeDatabaseException( OUTDATED_SCHEMA_ON_WORKER_ERROR % (SCHEMA_VERSION, version_info.current_version)