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

Don't upgrade a room if there's already an ongoing upgrade #4583

Closed
babolivier opened this issue Feb 7, 2019 · 6 comments
Closed

Don't upgrade a room if there's already an ongoing upgrade #4583

babolivier opened this issue Feb 7, 2019 · 6 comments
Assignees
Labels
A-Room-Upgrades z-bug (Deprecated Label)

Comments

@babolivier
Copy link
Contributor

Basically I tried upgrading a room at a time my server had some lag. Not seeing any kind of feedback in Riot after a short amount of time, I ran /upgraderoom a second time, which caused two tombstone events to be sent to the same v1 room, referencing two separate v3 rooms. Additionally, the UI got confused which caused me to left the wrong room, so the upgraded room isn't joinable by anyone.

Before initiating a room upgrade, Synapse should check if there's already an upgrade ongoing for the same room, and return an error if there is.

@anoadragon453
Copy link
Member

So we put a linearizer in place, which is great, but all it does is make one room upgrade happen after another, rather than preventing the second one from happening at all.

@anoadragon453
Copy link
Member

anoadragon453 commented Apr 11, 2019

Another problem seems to be that we can upgrade a room over and over again, even after it's already been upgraded.

@jryans
Copy link
Contributor

jryans commented Apr 11, 2019

Another problem seems to be that we can upgrade a room over and over again, even after it's already been upgraded.

There seems to be some debate about whether that's a bug or a feature: https://github.com/matrix-org/matrix-doc/issues/1937.

@anoadragon453
Copy link
Member

anoadragon453 commented Apr 11, 2019

ughhhhhhhhh

Edit: He brings up a good point though. We need to have some spec discussion before we go ahead and stop a room from being upgraded twice. However the issue of a room being upgraded twice at the same time can still be mitigated now (just not as cleanly).

@anoadragon453
Copy link
Member

anoadragon453 commented May 23, 2019

After some discussion we've decided to go with the following solutions to the following use cases:

  • Someone upgrades a room and then wants to upgrade the same room again (possibly to fix a botched upgrade)

Allow them to.

  • Person A and B upgrading a room at the exact same time

Let the first one received go through and send an error message to the second.

  • Person A upgrading a room, thinking something broke, and upgrading the same room again before the first go has completed

Send the same response to the second.

Will update #5051 to reflect this. Edit: Done.

anoadragon453 added a commit that referenced this issue Jun 25, 2019
Closes #4583

Does slightly less than #5045, which prevented a room from being upgraded multiple times, one after another. This PR still allows that, but just prevents two from happening at the same time.

Mostly just to mitigate the fact that servers are slow and it can take a moment for the room upgrade to actually complete. We don't want people sending another request to upgrade the room when really they just thought the first didn't go through.
@anoadragon453
Copy link
Member

anoadragon453 commented Jun 25, 2019

Fixed in #5051

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Room-Upgrades z-bug (Deprecated Label)
Projects
None yet
Development

No branches or pull requests

4 participants