backend: replace ratelimiter with x/time/rate. #3155
Conversation
|
@benma I made a mess on the other PR trying to merge master into it. I opened a new one with the same diff. I managed to test these changes also on the |
bznein
force-pushed
the
ratelimiting
branch
2 times, most recently
from
f6692ae to
d6b1e28
Compare
|
@benma friendly ping on this one :) |
go.mod
Outdated
| @@ -22,6 +22,7 @@ require ( | |||
| golang.org/x/crypto v0.32.0 | |||
| golang.org/x/mobile v0.0.0-20240716161057-1ad2df20a8b6 | |||
| golang.org/x/net v0.34.0 | |||
| golang.org/x/time v0.5.0 | |||
There was a problem hiding this comment.
why v0.5.0? v0.10.0 is available also (even v0.11.0 is, but that seems to require Go 1.23).
There was a problem hiding this comment.
Yeah I'm not sure, I think it was auto-added. Will change it
backend/rates/gecko.go
Outdated
| // to one of the supported exchange rates providers. | ||
| func apiRateLimit(baseURL string) time.Duration { | ||
| func apiRateLimit(baseURL string) float64 { |
There was a problem hiding this comment.
make the return type rate.Limit for clarity? then the callsite does not need to convert either.
backend/rates/rates.go
Outdated
| // To stay within acceptable rate limits defined by CoinGeckoRateLimit, callers can | ||
| // use util/ratelimit package. | ||
| // use https://pkg.go.dev/golang.org/x/time/rate package. |
There was a problem hiding this comment.
remove this part? users don't have to do anything, rate limiting is includd in the RatesUpdater
| @@ -275,36 +276,42 @@ func (updater *RateUpdater) updateLast(ctx context.Context) { | |||
| } | |||
|
|
|||
| var geckoRates map[string]map[string]float64 | |||
| callErr := updater.geckoLimiter.Call(ctx, "updateLast", func() error { | |||
| ctx, cancel := context.WithTimeout(ctx, 10*time.Second) | |||
There was a problem hiding this comment.
you missed this line below in your http request, which should time out.
backend/rates/rates.go
Outdated
| if callErr != nil { | ||
| updater.log.WithError(callErr).Errorf("updateLast") | ||
| if err := updater.geckoLimiter.Wait(ctx); err != nil { | ||
| updater.log.WithError(err).Error("could not wait for rate limiter") |
There was a problem hiding this comment.
please prefix all msgs with updateLast: like it used to be before, so the error is more easily identified in the logs.
| // Etherscan rate limits to one request per 0.2 seconds. | ||
| var CallInterval = 260 * time.Millisecond | ||
| var CallsPerSec = 3.8 |
There was a problem hiding this comment.
can unexport this, it's only used locally.
Removes the ratelimiter package and replaces its usages with x/time/rate.
Removes the ratelimiter package and replaces its
usages with x/time/rate.