From 5c1d301fd9e669b6704d54caeb1e8a3a223ba053 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 26 Jul 2018 14:12:00 +0100 Subject: [PATCH 1/3] Stop populating events.content This field is no longer read from, so we should stop populating it. Once we're happy that this doesn't break everything, and a rollback is unlikely, we can think about dropping the column. --- synapse/storage/events.py | 1 - .../delta/50/make_event_content_nullable.py | 93 +++++++++++++++++++ 2 files changed, 93 insertions(+), 1 deletion(-) create mode 100644 synapse/storage/schema/delta/50/make_event_content_nullable.py diff --git a/synapse/storage/events.py b/synapse/storage/events.py index 200f5ec95f..94515cd153 100644 --- a/synapse/storage/events.py +++ b/synapse/storage/events.py @@ -1189,7 +1189,6 @@ class EventsStore(EventsWorkerStore): "type": event.type, "processed": True, "outlier": event.internal_metadata.is_outlier(), - "content": encode_json(event.content).decode("UTF-8"), "origin_server_ts": int(event.origin_server_ts), "received_ts": self._clock.time_msec(), "sender": event.sender, diff --git a/synapse/storage/schema/delta/50/make_event_content_nullable.py b/synapse/storage/schema/delta/50/make_event_content_nullable.py new file mode 100644 index 0000000000..fa4a289514 --- /dev/null +++ b/synapse/storage/schema/delta/50/make_event_content_nullable.py @@ -0,0 +1,93 @@ +# -*- coding: utf-8 -*- +# Copyright 2018 New Vector Ltd +# +# 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. + +""" +We want to stop populating 'event.content', so we need to make it nullable. + +If this has to be rolled back, then the following should populate the missing data: + +Postgres: + + UPDATE events SET content=(ej.json::json)->'content' FROM event_json ej + WHERE ej.event_id = events.event_id AND + stream_ordering < ( + SELECT stream_ordering FROM events WHERE content IS NOT NULL + ORDER BY stream_ordering LIMIT 1 + ); + + UPDATE events SET content=(ej.json::json)->'content' FROM event_json ej + WHERE ej.event_id = events.event_id AND + stream_ordering > ( + SELECT stream_ordering FROM events WHERE content IS NOT NULL + ORDER BY stream_ordering DESC LIMIT 1 + ); + +SQLite: + + UPDATE events SET content=( + SELECT json_extract(json,'$.content') FROM event_json ej + WHERE ej.event_id = events.event_id + ) + WHERE + stream_ordering < ( + SELECT stream_ordering FROM events WHERE content IS NOT NULL + ORDER BY stream_ordering LIMIT 1 + ) + OR stream_ordering > ( + SELECT stream_ordering FROM events WHERE content IS NOT NULL + ORDER BY stream_ordering DESC LIMIT 1 + ); + +""" + +import logging + +from synapse.storage.engines import PostgresEngine + +logger = logging.getLogger(__name__) + + +def run_create(cur, database_engine, *args, **kwargs): + if isinstance(database_engine, PostgresEngine): + cur.execute(""" + ALTER TABLE events ALTER COLUMN content DROP NOT NULL; + """) + return + + # sqlite is an arse about this. ref: https://www.sqlite.org/lang_altertable.html + cur.execute("PRAGMA schema_version") + (oldver,) = cur.fetchone() + + cur.execute("SELECT sql FROM sqlite_master WHERE tbl_name='events' AND type='table'") + (oldsql,) = cur.fetchone() + sql = oldsql.replace("content TEXT NOT NULL", "content TEXT") + if sql == oldsql: + raise Exception("Couldn't find null constraint to drop in %s" % oldsql) + + logger.info("Replacing definition of 'events' with: %s", sql) + + cur.execute("PRAGMA writable_schema=ON") + + cur.execute( + "UPDATE sqlite_master SET sql=? WHERE tbl_name='events' AND type='table'", + (sql, ), + ) + + cur.execute("PRAGMA schema_version=%i" % (oldver+1,)) + cur.execute("PRAGMA writable_schema=OFF") + + +def run_upgrade(*args, **kwargs): + pass From 51d7df19158de0f21e659625bf43716ff0a700bc Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 26 Jul 2018 14:54:04 +0100 Subject: [PATCH 2/3] Create the column nullable There's no real point in ever making the column non-nullable, and doing so breaks the sytests. --- .../delta/50/make_event_content_nullable.py | 15 +++++++-------- synapse/storage/schema/full_schemas/16/im.sql | 7 ++++++- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/synapse/storage/schema/delta/50/make_event_content_nullable.py b/synapse/storage/schema/delta/50/make_event_content_nullable.py index fa4a289514..7d27342e39 100644 --- a/synapse/storage/schema/delta/50/make_event_content_nullable.py +++ b/synapse/storage/schema/delta/50/make_event_content_nullable.py @@ -60,6 +60,10 @@ logger = logging.getLogger(__name__) def run_create(cur, database_engine, *args, **kwargs): + pass + + +def run_upgrade(cur, database_engine, *args, **kwargs): if isinstance(database_engine, PostgresEngine): cur.execute(""" ALTER TABLE events ALTER COLUMN content DROP NOT NULL; @@ -67,27 +71,22 @@ def run_create(cur, database_engine, *args, **kwargs): return # sqlite is an arse about this. ref: https://www.sqlite.org/lang_altertable.html - cur.execute("PRAGMA schema_version") - (oldver,) = cur.fetchone() cur.execute("SELECT sql FROM sqlite_master WHERE tbl_name='events' AND type='table'") (oldsql,) = cur.fetchone() + sql = oldsql.replace("content TEXT NOT NULL", "content TEXT") if sql == oldsql: raise Exception("Couldn't find null constraint to drop in %s" % oldsql) logger.info("Replacing definition of 'events' with: %s", sql) + cur.execute("PRAGMA schema_version") + (oldver,) = cur.fetchone() cur.execute("PRAGMA writable_schema=ON") - cur.execute( "UPDATE sqlite_master SET sql=? WHERE tbl_name='events' AND type='table'", (sql, ), ) - cur.execute("PRAGMA schema_version=%i" % (oldver+1,)) cur.execute("PRAGMA writable_schema=OFF") - - -def run_upgrade(*args, **kwargs): - pass diff --git a/synapse/storage/schema/full_schemas/16/im.sql b/synapse/storage/schema/full_schemas/16/im.sql index ba5346806e..5f5cb8d01d 100644 --- a/synapse/storage/schema/full_schemas/16/im.sql +++ b/synapse/storage/schema/full_schemas/16/im.sql @@ -19,7 +19,12 @@ CREATE TABLE IF NOT EXISTS events( event_id TEXT NOT NULL, type TEXT NOT NULL, room_id TEXT NOT NULL, - content TEXT NOT NULL, + + -- 'content' used to be created NULLable, but as of delta 50 we drop that constraint. + -- the hack we use to drop the constraint doesn't work for an in-memory sqlite + -- database, which breaks the sytests. Hence, we no longer make it nullable. + content TEXT, + unrecognized_keys TEXT, processed BOOL NOT NULL, outlier BOOL NOT NULL, From 85531a06a27b0168b84d79fcc5102d2c204ba330 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 26 Jul 2018 14:55:29 +0100 Subject: [PATCH 3/3] changelog --- changelog.d/3614.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/3614.misc diff --git a/changelog.d/3614.misc b/changelog.d/3614.misc new file mode 100644 index 0000000000..356f28471e --- /dev/null +++ b/changelog.d/3614.misc @@ -0,0 +1 @@ +Stop populating events.content