From 119bee66ce96fa8df4d1dcfffe4eee4a54fcd9b9 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 11 Sep 2024 15:11:51 +0200 Subject: [PATCH] feat(sdk,ui): `SlidingSync::subscribe_to_rooms` has a new `cancel_in_flight_request` argument. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch adds a new `cancel_in_flight_request` argument to `SlidingSync::subscribe_to_rooms`, which tells the method cancel the in-flight request if any. This patch also updates `RoomListService::subscribe_to_rooms` to turn this new argument to `true` if the state machine isn't in a “starting” state. The problem it's solving is the following: * some apps starts the room list service * a first request is sent with `pos = None` * the server calculates a new session (which can be expensive) * the app subscribes to a set of rooms * a second request is immediately sent with `pos = None` again * the server does possibly NOT cancel its previous calculations, but starts a new session and its calculations This is pretty expensive for the server. This patch makes so that the immediate room subscriptions will be part of the second request, with the first request not being cancelled. --- crates/matrix-sdk-ui/src/notification_client.rs | 1 + .../matrix-sdk-ui/src/room_list_service/mod.rs | 9 ++++++++- crates/matrix-sdk/src/sliding_sync/mod.rs | 17 +++++++++-------- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/crates/matrix-sdk-ui/src/notification_client.rs b/crates/matrix-sdk-ui/src/notification_client.rs index 6269363daa2..11b881de6ea 100644 --- a/crates/matrix-sdk-ui/src/notification_client.rs +++ b/crates/matrix-sdk-ui/src/notification_client.rs @@ -380,6 +380,7 @@ impl NotificationClient { required_state, timeline_limit: Some(uint!(16)) })), + true, ); let mut remaining_attempts = 3; diff --git a/crates/matrix-sdk-ui/src/room_list_service/mod.rs b/crates/matrix-sdk-ui/src/room_list_service/mod.rs index 1b5a57a3a96..f6045837f9e 100644 --- a/crates/matrix-sdk-ui/src/room_list_service/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list_service/mod.rs @@ -396,7 +396,14 @@ impl RoomListService { settings.required_state.push((StateEventType::RoomCreate, "".to_owned())); } - self.sliding_sync.subscribe_to_rooms(room_ids, Some(settings)) + let cancel_in_flight_request = match self.state.get() { + State::Init | State::Recovering | State::Error { .. } | State::Terminated { .. } => { + false + } + State::SettingUp | State::Running => true, + }; + + self.sliding_sync.subscribe_to_rooms(room_ids, Some(settings), cancel_in_flight_request) } #[cfg(test)] diff --git a/crates/matrix-sdk/src/sliding_sync/mod.rs b/crates/matrix-sdk/src/sliding_sync/mod.rs index ba271019f95..fff27f60405 100644 --- a/crates/matrix-sdk/src/sliding_sync/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/mod.rs @@ -151,12 +151,13 @@ impl SlidingSync { &self, room_ids: &[&RoomId], settings: Option, + cancel_in_flight_request: bool, ) { let settings = settings.unwrap_or_default(); let mut sticky = self.inner.sticky.write().unwrap(); let room_subscriptions = &mut sticky.data_mut().room_subscriptions; - let mut skip_sync_loop = false; + let mut skip_over_current_sync_loop_iteration = false; for room_id in room_ids { // If the room subscription already exists, let's not @@ -172,11 +173,11 @@ impl SlidingSync { entry.insert((RoomSubscriptionState::default(), settings.clone())); - skip_sync_loop = true; + skip_over_current_sync_loop_iteration = true; } } - if skip_sync_loop { + if cancel_in_flight_request && skip_over_current_sync_loop_iteration { self.inner.internal_channel_send_if_possible( SlidingSyncInternalMessage::SyncLoopSkipOverCurrentIteration, ); @@ -1219,7 +1220,7 @@ mod tests { // Members are now synced! We can start subscribing and see how it goes. assert!(room0.are_members_synced()); - sliding_sync.subscribe_to_rooms(&[room_id_0, room_id_1], None); + sliding_sync.subscribe_to_rooms(&[room_id_0, room_id_1], None, true); // OK, we have subscribed to some rooms. Let's check on `room0` if members are // now marked as not synced. @@ -1260,7 +1261,7 @@ mod tests { // Members are synced, good, good. assert!(room0.are_members_synced()); - sliding_sync.subscribe_to_rooms(&[room_id_0], None); + sliding_sync.subscribe_to_rooms(&[room_id_0], None, false); // Members are still synced: because we have already subscribed to the // room, the members aren't marked as unsynced. @@ -1280,7 +1281,7 @@ mod tests { let room_id_2 = room_id!("!r2:bar.org"); // Subscribe to two rooms. - sliding_sync.subscribe_to_rooms(&[room_id_0, room_id_1], None); + sliding_sync.subscribe_to_rooms(&[room_id_0, room_id_1], None, false); { let sticky = sliding_sync.inner.sticky.read().unwrap(); @@ -1292,7 +1293,7 @@ mod tests { } // Subscribe to one more room. - sliding_sync.subscribe_to_rooms(&[room_id_2], None); + sliding_sync.subscribe_to_rooms(&[room_id_2], None, false); { let sticky = sliding_sync.inner.sticky.read().unwrap(); @@ -1314,7 +1315,7 @@ mod tests { } // Subscribe to one room again. - sliding_sync.subscribe_to_rooms(&[room_id_2], None); + sliding_sync.subscribe_to_rooms(&[room_id_2], None, false); { let sticky = sliding_sync.inner.sticky.read().unwrap();