Skip to content

Commit c4e653e

Browse files
Harden primary protection recovery
Require a full healthy window after process or leader startup, expose the blocking metric in check responses, and publish recovery latch state. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0fbc396f-32f9-449a-8537-6264e8665c31
1 parent d4fb5b7 commit c4e653e

5 files changed

Lines changed: 50 additions & 3 deletions

File tree

‎doc/mysql.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,9 @@ The ProxySQL query above is illustrative: deployments must provide a scalar quer
203203
- `FailOnNoHosts` changes the legacy no-host response from `HTTP 200` to `HTTP 500`. Enable it for primary and ProxySQL safety probes, where an empty roster cannot prove that work is safe.
204204
- `RecoveryThreshold` is the lower threshold that begins recovery after a cluster has throttled. It must not exceed `ThrottleThreshold`.
205205
- `RecoveryDurationMillis` is the continuous time the metric must remain at or below `RecoveryThreshold` before checks return `HTTP 200` again. A new error or threshold violation resets the recovery window.
206+
- A new process or leader starts configured recovery metrics in the recovering state. It must observe a complete healthy window before admitting work.
206207
- Metric sampling remains centralized in the `freno` leader and uses the existing metric cache. Application checks read the aggregated decision rather than querying the primary or ProxySQL for every batch.
208+
- Check responses include `MetricName`, identifying the replica, primary, or ProxySQL metric that blocked a composite decision. `recovery.mysql.<cluster>.active` reports whether the recovery latch is active.
207209

208210
Configuration loading rejects missing `RequiredClusters` references and dependency cycles.
209211

‎pkg/throttle/check.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,9 @@ func (check *ThrottlerCheck) checkAppMetricResult(appName string, storeType stri
103103
// all good!
104104
statusCode = http.StatusOK // 200
105105
}
106-
return NewCheckResult(statusCode, value, threshold, err)
106+
checkResult = NewCheckResult(statusCode, value, threshold, err)
107+
checkResult.MetricName = metricName
108+
return checkResult
107109
}
108110

109111
func (check *ThrottlerCheck) checkMySQLCluster(
@@ -134,10 +136,12 @@ func (check *ThrottlerCheck) checkMySQLCluster(
134136
continue
135137
}
136138
if requiredResult.StatusCode == http.StatusNotFound {
137-
return NewErrorCheckResult(
139+
checkResult := NewErrorCheckResult(
138140
http.StatusInternalServerError,
139141
fmt.Errorf("required MySQL cluster metric %q is unavailable", requiredCluster),
140142
)
143+
checkResult.MetricName = fmt.Sprintf("mysql/%s", requiredCluster)
144+
return checkResult
141145
}
142146
return requiredResult
143147
}

‎pkg/throttle/check_result.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ type CheckResult struct {
1111
StatusCode int `json:"StatusCode"`
1212
Value float64 `json:"Value"`
1313
Threshold float64 `json:"Threshold"`
14+
MetricName string `json:"MetricName,omitempty"`
1415
Error error `json:"-"`
1516
Message string `json:"Message"`
1617
}

‎pkg/throttle/throttler.go‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -486,16 +486,21 @@ func (throttler *Throttler) stabilizeMySQLMetric(clusterName string, metricResul
486486

487487
state, ok := throttler.mysqlRecoveryState[clusterName]
488488
if !ok {
489-
state = &mysqlClusterRecoveryState{}
489+
// A new process or leader has no recovery history. Require a complete
490+
// healthy window before admitting work rather than assuming the metric
491+
// was healthy before this process began observing it.
492+
state = &mysqlClusterRecoveryState{throttled: true}
490493
throttler.mysqlRecoveryState[clusterName] = state
491494
}
492495

493496
if err != nil || value > clusterSettings.ThrottleThreshold {
494497
state.throttled = true
495498
state.healthySince = time.Time{}
499+
metrics.GetOrRegisterGauge(fmt.Sprintf("recovery.mysql.%s.active", clusterName), nil).Update(1)
496500
return metricResult
497501
}
498502
if !state.throttled {
503+
metrics.GetOrRegisterGauge(fmt.Sprintf("recovery.mysql.%s.active", clusterName), nil).Update(0)
499504
return metricResult
500505
}
501506

@@ -505,20 +510,24 @@ func (throttler *Throttler) stabilizeMySQLMetric(clusterName string, metricResul
505510
}
506511
if value > recoveryThreshold {
507512
state.healthySince = time.Time{}
513+
metrics.GetOrRegisterGauge(fmt.Sprintf("recovery.mysql.%s.active", clusterName), nil).Update(1)
508514
return base.NewErrorMetricResult(value, base.RecoveryNotCompleteError)
509515
}
510516
if state.healthySince.IsZero() {
511517
state.healthySince = now
518+
metrics.GetOrRegisterGauge(fmt.Sprintf("recovery.mysql.%s.active", clusterName), nil).Update(1)
512519
return base.NewErrorMetricResult(value, base.RecoveryNotCompleteError)
513520
}
514521

515522
recoveryDuration := time.Duration(clusterSettings.RecoveryDurationMillis) * time.Millisecond
516523
if now.Sub(state.healthySince) < recoveryDuration {
524+
metrics.GetOrRegisterGauge(fmt.Sprintf("recovery.mysql.%s.active", clusterName), nil).Update(1)
517525
return base.NewErrorMetricResult(value, base.RecoveryNotCompleteError)
518526
}
519527

520528
state.throttled = false
521529
state.healthySince = time.Time{}
530+
metrics.GetOrRegisterGauge(fmt.Sprintf("recovery.mysql.%s.active", clusterName), nil).Update(0)
522531
return metricResult
523532
}
524533

‎pkg/throttle/throttler_test.go‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77

88
"github.com/github/freno/pkg/base"
99
"github.com/github/freno/pkg/config"
10+
metrics "github.com/rcrowley/go-metrics"
1011
"github.com/stretchr/testify/assert"
1112
)
1213

