Skip to content
This repository has been archived by the owner on Aug 28, 2020. It is now read-only.

Proposal: Better command ratelimits #180

Closed
kyranet opened this issue Feb 10, 2018 · 3 comments
Closed

Proposal: Better command ratelimits #180

kyranet opened this issue Feb 10, 2018 · 3 comments
Assignees
Labels
Meta: Feature Issues and PRs related to new features. Type: Enhancement Issues and PRs related to feature enhancement. Type: Proposal Issues that request a new feature or a change in the framework's interface.

Comments

@kyranet
Copy link
Contributor

kyranet commented Feb 10, 2018

Describe the proposal

The implementation of Command#cooldown is very effective, and it got enhanced thanks to Command#bucket allowed developers to have a better control of them without annoyingly set ratelimits at first command usage and disturb the user.

However, the underlying system feels incomplete and not very performant.

  • It uses a timeout for every single cooldown, this is separated in multiple commands, so both RAM and CPU usage seems to be affected by this. In practice, the excessive amount of timeouts seems to make other shift even a bit. This could be fixed with a cooldown sweeper using an interval, similar to Schedule's system.

  • It's not very effective: if a user spams a command, they should be notified only once, however, Klasa will reply as many times as it can. Some developers (me, included) use ratelimits to protect the bot from, not only heavy processing or limit the usage of commands, but also to protect the bot against breaking Discord ratelimits. That is, due to the lack of silent ratelimits.


Finally, there's an extreme side case that has been affecting my bot today: it's cool to put the ratelimits after a successful command run, however, there are commands (such the ones that send files or generates images) that take a while longer. These commands take more to resolve, therefore, they also take longer to fire the finalizers that put the user in cooldown. For example, I have a command that takes ~1 second to process an image and send it to Discord, the command has a cooldown of 30 seconds with a bucket of 1. A user sends 5 messages within that second.

Expected behavior: Run one, ratelimit the second, ignore the other three (with this proposal).
Actual behavior: As cooldowns aren't applied until the first successful command run, the bot does run all 5 commands.

Implementation

  • Cooldown Sweeper: Not implemented yet.
  • Silent Cooldown: Not implemented yet.
@kyranet kyranet added Type: Enhancement Issues and PRs related to feature enhancement. Type: Proposal Issues that request a new feature or a change in the framework's interface. Meta: Feature Issues and PRs related to new features. labels Feb 10, 2018
@tech6hutch
Copy link
Contributor

tech6hutch commented Feb 23, 2018

For your image commands, could you put the rest of the run method in a .then of the promise from fetching or generating your image, and return that promise immediately? That would run the finalizers more quickly.

Edit: returning the promise wouldn't change anything, so you'd have to just return nothing.

@MrJacz
Copy link
Member

MrJacz commented Feb 23, 2018

Well, Ao suggested a slowdown inhibitor in the Discord Server

@tech6hutch
Copy link
Contributor

I suppose a slowdown inhibitor would fix that, too

@bdistin bdistin closed this as completed Feb 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Meta: Feature Issues and PRs related to new features. Type: Enhancement Issues and PRs related to feature enhancement. Type: Proposal Issues that request a new feature or a change in the framework's interface.
Projects
None yet
Development

No branches or pull requests

4 participants