[core] Excluded drops due to group member seq shifting from counting as dropped

[ethouris]

Fixes #2879

The change: One call to CUDT::rcvDropTooLateUpTo was done in the group management so that all other links in the broadcast group are synchronized up to the sequence that was just "played" (the packet was delivered to the application). This may include, however, unreceived packets, which are this way counted as dropped. This modification uses a special parameter that allows to declare this drop-shift as "wink" (packets are delivered over an alternative link) and this way even if this shift required to drop the missing packets, they will not be counted as dropped in the stats.

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (ea8ea9f) 64.26% compared to head (00b19fd) 64.30%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2882      +/-   ##
==========================================
+ Coverage   64.26%   64.30%   +0.04%     
==========================================
  Files         101      101              
  Lines       17480    17481       +1     
==========================================
+ Hits        11233    11242       +9     
+ Misses       6247     6239       -8     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[maxsharabayko]

Those packets, that are present in the member's receiver buffer and could be read, but were read from another member should not be included in the pktRcvDrop statistics. However, if a packet was missing in the RCV buffer, but was read from another member socket, then it can be considered dropped.

In other words, missing packets on a member socket dropped by this operation still have to be counted as "dropped" in the case of a broadcast group.

[ethouris]

If the dropping has happened in the TSBPD thread call, then this drop will count.

Normally in case of broadcast mode it's unlikely that you have earlier a packet ready to play in one socket and a need of dropping in another, although it's theoretically possible in case of a very unlucky thread layout. For example:

P  A  B
0  x  .
1  .   x
2  .   x
3  .   .  

x = missing packet

In such a situation there exists a slight chance that packet 0 from B is preferred and then the next thread execution will happen at the time when packet 3 is ready to play and occassionally the thread for B link is selected. This way packets 1 and 2 will be skipped in order to reach 3, and then link A will be adjusted by dropping if necessary. Packets will be dropped on the group and will be also noted as dropped on the link B, as well as on the group.

As I said, there's only a slight chance for this to happen, but it still is. In all other situation, in case of symmetric losses, that preferably a link with more packets will be selected, only this one will drop, and if a second link had a symmetric loss, just a bit larger, then this link's larger drop list will be skipped with this fix, even if these same packets would be dropped on this link as well.

This problem will be mitigated with the common receiver buffer, although drop stats will be even more missing because with the common buffer you simply don't count losses if they are not symmetric.

[maxsharabayko]

Statistics must be deterministic and precise, not dependent on the thread layout. The rcvDropTooLateUpTo may return two values: the number of skipped (existing but discarded) packets and the number of dropped (missing) packets. If a socket is a member of a group, only dropped packets should be counted. In case of a single connection - probably both, although skipped packets could only occur if a several-packet message was allowed with the TSBPD mode enabled.

[ethouris]

So that's still staying true. The case when this function is called in "WINK" mode are used for group members only. This fix simply prevents packets from being counted as dropped, if the drop request was due to the needed synchronization of the other socket that provided the packets delivered to the application.

But even if these are not counted, they will still be counted as dropped on the link that has provided them, and as well counted as dropped in the group, in case when drops are symmetric.

My above description didn't suggest that the statistics depend on the random thread layout, but the SRT behavior does, and that the statistics simply follow that behavior.

In other words: drops will be counted on a link that performed the drops that effected with group drops due to being symmetric, but not on the other links, even if they dropped the same packets (due to being symmetric). This behavior holds also in a case described above when packets could theoretically be "mistakenly drops" (this problem will be solved with a common buffer), but with this fix if both links drop packets and they result in group drops, only one link will notify the drops - the one from which the packets were delivered to the group with drops. From all others they will be ignored.

If you want to have any "hybrid behavior" (between the old and the current one), that is, update dropped packets that were dropped symmetrically, but this time in every link that did drops, then it would have to request updating all other links with these dropped packets. But this will simply make that the number of dropped packets in all single socket stats will be equal to this value in the group stats and this way it makes no sense.

I personally think that statistics that make sense are the original drops of the link, regardless if they are symmetric or not (which is the old behavior) and the group keeps the symmetric drops only. Note also that with the solution for a common receiver buffer drops on single socket will not be counted at all because there's no way to do this (simply because there are no drops on single links at all, as well as a jump-over tracking would at best count the losses).

[maxsharabayko]

Just a note for possible reference in the future, the rcvDropTooLateUpTo also happens in CGroup::recv(..), should be marked WINKED as well.