When loading current ids, sort by `stream_id` to avoid incorrect overwrite and avoid errors caused by sorting alphabetical instance name which can be `null` (#13585)
When loading current ids, sort by stream ID so that we don't want to overwrite the `current_position` of an instance to a lower stream ID than we're actually at ([discussion](https://github.com/matrix-org/synapse/pull/13585#discussion_r951795379)). Previously, it sorted alphabetically by instance name which can be `null` and throw errors but more importantly, accomplishes nothing. Fixes the following startup error which is why I started looking into this area: ``` $ poetry run synapse_homeserver --config-path homeserver.yaml **************************************************************** Error during initialisation: '<' not supported between instances of 'NoneType' and 'str' There may be more information in the logs. **************************************************************** ``` Somehow my database ended up looking like the following, notice the `instance_name` is `null` in the db, and we can't sort `NoneType` things. Another question is why do we see the `instance_name` as `null` sometimes instead of `master` in monolith mode? ``` $ psql synapse synapse=# SELECT * FROM stream_positions; stream_name | instance_name | stream_id -----------------+---------------+----------- account_data | master | 1242 events | master | 1787 to_device | master | 58 presence_stream | master | 485638 receipts | master | 341 backfill | master | -139106 (6 rows) synapse=# SELECT instance_name, stream_id FROM receipts_linearized; instance_name | stream_id ---------------+----------- | 211 | 3 | 4 | 212 | 213 | 224 | 228 | 164 | 313 | 253 | 38 | 321 | 324 | 189 | 192 | 193 | 194 | 195 | 197 | 198 | 275 | 79 | 339 | 340 | 82 | 341 | 84 | 85 | 91 | 119 ```
This commit is contained in:
parent
c807b814ae
commit
b93bd95e8a
|
@ -0,0 +1 @@
|
|||
Fix loading the current stream position behind the actual position.
|
|
@ -460,8 +460,17 @@ class MultiWriterIdGenerator(AbstractStreamIdGenerator):
|
|||
# Cast safety: this corresponds to the types returned by the query above.
|
||||
rows.extend(cast(Iterable[Tuple[str, int]], cur))
|
||||
|
||||
# Sort so that we handle rows in order for each instance.
|
||||
rows.sort()
|
||||
# Sort by stream_id (ascending, lowest -> highest) so that we handle
|
||||
# rows in order for each instance because we don't want to overwrite
|
||||
# the current_position of an instance to a lower stream ID than
|
||||
# we're actually at.
|
||||
def sort_by_stream_id_key_func(row: Tuple[str, int]) -> int:
|
||||
(instance, stream_id) = row
|
||||
# If `stream_id` is ever `None`, we will see a `TypeError: '<'
|
||||
# not supported between instances of 'NoneType' and 'X'` error.
|
||||
return stream_id
|
||||
|
||||
rows.sort(key=sort_by_stream_id_key_func)
|
||||
|
||||
with self._lock:
|
||||
for (
|
||||
|
|
Loading…
Reference in New Issue