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

get_bundled_aggregations takes a few seconds even when there is no relations #13624

Closed
Tracked by #14284
MadLittleMods opened this issue Aug 25, 2022 · 4 comments
Closed
Tracked by #14284
Assignees
Labels
A-Messages-Endpoint /messages client API endpoint (`RoomMessageListRestServlet`) (which also triggers /backfill) A-Performance Performance, both client-facing and admin-facing A-Threads Threaded messages O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Blocks non-critical functionality, workarounds exist. T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.

Comments

@MadLittleMods
Copy link
Contributor

Mentioned in internal doc about speeding up /messages. Also see "2. Loading tons of events" in #13356


get_bundled_aggregations takes a few seconds even when there is no relations. This happens at the end of every /messages request.

For 500 messages that have 0 relations, reactions, threads, redactions, it takes 1 second to process. This is reproduced by running the many users, many messages Complement test I have in matrix-org/complement#443

Bunch of get_annotations_for_event and get_relations_for_event calls for each message:
Jaeger trace of get_bundled_aggregations that contains a bunch of get_annotations_for_event and get_relations_for_event

Potential solutions

Batch up the lookups. It looks like most are just the magnitude of the round-trip to and from the database for each of the messages.

@MadLittleMods MadLittleMods added A-Threads Threaded messages A-Messages-Endpoint /messages client API endpoint (`RoomMessageListRestServlet`) (which also triggers /backfill) A-Performance labels Aug 25, 2022
@DMRobertson DMRobertson added S-Minor Blocks non-critical functionality, workarounds exist. T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. O-Uncommon Most users are unlikely to come across this or unexpected workflow O-Occasional Affects or can be seen by some users regularly or most users rarely and removed O-Uncommon Most users are unlikely to come across this or unexpected workflow labels Aug 25, 2022
@clokep
Copy link
Member

clokep commented Aug 25, 2022

Batch up the lookups.

This was essentially the approach taken when I last looked at improving the performance here, see #11825 where this was done for edits and threads.

If the queries for references and annotations can't be batched we could at least run them in parallel (instead of in series) -- both on a per-event level and different relation level.

@clokep
Copy link
Member

clokep commented Nov 18, 2022

Batch up the lookups.

This was essentially the approach taken when I last looked at improving the performance here, see #11825 where this was done for edits and threads.

If the queries for references and annotations can't be batched we could at least run them in parallel (instead of in series) -- both on a per-event level and different relation level.

#14491 does this for annotations.

@clokep
Copy link
Member

clokep commented Nov 22, 2022

I'm going to close this, after deploying this change we saw an improvement in /message times a bit:

image

When looking at jaeger traces get_bundled_aggregations is now < 5% of a total /messages request.

@clokep clokep closed this as completed Nov 22, 2022
@MadLittleMods
Copy link
Contributor Author

MadLittleMods commented Nov 22, 2022

Looks like a great improvement! I don't really notice it in the overall /messages response time quantiles (first graph). But it is very obvious in the large room (>1000 members) response time quantiles (in the graph below the first one). And we see another drop in number of messages that take more than 10s

https://grafana.matrix.org/d/dYoRgTgVz/messages-timing?orgId=1&from=1669125091174&to=1669150291174

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Messages-Endpoint /messages client API endpoint (`RoomMessageListRestServlet`) (which also triggers /backfill) A-Performance Performance, both client-facing and admin-facing A-Threads Threaded messages O-Occasional Affects or can be seen by some users regularly or most users rarely S-Minor Blocks non-critical functionality, workarounds exist. T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.
Projects
None yet
Development

No branches or pull requests

3 participants