Skip to content

Comments

TaskQueue Fairness Rate Limit redo#8267

Merged
stephanos merged 2 commits intomainfrom
revert-8263-spk/revert-8135
Sep 3, 2025
Merged

TaskQueue Fairness Rate Limit redo#8267
stephanos merged 2 commits intomainfrom
revert-8263-spk/revert-8135

Conversation

@stephanos
Copy link
Contributor

@stephanos stephanos commented Sep 2, 2025

Reverts #8263 which reverted #8135

So this is #8135 plus a fix for flaky tests.

Comparing the results of the test runs (4 each) for the original (1st commit) and fix (2nd commit), I can only conclude that the issue has been addressed.

pm.rateLimitManager.Stop()
pm.versionedQueuesLock.Lock()
defer pm.versionedQueuesLock.Unlock()
pm.rateLimitManager.Stop()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change.

@stephanos stephanos marked this pull request as ready for review September 3, 2025 16:29
@stephanos stephanos requested a review from a team as a code owner September 3, 2025 16:29
Copy link
Contributor

@carlydf carlydf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved! Really nice job pinning this down 🙏

Nit: Could you change the title / add something to the title to indicate "revert but moved pm.rateLimitManager.Stop() after pm.versionedQueuesLock.Lock()" or "revert but with fix"

@stephanos stephanos changed the title Revert "Revert "TaskQueue Fairness Rate Limit (#8135)"" TaskQueue Fairness Rate Limit redo Sep 3, 2025
@stephanos stephanos merged commit 80ac2b7 into main Sep 3, 2025
177 of 179 checks passed
@stephanos stephanos deleted the revert-8263-spk/revert-8135 branch September 3, 2025 17:14
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