Add composite MySQL safety checks and stable recovery

[pradeepkintali]

Summary

• Compose replica lag with opt-in required primary-load and ProxySQL-capacity metrics.
• Fail closed when configured safety probes have no eligible hosts or required runtime metrics are unavailable.
• Require lower recovery thresholds and continuous healthy cooling periods, including after process/leader startup.
• Include the blocking metric name in composite responses and export recovery-latch metrics.
• Support canonical Vitess PRIMARY discovery (with deprecated MASTER input alias) while preserving the default replica behavior.
• Validate required-cluster references, dependency cycles, and tablet types at configuration loading.
• Document a staged rollout, client wait-budget coordination, and configuration rollback.

Configuration and rollout controls

RequiredClusters, FailOnNoHosts, and RecoveryDurationMillis are opt-in.
Existing clusters retain legacy behavior when these settings are absent.
There is no new global runtime feature switch in this PR.

The companion Puppet PR github/puppet#44086 initially generates shadow primary
probes with EnforcedClusters: [] and EnforceAll: false. Its October 8-15
threshold refresh is based on observed data, including incidents, not a verified
healthy baseline; no enforcement is enabled. This is collection-only shadowing, not a composite dry run: there are no composite would-reject counters. Probe validation and enforcement must be separate reviewed configuration changes.

Rollout plan

1. Deploy a binary containing this PR with existing configuration first.
2. Deploy shadow probes and validate writer discovery, permissions, probe cost,
   errors, and recovery gauges before adding required dependencies.
3. Review recent incident periods and historical host/shard coverage. Compare
   five-minute Datadog baselines with the instantaneous SQL probe.
4. Confirm callers safely retry/defer rejections and errors. Puppet proposes a
   30-second server recovery window; the Ruby client's default maximum wait is
   only 10 seconds. Do not silently enlarge every caller's wait budget.
5. Enable one reviewed cluster, exercise overload/probe loss/failover and both
   HTTP/memcache paths, then expand against owner-approved success criteria.

The #197 changes were merged into this feature branch, not directly into
master; a merged stacked PR does not establish a deployed server version.
The client switch in github/freno-client#37 is independent of server enforcement.

Rollback plan

Remove the primary dependency while preserving replica-lag checks. For the
companion Puppet configuration, remove the cluster from EnforcedClusters,
keep EnforceAll false, and deploy. Account for service notifications, restarts,
and startup recovery; this is not an instantaneous runtime toggle.

Validation

Original implementation validation:

• go test -count=1 ./...
• go vet ./pkg/base ./pkg/config ./pkg/throttle ./pkg/http ./pkg/mysql ./pkg/proxysql ./pkg/vitess
• Full-repository go vet ./... has pre-existing testing.T.Fatalf calls from non-test goroutines in internal/raft tests.

October 15 review follow-up makes PRIMARY canonical, retains MASTER as a deprecated input alias, updates discovery and companion Puppet generation together, and clarifies collection-only shadow rollout. The pinned Vitess dependency is unchanged.

Validation passed on both parent and stacked branches: vendored Go configuration/Vitess tests and vet, Go formatting, five parent and eleven stacked Puppet ERB specs, and full production rendering with 61 shadow primary probes, all 15 Vitess probes using PRIMARY, unchanged normal clusters with enforcement off, and selective canary enforcement verified in memory. git diff --check passed. These checks are not a full Puppet catalog/integration run.

Scope boundaries

This PR provides admission checks and recovery behavior, not universal database
load shedding. Each deployment owns metrics, thresholds, discovery, monitoring,
application concurrency, and safe fallback behavior. Fleet ProxySQL-capacity
enforcement is not included in the companion Puppet rollout.

[Copilot]

Copilot review overview

🟡 Changes recommended

Recovery semantics and direct-memcache behavior can bypass or inconsistently apply the new safety gates.

Review effort: Balanced
Findings: 1 · 3

Open (4)

• Removing IsValidReplica breaks source compatibility · New
• Direct client path bypasses required cluster checks · New
• Recovery state is not reset after leadership regain · New
• Recovery ignores the effective read threshold · New

What changed in this PR

Adds composite MySQL safety checks, recovery hysteresis, and configurable Vitess tablet discovery.

Changes:

• Composes replica checks with required primary and ProxySQL metrics.
• Adds fail-closed behavior, recovery thresholds, cooling periods, and metrics.
• Supports validated REPLICA, MASTER, and PRIMARY Vitess tablet configuration.

