throttler: fix the throttler when sending messages after messages are dropped

[davidsblom]

Fixes #1526

Pull Request

The throttler is dropping messages even after a sufficiently long quiet period. Instead of using the timestamp of the latest message, and the clock instead.

Type of change

Delete options that are not relevant.

• [x] Bug fix (non-breaking change which fixes an issue)

How has this change been tested?

Adding unit tests and ran a backtest.

[davidsblom]

What I've found thus far is that this line:

diff = self._timestamps[0] - self._timestamps[-1]

uses old timestamps when the latest message is sent.

[davidsblom]

Thinking out loud, maybe instead of a deque of timestamps, use a set of timestamps with a timer set to delete the timestamp when it expires at the end of the interval

[cjdsellers]

Thanks for providing the unit test @davidsblom thats really helpful.

I'll try to get to this soon, likely over the weekend.

[davidsblom]

@cjdsellers I think the issue is fixed now. Appreciate your review.

[cjdsellers]

Hey @davidsblom

This is great, thanks for digging in and providing the fix :pray:.

My only thought is, should we align the meaning of the local variables? i.e should we call the variable in the used method spread instead of diff, since its the result of the same equation? or maybe you have a different suggestion too?

        cdef int64_t spread = self._clock.timestamp_ns() - self._timestamps[-1]
        cdef int64_t diff = max_uint64(0, self._interval_ns - spread)
        cdef double used = <double>diff / <double>self._interval_ns

Otherwise I'm happy to merge and I can make a small local change if we see an opportunity for better variable naming. This component will be ported to Rust soon, so we'll likely reuse the naming there too.

[davidsblom]

Hi @cjdsellers , thanks for taking a look and merging it in. Makes indeed sense to rename the variables to be consistent. Feel free to make those changes if you have the time for it. The variable naming is a bit confusing indeed, and it took some time to fully grasp the meaning of each function.