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

[Docs] Need to illustrate the option Slack #121

Open
spencercjh opened this issue Dec 11, 2023 · 6 comments
Open

[Docs] Need to illustrate the option Slack #121

spencercjh opened this issue Dec 11, 2023 · 6 comments

Comments

@spencercjh
Copy link

spencercjh commented Dec 11, 2023

There is also a bug about it: #120

@aneeskA
Copy link

aneeskA commented Dec 12, 2023

Yes, please. What is the meaning of Slack in a leaky bucket algorithm? I have an observation where when the application do not Take() for a period of time, the rate limiting stops to apply on the next Take(). I saw a comment that WithoutSlack is the way to fix this problem. I want to make an educated decision. Please help with some documentation.

@spencercjh
Copy link
Author

There was a similar issue about it: #4 .

@rabbbit
Copy link
Contributor

rabbbit commented Jan 30, 2024

We'll be happy to accept PRs that update the documentation.

@spencercjh
Copy link
Author

spencercjh commented Jan 30, 2024

There is a Chinese blog to illustrate slack and #119, #120.

While it partially explains the context of the bug, it still lacks a direct explanation of slack and we are always approaching this issue from an observer's perspective, missing the original author's direct thoughts on this design about slack.

If no one can clarify this, please mention that slack defaults to 10 and the consequences it brings. Even consider modifying the usage example of this library to guide everyone new to use it with ratelimit.WithoutSlack.

If you agree with my comments, I can submit PRs to make contributions.

@rabbbit
Copy link
Contributor

rabbbit commented Mar 4, 2024

Apologies for the delay.

I see the points you're making. If you can put up the PRs, I'll accept them. Please make them small though - one for example, one for the documentation (perhaps in the FAQ)?

Thank you for the Chinese blog post. I'm surprised by the reported performance degradation, I'll try to run some more tests.

@spencercjh
Copy link
Author

Apologies for the delay.

I see the points you're making. If you can put up the PRs, I'll accept them. Please make them small though - one for example, one for the documentation (perhaps in the FAQ)?

Thank you for the Chinese blog post. I'm surprised by the reported performance degradation, I'll try to run some more tests.

Thanks for your reply. I will try to do some docs works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants