Skip to content
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

Rate limiting for streaming endpoints is behaving incorrectly #2852

Closed
bartekn opened this issue Jul 24, 2020 · 0 comments · Fixed by #4163
Closed

Rate limiting for streaming endpoints is behaving incorrectly #2852

bartekn opened this issue Jul 24, 2020 · 0 comments · Fixed by #4163

Comments

@bartekn
Copy link
Contributor

bartekn commented Jul 24, 2020

What version are you using?

1.6.0

What did you do?

@marcelosalloum was streaming historical data using /transactions endpoint. Request rate was under allowed quota.

What did you expect to see?

200 OK responses only.

What did you see instead?

Some 429 Too Many Requests responses.


After debugging the issue it seems that rate limiting is broken when streaming. Users pay 1 point per DB access while streaming:

func (we *web) streamHandler(jfn interface{}, sfn streamFunc, params interface{}) http.HandlerFunc {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
ctx := r.Context()
stream := sse.NewStream(ctx, w)
var oldHash [32]byte
for {
lastLedgerState := ledger.CurrentState()
// Rate limit the request if it's a call to stream since it queries the DB every second. See
// https://github.com/stellar/go/issues/715 for more details.
rateLimiter := we.rateLimiter
if rateLimiter != nil {
limited, _, err := rateLimiter.RateLimiter.RateLimit(rateLimiter.VaryBy.Key(r), 1)
if err != nil {
stream.Err(errors.Wrap(err, "RateLimiter error"))
return
}
if limited {
stream.Err(sse.ErrRateLimited)
return
}
}

but also 1 point for HTTP request (via middleware):
func (w *web) RateLimitMiddleware(next http.Handler) http.Handler {
if w.rateLimiter == nil {
return next
}
return w.rateLimiter.RateLimit(next)
}

This means that when streaming historical data the allowed request quota is actually 2x smaller.

@bartekn bartekn changed the title Rate limiting for streaming endpoints Rate limiting for streaming endpoints is behaving incorrectly Jul 24, 2020
@tamirms tamirms self-assigned this Dec 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants