Summary:
(1) The first change is the pacing rate calculation is simplified. It
removes the interval calculation and just uses the timer tick as the interval.
Then it calculates the burst size from there. For most cases these two
calculation should land at the same result, except when the
`cwnd < minBurstSize * tick / RTT`. In that case, the current calculation would
spread writes evenly across one RTT, assuming no new Ack arrives during the RTT;
while the new calculation uses the first a few ticks to finish the cwnd amount
of data.
(2) Then this diff changes how we compensate late timer. Now the pacer will
maintain a nextWriteTime_ and lastWriteTime_, which makes it easier to
calculate time elapsed since last write. Then each time writer tries to write,
it will be allowed to write timeElapsed * pacingRate. This is much more
intuitive than the current logic.
(3) The diff also adds pacing limited tracking into the pacer. An expected
pacing rate is cached when pacing rate is refreshed by congestion controller.
Then with packets sent out, Pacer keeps calculating the current send rate. When
the send rate is lower, Pacer sets pacingLimited_ to true. Otherwise false.
Only when the connection is not pacing limited, the lastWriteTime_ will be
packet sent time, otherwise it will be set to the last nextWriteTime_. In other
words: if the send rate is lower than expected, we use the expected send time
instead of real send time to calculate time elapsed, to allow higher late
timer compenstation, to give pacer a chance to catch up.
(4) Finally this diff removes the token collecting behavior in the pacer. I
think having tokens increaed, instead of reset, when an ack refreshes the pacing
rate or when we compensate late time, is quite confusing to some people. After
all the above changes, I found tperf can still sustain good throughput without
always increase tokens, and rally actualy gives even better results. So i think
we can remove this part of the pacer that's potentially very confusing to
people who don't know how we got there.
Reviewed By: mjoras
Differential Revision: D19252744
fbshipit-source-id: b83e4a01fc812fc52117f3ec0f5c3be1badf211f
Summary:
That's it. No more artificial limit. Even in a really fat pipe i
haven't seen us writing out a burst > 8 packets when the timer can schedule per
200 us.
Reviewed By: mjoras
Differential Revision: D17670054
fbshipit-source-id: 67535a6fae27defc7b6f79d76c9e5fa933b3923d
Summary:
Use the new Pacer interface in the transport where we currently
directly use CongestinoController interrace for paciner related APIs.
Reviewed By: mjoras
Differential Revision: D16918672
fbshipit-source-id: 410f72d567e71bdd3279e344f6e9abf5e132518e
Summary:
second try of this diff, now after fixing the uninitizlied
std::chrono::duration values in BBR and Copa
No need to pass the minimal interval any more since everywhere we just
pass the transportSettings value. Just that inside the function directly
Reviewed By: oesh
Differential Revision: D16985567
fbshipit-source-id: 3818371708633a324256e62b7f49bd6d1441e991
Summary:
No need to pass the minimal interval any more since everywhere we just
pass the transportSettings value. Just that inside the function directly
Reviewed By: mjoras
Differential Revision: D16773023
fbshipit-source-id: 22ada4f25d565e97e7fce27371a0e2240bbfe8c0
Summary:
Currently we use the number from transport setting directly inside
this function. There are congestion controllers that want to have a different
min cwnd value than the transport settings.
Reviewed By: siyengar
Differential Revision: D15292719
fbshipit-source-id: 69cdf9dea11c55907f0a511ebb3c000b48eac8f0