Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(event cache): sqlite persistent storage for the event cache 😎 #4308

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

bnjbvr
Copy link
Member

@bnjbvr bnjbvr commented Nov 21, 2024

This implements the following parts of #3280 (comment):

  • implement a sqlite backend for the event cache store
  • save events into the store as they come from sync
  • reload data from the store and use it to prefill the linked chunks for all rooms

For getting a nice demo in the multiverse client, a few shortcuts have been taken in the last commit, but these should be relatively easy to solve.

The demo shows the following:

  • open the client for the first time
  • observe the focused room is mostly empty
  • I manually back-paginate, getting some events back from the server
  • then, close the client
  • reopen it
  • observe the events are still there (they were persisted on disk).
Screencast.From.2024-11-21.18-09-05.mp4

More to come next week.

@bnjbvr
Copy link
Member Author

bnjbvr commented Nov 28, 2024

For what it's worth, to observers of this PR: this is being split, redesigned and reviewed in several smaller PRs, which all link to this one. This PR might be repurposed to represent the final step of the work done here, at some point.

bnjbvr and others added 8 commits December 3, 2024 16:27
And fix an issue that would cause a crash because a timeline wasn't
initialized and we tried to unwrap it later.
…ys return `true`.

The `add_or_update_remote_event` method always returns `true`. This
patch updates the method to return nothing, and cleans up the call sites
accordingly. This patch also adds comments to clarify the code flow.

The bool value returned by `add_or_update_remote_event` was supposed
to be `false` if the event was duplicated. First off, as soon as the
`Timeline` receives its events from the `EventCache` via `VectorDiff`,
the `event_cache::Deduplicator` will take care of deduplication,
so the `Timeline` won't have to handle that itself. Second,
`add_or_update_remote_event` was sometimes removing an event, but it
was re-inserting a new one immediately without returning `false`: it was
never returning `false` because a new event was always added.
This patch replaces `VecDeque<EventMeta>` by `AllRemoteEvents` which
is a wrapper type around `VecDeque<EventMeta>`, but this new type aims
at adding semantics API rather than a generic API. It also helps to
isolate the use of these values and to know precisely when and how they
are used.

As a first step, `AllRemoteEvents` implements a generic API to not break
the existing code. Next patches will revisit that a little bit step
by step.
This patch renames `AllRemoteEvents::back` to `last` so that it now gets
a specific semantics instead of being generic.
Having a mutable iterator can be dangerous and is probably too generic
regarding the safety we want to add around the `AllRemoteEvents` type.

This patch removes `iter_mut` and replaces it by its only use case:
`get_by_event_id_mut`.
@bnjbvr bnjbvr force-pushed the bnjbvr/sqlite-event-cache branch from baafadc to cfc0aff Compare December 3, 2024 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants