Open Hywan opened 7 months ago
Constraint: Pagination
The biggest difficulty is that the Timeline will ask for data that will be injected to another place (Timeline::paginate_backwards will see the results of RoomEventCache::backpaginate via RoomEventCache::subscribe). It's not easy to connect both.
For info, this is exactly the shape used in the Android SDK (hello @ganfra!). The timeline observes a reactive DB that is fed by other mechanisms. The actioner and the consumers are deconnected.
Lazy EventCache: Combine pagination and persistent storage
We must run a proper backpagination (with /messages) to see if events aren't missing from the federation that could have missed by /sync,
This is not mandatory. We must use the pagination token provided by the backend on /sync
, /messages
, /context
requests. The backend will provide late messages through this stream.
How to render or reorder a late message is a tricky question where we need help from the product. There is no good answer as far as I know:
I think most matrix clients take the second approach (which is also the lazy one). If we choose this path too, there might be things to do in the timeline module to flag late messages to offer a better UX. But again, we need a product decision first.
matrix_sdk::EventCache
is a new module that has been introduced with #3058. The idea is to manage all events inside the Rust SDK in a single place. An important user ofEventCache
is thematrix_sdk_ui::Timeline
.EventCache
uses a new data structure to organize all events,LinkedChunk
, since:3166
3230
This issue describes the plan to get a persistent storage for
EventCache
, along with the ability to make it reactive.Persistent storage
In order to create a persistent storage for
EventCache
, we need a mechanism that listens toLinkedChunk
updates, and map these updates to database operations (likeINSERT
,DELETE
and so on).We initially went to using a reactive mechanism (like
ObservableLinkedChunk
), but like any reactive mechanisms, we need to handle the lag.The lag, in this case, is pretty problematic. In case of a lag, the database will be out of sync —data will be missing—, and there is no easy way to get them again. In case of a lag, we might imagine to reset the database and to rewrite everything again, but not all events are loaded in memory. Alternatively, we may want to recompute all the differences between what is in memory and what is inside the database, but again, it's not an easy problem. Anyway, it implies more guards and more complexity.
Instead, we've decided to define a
LinkedChunkListener
trait, used byLinkedChunk
, onto which methods will be called on some particular operations, likeinsert_chunk
,remove_chunk
,insert_events
andremove_events
, that's probably all we need.The cons:
The pros:
Tasks
LinkedChunkListener
: https://github.com/matrix-org/matrix-rust-sdk/pull/3281EventCache
; write mode: from memory to persistent storage,LinkedChunk
from the storage; read mode: from persistent storage to memory,EventCache
.Reactive
EventCache
Right now, there is no satisfying way to get a stream of
EventCache
updates. The only mechanism that exists so far isRoomEventCache::subscribe
. It returns atokio::sync::mpsc::Receiver<RoomEventCacheUpdate>
.RoomEventCacheUpdate
is defined like so:https://github.com/matrix-org/matrix-rust-sdk/blob/ac0bc95c253c5d46fec31378016497f433e0067d/crates/matrix-sdk/src/event_cache/mod.rs#L816-L838
Constraint: Prepare for reconciliation
This is OK-ish for the moment (at the time of writing), but it will quickly show limitations, in particular with the reconciliation.
Why reconciliation is going to create a problem here? Because it's impossible to represent an update like: “remove item at position $p$, and insert item at position $q$”. The only possible updates so far are “clear” and “append”.
The assiduous reader (oh, hi^1) will think: “How does it work with back- or front-pagination?”. Thanks for asking. Let's make a detour.
Constraint: Pagination
Frontpagination isn't implemented yet (not something hard, just not here yet). Backpagination is done with
RoomEventCache::backpaginate
. It returns aBackPaginationOutcome
, defined like so:https://github.com/matrix-org/matrix-rust-sdk/blob/ac0bc95c253c5d46fec31378016497f433e0067d/crates/matrix-sdk/src/event_cache/mod.rs#L792-L814
Ah. Isn't it using
RoomEventCacheUpdate
? Well, no, because of theTimeline
! The API fromEventCache
has been extracted from theTimeline
. TheTimeline
had and still has 2 sources of updates:/sync
and/messages
for pagination.Would it be hard to switch to a single
Stream<Item = SyncTimelineEvent>
? Well. Yes and no./sync
, theTimeline
listens toRoomEventCache::subscribe
:https://github.com/matrix-org/matrix-rust-sdk/blob/ac0bc95c253c5d46fec31378016497f433e0067d/crates/matrix-sdk-ui/src/timeline/builder.rs#L133-L137
/messages
, theTimeline
has a dedicated mechanism that calls/messages
in a loop until $n$TimelineItem
s are received. This is different of $n$SyncTimelineItem
, because some events are state events and cannot be displayed to the user:https://github.com/matrix-org/matrix-rust-sdk/blob/ac0bc95c253c5d46fec31378016497f433e0067d/crates/matrix-sdk-ui/src/timeline/pagination.rs#L49-L51
What do we want? A single source of data for
Timeline
. What does it require? Change the backpagination mechanism. Not a big deal, it's doable. The biggest difficulty is that theTimeline
will ask for data that will be injected to another place (Timeline::paginate_backwards
will see the results ofRoomEventCache::backpaginate
viaRoomEventCache::subscribe
). It's not easy to connect both. How to do it? Glad you ask.The solution: Step 1
LinkedChunk
must expose aStream<Item = Vec<VectorDiff<SyncTimelineEvent>>>
-like API, something like:Easy right? Well. No.
LinkedChunk
is not aVector
. The algorithm is going to be fun here.LinkedChunk::subscribe_as_vector
should fake it's aVector
and should emitVectorDiff
, à laeyeball_im::ObservableVector
.Of course,
RoomEventCache::subscribe
must be rewritten to useLinkedChunk::subscribe_as_vector
.The solution: Step 2
Timeline
must listen toRoomEventCache::subscribe
but for all updates. Then,Timeline
will mapStream<Item = Vec<VectorDiff<SyncTimelineItem>>>
intoTimelineItem
s that will be inserted/deleted/moved in the correct places inside its ownObservableVector<TimelineItem>
insideTimelineInnerState
:https://github.com/matrix-org/matrix-rust-sdk/blob/ac0bc95c253c5d46fec31378016497f433e0067d/crates/matrix-sdk-ui/src/timeline/inner/state.rs#L71-L75
This is going to be delicate.
Tasks
The 2 following lists can be done in parallel:
Tasks on
EventCache
:LinkedChunk::subscribe_as_vector
:Tasks on
EventCache
andTimeline
:Timeline
toEventCache
so thatEventCache
is the only place to have pagination logics.Timeline
must entirely depend onEventCache
.The following list must be done after the 2 previous lists:
RoomEventCache::subscribe
to useLinkedChunk::subscribe_as_vector
:RoomEventCacheUpdate
to include theVectorDiff
, and theTimeline
must handle theseVectorDiff
itself (there is already aTimelineItemPosition
type, it must be expanded a little bit)Timeline
to handle aStream<Item = Vec<VectorDiff<SyncTimelineItem>>>
.Lazy
EventCache
: Combine pagination and persistent storageBefore being able to enable persistent storage in
EventCache
, one last problem must be addressed.RoomEventCache
will useRoomEvents
(which usesLinkedChunk
) to load events from the persistent storage. That's fine. However, we don't want to load all events from the persistent storage. Imagine rooms with 1'000'000 events: do we want to load all the events in memory? Absolutely no!RoomEventCache
must be lazy: it must load only the $n$ newest chunks from the persistent storage.OK, but what happens when we backpaginate?
/messages
) to see if events aren't missing from the federation that could have missed by/sync
,Once we have this mechanism in-place, we can enable the persistent storage for
EventCache
.Tasks
RoomEventCache
is lazy: load chunks on-demand,RoomEventCache
is able to backpaginate from its persistent storage and from/messages
at the same time,EventCache
🎉.Naive reconciliation
To start with, we can implement a super native reconciliation algorithm that will simply remove duplicated events. For a first shot, it's totally fine. A better reconciliation algorithm can be defined later. For the record, what is present now is the remove duplicated events approach.
What we want to ensure is that the reconciliation algorithm will modify events in
LinkedChunk
, which will propagate to the persistent storage viaLinkedChunkListener
and to theTimeline
viaRoomEventCache::subscribe
. That's one of the main goal of this proposed designed.Tasks
Conclusion
This plan provides a solution to support a reconciliation mechanism in a single place. It will benefit to other users, like
Timeline
. TheTimeline
needs to be refactored due to having 2 source of updates, one for/sync
(live events), and one for/messages
(front- and back-paginations of old events).Because
EventCache
will be reactive, it will simplify a lot all the testing aspects:EventCache
will be easy to test: assert a stream,Timeline
will be super easy to test without having to mock a server will all its possible requests: simply generate a stream that will emit what we want to test exactly, it's a matter of writing the following code for the inputs of theTimeline
:Timeline
.Bonus
LinkedChunk
uses mutation-based testing and property-based testing: