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

Allow /createRoom to be run against event creator #7867

Closed
erikjohnston opened this issue Jul 16, 2020 · 4 comments · Fixed by #10564
Closed

Allow /createRoom to be run against event creator #7867

erikjohnston opened this issue Jul 16, 2020 · 4 comments · Fixed by #10564
Labels
A-Create-Room A-Performance Performance, both client-facing and admin-facing T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. X-Release-Blocker Must be resolved before making a release z-p2 (Deprecated Label)

Comments

@erikjohnston
Copy link
Member

Currently it has to point at master, purely because store_room calls self._public_room_id_gen.get_next(). We should try and figure out a way that we can run /createRoom without interacting with master if possible.

@anoadragon453
Copy link
Member

Would this involve modifying/splitting out self._public_room_id_gen into a MultiWriterIdGenerator instance, similar to what was done for presence's ID generator here?

@erikjohnston erikjohnston added the T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. label Jul 23, 2021
@anoadragon453
Copy link
Member

Would this involve modifying/splitting out self._public_room_id_gen into a MultiWriterIdGenerator instance, similar to what was done for presence's ID generator here?

I believe so. One needs to:

  1. Convert self._public_room_id_gen to a MultiWriterIdGenerator such that it can be incremented by more than one process.
  2. Move the store_room storage method to RoomWorkerStore.

With that, we could also explore moving room upgrade tasks to workers, as they also rely on being able to create a room.

@anoadragon453
Copy link
Member

#9044 is another good example of a PR moving a StreamIdGenerator / SlavedIdTracker pair to a MultiWriterIdGenerator.

However in the process of investigating what would be involved, @erikjohnston noticed that this stream doesn't actually seem to be utilised anywhere anymore. It likely stemmed from the days of when we used to paginate the public rooms list with stream IDs, instead of an arbitrary pagination today - which we have today.

As such, the solution here is simply to just remove public_room_list_stream, rather than needing to convert it. That will then unblock moving /createRoom off of the main process.

As with all potentially risky database changes, the plan is to:

  1. Remove reading and writing to public_room_list_stream, but keep the table around just in case.
  2. Ship a release of Synapse and let that change sit for a bit. Move /createRoom off the main process at the same time.
  3. Create a migration to remove the table and its contents.
  4. Ship another release.

@anoadragon453 anoadragon453 added the X-Release-Blocker Must be resolved before making a release label Aug 17, 2021
@anoadragon453
Copy link
Member

Fixed by #10564.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Create-Room A-Performance Performance, both client-facing and admin-facing T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. X-Release-Blocker Must be resolved before making a release z-p2 (Deprecated Label)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants