atagen

matrix-rust-sdk

public
Star0
Fork0
Watch0
23 commits4 branches0 tags0 releases16.6 MiB

Commit fb4e6801445b

tarball

atagen <boss@atagen.co>2026-10-25 08:58 UTC

1 file changed+129-105
sqlite: read the event cache through the read pool again

72db3bb7d moved load_all_chunks, load_last_chunk, load_previous_chunk, the metadata load, filter_duplicated_events and find_event_relations into immediate write transactions only so register_linked_chunk_owner could run its one-time INSERT, which serialised every read behind every write. Registration stays with handle_linked_chunk_updates, and reads instead check linked_chunk_is_stale so legacy rows under a cleared room stay invisible until the next write deletes them.
crates/matrix-sdk-sqlite/src/event_cache_store.rs+129−105
@@ -1037,12 +1037,10 @@ impl EventCacheStore for SqliteEventCacheStore {
10371037
10381038 let this = self.clone();
10391039
1040- let result = with_immediate_transaction(self, move |txn| -> Result<_> {
1041- register_linked_chunk_owner(
1042- txn,
1043- &hashed_linked_chunk_id,
1044- &hashed_room_id,
1045- )?;
1040+ let result = self.read().await?.with_transaction(move |txn| -> Result<_> {
1041+ if linked_chunk_is_stale(txn, &hashed_linked_chunk_id, &hashed_room_id)? {
1042+ return Ok(Default::default());
1043+ }
10461044
10471045 let mut items = Vec::new();
10481046
@@ -1084,51 +1082,55 @@ impl EventCacheStore for SqliteEventCacheStore {
10841082 let linked_chunk_id = linked_chunk_id.to_owned();
10851083 let hashed_room_id = self.encode_key(keys::LINKED_CHUNKS, linked_chunk_id.room_id());
10861084
1087- with_immediate_transaction(self, move |txn| -> Result<_> {
1088- register_linked_chunk_owner(txn, &hashed_linked_chunk_id, &hashed_room_id)?;
1089-
1090- // We want to collect the metadata about each chunk (id, next, previous), and
1091- // for event chunks, the number of events in it. For gaps, the
1092- // number of events is 0, by convention.
1093- //
1094- // We've tried different strategies over time:
1095- // - use a `LEFT JOIN` + `COUNT`, which was extremely inefficient because it
1096- // caused a full table traversal for each chunk, including for gaps which
1097- // don't have any events. This happened in
1098- // https://github.com/matrix-org/matrix-rust-sdk/pull/5225.
1099- // - use a `CASE` statement on the chunk's type: if it's an event chunk, run an
1100- // additional `SELECT` query. It was an immense improvement, but still caused
1101- // one select query per event chunk. This happened in
1102- // https://github.com/matrix-org/matrix-rust-sdk/pull/5411.
1103- //
1104- // The current solution is to run two queries:
1105- // - one to get each chunk and its number of events, by doing a single `SELECT`
1106- // query over the `event_chunks` table, grouping by chunk ids. This gives us a
1107- // list of `(chunk_id, num_events)` pairs, which can be transformed into a
1108- // hashmap.
1109- // - one to get each chunk's metadata (id, previous, next, type) from the
1110- // database with a `SELECT`, and then use the hashmap to get the number of
1111- // events.
1112- //
1113- // This strategy minimizes the number of queries to the database, and keeps them
1114- // super simple, while doing a bit more processing here, which is much faster.
1115-
1116- let num_events_by_chunk_ids = txn
1117- .prepare(
1118- r#"
1085+ self.read()
1086+ .await?
1087+ .with_transaction(move |txn| -> Result<_> {
1088+ if linked_chunk_is_stale(txn, &hashed_linked_chunk_id, &hashed_room_id)? {
1089+ return Ok(Default::default());
1090+ }
1091+
1092+ // We want to collect the metadata about each chunk (id, next, previous), and
1093+ // for event chunks, the number of events in it. For gaps, the
1094+ // number of events is 0, by convention.
1095+ //
1096+ // We've tried different strategies over time:
1097+ // - use a `LEFT JOIN` + `COUNT`, which was extremely inefficient because it
1098+ // caused a full table traversal for each chunk, including for gaps which
1099+ // don't have any events. This happened in
1100+ // https://github.com/matrix-org/matrix-rust-sdk/pull/5225.
1101+ // - use a `CASE` statement on the chunk's type: if it's an event chunk, run an
1102+ // additional `SELECT` query. It was an immense improvement, but still caused
1103+ // one select query per event chunk. This happened in
1104+ // https://github.com/matrix-org/matrix-rust-sdk/pull/5411.
1105+ //
1106+ // The current solution is to run two queries:
1107+ // - one to get each chunk and its number of events, by doing a single `SELECT`
1108+ // query over the `event_chunks` table, grouping by chunk ids. This gives us a
1109+ // list of `(chunk_id, num_events)` pairs, which can be transformed into a
1110+ // hashmap.
1111+ // - one to get each chunk's metadata (id, previous, next, type) from the
1112+ // database with a `SELECT`, and then use the hashmap to get the number of
1113+ // events.
1114+ //
1115+ // This strategy minimizes the number of queries to the database, and keeps them
1116+ // super simple, while doing a bit more processing here, which is much faster.
1117+
1118+ let num_events_by_chunk_ids = txn
1119+ .prepare(
1120+ r#"
11191121 SELECT ec.chunk_id, COUNT(ec.event_id)
11201122 FROM event_chunks as ec
11211123 WHERE ec.linked_chunk_id = ?
11221124 GROUP BY ec.chunk_id
11231125 "#,
1124- )?
1125- .query_map((&hashed_linked_chunk_id,), |row| {
1126- Ok((row.get::<_, u64>(0)?, row.get::<_, usize>(1)?))
1127- })?
1128- .collect::<Result<HashMap<_, _>, _>>()?;
1126+ )?
1127+ .query_map((&hashed_linked_chunk_id,), |row| {
1128+ Ok((row.get::<_, u64>(0)?, row.get::<_, usize>(1)?))
1129+ })?
1130+ .collect::<Result<HashMap<_, _>, _>>()?;
11291131
1130- txn.prepare(
1131- r#"
1132+ txn.prepare(
1133+ r#"
11321134 SELECT
11331135 lc.id,
11341136 lc.previous,
@@ -1137,39 +1139,39 @@ impl EventCacheStore for SqliteEventCacheStore {
11371139 FROM linked_chunks as lc
11381140 WHERE lc.linked_chunk_id = ?
11391141 ORDER BY lc.id"#,
1140- )?
1141- .query_map((&hashed_linked_chunk_id,), |row| {
1142- Ok((
1143- row.get::<_, u64>(0)?,
1144- row.get::<_, Option<u64>>(1)?,
1145- row.get::<_, Option<u64>>(2)?,
1146- row.get::<_, String>(3)?,
1147- ))
1148- })?
1149- .map(|data| -> Result<_> {
1150- let (id, previous, next, chunk_type) = data?;
1151-
1152- // Note: since a gap has 0 events, an alternative could be to *not* retrieve
1153- // the chunk type, and just let the hashmap lookup fail for gaps. However,
1154- // benchmarking shows that this is slightly slower than matching the chunk
1155- // type (around 1%, so in the realm of noise), so we keep the explicit
1156- // check instead.
1157- let num_items = if chunk_type == CHUNK_TYPE_GAP_TYPE_STRING {
1158- 0
1159- } else {
1160- num_events_by_chunk_ids.get(&id).copied().unwrap_or(0)
1161- };
1142+ )?
1143+ .query_map((&hashed_linked_chunk_id,), |row| {
1144+ Ok((
1145+ row.get::<_, u64>(0)?,
1146+ row.get::<_, Option<u64>>(1)?,
1147+ row.get::<_, Option<u64>>(2)?,
1148+ row.get::<_, String>(3)?,
1149+ ))
1150+ })?
1151+ .map(|data| -> Result<_> {
1152+ let (id, previous, next, chunk_type) = data?;
11621153
1163- Ok(ChunkMetadata {
1164- identifier: ChunkIdentifier::new(id),
1165- previous: previous.map(ChunkIdentifier::new),
1166- next: next.map(ChunkIdentifier::new),
1167- num_items,
1154+ // Note: since a gap has 0 events, an alternative could be to *not* retrieve
1155+ // the chunk type, and just let the hashmap lookup fail for gaps. However,
1156+ // benchmarking shows that this is slightly slower than matching the chunk
1157+ // type (around 1%, so in the realm of noise), so we keep the explicit
1158+ // check instead.
1159+ let num_items = if chunk_type == CHUNK_TYPE_GAP_TYPE_STRING {
1160+ 0
1161+ } else {
1162+ num_events_by_chunk_ids.get(&id).copied().unwrap_or(0)
1163+ };
1164+
1165+ Ok(ChunkMetadata {
1166+ identifier: ChunkIdentifier::new(id),
1167+ previous: previous.map(ChunkIdentifier::new),
1168+ next: next.map(ChunkIdentifier::new),
1169+ num_items,
1170+ })
11681171 })
1172+ .collect::<Result<Vec<_>, _>>()
11691173 })
1170- .collect::<Result<Vec<_>, _>>()
1171- })
1172- .await
1174+ .await
11731175 }
11741176
11751177 #[instrument(skip(self))]
@@ -1186,12 +1188,10 @@ impl EventCacheStore for SqliteEventCacheStore {
11861188
11871189 let this = self.clone();
11881190
1189- with_immediate_transaction(self, move |txn| -> Result<_> {
1190- register_linked_chunk_owner(
1191- txn,
1192- &hashed_linked_chunk_id,
1193- &hashed_room_id,
1194- )?;
1191+ self.read().await?.with_transaction(move |txn| -> Result<_> {
1192+ if linked_chunk_is_stale(txn, &hashed_linked_chunk_id, &hashed_room_id)? {
1193+ return Ok((None, ChunkIdentifierGenerator::new_from_scratch()));
1194+ }
11951195
11961196 // Find the latest chunk identifier to generate a `ChunkIdentifierGenerator`, and count the number of chunks.
11971197 let (observed_max_identifier, number_of_chunks) = txn
@@ -1287,12 +1287,10 @@ impl EventCacheStore for SqliteEventCacheStore {
12871287
12881288 let this = self.clone();
12891289
1290- with_immediate_transaction(self, move |txn| -> Result<_> {
1291- register_linked_chunk_owner(
1292- txn,
1293- &hashed_linked_chunk_id,
1294- &hashed_room_id,
1295- )?;
1290+ self.read().await?.with_transaction(move |txn| -> Result<_> {
1291+ if linked_chunk_is_stale(txn, &hashed_linked_chunk_id, &hashed_room_id)? {
1292+ return Ok(Default::default());
1293+ }
12961294
12971295 // Find the chunk before the chunk identified by `before_chunk_identifier`.
12981296 let Some((chunk_identifier, previous_chunk, next_chunk, chunk_type)) = txn
@@ -1369,12 +1367,10 @@ impl EventCacheStore for SqliteEventCacheStore {
13691367 let linked_chunk_id = linked_chunk_id.to_owned();
13701368 let hashed_room_id = self.encode_key(keys::LINKED_CHUNKS, linked_chunk_id.room_id());
13711369
1372- with_immediate_transaction(self, move |txn| -> Result<_> {
1373- register_linked_chunk_owner(
1374- txn,
1375- &hashed_linked_chunk_id,
1376- &hashed_room_id,
1377- )?;
1370+ self.read().await?.with_transaction(move |txn| -> Result<_> {
1371+ if linked_chunk_is_stale(txn, &hashed_linked_chunk_id, &hashed_room_id)? {
1372+ return Ok(Default::default());
1373+ }
13781374
13791375 txn.chunk_large_query_over(events, None, move |txn, events| {
13801376 let query = format!(
@@ -1485,19 +1481,23 @@ impl EventCacheStore for SqliteEventCacheStore {
14851481 let filters = filters.map(ToOwned::to_owned);
14861482 let store = self.clone();
14871483
1488- with_immediate_transaction(self, move |txn| -> Result<_> {
1489- register_linked_chunk_owner(txn, &hashed_linked_chunk_id, &hashed_room_id)?;
1484+ self.read()
1485+ .await?
1486+ .with_transaction(move |txn| -> Result<_> {
1487+ if linked_chunk_is_stale(txn, &hashed_linked_chunk_id, &hashed_room_id)? {
1488+ return Ok(Default::default());
1489+ }
14901490
1491- find_event_relations_transaction(
1492- store,
1493- hashed_room_id,
1494- hashed_linked_chunk_id,
1495- event_id,
1496- filters,
1497- txn,
1498- )
1499- })
1500- .await
1491+ find_event_relations_transaction(
1492+ store,
1493+ hashed_room_id,
1494+ hashed_linked_chunk_id,
1495+ event_id,
1496+ filters,
1497+ txn,
1498+ )
1499+ })
1500+ .await
15011501 }
15021502
15031503 #[instrument(skip(self))]
@@ -1791,6 +1791,30 @@ fn insert_chunk(
17911791 Ok(())
17921792 }
17931793
1794+/// Rows of a linked chunk that no write registered before its room was cleared
1795+/// are deleted by the next write, so reads must already treat them as gone.
1796+fn linked_chunk_is_stale(
1797+ txn: &Transaction<'_>,
1798+ linked_chunk_id: &Key,
1799+ room_id: &Key,
1800+) -> rusqlite::Result<bool> {
1801+ let registered = txn.query_row(
1802+ "SELECT EXISTS(SELECT 1 FROM linked_chunk_rooms WHERE linked_chunk_id = ?)",
1803+ [linked_chunk_id],
1804+ |row| row.get::<_, bool>(0),
1805+ )?;
1806+
1807+ if registered {
1808+ return Ok(false);
1809+ }
1810+
1811+ txn.query_row(
1812+ "SELECT EXISTS(SELECT 1 FROM cleared_rooms WHERE room_id = ?)",
1813+ [room_id],
1814+ |row| row.get::<_, bool>(0),
1815+ )
1816+}
1817+
17941818 fn register_linked_chunk_owner(
17951819 txn: &Transaction<'_>,
17961820 linked_chunk_id: &Key,