summary refs log tree commit diff
diff options
context:
space:
mode:
authorSean Quah <8349537+squahtx@users.noreply.github.com>2023-01-25 13:38:53 +0000
committerGitHub <noreply@github.com>2023-01-25 13:38:53 +0000
commita63d4cc9e96c1f5bb9c5bb9fc9119fb137de3b1b (patch)
tree664de7b53d6e04cc31428f015752fc32ee840192
parentDocument the export user data command. (#14883) (diff)
downloadsynapse-a63d4cc9e96c1f5bb9c5bb9fc9119fb137de3b1b.tar.xz
Make sqlite database migrations transactional again (#14910)
#13873 introduced a regression which causes sqlite database migrations
to no longer run inside a transaction. Wrap them in a transaction again,
to avoid database corruption when migrations are interrupted.

Fixes #14909.

Signed-off-by: Sean Quah <seanq@matrix.org>
-rw-r--r--changelog.d/14910.bugfix1
-rw-r--r--synapse/storage/engines/_base.py3
-rw-r--r--synapse/storage/engines/sqlite.py5
3 files changed, 7 insertions, 2 deletions
diff --git a/changelog.d/14910.bugfix b/changelog.d/14910.bugfix
new file mode 100644
index 0000000000..f1f34cd6ba
--- /dev/null
+++ b/changelog.d/14910.bugfix
@@ -0,0 +1 @@
+Fix a regression introduced in Synapse 1.69.0 which can result in database corruption when database migrations are interrupted on sqlite.
diff --git a/synapse/storage/engines/_base.py b/synapse/storage/engines/_base.py
index 70e594a68f..bc9ca3a53c 100644
--- a/synapse/storage/engines/_base.py
+++ b/synapse/storage/engines/_base.py
@@ -132,6 +132,9 @@ class BaseDatabaseEngine(Generic[ConnectionType, CursorType], metaclass=abc.ABCM
         """Execute a chunk of SQL containing multiple semicolon-delimited statements.
 
         This is not provided by DBAPI2, and so needs engine-specific support.
+
+        Some database engines may automatically COMMIT the ongoing transaction both
+        before and after executing the script.
         """
         ...
 
diff --git a/synapse/storage/engines/sqlite.py b/synapse/storage/engines/sqlite.py
index 14260442b6..2f7df85ce4 100644
--- a/synapse/storage/engines/sqlite.py
+++ b/synapse/storage/engines/sqlite.py
@@ -135,13 +135,14 @@ class Sqlite3Engine(BaseDatabaseEngine[sqlite3.Connection, sqlite3.Cursor]):
         > than one statement with it, it will raise a Warning. Use executescript() if
         > you want to execute multiple SQL statements with one call.
 
-        Though the docs for `executescript` warn:
+        The script is wrapped in transaction control statemnets, since the docs for
+        `executescript` warn:
 
         > If there is a pending transaction, an implicit COMMIT statement is executed
         > first. No other implicit transaction control is performed; any transaction
         > control must be added to sql_script.
         """
-        cursor.executescript(script)
+        cursor.executescript(f"BEGIN TRANSACTION;\n{script}\nCOMMIT;")
 
 
 # Following functions taken from: https://github.com/coleifer/peewee