Include thread information when sending receipts over federation.

[clokep]

Send the thread ID as part of the receipt meta data over federation.

This gets a bit complicated since the same room / receipt type / user could appear multiple times with different thread IDs and we don't want to stomp over each other. To avoid this we send them as separate EDUs.

Fixes #14459

This was missed as part of #12550 somehow.

[squahtx]

The changes in the PR look good.

---

There's a limit on EDUs per transaction defined as MAX_EDUS_PER_TRANSACTION.
The comments say that this limit is specced and enforced by the receiving homeserver.

synapse/synapse/federation/sender/per_destination_queue.py

Lines 45 to 46 in 299b00d

	# This is defined in the Matrix spec and enforced by the receiver.
	MAX_EDUS_PER_TRANSACTION = 100

Now that we can return more than 1 read receipt EDU, it's possible for us to exceed that limit.
I think we may want some additional changes to avoid having our transactions rejected. Some kind of limit parameter to _get_rr_edus maybe?

synapse/synapse/federation/sender/per_destination_queue.py

Lines 717 to 720 in 299b00d

	# if we've decided to send a transaction anyway, and we have room, we
	# may as well send any pending RRs
	if len(pending_edus) < MAX_EDUS_PER_TRANSACTION:
	pending_edus.extend(self.queue._get_rr_edus(force_flush=True))

[clokep]

There's a limit on EDUs per transaction defined as MAX_EDUS_PER_TRANSACTION.
The comments say that this limit is specced and enforced by the receiving homeserver.

Ooooo good-catch! 👍

[clokep]

@squahtx Please take another look. I attempted to clarify some of the comments!

[squahtx]

Thanks for improving the comments! Some more thoughts.

[squahtx]

Looks a lot easier to understand!

AIUI even though queue_read_receipt looks linear time now, we'll put new receipts into the first EDU most of the time. It's only when a user stacks up receipts for lots of threads in the same room that we'll do a linear scan.

[clokep]

AIUI even though queue_read_receipt looks linear time now, we'll put new receipts into the first EDU most of the time. It's only when a user stacks up receipts for lots of threads in the same room that we'll do a linear scan.

That's the hope, yes. 👍 Note that (by default) receipts will only be queued for up to 2 seconds before being sent anyway (

synapse/synapse/federation/sender/__init__.py

Lines 611 to 615 in 09b5888

	# The first receipt in a given room is sent out immediately, at time T0. Any
	# further receipts are, in theory, batched up for N seconds, where N is calculated
	# based on the number of servers in the room to achieve a transaction frequency
	# of around 50Hz. So, for example, if there were 100 servers in the room, then
	# N would be 100 / 50Hz = 2 seconds.

). Is there any comments or something I should update?

[squahtx]

Is there any comments or something I should update?

Nope, just checking I understand the performance characteristics right.

[squahtx]

LGTM