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

After a user redacted their own join event from HQ, Android DoSes us with /context requests #1859

Closed
ara4n opened this issue Jan 7, 2018 · 6 comments

Comments

@ara4n
Copy link
Member

ara4n commented Jan 7, 2018

We saw 2061 requests to '/_matrix/client/r0/rooms/%21cURbafjkfsMDVwdRDQ%3Amatrix.org/context/%2415153218301304484Fpgds%3Amatrix.org?limit=1' from Riot/Android clients after a user redacted that event (they sent 3 redactions, as presumably it didn't seem to work). This looks to have contributed to taking out the main synapse process for ~30 minutes

@ara4n
Copy link
Member Author

ara4n commented Jan 7, 2018

screen shot 2018-01-07 at 12 11 33

^ the carnage this caused on CPU on the context servlet

@ylecollen
Copy link
Contributor

ylecollen commented Jan 8, 2018

The main problem is the performance of room state events redaction
-> either the client lets the server does the job : it looks weird that only 2061 requests are enough to stuck the server for 30 minutes
-> or the android stores the state events. It means to save about 13000 state events for Matrix HQ, load them in redaction case and recompute the new room state status.
On low / middle range devices, the storage use explodes + many devices already reported that the application crashed because of the 13000 events loading in RAM.

@ara4n
Copy link
Member Author

ara4n commented Jan 8, 2018

why is it calling /context at all, though? the right API to recompute the state is /state (which may be less heavy)?

@ylecollen
Copy link
Contributor

ylecollen commented Jan 8, 2018

@ara4n

For the both mobile clients,

When an event is redacted (from the sync)

-> check if the event is known, if it is a room state, compute the new room state from the state events list stored in the application store (disabled on Android because of out of memory crashes)

-> if the event is not known, a "getContent of event" request is triggered to retrieve the message type (this is your server overload issue). If a room state event, we perform a room initial sync (because we don't know which updates it implies).

The context request is done because there is no other way to retrieve a message content from its room id / event id.

@richvdh
Copy link
Member

richvdh commented Jan 9, 2018

(the agreed solution is to implement /rooms/{roomId}/event/{eventId} on the server and have the mobile client use that instead of the more heavy-weight .../context/...)

@ara4n ara4n added P1 and removed P2 labels Jan 9, 2018
ylecollen pushed a commit to matrix-org/matrix-android-sdk that referenced this issue Jan 10, 2018
After a user redacted their own join event from HQ, Android DoSes us with /context requests
@ylecollen
Copy link
Contributor

ylecollen commented Jan 10, 2018

@richvdh @ara4n
getContextOfEvent is replaced by getEvent which
1- tries /rooms/{roomId}/event/{eventId}
2- if the new request is not supported, tries /events/{eventId} (which still works)
3- if "/events/{eventId} " is not supported, use the /context request

we need to be backward compatible until all the servers are updated.

ylecollen pushed a commit to matrix-org/matrix-android-sdk that referenced this issue Jan 16, 2018
After a user redacted their own join event from HQ, Android DoSes us with /context requests

(cherry picked from commit c1c842c)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants