-
-
Notifications
You must be signed in to change notification settings - Fork 0
Simplify & expand Wait methods, remove FIFO #31
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR removes the FIFO (First-In-First-Out) ordering guarantee from the Wait and WaitN methods in the rate limiter library, addressing issue #30. The change simplifies the implementation by eliminating the waiter queue mechanism.
- Remove FIFO ordering guarantees from Wait/WaitN methods
- Delete waiter queue implementation and associated data structures
- Clean up unused syncMap methods that were only needed for waiter management
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| limiter.go | Remove waiters field from Limiter struct |
| limiter_wait.go | Remove waiter queue logic and FIFO ordering implementation |
| limiter_wait_test.go | Restructure tests and remove FIFO-specific test cases |
| syncmap.go | Remove unused syncMap type and associated methods |
| syncmap_test.go | Remove tests for deleted syncMap functionality |
More cleanup to do I think
We only care about ctx.Done. Simpler, and less likely to have clock inconsistencies.
c0473c1 to
8476250
Compare
484cf9b to
73c3e04
Compare
ctx first is idiomatic, changed waitWithDetails too
446deda to
a56db73
Compare
a56db73 to
13c9e35
Compare
Addresses #30, read that for justifications. tl;dr a flaky promise is worse than no promise.
Remove the waiter queue, and therefore any FIFO, from the
Waitmethods.Clean up now-unused syncMap methods.
…and implement
Wait*with details, debugs, and onLimiters. More than planned.