Device list updates to be sent over federation will be skipped if there are too many at one time

[reivilibre]

get_device_updates_by_remote always returns the next stream position as the latest device stream token, even in cases where it is limited to a maximum number of rows to return.
The effect of this is that any device updates that don't fit within one limit-sized batch will be skipped.

Discovered whilst looking into #11719. Due to the nature of my fix to #11719, I'll likely fix them both at the same time.

[DMRobertson]

I wonder if this partly explains #8631?

[reivilibre]

Seems it's not quite as simple as it looked to be at the start; the plot thickens.

device_lists_stream doesn't have a unique index on stream_id, and test_get_device_updates_by_remote claims in a comment that it is adding two device updates with the same stream ID. (fun fact: at least on my test run, they're using sequential IDs — not the same stream ID :/)

That's all well and good, but if there are multiple rows for the same stream ID and you're paginating by stream ID (as is done currently, despite the fact that it always skips to the end) with a limit on the number of rows, that gets awkward.

Annoyingly it looks like they do overlap in practice (or have done so in the past); querying LP.net:

synapse=# select count(distinct stream_id) from device_lists_stream ;
 count  
--------
 232366
(1 row)
synapse=# select count(stream_id) from device_lists_stream ;
 count  
--------
 266670
(1 row)

[reivilibre]

I notice that 'recent' entries in the table on LP.net don't seem to reuse stream_ids, though. Maybe this is something that happened in the past but doesn't anymore :/.

synapse=# select stream_id from device_lists_stream order by stream_id desc limit 1;
 stream_id 
-----------
   3902416
(1 row)

synapse=# select count(stream_id) from device_lists_stream where stream_id > 902416;
 count  
--------
 176803
(1 row)

synapse=# select count(distinct stream_id) from device_lists_stream where stream_id > 902416;
 count  
--------
 176803
(1 row)

[reivilibre]

Rich kindly pointed me to #7010 — looks like the stream_ids were changed to not get shared between multiple rows (though historical rows are of course still there; can probably gloss over those).

[babolivier]

I assume this issue can be closed now that #11729 has been merged?

[reivilibre]

I assume this issue can be closed now that #11729 has been merged?

This part always gets me — think it has to hit develop and then it'll close itself