From 3f618c0485144b616ba27ab835c201e487cd313a Mon Sep 17 00:00:00 2001 From: Zakhar Bessarab Date: Wed, 17 Apr 2024 11:38:19 +0400 Subject: [PATCH] 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 * Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/5565 Signed-off-by: hagen1778 --------- Signed-off-by: Zakhar Bessarab Signed-off-by: hagen1778 Co-authored-by: hagen1778 --- app/vmauth/main.go | 8 -------- docs/CHANGELOG.md | 1 + 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/app/vmauth/main.go b/app/vmauth/main.go index bcfc275f18..202b908fcb 100644 --- a/app/vmauth/main.go +++ b/app/vmauth/main.go @@ -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) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index bf8dc87225..c383aa416b 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -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)