Skip to content

Commit

Permalink
feat(sdk,ui): SlidingSync::subscribe_to_rooms has a new `cancel_in_…
Browse files Browse the repository at this point in the history
…flight_request` argument.

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.
  • Loading branch information
Hywan committed Sep 16, 2024
1 parent 4fd4410 commit 119bee6
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 9 deletions.
1 change: 1 addition & 0 deletions crates/matrix-sdk-ui/src/notification_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,7 @@ impl NotificationClient {
required_state,
timeline_limit: Some(uint!(16))
})),
true,
);

let mut remaining_attempts = 3;
Expand Down
9 changes: 8 additions & 1 deletion crates/matrix-sdk-ui/src/room_list_service/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
17 changes: 9 additions & 8 deletions crates/matrix-sdk/src/sliding_sync/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,12 +151,13 @@ impl SlidingSync {
&self,
room_ids: &[&RoomId],
settings: Option<http::request::RoomSubscription>,
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
Expand All @@ -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,
);
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand Down

0 comments on commit 119bee6

Please sign in to comment.