Skip to content

Commit 91f5968

Browse files
hoffmaenhoffmaen
committed
* Minor improvements and refactoring
1 parent 2d02a51 commit 91f5968

File tree

2 files changed

+121
-15
lines changed

2 files changed

+121
-15
lines changed

‎src/code.cloudfoundry.org/gorouter/route/hash_based.go‎

Lines changed: 43 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,17 @@ func (h *HashBased) Next(attempt int) *Endpoint {
5858
return endpoint
5959
}
6060

61+
if len(h.pool.endpoints) == 0 {
62+
h.logger.Warn("hash-based-routing-pool-empty", slog.String("host", h.pool.host))
63+
return nil
64+
}
65+
66+
endpoint = h.getSingleEndpoint()
67+
if endpoint != nil {
68+
h.lastEndpoint = endpoint
69+
return endpoint
70+
}
71+
6172
if h.pool.HashLookupTable == nil {
6273
h.logger.Error("hash-based-routing-failed", slog.String("host", h.pool.host), log.ErrAttr(errors.New("Lookup table is empty")))
6374
return nil
@@ -89,11 +100,6 @@ func (h *HashBased) Next(attempt int) *Endpoint {
89100
}
90101

91102
func (h *HashBased) findEndpoint(index uint64, attempt int) *Endpoint {
92-
maxIterations := len(h.pool.endpoints)
93-
if maxIterations == 0 {
94-
return nil
95-
}
96-
97103
// Ensure we don't exceed the lookup table size
98104
lookupTableSize := h.pool.HashLookupTable.GetLookupTableSize()
99105

@@ -128,10 +134,9 @@ func (h *HashBased) findEndpoint(index uint64, attempt int) *Endpoint {
128134

129135
lastEndpointPrivateId = id
130136

131-
e := endpointElem.endpoint
132-
if h.pool.HashRoutingProperties.BalanceFactor <= 0 || !h.isOverloaded(e) {
137+
if h.pool.HashRoutingProperties.BalanceFactor <= 0 || !h.isImbalancedOrOverloaded(endpointElem) {
133138
h.lastLookupTableIndex = currentIndex
134-
return e
139+
return endpointElem.endpoint
135140
}
136141

137142
currentIndex = (currentIndex + 1) % lookupTableSize
@@ -141,11 +146,24 @@ func (h *HashBased) findEndpoint(index uint64, attempt int) *Endpoint {
141146
return nil
142147
}
143148

144-
func (h *HashBased) isOverloaded(e *Endpoint) bool {
145-
avgLoad := h.CalculateAverageLoad()
149+
func (h *HashBased) isImbalancedOrOverloaded(e *endpointElem) bool {
150+
endpoint := e.endpoint
151+
return h.IsImbalancedOrOverloaded(endpoint, e.isOverloaded())
152+
}
153+
154+
func (h *HashBased) IsImbalancedOrOverloaded(endpoint *Endpoint, isEndpointOverloaded bool) bool {
155+
avgNumberOfInFlightRequests := h.CalculateAverageLoad()
156+
currentInFlightRequestCount := endpoint.Stats.NumberConnections.Count()
146157
balanceFactor := h.pool.HashRoutingProperties.BalanceFactor
147-
if float64(e.Stats.NumberConnections.Count())/avgLoad > balanceFactor {
148-
h.logger.Info("hash-based-routing-endpoint-overloaded", slog.String("host", h.pool.host), slog.String("endpoint-id", e.PrivateInstanceId), slog.Int64("endpoint-connections", e.Stats.NumberConnections.Count()), slog.Float64("average-load", avgLoad))
158+
159+
if isEndpointOverloaded {
160+
h.logger.Debug("hash-based-routing-endpoint-overloaded", slog.String("host", h.pool.host), slog.String("endpoint-id", endpoint.PrivateInstanceId), slog.Int64("endpoint-connections", currentInFlightRequestCount))
161+
return true
162+
}
163+
164+
// Check if avgNumberOfInFlightRequests is 0 to avoid division by 0
165+
if avgNumberOfInFlightRequests == 0 || float64(currentInFlightRequestCount)/avgNumberOfInFlightRequests > balanceFactor {
166+
h.logger.Debug("hash-based-routing-endpoint-imbalanced", slog.String("host", h.pool.host), slog.String("endpoint-id", endpoint.PrivateInstanceId), slog.Int64("endpoint-connections", endpoint.Stats.NumberConnections.Count()), slog.Float64("average-load", avgNumberOfInFlightRequests))
149167
return true
150168
}
151169
return false
@@ -203,6 +221,7 @@ func (h *HashBased) PostRequest(e *Endpoint) {
203221
e.Stats.NumberConnections.Decrement()
204222
}
205223

224+
// CalculateAverageLoad computes the average number of in-flight requests across all endpoints in the pool.
206225
func (h *HashBased) CalculateAverageLoad() float64 {
207226
if len(h.pool.endpoints) == 0 {
208227
return 0
@@ -217,3 +236,15 @@ func (h *HashBased) CalculateAverageLoad() float64 {
217236

218237
return float64(currentInFlightRequestCount) / float64(len(h.pool.endpoints))
219238
}
239+
240+
func (h *HashBased) getSingleEndpoint() *Endpoint {
241+
if len(h.pool.endpoints) == 1 {
242+
e := h.pool.endpoints[0]
243+
if e.isOverloaded() {
244+
return nil
245+
}
246+
247+
return e.endpoint
248+
}
249+
return nil
250+
}

‎src/code.cloudfoundry.org/gorouter/route/hash_based_test.go‎

Lines changed: 78 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,11 @@ import (
66
"time"
77

88
"code.cloudfoundry.org/gorouter/config"
9-
109
"code.cloudfoundry.org/gorouter/route"
1110
"code.cloudfoundry.org/gorouter/test_util"
1211
. "github.com/onsi/ginkgo/v2"
1312
. "github.com/onsi/gomega"
13+
"github.com/onsi/gomega/gbytes"
1414
)
1515

1616
var _ = Describe("HashBased", func() {
@@ -257,7 +257,83 @@ var _ = Describe("HashBased", func() {
257257
Expect(endpoint.Stats.NumberConnections.Count()).To(Equal(initialCount - 1))
258258
})
259259
})
260-
Describe("CalculateAverageLoad", func() {
260+
Describe("IsImbalancedOrOverloaded", func() {
261+
var iter *route.HashBased
262+
var endpoints []*route.Endpoint
263+
264+
BeforeEach(func() {
265+
iter = route.NewHashBased(logger.Logger, pool, "", false, false, "").(*route.HashBased)
266+
})
267+
268+
Context("when endpoints have a lot of in-flight requests", func() {
269+
var e1, e2, e3 *route.Endpoint
270+
BeforeEach(func() {
271+
e1 = route.NewEndpoint(&route.EndpointOpts{Host: "1.2.3.4", Port: 5678, LoadBalancingAlgorithm: "hash", HashHeaderName: "tenant-id", HashBalanceFactor: 1.2, PrivateInstanceId: "ID1"})
272+
e2 = route.NewEndpoint(&route.EndpointOpts{Host: "2.2.3.4", Port: 5678, LoadBalancingAlgorithm: "hash", HashHeaderName: "tenant-id", HashBalanceFactor: 1.2, PrivateInstanceId: "ID2"})
273+
e3 = route.NewEndpoint(&route.EndpointOpts{Host: "3.2.3.4", Port: 5678, LoadBalancingAlgorithm: "hash", HashHeaderName: "tenant-id", HashBalanceFactor: 1.2, PrivateInstanceId: "ID3"})
274+
endpoints = []*route.Endpoint{e1, e2, e3}
275+
for _, e := range endpoints {
276+
pool.Put(e)
277+
}
278+
279+
})
280+
It("mark the endpoint as overloaded", func() {
281+
for i := 0; i < 500; i++ {
282+
iter.PreRequest(e1)
283+
}
284+
// in general 500 in flight requests counted by e1
285+
Expect(iter.IsImbalancedOrOverloaded(e1, true)).To(BeTrue())
286+
})
287+
It("do not mark as imbalanced if every endpoint has 499 in-flight requests", func() {
288+
for i := 0; i < 498; i++ {
289+
iter.PreRequest(e1)
290+
}
291+
for i := 0; i < 498; i++ {
292+
iter.PreRequest(e2)
293+
}
294+
for i := 0; i < 498; i++ {
295+
iter.PreRequest(e3)
296+
}
297+
// in general 500 in flight requests counted by e1
298+
Expect(iter.IsImbalancedOrOverloaded(e1, false)).To(BeFalse())
299+
})
300+
301+
It("mark endpoint as overloaded if every endpoint has 500 in-flight requests", func() {
302+
for i := 0; i < 499; i++ {
303+
iter.PreRequest(e1)
304+
}
305+
for i := 0; i < 499; i++ {
306+
iter.PreRequest(e2)
307+
}
308+
for i := 0; i < 499; i++ {
309+
iter.PreRequest(e3)
310+
}
311+
// in general 500 in flight requests counted by e1
312+
Expect(iter.IsImbalancedOrOverloaded(e1, true)).To(BeTrue())
313+
Eventually(logger).Should(gbytes.Say("hash-based-routing-endpoint-overloaded"))
314+
Expect(iter.IsImbalancedOrOverloaded(e2, true)).To(BeTrue())
315+
Expect(iter.IsImbalancedOrOverloaded(e3, true)).To(BeTrue())
316+
317+
})
318+
It("mark as imbalanced if it has more in-flight requests", func() {
319+
for i := 0; i < 300; i++ {
320+
iter.PreRequest(e1)
321+
}
322+
for i := 0; i < 200; i++ {
323+
iter.PreRequest(e2)
324+
}
325+
for i := 0; i < 200; i++ {
326+
iter.PreRequest(e3)
327+
}
328+
Expect(iter.IsImbalancedOrOverloaded(e1, false)).To(BeTrue())
329+
Eventually(logger).Should(gbytes.Say("hash-based-routing-endpoint-imbalanced"))
330+
Expect(iter.IsImbalancedOrOverloaded(e2, false)).To(BeFalse())
331+
Expect(iter.IsImbalancedOrOverloaded(e3, false)).To(BeFalse())
332+
})
333+
})
334+
})
335+
336+
Describe("CalculateAverageNumberOfConnections", func() {
261337
var iter *route.HashBased
262338
var endpoints []*route.Endpoint
263339

@@ -336,7 +412,6 @@ type MockHashLookupTable struct {
336412

337413
// NewMockHashLookupTable creates a new mock lookup table with predefined mappings
338414
func NewMockHashLookupTable(lookupTable []int, endpointList []string) *MockHashLookupTable {
339-
340415
return &MockHashLookupTable{
341416
lookupTable: lookupTable,
342417
endpointList: endpointList,

0 commit comments

Comments
 (0)