Skip to content

Commit c967958

Browse files
Copilotalexisjcarr
andcommitted
Fix bug: Exclude hosts with non-OK HTTP status from lag calculation
Previously, only hosts returning 404 were excluded from lag calculations. This caused hosts returning other error codes (500, 503, etc.) to still be used in lag calculations, even though they were unhealthy. Changed the condition to exclude any host with non-OK (non-200) HTTP status. - Updated pkg/throttle/mysql.go to check for != StatusOK instead of == StatusNotFound - Added test case to verify 500 and 503 status codes are properly excluded Co-authored-by: alexisjcarr <[email protected]>
1 parent 9ff4c7d commit c967958

File tree

2 files changed

+52
-1
lines changed

2 files changed

+52
-1
lines changed

pkg/throttle/mysql.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ func aggregateMySQLProbes(
2121
// so it's safe to iterate it
2222
probeValues := []float64{}
2323
for _, probe := range *probes {
24-
if clusterInstanceHttpChecksMap[mysql.MySQLHttpCheckHashKey(clusterName, &probe.Key)] == http.StatusNotFound {
24+
if clusterInstanceHttpChecksMap[mysql.MySQLHttpCheckHashKey(clusterName, &probe.Key)] != http.StatusOK {
2525
continue
2626
}
2727
instanceMetricResult, ok := instanceResultsMap[mysql.GetClusterInstanceKey(clusterName, &probe.Key)]

pkg/throttle/mysql_status_test.go

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
package throttle
2+
3+
import (
4+
"net/http"
5+
"testing"
6+
7+
"github.com/github/freno/pkg/base"
8+
"github.com/github/freno/pkg/mysql"
9+
10+
test "github.com/outbrain/golib/tests"
11+
)
12+
13+
func TestHttpStatusExclusion(t *testing.T) {
14+
clusterName := "c0"
15+
k1 := mysql.GetClusterInstanceKey(clusterName, &key1)
16+
k2 := mysql.GetClusterInstanceKey(clusterName, &key2)
17+
k3 := mysql.GetClusterInstanceKey(clusterName, &key3)
18+
k4 := mysql.GetClusterInstanceKey(clusterName, &key4)
19+
20+
instanceResults := mysql.InstanceMetricResultMap{
21+
k1: base.NewSimpleMetricResult(1.2),
22+
k2: base.NewSimpleMetricResult(1.7),
23+
k3: base.NewSimpleMetricResult(0.3),
24+
k4: base.NewSimpleMetricResult(2.5),
25+
}
26+
27+
var probeList mysql.Probes = map[mysql.InstanceKey](*mysql.Probe){}
28+
for ck := range instanceResults {
29+
probeList[ck.Key] = &mysql.Probe{Key: ck.Key}
30+
}
31+
32+
// Test: 500 status should exclude host with worst lag
33+
httpStatuses := mysql.ClusterInstanceHttpCheckResultMap{
34+
mysql.MySQLHttpCheckHashKey(clusterName, &key1): http.StatusOK,
35+
mysql.MySQLHttpCheckHashKey(clusterName, &key2): http.StatusOK,
36+
mysql.MySQLHttpCheckHashKey(clusterName, &key3): http.StatusOK,
37+
mysql.MySQLHttpCheckHashKey(clusterName, &key4): http.StatusInternalServerError,
38+
}
39+
40+
result := aggregateMySQLProbes(&probeList, clusterName, instanceResults, httpStatuses, 0, false, 0)
41+
lagValue, err := result.Get()
42+
test.S(t).ExpectNil(err)
43+
test.S(t).ExpectEquals(lagValue, 1.7)
44+
45+
// Test: 503 status should also exclude
46+
httpStatuses[mysql.MySQLHttpCheckHashKey(clusterName, &key2)] = http.StatusServiceUnavailable
47+
result = aggregateMySQLProbes(&probeList, clusterName, instanceResults, httpStatuses, 0, false, 0)
48+
lagValue, err = result.Get()
49+
test.S(t).ExpectNil(err)
50+
test.S(t).ExpectEquals(lagValue, 1.2)
51+
}

0 commit comments

Comments
 (0)