matrix-rust-sdk icon indicating copy to clipboard operation
matrix-rust-sdk copied to clipboard

Follow an ignore/unignore user event with an initial sync

Open Velin92 opened this issue 2 years ago • 7 comments

TODO is here: https://github.com/matrix-org/matrix-rust-sdk/blob/09fc258f9a633996831a7b2e1b9b267e73fba54f/crates/matrix-sdk/src/account.rs#L814

Velin92 avatar Mar 24 '23 09:03 Velin92

I just spent a few hours trying to figure out why ignore lists were seemingly wrong after calling ignore() or unignore() for a given user ID, even though the actual server call to ignore/unignore did properly work. Then I found this comment in the code and stumbled upon this issue.

I'm happy to work on this, but am not sure where exactly to start; would appreciate some pointers.

On a related note, I also feel that it's unfortunate and inefficient that the matrix_sdk_ui's Timeline logic causes all timelines in the entire account to be fully cleared upon a change to the ignore list. That surprised me, although I do understand why it's done.

kevinaboos avatar Jul 22 '24 22:07 kevinaboos

On a related note, I also feel that it's unfortunate and inefficient that the matrix_sdk_ui's Timeline logic causes all timelines in the entire account to be fully cleared upon a change to the ignore list. That surprised me, although I do understand why it's done.

Indeed, modulo it's happening in the event cache now.

So, when ignoring a user, we could do slightly better by removing only events from the ignored users in all the rooms we know about, and that would avoid the full flush. But when unignoring a user, we don't quite have a choice, because we don't which rooms may include events from the unignored user, only the server may tell us that.

bnjbvr avatar Jul 23 '24 10:07 bnjbvr

But when unignoring a user, we don't quite have a choice, because we don't which rooms may include events from the unignored user, only the server may tell us that.

True, that's fair.

In general, needing to clear all the timeline caches is okay, but I need a way to actually tell the Client or SlidingSync instance to re-fetch everything. Any ideas on how to do that?

kevinaboos avatar Jul 23 '24 18:07 kevinaboos

So there's no way to do that in the client, as far as I know (except if you restart the sync completely, after dropping all the local caches, etc.). On the other hand, you can do it a room granularity level, using back-pagination: the server will properly filter out events from ignored users, in that case.

bnjbvr avatar Jul 25 '24 13:07 bnjbvr

using back-pagination

oh that's a clever idea! Thanks, I didn't think about using pagination; I'll give it a shot.

kevinaboos avatar Jul 29 '24 21:07 kevinaboos

update: it works well, thanks @bnjbvr!

Note that I'd still be interested in helping to fix this issue within the SDK itself; I'm just not sure where to begin.

kevinaboos avatar Aug 01 '24 17:08 kevinaboos

Nice! I think the way to go here would be to have the event cache automatically trigger back-pagination (as it then remembers the events that have been back-paginated, so re-opening a previously-cleared room will include the back-paginated events).

However, we cannot do this at scale: imagine a user with 4000 rooms ignoring another user; this would trigger lots and lots of back-paginations, and because of the lack of persistent storage for the event cache (#3280), all those back-pagination results would be lost after an app restart.

Nevertheless: implementing some kind of support for "automatic", background back-pagination is one step along the way. The idea is that a room event cache could decide to trigger back-pagination on its own, while right now, this requires a manual function call through an API.

This would probably require having an optional tokio task JoinHandle in a RoomEventCacheInner instance, so we know whether a back-pagination is already happening in the room or not. If that's the case and the task is not cancelled/done, then we'd have any manual back-pagination wait on that task, instead of starting a new back-pagination from start. Otherwise, we could have manual back-pagination fill in that task, and we could spawn this task from within a room event cache, based on Heuristics™ (e.g. a user has been ignored). Having the machinery without implementing any heuristics triggering automatic back-paginations would be a great first step.

bnjbvr avatar Oct 17 '24 08:10 bnjbvr