When the max number of to_device messages for a sync is reached, the next sync does not return immediately

[BillCarsonFr]

It appears that when you have reached the maximum number of to_device per sync, the next call to sync does not return immediatly with the remaining to_devices.
There must be new incoming data in order for the sync to trigger and return.

Expected:
If there are pending to_devices to deliver the sync should return immediatly instead of waiting for timeout or new data?

[babolivier]

For extra context, we currently limit to 100 to_device messages in a sync. We return immediately (with the remaining messages) when we receive new data (i.e. data that's been created since the previous sync returned) but if we have leftover to_device messages in the database we don't count it as new data and don't return them until either new data comes in or we reach the 30s timeout.

If we have leftover to_device messages we couldn't fit in the previous sync, we definitely should return the following sync immediately with them.

[bmarty]

We want the Matrix protocol to have good performance, and so the Matrix clients, so I would rise up the severity of this issue, S-Minor is not enough to me.
Some workaround have been implemented on some clients (JS and iOS), but not on Android. I will create an issue there to implement the workaround: set the timeout parameter of sync request to 0 until the number of toDevice event is 0.

[richvdh]

It seems counterproductive for clients to have to spend time implementing workarounds for server-side bugs. @callahad: can we fit this in?

[callahad]

Bumping severity + priority per discussion above. @DMRobertson may be able to look at this once he finishes up work on another sync issue.

[BillCarsonFr]

Slightly off-topic, but currently clients will do a catchup sync until the to_device array is empty/null. During this period client won't upload new otk (because their might be pre key olm message pending and we don't want to rotate keys to early).

The thing is that client might be doing that endlessly if there are always new to_device events.

I wonder if we don't need a limited field for to device..

[erikjohnston]

I was only able to reproduce this in unit tests on SQLite. @BillCarsonFr were you experiencing this on servers using postgres?

[erikjohnston]

So looks like the bug I fixed in #11966 is a recent regression and hasn't made it to a release yet.

Can someone confirm if this issue is still present?

[richvdh]

Can someone confirm if this issue is still present?

given nobody is reporting this any more, I'm going to assume it is now resolved. We can reopen if anyone can still reproduce.