Skip to content

Conversation

@clipperhouse
Copy link
Owner

@clipperhouse clipperhouse commented Aug 20, 2025

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 Wait methods.

Clean up now-unused syncMap methods.

…and implement Wait* with details, debugs, and on Limiters. More than planned.

@clipperhouse clipperhouse requested a review from Copilot August 20, 2025 20:53
Copy link

Copilot AI left a 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.
@clipperhouse clipperhouse force-pushed the clipperhouse/30/remove-wait-fifo branch from c0473c1 to 8476250 Compare August 21, 2025 16:58
@clipperhouse clipperhouse force-pushed the clipperhouse/30/remove-wait-fifo branch 3 times, most recently from 484cf9b to 73c3e04 Compare August 21, 2025 21:34
ctx first is idiomatic, changed waitWithDetails too
@clipperhouse clipperhouse force-pushed the clipperhouse/30/remove-wait-fifo branch 6 times, most recently from 446deda to a56db73 Compare August 22, 2025 02:38
@clipperhouse clipperhouse force-pushed the clipperhouse/30/remove-wait-fifo branch from a56db73 to 13c9e35 Compare August 22, 2025 02:54
@clipperhouse clipperhouse changed the title Remove FIFO on Wait Simplify & expand Wait methods, remove FIFO Aug 22, 2025
@clipperhouse clipperhouse merged commit 2493707 into main Aug 22, 2025
9 checks passed
@clipperhouse clipperhouse deleted the clipperhouse/30/remove-wait-fifo branch August 22, 2025 16:51
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

Successfully merging this pull request may close these issues.

2 participants