Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate or implement internal rate limiting? #190

Open
udoprog opened this issue Dec 29, 2019 · 2 comments
Open

Deprecate or implement internal rate limiting? #190

udoprog opened this issue Dec 29, 2019 · 2 comments

Comments

@udoprog
Copy link
Contributor

udoprog commented Dec 29, 2019

#182 was fixed, but I didn't implement rate limiting using burst windows. As per the comment in #184, I raised the question whether this project should provide a built-in rate limiter at all or if we should encourage the user to use an external project like leaky-bucket (full disclosure: my project).

The following is an example of how an external rate limiter is used for anyone who is curious. Unfortunately this hides a lot of utility functions of Sender (like send_cap_req) and it's not straight forward to make async functions like acquire below pluggable. async functions are not supported in traits, nor do we have existential types yet so the anonymous future can be named.

The benefit however is that the rate limiter is an async function that can be await:ed at any point. You have full control over its configuration, use, and implementation. Rate limiting happens before the send is buffered which makes sure that the rest of your application is back pressured when necessary.

use anyhow::Result;
use irc::{Message, Sender};
use leaky_bucket::LeakyBucket;

struct RateLimitedSender {
    sender: irc::Sender,
    rate_limiter: LeakyBucket,
}

impl RateLimitedSender {
    pub fn new(sender: Sender) -> Result<Self> {
        Ok(Self {
            sender,
            leaky_bucket: LeakyBucket::builder()
                .max(100)
                .refill_interval(Duration::from_secs(10))
                .refill_amount(100)
                .build()?,
        })
    }

    pub async fn send(m: impl Into<Message>) -> Result<()> {
        self.rate_limiter.acquire(1).await?;
        self.sender.send(m);
        Ok(())
    }
}
@udoprog
Copy link
Contributor Author

udoprog commented Dec 29, 2019

Related: #191

@aatxe
Copy link
Owner

aatxe commented Dec 31, 2019

I'm fine with presenting the set up for rate limiting differently (including having it be more directly optional), but I'd definitely like to include an out-of-the-box solution within the IRC crate if possible.

wmfgerrit pushed a commit to wikimedia/wikimedia-irc-ircservserv that referenced this issue May 24, 2021
Sleep 1s in between sending commands in sync_channel() will hopefully
be good enough while the `irc` crate doesn't actually have ratelimiting
(see aatxe/irc#190).

Calling the send_ function just queues the message to send, but
hopefully waiting a second between commands gives enough time for
limiting to be enough to avoid flooding.

Change-Id: Id81d77699bedb1bc1634b0ccc28423bf7a15e772
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants