Skip to content

Commit fee4c3f

Browse files
committed
Record max pruned ID in a separate table
Rather than trying to infer it from the minimum ID in the table. We were seeing issues in CI due to not all device list updates having entries in the `device_lists_changes_in_rooms` table due to the user not being in any rooms. This meant that the returned minimum ID was larger than expected, causing failures when calling `get_all_device_list_changes(..)`. By explicitly tracking the max pruned ID, we don't have to worry about problems with trying to infer the actual max pruned ID.
1 parent 524b93f commit fee4c3f

3 files changed

Lines changed: 86 additions & 30 deletions

File tree

synapse/storage/databases/main/devices.py

Lines changed: 43 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1745,16 +1745,21 @@ def get_devices_not_accessed_since_txn(
17451745

17461746
return devices
17471747

1748-
def _get_min_device_lists_changes_in_room_txn(self, txn: LoggingTransaction) -> int:
1749-
"""Returns the minimum stream ID that we have entries for
1750-
`device_lists_changes_in_room`
1748+
def _get_max_pruned_device_lists_changes_in_room_txn(
1749+
self, txn: LoggingTransaction
1750+
) -> int:
1751+
"""Returns the maximum stream ID that has been pruned from
1752+
`device_lists_changes_in_room`.
1753+
1754+
Any queries for stream IDs less than this value cannot be answered
1755+
completely, as the data has been deleted.
17511756
"""
17521757

17531758
return self.db_pool.simple_select_one_onecol_txn(
17541759
txn,
1755-
table="device_lists_changes_in_room",
1760+
table="device_lists_changes_in_room_max_pruned_stream_id",
17561761
keyvalues={},
1757-
retcol="COALESCE(MIN(stream_id), 0)",
1762+
retcol="stream_id",
17581763
allow_none=False,
17591764
)
17601765

@@ -1783,9 +1788,11 @@ async def get_device_list_changes_in_rooms(
17831788
def _get_device_list_changes_in_rooms_txn(
17841789
txn: LoggingTransaction,
17851790
) -> set[str] | None:
1786-
# Check if the from_token is too old.
1787-
lowest_known_stream_id = self._get_min_device_lists_changes_in_room_txn(txn)
1788-
if lowest_known_stream_id > from_token.stream:
1791+
# Check if the from_token is too old (i.e. data has been pruned).
1792+
max_pruned_stream_id = (
1793+
self._get_max_pruned_device_lists_changes_in_room_txn(txn)
1794+
)
1795+
if max_pruned_stream_id > from_token.stream:
17891796
return None
17901797

17911798
changes: set[str] = set()
@@ -1831,10 +1838,16 @@ async def get_all_device_list_changes(self, from_id: int, to_id: int) -> set[str
18311838
def _get_all_device_list_changes_txn(
18321839
txn: LoggingTransaction,
18331840
) -> set[str] | None:
1834-
# Check if the from_token is too old. We do this each time as we may
1835-
# prune the table in between runs.
1836-
lowest_known_stream_id = self._get_min_device_lists_changes_in_room_txn(txn)
1837-
if lowest_known_stream_id > from_id:
1841+
# Check if the from_token is too old (i.e. data has been pruned).
1842+
max_pruned_stream_id = (
1843+
self._get_max_pruned_device_lists_changes_in_room_txn(txn)
1844+
)
1845+
if max_pruned_stream_id > from_id:
1846+
logger.warning(
1847+
"Given stream ID is too old %d < %d",
1848+
from_id,
1849+
max_pruned_stream_id,
1850+
)
18381851
return None
18391852

18401853
sql = """
@@ -1851,7 +1864,7 @@ def _get_all_device_list_changes_txn(
18511864
)
18521865

18531866
if room_ids is None:
1854-
raise Exception("Given stream ID is too old")
1867+
raise Exception(f"Given stream ID is too old {from_id}")
18551868

18561869
return room_ids
18571870

@@ -1870,9 +1883,11 @@ async def get_device_list_changes_in_room(
18701883
def get_device_list_changes_in_room_txn(
18711884
txn: LoggingTransaction,
18721885
) -> Collection[tuple[str, str]] | None:
1873-
# Check if the from_token is too old.
1874-
lowest_known_stream_id = self._get_min_device_lists_changes_in_room_txn(txn)
1875-
if lowest_known_stream_id > min_stream_id:
1886+
# Check if the from_token is too old (i.e. data has been pruned).
1887+
max_pruned_stream_id = (
1888+
self._get_max_pruned_device_lists_changes_in_room_txn(txn)
1889+
)
1890+
if max_pruned_stream_id > min_stream_id:
18761891
return None
18771892

18781893
sql = """
@@ -2511,10 +2526,9 @@ def get_prune_before_stream_id_txn(txn: LoggingTransaction) -> int | None:
25112526
if prune_before_stream_id is None:
25122527
return
25132528

2514-
# Get the max stream ID that we have in the table, so that we avoid
2515-
# deleting it. We want to keep the max stream ID so that the minimum
2516-
# stream ID can be calculated in
2517-
# `_get_min_device_lists_changes_in_room`.
2529+
# Get the max stream ID in the table so we avoid deleting it. We need
2530+
# to keep the latest row so that we can calculate the maximum stream ID
2531+
# used.
25182532
max_stream_id = await self.db_pool.simple_select_one_onecol(
25192533
table="device_lists_changes_in_room",
25202534
keyvalues={},
@@ -2586,6 +2600,15 @@ def prune_device_lists_changes_in_room_txn(txn: LoggingTransaction) -> int:
25862600
await self.clock.sleep(Duration(milliseconds=100))
25872601

25882602
if num_rows_deleted:
2603+
# Update the max pruned stream ID tracking table so that the
2604+
# safety check knows data up to this point has been deleted.
2605+
await self.db_pool.simple_update_one(
2606+
table="device_lists_changes_in_room_max_pruned_stream_id",
2607+
keyvalues={},
2608+
updatevalues={"stream_id": prune_before_stream_id},
2609+
desc="prune_device_lists_changes_in_room_update_max_pruned",
2610+
)
2611+
25892612
logger.info(
25902613
"Pruned %d rows from device_lists_changes_in_room", num_rows_deleted
25912614
)
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
--
2+
-- This file is licensed under the Affero General Public License (AGPL) version 3.
3+
--
4+
-- Copyright (C) 2026 Element Creations Ltd
5+
--
6+
-- This program is free software: you can redistribute it and/or modify
7+
-- it under the terms of the GNU Affero General Public License as
8+
-- published by the Free Software Foundation, either version 3 of the
9+
-- License, or (at your option) any later version.
10+
--
11+
-- See the GNU Affero General Public License for more details:
12+
-- <https://www.gnu.org/licenses/agpl-3.0.html>.
13+
14+
15+
-- Tracks the maximum stream_id that has been deleted (pruned) from the
16+
-- device_lists_changes_in_room table. This is used to determine whether
17+
-- it's safe to read from that table for a given stream_id — if the
18+
-- requested stream_id is <= the value here, the data has been pruned and
19+
-- the table cannot provide a complete answer.
20+
--
21+
-- This replaces the previous approach of using MIN(stream_id) on the
22+
-- device_lists_changes_in_room table, which incorrectly returned 0 when
23+
-- the table was empty.
24+
CREATE TABLE IF NOT EXISTS device_lists_changes_in_room_max_pruned_stream_id (
25+
Lock CHAR(1) NOT NULL DEFAULT 'X' UNIQUE,
26+
stream_id BIGINT NOT NULL
27+
);
28+
29+
-- Seed with the current minimum stream_id minus 1, or 0 if the table is empty.
30+
INSERT INTO device_lists_changes_in_room_max_pruned_stream_id (stream_id)
31+
SELECT COALESCE(MIN(stream_id) - 1, 0) FROM device_lists_changes_in_room;

tests/storage/test_devices.py

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -399,12 +399,12 @@ def get_devices_in_room_status() -> tuple[int, str]:
399399
self.assertEqual(count, 10 * len(room_ids))
400400
self.assertEqual(min_device_id, "device_id0")
401401

402-
# Record the minimum stream ID before pruning, so we can check that this
403-
# correctly updates after pruning (as it is cached).
404-
starting_min_device_lists_id = self.get_success(
402+
# Record the max pruned stream ID before pruning, so we can check
403+
# that this correctly updates after pruning.
404+
starting_max_pruned_id = self.get_success(
405405
self.store.db_pool.runInteraction(
406-
"get_min_device_lists_changes_in_room",
407-
self.store._get_min_device_lists_changes_in_room_txn,
406+
"get_max_pruned_device_lists_changes_in_room",
407+
self.store._get_max_pruned_device_lists_changes_in_room_txn,
408408
)
409409
)
410410

@@ -442,14 +442,16 @@ def get_devices_in_room_status() -> tuple[int, str]:
442442
self.reactor.advance(Duration(milliseconds=110).as_secs())
443443

444444
count, min_device_id = get_devices_in_room_status()
445+
# We should always keep the most recent entries so that we can
446+
# calculate the maximum stream ID used.
445447
self.assertEqual(count, len(room_ids))
446448
self.assertEqual(min_device_id, "device_id19")
447449

448-
# Check that the minimum stream ID cache has been advanced after pruning.
449-
min_device_lists_id = self.get_success(
450+
# Check that the max pruned stream ID has been advanced after pruning.
451+
max_pruned_id = self.get_success(
450452
self.store.db_pool.runInteraction(
451-
"get_min_device_lists_changes_in_room",
452-
self.store._get_min_device_lists_changes_in_room_txn,
453+
"get_max_pruned_device_lists_changes_in_room",
454+
self.store._get_max_pruned_device_lists_changes_in_room_txn,
453455
)
454456
)
455-
self.assertGreater(min_device_lists_id, starting_min_device_lists_id)
457+
self.assertGreater(max_pruned_id, starting_max_pruned_id)

0 commit comments

Comments
 (0)