Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Rework relations storage and background update to handle any relation type #11387

Closed
clokep opened this issue Nov 18, 2021 · 3 comments · Fixed by #11391
Closed

Rework relations storage and background update to handle any relation type #11387

clokep opened this issue Nov 18, 2021 · 3 comments · Fixed by #11391
Assignees
Labels
A-Threads Threaded messages T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.

Comments

@clokep
Copy link
Member

clokep commented Nov 18, 2021

The event_thread_relation background update is fairly heavy and only looks for the unstable io.element.thread relation. We will need to essentially re-run this when MSC3440 is stabilized to search for m.thread relations instead.

It seems a better way to do this would be to store unknown relation types into the database (and potentially filter them on fetch) so that we do not need to crawl all events in the future when more relation types are added. If a future relation has additional data which must be processed, similar to the key field of m.annotation, then this would allow easily re-processing only the events with that relation.

Note that this was previously discussed at #11088 (comment) but rejected. This is being reconsidered in light of #11375 and other problems with running this background update.

The overall plan for how to accomplish this would be:

  1. Replace the current background update with a no-op, this allows any servers which have not finished it to give up.
  2. Write a new background update that does something similar, but pulls in any relation type.
  3. Rework the code in _handle_event_relations to save any relation of new events (instead of only known relations).
  4. (Optionally) Modify places where we fetch relations to limit to only "known" relations.

This also has the benefit of allowing for easier experimentation with unknown relations. Synapse would (somewhat obviously) not include those in bundled relations since the format of those is custom per relation type.

@clokep clokep added T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. A-Threads Threaded messages labels Nov 18, 2021
@clokep clokep self-assigned this Nov 18, 2021
@reivilibre
Copy link
Contributor

If I understand, this also seems like it is necessary for supporting out-of-spec (third-party) relation types, so I must say this seems basically required for extensibility

@clokep
Copy link
Member Author

clokep commented Nov 18, 2021

If I understand, this also seems like it is necessary for supporting out-of-spec (third-party) relation types, so I must say this seems basically required for extensibility

Yes, this is what the last sentence in the description is trying to say!

@clokep
Copy link
Member Author

clokep commented Nov 18, 2021

I should note that the background update that is being modified was added in #11181.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Threads Threaded messages T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants