From 4c2ceacb4d4b84093bd5e67a8d80e193a241aaba Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 14 Jun 2024 12:26:29 +0100 Subject: [PATCH 1/4] Automatically apply SQL for inconsistent sequence Rather than forcing the server operator to apply the SQL manually. This should be safe, as there should be only one writer for these sequences. --- docs/postgres.md | 10 --------- synapse/storage/util/sequence.py | 35 ++++++++++++----------------- tests/storage/test_id_generators.py | 14 +++++++----- 3 files changed, 23 insertions(+), 36 deletions(-) diff --git a/docs/postgres.md b/docs/postgres.md index 4b2ba38275..d06f0cda10 100644 --- a/docs/postgres.md +++ b/docs/postgres.md @@ -255,13 +255,3 @@ however extreme care must be taken to avoid database corruption. Note that the above may fail with an error about duplicate rows if corruption has already occurred, and such duplicate rows will need to be manually removed. - -### Fixing inconsistent sequences error - -Synapse uses Postgres sequences to generate IDs for various tables. A sequence -and associated table can get out of sync if, for example, Synapse has been -downgraded and then upgraded again. - -To fix the issue shut down Synapse (including any and all workers) and run the -SQL command included in the error message. Once done Synapse should start -successfully. diff --git a/synapse/storage/util/sequence.py b/synapse/storage/util/sequence.py index f57e7ec41c..542263df74 100644 --- a/synapse/storage/util/sequence.py +++ b/synapse/storage/util/sequence.py @@ -36,21 +36,6 @@ logger = logging.getLogger(__name__) -_INCONSISTENT_SEQUENCE_ERROR = """ -Postgres sequence '%(seq)s' is inconsistent with associated -table '%(table)s'. This can happen if Synapse has been downgraded and -then upgraded again, or due to a bad migration. - -To fix this error, shut down Synapse (including any and all workers) -and run the following SQL: - - SELECT setval('%(seq)s', ( - %(max_id_sql)s - )); - -See docs/postgres.md for more information. -""" - _INCONSISTENT_STREAM_ERROR = """ Postgres sequence '%(seq)s' is inconsistent with associated stream position of '%(stream_name)s' in the 'stream_positions' table. @@ -169,14 +154,16 @@ def check_consistency( if row: max_in_stream_positions = row[0] - txn.close() - # If `is_called` is False then `last_value` is actually the value that # will be generated next, so we decrement to get the true "last value". if not is_called: last_value -= 1 if max_stream_id > last_value: + # The sequence is lagging behind the tables. This is probably due to + # rolling back to a version before the sequence was used and then + # forwards again. We resolve this by setting the sequence to the + # right value. logger.warning( "Postgres sequence %s is behind table %s: %d < %d", self._sequence_name, @@ -184,10 +171,16 @@ def check_consistency( last_value, max_stream_id, ) - raise IncorrectDatabaseSetup( - _INCONSISTENT_SEQUENCE_ERROR - % {"seq": self._sequence_name, "table": table, "max_id_sql": table_sql} - ) + + sql = f""" + SELECT setval('{self._sequence_name}', GREATEST( + (SELECT last_value FROM {self._sequence_name}), + ({table_sql}) + )); + """ + txn.execute(sql) + + txn.close() # If we have values in the stream positions table then they have to be # less than or equal to `last_value` diff --git a/tests/storage/test_id_generators.py b/tests/storage/test_id_generators.py index f0307252f3..0765d631bf 100644 --- a/tests/storage/test_id_generators.py +++ b/tests/storage/test_id_generators.py @@ -28,7 +28,6 @@ LoggingDatabaseConnection, LoggingTransaction, ) -from synapse.storage.engines import IncorrectDatabaseSetup from synapse.storage.types import Cursor from synapse.storage.util.id_generators import MultiWriterIdGenerator from synapse.storage.util.sequence import ( @@ -525,7 +524,7 @@ async def _get_next_async() -> None: self.assertEqual(id_gen_5.get_current_token_for_writer("third"), 6) def test_sequence_consistency(self) -> None: - """Test that we error out if the table and sequence diverges.""" + """Test that we correct sequence if the table and sequence diverges.""" # Prefill with some rows self._insert_row_with_id("master", 3) @@ -536,9 +535,14 @@ def _insert(txn: Cursor) -> None: self.get_success(self.db_pool.runInteraction("_insert", _insert)) - # Creating the ID gen should error - with self.assertRaises(IncorrectDatabaseSetup): - self._create_id_generator("first") + # Creating the ID gen should now fix the inconsistency + id_gen = self._create_id_generator() + + async def _get_next_async() -> None: + async with id_gen.get_next() as stream_id: + self.assertEqual(stream_id, 27) + + self.get_success(_get_next_async()) def test_minimal_local_token(self) -> None: self._insert_rows("first", 3) From b32cd23b905dec8f605ace3fbeac37e858d565d6 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 14 Jun 2024 12:28:59 +0100 Subject: [PATCH 2/4] Newsfile --- changelog.d/17305.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/17305.misc diff --git a/changelog.d/17305.misc b/changelog.d/17305.misc new file mode 100644 index 0000000000..cb6b9504b3 --- /dev/null +++ b/changelog.d/17305.misc @@ -0,0 +1 @@ +When rolling back to a previous Synapse version and then forwards again to this release, don't require server operators to manually run SQL. From 4bd969ba55a115df63a3ec7dc82365317c7da5ca Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 14 Jun 2024 16:07:47 +0100 Subject: [PATCH 3/4] Update tests/storage/test_id_generators.py Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> --- tests/storage/test_id_generators.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/storage/test_id_generators.py b/tests/storage/test_id_generators.py index 0765d631bf..9be2923e6f 100644 --- a/tests/storage/test_id_generators.py +++ b/tests/storage/test_id_generators.py @@ -524,7 +524,7 @@ async def _get_next_async() -> None: self.assertEqual(id_gen_5.get_current_token_for_writer("third"), 6) def test_sequence_consistency(self) -> None: - """Test that we correct sequence if the table and sequence diverges.""" + """Test that we correct the sequence if the table and sequence diverges.""" # Prefill with some rows self._insert_row_with_id("master", 3) From d065598c99d4b8acf04f645f72c47465f688ff7f Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 14 Jun 2024 16:08:43 +0100 Subject: [PATCH 4/4] Update logs --- synapse/storage/util/sequence.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/util/sequence.py b/synapse/storage/util/sequence.py index 542263df74..c4c0602b28 100644 --- a/synapse/storage/util/sequence.py +++ b/synapse/storage/util/sequence.py @@ -165,7 +165,7 @@ def check_consistency( # forwards again. We resolve this by setting the sequence to the # right value. logger.warning( - "Postgres sequence %s is behind table %s: %d < %d", + "Postgres sequence %s is behind table %s: %d < %d. Updating sequence.", self._sequence_name, table, last_value,