app/vmauth: do not increment backend_errors when hitting concurrency limit (#6078)

* app/vmauth: do not increment backend_errors when hitting concurrency limit

Previously, both "vmauth_concurrent_requests_limit_reached_total" and "vmauth_user_request_backend_errors_total" were incremented.
This was based on the assumption that if concurrency limit is hit the backend must be failing to handle the request thus meaning an error.

This assumption does not work in case the endpoint can be overloaded by the misbehaving client sending too many requests within the timeframe.

Signed-off-by: Zakhar Bessarab <z.bessarab@victoriametrics.com>

* Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5565

Signed-off-by: hagen1778 <roman@victoriametrics.com>

---------

Signed-off-by: Zakhar Bessarab <z.bessarab@victoriametrics.com>
Signed-off-by: hagen1778 <roman@victoriametrics.com>
Co-authored-by: hagen1778 <roman@victoriametrics.com>
This commit is contained in:
Zakhar Bessarab 2024-04-17 11:38:19 +04:00 committed by Aliaksandr Valialkin
parent 60a447bb76
commit 3f618c0485
No known key found for this signature in database
GPG key ID: 52C003EE2BCDB9EB
2 changed files with 1 additions and 8 deletions

View file

@ -161,20 +161,12 @@ func processUserRequest(w http.ResponseWriter, r *http.Request, ui *UserInfo) {
if err := ui.beginConcurrencyLimit(); err != nil {
handleConcurrencyLimitError(w, r, err)
<-concurrencyLimitCh
// Requests failed because of concurrency limit must be counted as errors,
// since this usually means the backend cannot keep up with the current load.
ui.backendErrors.Inc()
return
}
default:
concurrentRequestsLimitReached.Inc()
err := fmt.Errorf("cannot serve more than -maxConcurrentRequests=%d concurrent requests", cap(concurrencyLimitCh))
handleConcurrencyLimitError(w, r, err)
// Requests failed because of concurrency limit must be counted as errors,
// since this usually means the backend cannot keep up with the current load.
ui.backendErrors.Inc()
return
}
processRequest(w, r, ui)

View file

@ -31,6 +31,7 @@ See also [LTS releases](https://docs.victoriametrics.com/lts-releases/).
## tip
* BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert.html): supported any status codes from the range 200-299 from alertmanager. Previously, only 200 status code considered a successful action. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6110).
* BUGFIX: [vmauth](https://docs.victoriametrics.com/vmauth/): don't treat concurrency limit hit as an error of the backend. Previously, hitting the concurrency limit would increment both `vmauth_concurrent_requests_limit_reached_total` and `vmauth_user_request_backend_errors_total` counters. Now, only concurrency limit counter is incremented. Updates [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5565).
## [v1.100.1](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.100.1)