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 2 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.
20 changes: 9 additions & 11 deletions synapse/storage/prepare_database.py
Expand Up @@ -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:
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.

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
65 changes: 65 additions & 0 deletions 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)