Stop populating unused table local_invites.
#7793
Conversation
This table is no longer used, so we may as well stop populating it. Removing it would prevent people rolling back to older releases of Synapse, so that can happen in a future release.
richvdh
force-pushed
the
rav/drop_local_invites
branch
from
6e58450 to
5900ae4
Compare
| " room_id = ? AND invitee = ? AND locally_rejected is NULL" | ||
| " AND replaced_by is NULL" | ||
| ) | ||
|
|
||
| def f(txn, stream_ordering): |
There was a problem hiding this comment.
(I know that stream_ordering is unused here, but I'm about to hook into it in another PR)
| @@ -361,7 +361,6 @@ def _purge_room_txn(self, txn, room_id): | |||
| "event_push_summary", | |||
| "pusher_throttle", | |||
| "group_summary_rooms", | |||
| "local_invites", | |||
There was a problem hiding this comment.
Would we still want to potentially purge the table until it is fully removed from the database? I doubt it matters much either way, but...
There was a problem hiding this comment.
I'm minded to say "no". Say we leave this line here for now and drop the table in Synapse 1.17.0: that will mean that anybody trying to roll back from 1.17.0 to 1.16.0 will end up with broken purges.
OTOH leaving it here seems fairly harmless, especially if we actually drop the table soon.
There was a problem hiding this comment.
I can see it going either way. I guess my thought was to think about why an admin might be purging a room and if it could leave them liable to have information in the local_invites table still.
I think removing it from the purges is the simplest way for us to handle rollbacks, however. I'm convinced!
| and not backfilled | ||
| and event.internal_metadata.is_outlier() | ||
| and event.internal_metadata.is_out_of_band_membership() |
There was a problem hiding this comment.
This logic looks slightly different not sure if that was done on purpose as part of this change or is introducing a bug.
not backfilled and (not outlier or OOB membership) becomes not backfilled and outlier and OOB membership.
There was a problem hiding this comment.
Ah, I see it is a combination of this clause + the is outlier clause below. 👍
There was a problem hiding this comment.
yeah, I hope it is the same thing, but a second pair of eyes to confirm that is much appreciated.
There was a problem hiding this comment.
I went through it again and I believe it is correct!
|
thanks patrick! |
* commit '43726783e': (22 commits) 1.17.0rc1 Fix some spelling mistakes / typos. (#7811) `update_membership` declaration: now always returns an event id. (#7809) Improve stacktraces from exceptions in background processes (#7808) Fix `can only concatenate list (not "tuple") to list` exception (#7810) Pass original request headers from workers to the main process. (#7797) Generate real events when we reject invites (#7804) Add `HomeServer.signing_key` property (#7805) Revert "Update the installation docs on apt-transport-https (#7801)" Do not use simplejson in Synapse. (#7800) Stop passing bytes when dumping JSON (#7799) Update the installation docs on apt-transport-https (#7801) shuffle changelog slightly Change Caddy links (old is deprecated) (#7789) Stop populating unused table `local_invites`. (#7793) Refactor getting replication updates from database v2. (#7740) Add libwebp dependency to Dockerfile (#7791) Add documentation for JWT login type and improve sample config. (#7776) Convert the appservice handler to async/await. (#7775) Don't ignore `set_tweak` actions with no explicit `value`. (#7766) ...
This table is no longer used, so we may as well stop populating it. Removing it
would prevent people rolling back to older releases of Synapse, so that can
happen in a future release.
(It got replaced by
local_current_membership, ftr.)