Remove obsolete and experimental /sync/e2ee endpoint.
#18583
Conversation
The endpoint was part of experiments for MSC3575 but does not feature in that MSC.
Signed-off-by: Olivier 'reivilibre <[email protected]>
|
cc @MadLittleMods please check if this removal matches your understanding for the future |
| @@ -630,177 +628,6 @@ async def encode_room( | |||
| return result | |||
|
|
|||
|
|
|||
| class SlidingSyncE2eeRestServlet(RestServlet): | |||
There was a problem hiding this comment.
> cc @MadLittleMods please check if this removal matches your understanding for the future
I think removing /sync/e2ee is fine. Here is what I wrote about it before:
Really feels like this
/sync/e2eeendpoint should just be a sliding sync loop with only the e2ee extension.But I assume we're making
/sync/e2eein Synapse right now to lighten up the pressure for the Sliding Sync proxy in the mean-time.-- @MadLittleMods, Comment on the Native Sliding Sync simplification document
Re-stated:
My hunch is that this endpoint is probably more of a stepping stone to relieve pressure on the Sliding Sync proxy now and we will end up handling E2EE stuff in main Sliding Sync endpoint itself with the extensions.
But we weren't even able to use it to relieve pressure back then because an interaction problem with /sync v2 (which the Sliding Sync proxy used) and To-Device messages, see #17167 (comment)
Additionally, the endpoint may be sufficiently heavyweight and just as slow (I don't think we really investigated this point after the fact as I'm not sure this endpoint was even used):
We're doing all the same work for this new endpoint as the old sync v2 so the performance is bound to be about the same. Which also means To-Device events will probably get backed up by other events above the same. I did add a filter to only work with membership events which might help.
Are clients using /sync/e2ee?
I assume you've checked if the Element X clients are using this at all and have asked whether it was useful? Those would be the only clients using this endpoint.
There was a problem hiding this comment.
I did not see any usage on matrix.org, I also searched in Ruma (and in Conduwuit in case they implement the same), without finding this endpoint implemented.
I haven't asked them if it would be useful, but I guess they are getting by fine without it.
| @@ -32,11 +32,6 @@ | |||
| ("sync_endpoint", "experimental_features"), | |||
| [ | |||
| ("/sync", {}), | |||
| ( | |||
There was a problem hiding this comment.
This does not need to be parametrised anymore
tests/rest/client/test_sync.py
Outdated
There was a problem hiding this comment.
All those tests don't need to be parametrised anymore
Introduced in: #17167
Remove obsolete
/sync/e2eeexperimental endpointThe endpoint was part of experiments for MSC3575 but does not feature in
that MSC.