search

uber-go / ratelimit

A Go blocking leaky-bucket rate limit implementation
MIT License
4.32k stars 300 forks source link

Restore int64 based atomic rate limiter #94

Closed storozhukBM closed 2 years ago

storozhukBM commented 2 years ago

This limiter was introduced and merged in the following PR #85 Later @twelsh-aw found an issue with this implementation #90 So @rabbbit reverted this change in #91

Our tests did not detect this issue, so we have a separate PR #93 that enhances our tests approach to detect potential errors better. With this PR, we want to restore the int64-based atomic rate limiter implementation as a non-default rate limiter and then check that #93 will detect the bug. Right after it, I'll open a subsequent PR to fix this bug.

codecov[bot] commented 2 years ago

Codecov Report

Merging #94 (e3d4637) into main (029273d) will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main       #94   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            3         4    +1     
  Lines           70        97   +27     
=========================================
+ Hits            70        97   +27
Impacted Files Coverage Δ
limiter_atomic_int64.go 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 029273d...e3d4637. Read the comment docs.

rabbbit commented 2 years ago

Thanks!