Convert delete room admin API to async endpoint

[dklimpel]

Fixes: #10797, #6934

Related to:

• Shuting down a room with 4k+ users takes more than 2h #7651
• Room deletion (shutdown) fail in a constant loop due to non-serializable access caused by PostgreSQL isolation levels #10294
• Host dead after caches seem to break possibly due to multiple room delete commands in short period of time #10792

I have created two new endpoints:

• DELETE /_synapse/admin/v2/rooms/<room_id>
• GET /_synapse/admin/v2/rooms/<room_id>/delete_status

changes

• moved functionality of RoomShutdownHandler into PaginationHandler
• merged shutdown and purge room into one function/process
• did not touched old functions, it should be removed after the new api is stable for a few month
• added seperate unit tests
• added result information to PurgeStatus

Questions

• Is there a chance to define the new endpoint not as final to improve it in near feature if there are issues?
• I do not know why the purge history endpoint creates a new <purge_id>. I have used the room_id instead. IMO this is easier for admins, because they do not have to memory a new id to get the status of process.
• test test_delete_same_room_twice is flaky, do you have a hint to improve it? Otherwise I have to remove it.

Pull Request Checklist

• Pull request is based on the develop branch
• Pull request includes a changelog file. The entry should:
  • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
  • Use markdown where necessary, mostly for code blocks.
  • End with either a period (.) or an exclamation mark (!).
  • Start with a capital letter.
• Pull request includes a sign off
• Code style is correct (run the linters)

Signed-off-by: Dirk Klimpel dirk@klimpel.org

[richvdh]

Is there a chance to define the new endpoint not as final to improve it in near feature if there are issues?

I think the best way to do this is by flagging it as experimental and "subject to change" in the documentation.

[richvdh]

I do not know why the purge history endpoint creates a new <purge_id>. I have used the room_id instead. IMO this is easier for admins, because they do not have to memory a new id to get the status of process.

I think one reason for this is because there can be multiple requests for the same room. For example, the first request might not go back far enough, or might have failed. By giving each request its own id, you can make sure that you're getting the results for the right request.

I hear your point that it's harder for external tools to use, though. Happy for it to just use room id for now.

[dklimpel]

Ok. That is a good point.
Maybe it makes sense to create an id for the uniqueness and to query the last status of the room with the room_id?

[dklimpel]

Probably I have to adjust this PR after merging

• Allow admins to proactively block rooms #11228

[dklimpel]

Do you have any better idea for the name of the returned object then delete_status?

{
    "delete_status": [
        {
            "purge_id": "purgeid1",
            "status": "failed",
            "result": {
                "kicked_users": [],
                "failed_to_kick_users": [],
                "local_aliases": [],
                "new_room_id": null
            }
        }
    ]
}

[richvdh]

wait, what? why are we removing this ability? It's important to be able to pre-emptively block rooms.

[dklimpel]

That ist not removed. The ability in Version 1 is created in #11228 and there is also an issue #11283.
IMO a simple block of a room with a bg task is not a very good usage. And it is implemented with raise and catching exceptions. I think v1 of this API should be removed after publishing the block API.

[richvdh]

hrm. Ok, let's leave it for now.

[richvdh]

fixes: #6934

[richvdh]

🎉 thanks @dklimpel , let's get this thing shipped!

[dklimpel]

🎉 thanks @dklimpel , let's get this thing shipped!

Thank you for your patience. The PR is unusually large and complex.