@@ -247,6 +248,7 @@ func TestCheckMySQLRequiredClusters(t *testing.T) {
247248
assert.Equal(t, http.StatusTooManyRequests, result.StatusCode)
248249
assert.Equal(t, 60.0, result.Value)
249250
assert.Equal(t, 50.0, result.Threshold)
251+
assert.Equal(t, "mysql/primary", result.MetricName)
250252
})
251253

252254
t.Run("ProxySQL cluster blocks admission", func(t *testing.T) {
@@ -275,6 +277,7 @@ func TestCheckMySQLRequiredClusters(t *testing.T) {
275277
result := check.Check("transitions", "mysql", "replicas", "", &CheckFlags{OKIfNotExists: true})
276278
assert.Equal(t, http.StatusInternalServerError, result.StatusCode)
277279
assert.EqualError(t, result.Error, `required MySQL cluster metric "primary" is unavailable`)
280+
assert.Equal(t, "mysql/primary", result.MetricName)
278281
})
279282
}
280283

@@ -337,3 +340,31 @@ func TestStabilizeMySQLMetricResetsRecoveryWindow(t *testing.T) {
337340
_, err := result.Get()
338341
assert.Equal(t, base.RecoveryNotCompleteError, err)
339342
}
343+
344+
func TestStabilizeMySQLMetricRequiresHealthyWindowAfterStartup(t *testing.T) {
345+
originalSettings := config.Settings().Stores.MySQL
346+
defer func() {
347+
config.Settings().Stores.MySQL = originalSettings
348+
}()
349+
350+
config.Settings().Stores.MySQL = config.MySQLConfigurationSettings{
351+
Clusters: map[string]*config.MySQLClusterConfigurationSettings{
352+
"primary": {
353+
ThrottleThreshold: 50.0,
354+
RecoveryDurationMillis: 5000,
355+
},
356+
},
357+
}
358+
359+
throttler := NewThrottler()
360+
start := time.Date(2026, time.September, 23, 12, 0, 0, 0, time.UTC)
361+
362+
result := throttler.stabilizeMySQLMetric("primary", base.NewSimpleMetricResult(30), start)
363+
_, err := result.Get()
364+
assert.Equal(t, base.RecoveryNotCompleteError, err)
365+
366+
result = throttler.stabilizeMySQLMetric("primary", base.NewSimpleMetricResult(30), start.Add(5*time.Second))
367+
_, err = result.Get()
368+
assert.Nil(t, err)
369+
assert.Equal(t, int64(0), metrics.Get("recovery.mysql.primary.active").(metrics.Gauge).Value())
370+
}

0 commit comments

Comments
 (0)