Batch fetch bundled annotations

[clokep]

This applies similar changes for annotations as we did for edits (#11660) and threads (#11752) -- namely we make a query per group of events instead of per individual event.

Please see inline comments for some more info.

See #13624.

[clokep]

The room_id is now check as part of the query (we check the parent and child event have the same room ID).

[MadLittleMods]

Using the artificial Complement test from matrix-org/complement#443 which has zero relations, get_bundled_aggregations has gone from ~1000ms -> ~600ms (500-700ms) with this PR 📉

All of the get_annotations_for_event calls have been boiled down to a single get_annotations_for_events 💪 and @clokep suspects he can do the same thing for the m.reference (which is the remaining get_relations_for_event bulk)

Trace JSON (drag and drop onto Jaeger under the JSON File tab): https://gist.github.com/MadLittleMods/b1fb31f5527b90f59d69111f8f2c2a8b

To reproduce my results:

Expand for reproduction steps

Have Jaeger running locally to collect the spans/traces. If you don't have one, you can spin one up with Docker:

docker run -d --name jaeger \
  -e COLLECTOR_ZIPKIN_HOST_PORT=:9411 \
  -e COLLECTOR_OTLP_ENABLED=true \
  -p 6831:6831/udp \
  -p 6832:6832/udp \
  -p 5778:5778 \
  -p 5775:5775/udp \
  -p 16686:16686 \
  -p 4317:4317 \
  -p 4318:4318 \
  -p 14250:14250 \
  -p 14268:14268 \
  -p 14269:14269 \
  -p 9411:9411 \
  jaegertracing/all-in-one:1.35

Get the right Synapse code in place:

# Use the OpenTelemetry PR as a base so we can trace in the Complement Docker tests
git checkout madlittlemods/11850-migrate-to-opentelemetry
# Create a new scratch branch
git checkout -b madlittlemods/test-14491-faster-scratch
# Merge this PR in
git merge clokep/batch-aggs

Add the following config to docker/conf/homeserver.yaml#L189-L206

synapse/docker/conf/homeserver.yaml

Lines 189 to 206 in 04de9ea

	# foo
	tracing:
	enabled: true
	sample_rate: 1
	homeserver_whitelist:
	- ".*"
	jaeger_exporter_config:
	agent_host_name: host.docker.internal
	agent_port: 6831
	# Split UDP packets (UDP_PACKET_MAX_LENGTH is set to 65k in OpenTelemetry)
	udp_split_oversized_batches: true
	# If you define a collector, it will communicate directly to the collector,
	# bypassing the agent
	#
	# It does not seem like the agent can keep up with the massive UDP load
	# (1065 spans in one trace) so lets just use the HTTP collector endpoint
	# instead which seems to work.
	collector_endpoint: "http://host.docker.internal:14268/api/traces?format=jaeger.thrift"

Go to your Complement checkout and get the relevant test:

git checkout madlittlemods/room-messages

Back to Synapse to test:

# This will take a while to run the first time while it sets up the massive amount of rooms, senders, and messages on `hs1`.
TEST_ONLY_IGNORE_POETRY_LOCKFILE=1 TEST_ONLY_SKIP_DEP_HASH_VERIFICATION=1 COMPLEMENT_DEBUG=1 COMPLEMENT_KEEP_BLUEPRINTS="fed.perf_many_messages.hs1" COMPLEMENT_ALWAYS_PRINT_SERVER_LOGS=1 COMPLEMENT_DIR=../complement ./scripts-dev/complement.sh -run TestMessagesOverFederation -p 1

Visit http://localhost:16686/search?limit=20&lookback=1h&operation=RoomMessageListRestServlet&service=hs2%20master to find the trace in Jaeger

[clokep]

We lose checking the limit here, meaning that we could return 100 keys for a single event ID.

Is that OK? Do we want to try to enforce it a different way? (If so --how?)

[erikjohnston]

I'd be tempted to enforce this in python, so at the very least its not going down the APIs.

[clokep]

Done! I chose to do it in the get_aggregation_groups_for_events function since that's cached. If you have a bunch of ignored users this could mean you end up with < the limit of bundled annotations when there is actually more than the limit. I think this is enough of an edge case that it is unlikely to be hit though.

[erikjohnston]

I'd be tempted to enforce this in python, so at the very least its not going down the APIs.