element-hq / element-hq/synapse

portdb of new stream tables (thread subscriptions, sticky events) when exactly 1 row is inside can trigger startup failure

Open
#19,671 2 comments 0 reactions 0 assignees View on GitHub
A-Database A-Portdb T-Defect
Dominant language
Python
Stars
4.6k
Forks
600
Avg merge
5d 22h
Merged PRs (30d)
51

Description

[Discussed in `#synapse-dev:matrix.org`](https://matrix.to/#/!i5D5LLct_DYG-4hQprLzrxdbZ580U9UB6AEgFnk6rZQ/$Z3nqbH0Qy21FWC3qJOim6LSRCRpJ3pxV5DLXm98IA6I?via=element.io&via=matrix.org&via=beeper.com) with roots in https://github.com/element-hq/synapse/pull/19558#discussion_r3013184415. STUB.

The `MultiWriterIdGenerator`, on SQLite and Postgres it starts with a different number

I think SQLite starts at 1, Postgres at 2

The way we create new Postgres sequences in the database is bent around this fact, because we `SELECT nextval()` on the sequence to advance it to position 2

When you port a DB that has 1 row of data, the stream fact row has id=1 in it, but the Postgres sequence is at 2.
This causes the error on startup:

```python
_INCONSISTENT_STREAM_ERROR = """
Postgres sequence '%(seq)s' is inconsistent with associated stream position
of '%(stream_name)s' in the 'stream_positions' table.

This is likely a programming error and should be reported at
https://github.com/matrix-org/synapse.

A temporary workaround to fix this error is to shut down Synapse (including
any and all workers) and run the following SQL:

DELETE FROM stream_positions WHERE stream_name = '%(stream_name)s';

This will need to be done every time the server is restarted.
"""
...
# If we have values in the stream positions table then they have to be
# less than or equal to `last_value`
if max_in_stream_positions and max_in_stream_positions > last_value:
raise IncorrectDatabaseSetup(
_INCONSISTENT_STREAM_ERROR
% {"seq": self._sequence_name, "stream_name": stream_name}
)
```

On the other hand, if the Postgres sequence falls behind the highest number in the fact row, it prints a WARN and advances it for you, because it's some safety around rollbacks

```python
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. Updating sequence.",
self._sequence_name,
table,
last_value,
max_stream_id,
)

sql = f"""
SELECT setval('{self._sequence_name}', GREATEST(
(SELECT last_value FROM {self._sequence_name}),
({table_sql})
));
"""
txn.execute(sql)
```

So this problem probably happens with any of the new streams (thread subs, sticky events, quarantined media) where someone adds 1 row in SQLite and then ports.

Discovered in https://github.com/element-hq/synapse/pull/19558#discussion_r3013184415 where the PR adds a new stream for quarantined media, populated from the existing quarantined media. The portdb sample database has exactly 1 piece of quarantined media in it.

Contributor guide

Open the contributing guide

Research direction

Locate the MultiWriterIdGenerator startup checks for stream_positions and the PostgreSQL sequence handling, then reproduce the portdb case with exactly one row in a new stream such as quarantined media. Compare SQLite and PostgreSQL positions and verify that a ported database starts successfully without requiring the documented DELETE workaround.

Written by the indexing model from the issue text.

Assessment

Tech stack
postgresql, python, sqlite
Domain
backend, databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.