From 8295f7eb346f157f6868e1cb679b88bf4249dbca Mon Sep 17 00:00:00 2001
From: Aliaksandr Valialkin <valyala@victoriametrics.com>
Date: Tue, 12 Nov 2024 16:38:10 +0100
Subject: [PATCH] app/vmauth: simplify the logic for the fix at
 a0a154511a1f26035cd714987fc2924e70087f96

The fix at a0a154511a1f26035cd714987fc2924e70087f96 looks too complicated and fragile:

- It moves buMin initialization to the place, which is far from its usage.
- It embeds unclear logic on selecting the proper buMin if it is broken,
  into unrelated loop.

The actual fix must be more clear:

$ git diff 95acca6b5223c8e92c82935183cf1159b9046196 -- app/vmauth/

-               if n := bu.concurrentRequests.Load(); n < minRequests {
+               if n := bu.concurrentRequests.Load(); n < minRequests || buMin.isBroken() {

This should simplify further maintenance of this code.

Updates https://github.com/VictoriaMetrics/VictoriaMetrics/pull/7489
Updates https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3061
---
 app/vmauth/auth_config.go | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/app/vmauth/auth_config.go b/app/vmauth/auth_config.go
index 018dce6ba4..63f177ef5d 100644
--- a/app/vmauth/auth_config.go
+++ b/app/vmauth/auth_config.go
@@ -462,17 +462,12 @@ func getLeastLoadedBackendURL(bus []*backendURL, atomicCounter *atomic.Uint32) *
 
 	// Slow path - select other backend urls.
 	n := atomicCounter.Add(1) - 1
-	buMin := bus[n%uint32(len(bus))]
 	for i := uint32(0); i < uint32(len(bus)); i++ {
 		idx := (n + i) % uint32(len(bus))
 		bu := bus[idx]
 		if bu.isBroken() {
 			continue
 		}
-		if buMin.isBroken() {
-			// verify that buMin isn't set as broken
-			buMin = bu
-		}
 		if bu.concurrentRequests.Load() == 0 {
 			// Fast path - return the backend with zero concurrently executed requests.
 			// Do not use CompareAndSwap() instead of Load(), since it is much slower on systems with many CPU cores.
@@ -482,12 +477,13 @@ func getLeastLoadedBackendURL(bus []*backendURL, atomicCounter *atomic.Uint32) *
 	}
 
 	// Slow path - return the backend with the minimum number of concurrently executed requests.
+	buMin := bus[n%uint32(len(bus))]
 	minRequests := buMin.concurrentRequests.Load()
 	for _, bu := range bus {
 		if bu.isBroken() {
 			continue
 		}
-		if n := bu.concurrentRequests.Load(); n < minRequests {
+		if n := bu.concurrentRequests.Load(); n < minRequests || buMin.isBroken() {
 			buMin = bu
 			minRequests = n
 		}