From 8dd637c3669f7f838c364df5cbf2fb1e7a58f50c Mon Sep 17 00:00:00 2001 From: meiji163 Date: Tue, 17 Jun 2025 22:47:28 -0700 Subject: [PATCH 1/7] add bypass --- pkg/http/api.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkg/http/api.go b/pkg/http/api.go index 39b4c4c..f78d127 100644 --- a/pkg/http/api.go +++ b/pkg/http/api.go @@ -62,13 +62,16 @@ type APIImpl struct { throttlerCheck *throttle.ThrottlerCheck consensusService group.ConsensusService hostname string + bypassEnabled bool } // NewAPIImpl creates a new instance of the API implementation func NewAPIImpl(throttlerCheck *throttle.ThrottlerCheck, consensusService group.ConsensusService) *APIImpl { + bypassEnabled := (os.Getenv("FRENO_BYPASS_ENABLED") != "") api := &APIImpl{ throttlerCheck: throttlerCheck, consensusService: consensusService, + bypassEnabled: bypassEnabled, } if hostname, err := os.Hostname(); err == nil { api.hostname = hostname @@ -165,6 +168,12 @@ func (api *APIImpl) check(w http.ResponseWriter, r *http.Request, ps httprouter. } flags.LowPriority = (r.URL.Query().Get("p") == "low") + if api.bypassEnabled { + checkResult := throttle.NewCheckResult(http.StatusOK, 0, 0, nil) + api.respondToCheckRequest(w, r, checkResult) + return + } + checkResult := api.throttlerCheck.Check(appName, storeType, storeName, remoteAddr, flags) if checkResult.StatusCode == http.StatusNotFound && flags.OKIfNotExists { checkResult.StatusCode = http.StatusOK // 200 From 38d19aee60ef3a0118cd68a35faf55f6f69637f7 Mon Sep 17 00:00:00 2001 From: meiji163 Date: Tue, 17 Jun 2025 23:22:03 -0700 Subject: [PATCH 2/7] Update pkg/http/api.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- pkg/http/api.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/http/api.go b/pkg/http/api.go index f78d127..cbbcf3d 100644 --- a/pkg/http/api.go +++ b/pkg/http/api.go @@ -67,7 +67,10 @@ type APIImpl struct { // NewAPIImpl creates a new instance of the API implementation func NewAPIImpl(throttlerCheck *throttle.ThrottlerCheck, consensusService group.ConsensusService) *APIImpl { - bypassEnabled := (os.Getenv("FRENO_BYPASS_ENABLED") != "") + bypassEnabled, err := strconv.ParseBool(os.Getenv("FRENO_BYPASS_ENABLED")) + if err != nil { + bypassEnabled = false + } api := &APIImpl{ throttlerCheck: throttlerCheck, consensusService: consensusService, From 934910ce132e79396efb54b3b4aff2466e03f738 Mon Sep 17 00:00:00 2001 From: meiji163 Date: Wed, 18 Jun 2025 17:14:15 -0700 Subject: [PATCH 3/7] Revert "add bypass" This reverts commit 8dd637c3669f7f838c364df5cbf2fb1e7a58f50c. --- pkg/http/api.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/pkg/http/api.go b/pkg/http/api.go index cbbcf3d..39b4c4c 100644 --- a/pkg/http/api.go +++ b/pkg/http/api.go @@ -62,19 +62,13 @@ type APIImpl struct { throttlerCheck *throttle.ThrottlerCheck consensusService group.ConsensusService hostname string - bypassEnabled bool } // NewAPIImpl creates a new instance of the API implementation func NewAPIImpl(throttlerCheck *throttle.ThrottlerCheck, consensusService group.ConsensusService) *APIImpl { - bypassEnabled, err := strconv.ParseBool(os.Getenv("FRENO_BYPASS_ENABLED")) - if err != nil { - bypassEnabled = false - } api := &APIImpl{ throttlerCheck: throttlerCheck, consensusService: consensusService, - bypassEnabled: bypassEnabled, } if hostname, err := os.Hostname(); err == nil { api.hostname = hostname @@ -171,12 +165,6 @@ func (api *APIImpl) check(w http.ResponseWriter, r *http.Request, ps httprouter. } flags.LowPriority = (r.URL.Query().Get("p") == "low") - if api.bypassEnabled { - checkResult := throttle.NewCheckResult(http.StatusOK, 0, 0, nil) - api.respondToCheckRequest(w, r, checkResult) - return - } - checkResult := api.throttlerCheck.Check(appName, storeType, storeName, remoteAddr, flags) if checkResult.StatusCode == http.StatusNotFound && flags.OKIfNotExists { checkResult.StatusCode = http.StatusOK // 200 From 0ab8e8259059c11b4700d5fd29a5ff5f8afa1b2b Mon Sep 17 00:00:00 2001 From: meiji163 Date: Wed, 18 Jun 2025 17:13:02 -0700 Subject: [PATCH 4/7] bypass on no hosts found --- cmd/freno/main.go | 8 ++++++++ pkg/throttle/throttler.go | 6 ++++++ 2 files changed, 14 insertions(+) diff --git a/cmd/freno/main.go b/cmd/freno/main.go index 3bf0a7f..4c5105c 100644 --- a/cmd/freno/main.go +++ b/cmd/freno/main.go @@ -11,6 +11,8 @@ import ( "github.com/github/freno/pkg/http" "github.com/github/freno/pkg/throttle" "github.com/outbrain/golib/log" + "os" + "strconv" ) // AppVersion has to be filled by ldflags: @@ -120,6 +122,12 @@ func httpServe() error { throttler := throttle.NewThrottler() log.Infof("Starting consensus service") log.Infof("- forced leadership: %+v", group.ForceLeadership) + + bypassEnabled, err := strconv.ParseBool(os.Getenv("FRENO_BYPASS_ENABLED")) + if err != nil { + bypassEnabled = false + } + throttler.BypassOnNoHostsFound = bypassEnabled consensusServiceProvider, err := group.NewConsensusServiceProvider(throttler) if err != nil { return err diff --git a/pkg/throttle/throttler.go b/pkg/throttle/throttler.go index e0d103c..3dd020d 100644 --- a/pkg/throttle/throttler.go +++ b/pkg/throttle/throttler.go @@ -78,6 +78,8 @@ type Throttler struct { nonLowPriorityAppRequestsThrottled *cache.Cache httpClient *http.Client + + BypassOnNoHostsFound bool } func NewThrottler() *Throttler { @@ -331,6 +333,10 @@ func (throttler *Throttler) refreshMySQLInventory() error { } } if len(totalHosts) == 0 { + if throttler.bypassOnNoHostsFound { + log.Debugf("No hosts for pool: %+v, but bypass is enabled", poolName) + return nil + } return log.Errorf("Unable to get any HAproxy hosts for pool: %+v", poolName) } From 891cfa17367ef408a77c1b9671e407f68d925d3b Mon Sep 17 00:00:00 2001 From: meiji163 Date: Wed, 18 Jun 2025 17:17:21 -0700 Subject: [PATCH 5/7] fix --- pkg/throttle/throttler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/throttle/throttler.go b/pkg/throttle/throttler.go index 3dd020d..b602929 100644 --- a/pkg/throttle/throttler.go +++ b/pkg/throttle/throttler.go @@ -333,7 +333,7 @@ func (throttler *Throttler) refreshMySQLInventory() error { } } if len(totalHosts) == 0 { - if throttler.bypassOnNoHostsFound { + if throttler.BypassOnNoHostsFound { log.Debugf("No hosts for pool: %+v, but bypass is enabled", poolName) return nil } From 169ec08796f507cc222b3ceaa261a608dba35f47 Mon Sep 17 00:00:00 2001 From: meiji163 Date: Wed, 18 Jun 2025 18:16:41 -0700 Subject: [PATCH 6/7] handle empty probes --- pkg/throttle/mysql.go | 4 ++++ pkg/throttle/mysql_test.go | 49 ++++++++++++++++++++++---------------- pkg/throttle/throttler.go | 2 +- 3 files changed, 33 insertions(+), 22 deletions(-) diff --git a/pkg/throttle/mysql.go b/pkg/throttle/mysql.go index 2489b34..532440b 100644 --- a/pkg/throttle/mysql.go +++ b/pkg/throttle/mysql.go @@ -16,6 +16,7 @@ func aggregateMySQLProbes( ignoreHostsCount int, ignoreDialTcpErrors bool, ignoreHostsThreshold float64, + bypassOnNohosts bool, ) (worstMetric base.MetricResult) { // probes is known not to change. It can be *replaced*, but not changed. // so it's safe to iterate it @@ -46,6 +47,9 @@ func aggregateMySQLProbes( probeValues = append(probeValues, value) } if len(probeValues) == 0 { + if bypassOnNohosts { + return base.NewSimpleMetricResult(0.0) + } return base.NoHostsMetricResult } diff --git a/pkg/throttle/mysql_test.go b/pkg/throttle/mysql_test.go index 2b13f1a..24c6f5a 100644 --- a/pkg/throttle/mysql_test.go +++ b/pkg/throttle/mysql_test.go @@ -54,41 +54,48 @@ func TestAggregateMySQLProbesNoErrors(t *testing.T) { probes[clusterKey.Key] = &mysql.Probe{Key: clusterKey.Key} } { - worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 0, false, 0) + worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 0, false, 0, false) value, err := worstMetric.Get() test.S(t).ExpectNil(err) test.S(t).ExpectEquals(value, 1.7) } { - worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 1, false, 0) + worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 1, false, 0, false) value, err := worstMetric.Get() test.S(t).ExpectNil(err) test.S(t).ExpectEquals(value, 1.2) } { - worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 2, false, 0) + worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 2, false, 0, false) value, err := worstMetric.Get() test.S(t).ExpectNil(err) test.S(t).ExpectEquals(value, 1.1) } { - worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 3, false, 0) + worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 3, false, 0, false) value, err := worstMetric.Get() test.S(t).ExpectNil(err) test.S(t).ExpectEquals(value, 0.6) } { - worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 4, false, 0) + worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 4, false, 0, false) value, err := worstMetric.Get() test.S(t).ExpectNil(err) test.S(t).ExpectEquals(value, 0.3) } { - worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 5, false, 0) + worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 5, false, 0, false) value, err := worstMetric.Get() test.S(t).ExpectNil(err) test.S(t).ExpectEquals(value, 0.3) } + { + var emptyProbes mysql.Probes = map[mysql.InstanceKey](*mysql.Probe){} + worstMetric := aggregateMySQLProbes(&emptyProbes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 5, false, 0, true) + value, err := worstMetric.Get() + test.S(t).ExpectNil(err) + test.S(t).ExpectEquals(value, 0.0) + } } func TestAggregateMySQLProbesNoErrorsIgnoreHostsThreshold(t *testing.T) { @@ -117,37 +124,37 @@ func TestAggregateMySQLProbesNoErrorsIgnoreHostsThreshold(t *testing.T) { probes[clusterKey.Key] = &mysql.Probe{Key: clusterKey.Key} } { - worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 0, false, 1.0) + worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 0, false, 1.0, false) value, err := worstMetric.Get() test.S(t).ExpectNil(err) test.S(t).ExpectEquals(value, 1.7) } { - worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 1, false, 1.0) + worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 1, false, 1.0, false) value, err := worstMetric.Get() test.S(t).ExpectNil(err) test.S(t).ExpectEquals(value, 1.2) } { - worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 2, false, 1.0) + worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 2, false, 1.0, false) value, err := worstMetric.Get() test.S(t).ExpectNil(err) test.S(t).ExpectEquals(value, 1.1) } { - worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 3, false, 1.0) + worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 3, false, 1.0, false) value, err := worstMetric.Get() test.S(t).ExpectNil(err) test.S(t).ExpectEquals(value, 0.6) } { - worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 4, false, 1.0) + worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 4, false, 1.0, false) value, err := worstMetric.Get() test.S(t).ExpectNil(err) test.S(t).ExpectEquals(value, 0.6) } { - worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 5, false, 1.0) + worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 5, false, 1.0, false) value, err := worstMetric.Get() test.S(t).ExpectNil(err) test.S(t).ExpectEquals(value, 0.6) @@ -180,19 +187,19 @@ func TestAggregateMySQLProbesWithErrors(t *testing.T) { probes[clusterKey.Key] = &mysql.Probe{Key: clusterKey.Key} } { - worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 0, false, 0) + worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 0, false, 0, false) _, err := worstMetric.Get() test.S(t).ExpectNotNil(err) test.S(t).ExpectEquals(err, base.NoSuchMetricError) } { - worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 1, false, 0) + worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 1, false, 0, false) value, err := worstMetric.Get() test.S(t).ExpectNil(err) test.S(t).ExpectEquals(value, 1.7) } { - worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 2, false, 0) + worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 2, false, 0, false) value, err := worstMetric.Get() test.S(t).ExpectNil(err) test.S(t).ExpectEquals(value, 1.2) @@ -200,19 +207,19 @@ func TestAggregateMySQLProbesWithErrors(t *testing.T) { instanceResultsMap[key1cluster] = base.NoSuchMetric { - worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 0, false, 0) + worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 0, false, 0, false) _, err := worstMetric.Get() test.S(t).ExpectNotNil(err) test.S(t).ExpectEquals(err, base.NoSuchMetricError) } { - worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 1, false, 0) + worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 1, false, 0, false) _, err := worstMetric.Get() test.S(t).ExpectNotNil(err) test.S(t).ExpectEquals(err, base.NoSuchMetricError) } { - worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 2, false, 0) + worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 2, false, 0, false) value, err := worstMetric.Get() test.S(t).ExpectNil(err) test.S(t).ExpectEquals(value, 1.7) @@ -245,13 +252,13 @@ func TestAggregateMySQLProbesWithHttpChecks(t *testing.T) { probes[clusterKey.Key] = &mysql.Probe{Key: clusterKey.Key} } { - worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 0, false, 0) + worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 0, false, 0, false) _, err := worstMetric.Get() test.S(t).ExpectNil(err) } { clusterInstanceHttpCheckResultMap[mysql.MySQLHttpCheckHashKey(clusterName, &key2)] = http.StatusNotFound - worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 0, false, 0) + worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 0, false, 0, false) value, err := worstMetric.Get() test.S(t).ExpectNil(err) test.S(t).ExpectEquals(value, 1.2) @@ -260,7 +267,7 @@ func TestAggregateMySQLProbesWithHttpChecks(t *testing.T) { for hashKey := range clusterInstanceHttpCheckResultMap { clusterInstanceHttpCheckResultMap[hashKey] = http.StatusNotFound } - worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 0, false, 0) + worstMetric := aggregateMySQLProbes(&probes, clusterName, instanceResultsMap, clusterInstanceHttpCheckResultMap, 0, false, 0, false) _, err := worstMetric.Get() test.S(t).ExpectNotNil(err) } diff --git a/pkg/throttle/throttler.go b/pkg/throttle/throttler.go index b602929..8410f97 100644 --- a/pkg/throttle/throttler.go +++ b/pkg/throttle/throttler.go @@ -447,7 +447,7 @@ func (throttler *Throttler) aggregateMySQLMetrics() error { metricName := fmt.Sprintf("mysql/%s", clusterName) ignoreHostsCount := throttler.mysqlInventory.IgnoreHostsCount[clusterName] ignoreHostsThreshold := throttler.mysqlInventory.IgnoreHostsThreshold[clusterName] - aggregatedMetric := aggregateMySQLProbes(probes, clusterName, throttler.mysqlInventory.InstanceKeyMetrics, throttler.mysqlInventory.ClusterInstanceHttpChecks, ignoreHostsCount, config.Settings().Stores.MySQL.IgnoreDialTcpErrors, ignoreHostsThreshold) + aggregatedMetric := aggregateMySQLProbes(probes, clusterName, throttler.mysqlInventory.InstanceKeyMetrics, throttler.mysqlInventory.ClusterInstanceHttpChecks, ignoreHostsCount, config.Settings().Stores.MySQL.IgnoreDialTcpErrors, ignoreHostsThreshold, throttler.BypassOnNoHostsFound) go throttler.aggregatedMetrics.Set(metricName, aggregatedMetric, cache.DefaultExpiration) if throttler.memcacheClient != nil { go func() { From 2a2d27a2325e9274c49b59ee9ec298ebda7abc77 Mon Sep 17 00:00:00 2001 From: meiji163 Date: Wed, 18 Jun 2025 18:18:41 -0700 Subject: [PATCH 7/7] add info logging --- cmd/freno/main.go | 4 ++++ pkg/throttle/throttler.go | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/cmd/freno/main.go b/cmd/freno/main.go index 4c5105c..5c7a8e9 100644 --- a/cmd/freno/main.go +++ b/cmd/freno/main.go @@ -125,8 +125,12 @@ func httpServe() error { bypassEnabled, err := strconv.ParseBool(os.Getenv("FRENO_BYPASS_ENABLED")) if err != nil { + log.Infof("error parsing FRENO_BYPASS_ENABLED: %s", os.Getenv("FRENO_BYPASS_ENABLED")) bypassEnabled = false } + if bypassEnabled { + log.Info("- bypass on no hosts is enabled!") + } throttler.BypassOnNoHostsFound = bypassEnabled consensusServiceProvider, err := group.NewConsensusServiceProvider(throttler) if err != nil { diff --git a/pkg/throttle/throttler.go b/pkg/throttle/throttler.go index 8410f97..58cd006 100644 --- a/pkg/throttle/throttler.go +++ b/pkg/throttle/throttler.go @@ -334,7 +334,7 @@ func (throttler *Throttler) refreshMySQLInventory() error { } if len(totalHosts) == 0 { if throttler.BypassOnNoHostsFound { - log.Debugf("No hosts for pool: %+v, but bypass is enabled", poolName) + log.Infof("No hosts for pool: %+v, but bypass is enabled", poolName) return nil } return log.Errorf("Unable to get any HAproxy hosts for pool: %+v", poolName)