-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Properly setup more sequences in portdb #16043
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Fix a long-standing bug where the `synapse_port_db` failed to configure sequences for application services and partial stated rooms. |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -761,14 +761,18 @@ def alter_table(txn: LoggingTransaction) -> None: | |||||
|
||||||
# Step 2. Set up sequences | ||||||
# | ||||||
# We do this before porting the tables so that event if we fail half | ||||||
# We do this before porting the tables so that even if we fail half | ||||||
# way through the postgres DB always have sequences that are greater | ||||||
# than their respective tables. If we don't then creating the | ||||||
# `DataStore` object will fail due to the inconsistency. | ||||||
self.progress.set_state("Setting up sequence generators") | ||||||
await self._setup_state_group_id_seq() | ||||||
await self._setup_user_id_seq() | ||||||
await self._setup_events_stream_seqs() | ||||||
await self._setup_sequence( | ||||||
"un_partial_stated_event_stream_sequence", | ||||||
("un_partial_stated_event_stream",), | ||||||
) | ||||||
await self._setup_sequence( | ||||||
"device_inbox_sequence", ("device_inbox", "device_federation_outbox") | ||||||
) | ||||||
|
@@ -779,6 +783,11 @@ def alter_table(txn: LoggingTransaction) -> None: | |||||
await self._setup_sequence("receipts_sequence", ("receipts_linearized",)) | ||||||
await self._setup_sequence("presence_stream_sequence", ("presence_stream",)) | ||||||
await self._setup_auth_chain_sequence() | ||||||
await self._setup_sequence( | ||||||
"application_services_txn_id_seq", | ||||||
("application_services_txns",), | ||||||
"txn_id", | ||||||
DMRobertson marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
) | ||||||
|
||||||
# Step 3. Get tables. | ||||||
self.progress.set_state("Fetching tables") | ||||||
|
@@ -1083,7 +1092,10 @@ def _setup_events_stream_seqs_set_pos(txn: LoggingTransaction) -> None: | |||||
) | ||||||
|
||||||
async def _setup_sequence( | ||||||
self, sequence_name: str, stream_id_tables: Iterable[str] | ||||||
self, | ||||||
sequence_name: str, | ||||||
stream_id_tables: Iterable[str], | ||||||
column_name: str = "stream_id", | ||||||
) -> None: | ||||||
"""Set a sequence to the correct value.""" | ||||||
current_stream_ids = [] | ||||||
|
@@ -1093,7 +1105,7 @@ async def _setup_sequence( | |||||
await self.sqlite_store.db_pool.simple_select_one_onecol( | ||||||
table=stream_id_table, | ||||||
keyvalues={}, | ||||||
retcol="COALESCE(MAX(stream_id), 1)", | ||||||
retcol=f"COALESCE(MAX({column_name}), 1)", | ||||||
Comment on lines
-1096
to
+1108
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is fine as-is, but I have some thoughts as an aside. The error message quoted in the logs of #16040 uses I also vaguely remember reading some stream code that suggests streams could count backwards. synapse/synapse/storage/util/id_generators.py Lines 180 to 181 in 02f74f3
If negative-step streams are used gets used, I think this logic will be broken. But I suspect we never use it (and would vote to remove it). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we at some point changed these to start at (Note that this coalesces to |
||||||
allow_none=True, | ||||||
), | ||||||
) | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not quite sure why the
auth_chain_sequence
one has a separate method, it seems almost identical. 🤷