File	Description
pkg/​base/​throttle_metric.go	Adds recovery-state metric errors.
pkg/​config/​config_test.go	Tests new configuration validation.
pkg/​config/​mysql_config.go	Adds composite and recovery settings.
pkg/​config/​vitess_config.go	Validates tablet types.
pkg/​throttle/​check.go	Evaluates required clusters.
pkg/​throttle/​check_result.go	Reports the blocking metric.
pkg/​throttle/​throttler.go	Implements recovery latching.
pkg/​throttle/​throttler_test.go	Tests composition and recovery.
pkg/​vitess/​api_client.go	Filters configured tablet types.
pkg/​vitess/​api_client_test.go	Tests primary discovery.
doc/​mysql.md	Documents configuration and rollout scope.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[andyedison]

This feels like an easy to miss self inflicted problem that could happen if you added clusters at the same time as adding to RequiredClusters. Maybe adding a ShadowRequiredClusters could have a built in way of adding requirements but in a safer way? It could still emit would reject counters but not actually reject writes?

[pradeepkintali]

Thanks for flagging this rollout hazard. I checked the companion Puppet configuration and rendered the full production configuration: EnforcedClusters: [] and EnforceAll: false generate the 61 standalone primary probes without changing any normal cluster configuration or adding required dependencies. Probe self-check failures, aggregated metrics, and recovery telemetry can be observed before activation.

For this collection-only rollout, I have kept the existing controls rather than adding ShadowRequiredClusters to the server API. I tightened the server and Puppet rollout docs to explicitly require probe deployment/validation first and enforcement in a separate reviewed configuration change, so adding probes and enforcing them together is not the rollout path.

To be clear, the current shadow mode is not a composite dry run and does not emit composite would-reject counters. Your proposed setting would add that capability; I am deferring that feature rather than claiming the existing telemetry is equivalent. No enforcement was enabled. Full production renders on both Puppet branches confirm normal clusters remain unchanged with the empty allowlist, and selective enforcement changes only the chosen canary.

[andyedison]

The safeguards you're relying on (EnforcedClusters, EnforceAll, full-render checks, a separate reviewed activation change) all live in our internal configuration management, not in freno. Anyone running freno outside GitHub gets none of them. In freno itself, enforcement is just a RequiredClusters edit with no dry run, and the caveats sit at the bottom of a long section mixed in with GitHub-specific rollout steps.

If ShadowRequiredClusters stays out of this PR, the public repo needs to carry the safety information itself:

1. Move GitHub-internal rollout content out of doc/mysql.md. EnforcedClusters/EnforceAll, the Datadog baselines, the fleet 30-second window, and the Add composite MySQL safety checks and stable recovery #196/Harden primary protection recovery and Vitess discovery #197 branch history aren't freno concepts. Someone outside GitHub will look for EnforcedClusters in the freno config and won't find it. That material belongs in the internal deployment docs alongside that config.
2. Start the section with a short "Behavior changes" callout that applies to any deployment:
   • Any cluster with RecoveryDurationMillis > 0 returns 429 for at least that window after every process start and leader change, even with no RequiredClusters. freno can't reload config without a restart (Configuration.Reload() has no callers), so every config change, including a rollback, triggers this.
   • RequiredClusters fails closed. If a dependency can't be collected (wrong discovery, missing grants, or an empty host list with FailOnNoHosts), checks for the cluster that requires it return 500.
   • While any dependency is unhealthy, that cluster's memcache key is deleted.
   • There is no dry run. A config change that adds a probe and references it from RequiredClusters at the same time can block writes immediately.
   • Clients with a wait budget shorter than RecoveryDurationMillis will time out during recovery.
3. Log the composite config at startup. Write one line for each cluster that sets RequiredClusters, RecoveryDurationMillis, or FailOnNoHosts, so operators see it in the logs and not only in the docs.
4. Make it visible from README.md. freno ships from master without release notes, so anyone pulling master gets this with no announcement. Link the callout from the MySQL section of README.md (next to "Read more about freno and MySQL throttling"), so people see it without reading to the end of doc/mysql.md.

ShadowRequiredClusters is still the real fix. A would-reject counter is the only way any deployment can validate a dependency before it starts blocking writes. If it's deferred, please open an issue for it and link it from the doc.