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

Drop state_events and rejections tables #11496

Open
3 of 5 tasks
richvdh opened this issue Dec 2, 2021 · 1 comment
Open
3 of 5 tasks

Drop state_events and rejections tables #11496

richvdh opened this issue Dec 2, 2021 · 1 comment
Labels
A-Database DB stuff like queries, migrations, new/remove columns, indexes, unexpected entries in the db T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. Z-Future-Maintenance Things that can't yet be done, but will need cleaning up in a couple of months/releases

Comments

@richvdh
Copy link
Member

richvdh commented Dec 2, 2021

Both state_events and rejections are tables which add a single piece of data to a subset of events (state_key and reason, respectively).

I believe this would better handled with nullable columns in the events table:

  • state_events and rejections are never read from without a join to events, so we can save a join whereever we read from them
  • it's not like adding the data to events is going to transform it from a small, cacheable table to something huge (so I don't expect much difference in the case where we don't join to rejections or state_events). In any case, when we look in events we normally want the rejection status too.
  • obviously, populating three tables (and maintaining two extra indexes) is extra work to do when storing events.

However, getting from here to there is not an easy task by any means, and will take quite a bit of work to do in a way that doesn't break backwards compatibility. Here are the tasks as far as I can see them:

Each of those steps will require SCHEMA_VERSION and/or SCHEMA_COMPAT version updates to ensure that we never break compatibility with the previous Synapse release.

@reivilibre reivilibre added the T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. label Dec 9, 2021
richvdh added a commit that referenced this issue May 28, 2022
This method was introduced in #12852. It is using the `state_key` column from
the `events` table, which is not (yet) reliable (see #11496).
richvdh added a commit that referenced this issue May 30, 2022
This method was introduced in #12852. It is using the `state_key` column from
the `events` table, which is not (yet) reliable (see #11496).
@richvdh
Copy link
Member Author

richvdh commented Oct 10, 2022

It's no longer entirely clear that putting rejections into the events table is a good idea... it is possible for events to change rejection status (thanks to faster-joins) and in particular, different servers may have different perspectives on whether an event has been rejected or not. The implication is that, if we were to move to a model where a single event store could back multiple server deployments, the rejection status would have to go and live somewhere else.

@MadLittleMods MadLittleMods added A-Database DB stuff like queries, migrations, new/remove columns, indexes, unexpected entries in the db Z-Future-Maintenance Things that can't yet be done, but will need cleaning up in a couple of months/releases labels Jul 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Database DB stuff like queries, migrations, new/remove columns, indexes, unexpected entries in the db T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. Z-Future-Maintenance Things that can't yet be done, but will need cleaning up in a couple of months/releases
Projects
None yet
Development

No branches or pull requests

3 participants