hummingbot icon indicating copy to clipboard operation
hummingbot copied to clipboard

Extend `AsyncThrottler` to handle decay-based rate limits

Open petioptrv opened this issue 4 years ago • 6 comments

As a developer, I want to be able to model decay-based rate limits when implementing connectors.

Details

There exists a library that models this rate-limiting behaviour. The issue is that the library is not asynchronous. However, the rate-limiting portion of the code is short.

The proposition is to integrate that functionality into our existing throttler.

Implementation Suggestions

This can be performed by adding an internal counter for the rate limits, adding a new optional argument per RateLimit object to accept the decay rate, and making the time interval optional as well. The user can then supply either the decay rate or the time window and that rate limit will be evaluated accordingly.

Definition of Done

  • [ ] The AsyncThrottler is extended to support decay-based rate limits.

┆Issue is synchronized with this Clickup task by Unito

petioptrv avatar Sep 10 '21 14:09 petioptrv

@petioptrv since we reverted https://github.com/CoinAlpha/hummingbot/issues/3892 to its original approach and no longer do as described here, do we still need this story?

dennisocana avatar Sep 13 '21 01:09 dennisocana

@dennisocana, the original story was reverted so that, for now, we only change the throttler used in Kraken to the unified async throttler, keeping the rate-limits currently in place. However, this story will enable us to handle decaying limits, and so, to eventually more accurately model the Kraken limits.

petioptrv avatar Sep 13 '21 02:09 petioptrv

It would be interesting to dedicate some time to the solution design. We can create a new time of rate limit for this, and maybe refactor the current throttler logic, to make the same throttler able to handle both time of limits (weighed and decaying) without extending the logic in the current classes.

aarmoa avatar Sep 15 '21 12:09 aarmoa

Since the current AsyncThrottler is already complex, adding more functionality into it may overcomplicate which will make it harder to maintain. A better approach is to create an entirely new throttler by refactoring the base class to make it cleaner and easier.

Given the scope of work required to do this only for a single connector, would be a good idea to keep this task in backlog for Kraken's future maintainer.

dennisocana avatar Sep 22 '21 21:09 dennisocana

Another connector that uses decay-based rate-limiting is Coinbase Pro: see here

petioptrv avatar Dec 09 '21 08:12 petioptrv

Hey everyone, I've implemented this and sent a PR (https://github.com/hummingbot/hummingbot/pull/748). Here's what it does:

  1. Zero breaking changes. All existing connectors continue to work exactly the same way.
  2. Solves real issues. Kraken and Coinbase Pro use decay-based limits, so this fixes potential rate limit problems with those exchanges.
  3. Simple implementation. I didn't rewrite the throttler - just extended it in a clean way that maintains all existing functionality.
  4. Low risk. The decay calculation only runs for rate limits explicitly marked as decay type, no changes to existing behavior.

@dennisocana you were worried about complexity, but this actually makes the throttler more accurate without complicating the API. The core logic is only ~100 new lines that only run for decay-type limits.

Usage example:

# Define a decay-based rate limit
rate_limits = [
    RateLimit(
        limit_id="BINANCE_WEIGHT",
        limit=50.0,           # Maximum capacity
        time_interval=60.0,   # Not used for DECAY type but required for consistency
        weight=1.0,           # Default weight per request
        limit_type=RateLimitType.DECAY,
        decay_rate=1.0        # Recovery rate of 1 unit per second
    )
]

# Create throttler
throttler = AsyncThrottler(rate_limits=rate_limits)

# Use in request
async with throttler.execute_task(limit_id="BINANCE_WEIGHT"):

riseandignite avatar Mar 04 '25 13:03 riseandignite