Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Fix rolling back when using workers #11255

Merged
merged 4 commits into from Nov 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/11255.bugfix
@@ -0,0 +1 @@
Fix rolling back Synapse version when using workers.
23 changes: 12 additions & 11 deletions synapse/storage/prepare_database.py
Expand Up @@ -133,22 +133,23 @@ 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:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change from != to < is the real fix here, I believe?

Is it worth adding a comment saying "If the database hasn't been updated yet, refuse to start" or something?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's pretty much what the comment at line 134 is saying, though I kindof agree that it might be clearer to move it down now.

# 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)
)

_upgrade_existing_database(
cur,
version_info,
database_engine,
config,
databases=databases,
)
else:
logger.info("%r: Initialising new database", databases)

Expand Down
69 changes: 69 additions & 0 deletions tests/storage/test_rollback_worker.py
@@ -0,0 +1,69 @@
# 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",
)

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)

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",
)

cur = db_conn.cursor()
cur.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)