_sendBps overstated in PeerState.java

[jogger]

Bytes sent are double counted when the send window is rolled. Might negatively affect the b/w limiter, especially when on low b/w.

I have also removed excessive flow control and straightened out use of now(). Patch to follow after test.

[Zlatin Balevsky]

I don't immediately see how it's overstated, but the only place _sendBps is used is in the "udp.sendBps" metric.. which is not used for any decision making anywhere.

[jogger]

Look at allocateSendingBytes(). size parameter is twice added when interval is rolled over. And size runs against sendWindowBytes, could have some effect.

[Zlatin Balevsky]

It's added twice to _sendBytes, but that field is zeroed-out when the window is rolled over. Or am I missing something?

[jogger]

Yep, so size is going twice into sendBps, into the old interval as well as into the new one. But the effect is only cosmetic, going into some debug statement only. Will post the patch anyway, as it cleans up some spots.

[zzz]

@OP I'm about to turn my attention to this ticket. Let me know if you're going to provide a patch (or if you aren't)

[jogger]

@zzz good you ask: This overlaps with #2427 where I have provided a patch. This one here turned out to be cosmetic, for full stats only.

Going over it, after introducing the sliding window over there, now there is a glitch. I will comment #2427.

[zzz]

OK. zab is feeding me tickets to review, this one was on the list, and #2427 was not, and wasn't referenced here either.

So I still need an answer to my question in comment 6, even if this ticket is "cosmetic":

Are you going to be providing a patch, or should we be doing it, or should this ticket simply be closed without change, or are the changes covered in #2427 ?

[jogger]

Changes covered in #2427. Can be closed or made a subticket.

[zzz]

Closed at OP's request, see #2427